[BitcoinV2]: Allow to preImageHash and compile transaction throug…

…h Bitcoin legacy interface (#3979) * feat(bitcoin): Fix Taproot signing when tx contains Segwit UTXOs too * feat(bitcoin): Forward preImageHashes, compile requests to Rust * feat(bitcoin): Fix clippy warnings
trustwallet · Aug 9, 2024 · a9eb114 · a9eb114
1 parent a05c01a
commit a9eb114
Show file tree

Hide file tree

Showing 14 changed files with 311 additions and 47 deletions.
diff --git a/rust/frameworks/tw_utxo/src/address/legacy.rs b/rust/frameworks/tw_utxo/src/address/legacy.rs
@@ -97,16 +97,17 @@ impl LegacyAddress {
     }
 
     pub fn prefix(&self) -> u8 {
-        self.bytes()[0]
+        self.0.as_ref()[0]
     }
 
+    /// Address bytes excluding the prefix (skip first byte).
     pub fn bytes(&self) -> &[u8] {
-        self.0.as_ref()
+        &self.0.as_ref()[1..]
     }
 
     pub fn payload(&self) -> H160 {
-        debug_assert_eq!(self.bytes().len(), H160::LEN + 1);
-        H160::try_from(&self.0.as_ref()[1..]).expect("Legacy address must be exactly 21 bytes")
+        debug_assert_eq!(self.bytes().len(), H160::LEN);
+        H160::try_from(self.bytes()).expect("Legacy address must be exactly 20 bytes")
     }
 }
 

diff --git a/rust/frameworks/tw_utxo/src/modules/sighash_computer.rs b/rust/frameworks/tw_utxo/src/modules/sighash_computer.rs
@@ -80,6 +80,7 @@ where
                         (sighash, None)
                     },
                     SigningMethod::Taproot => {
+                        // TODO Move `tr_spent_amounts` and `tr_spent_script_pubkeys` logic to `Transaction::preimage_taproot_tx()`.
                         let tr_spent_amounts: Vec<Amount> = unsigned_tx
                             .input_args()
                             .iter()
@@ -89,7 +90,15 @@ where
                         let tr_spent_script_pubkeys: Vec<Script> = unsigned_tx
                             .input_args()
                             .iter()
-                            .map(|utxo| utxo.script_pubkey.clone())
+                            .map(|utxo| {
+                                if utxo.signing_method == SigningMethod::Taproot {
+                                    // Taproot UTXOs scriptPubkeys should be signed as is.
+                                    utxo.script_pubkey.clone()
+                                } else {
+                                    // Use the original scriptPubkey declared in the unspent output.
+                                    utxo.prevout_script_pubkey.clone()
+                                }
+                            })
                             .collect();
 
                         let tr = UtxoTaprootPreimageArgs {

diff --git a/rust/frameworks/tw_utxo/src/transaction/asset/brc20.rs b/rust/frameworks/tw_utxo/src/transaction/asset/brc20.rs
@@ -1,7 +1,8 @@
 use super::ordinal::OrdinalsInscription;
+use bitcoin::hashes::Hash;
 use std::ops::Deref;
 use tw_coin_entry::error::prelude::*;
-use tw_hash::H264;
+use tw_hash::{H256, H264};
 
 #[derive(Debug, Clone)]
 pub struct Brc20Ticker(String);
@@ -47,6 +48,14 @@ impl BRC20TransferInscription {
         let inscription = OrdinalsInscription::new(BRC20_MIME, payload.as_bytes(), recipient)?;
         Ok(BRC20TransferInscription(inscription))
     }
+
+    pub fn merkle_root(&self) -> SigningResult<H256> {
+        self.spend_info
+            .merkle_root()
+            .map(|root| H256::from(root.to_byte_array()))
+            .or_tw_err(SigningErrorType::Error_internal)
+            .context("No merkle root of the BRC20 Transfer spend info")
+    }
 }
 
 #[cfg(test)]

diff --git a/rust/frameworks/tw_utxo/src/transaction/mod.rs b/rust/frameworks/tw_utxo/src/transaction/mod.rs
@@ -50,6 +50,11 @@ pub struct UtxoTaprootPreimageArgs {
 /// UTXO signing arguments contain all info required to sign a UTXO (Unspent Transaction Output).
 #[derive(Clone)]
 pub struct UtxoToSign {
+    /// Original `scriptPubkey` specified in the unspent transaction output.
+    /// May be the same or different from [`UtxoToSign::script_pubkey`].
+    pub prevout_script_pubkey: Script,
+    /// `scriptPubkey` with which the UTXO needs to be signed.
+    /// For example, if [`UtxoToSign::original_script_pubkey`] is P2WPKH, then [`UtxoToSign::script_pubkey`] will be P2PKH.
     pub script_pubkey: Script,
     pub signing_method: SigningMethod,
     pub spending_data_constructor: SpendingDataConstructor,

diff --git a/rust/frameworks/tw_utxo/src/transaction/standard_transaction/builder/output.rs b/rust/frameworks/tw_utxo/src/transaction/standard_transaction/builder/output.rs
@@ -6,7 +6,6 @@ use crate::{
         transaction_parts::Amount,
     },
 };
-use bitcoin::hashes::Hash;
 use tw_coin_entry::error::prelude::*;
 use tw_hash::{ripemd::bitcoin_hash_160, sha2::sha256, H160, H256};
 use tw_keypair::{ecdsa, schnorr};
@@ -132,15 +131,7 @@ impl OutputBuilder {
         let pubkey_data = pubkey.compressed();
         let ticker = Brc20Ticker::new(ticker)?;
         let transfer = BRC20TransferInscription::new(&pubkey_data, &ticker, &value)?;
-
-        let merkle_root: H256 = transfer
-            .spend_info
-            .merkle_root()
-            .or_tw_err(SigningErrorType::Error_internal)
-            .context("No merkle root of the BRC20 Transfer spend info")?
-            .to_byte_array()
-            .into();
-
+        let merkle_root = transfer.merkle_root()?;
         Ok(self.p2tr_script_path(pubkey, merkle_root))
     }
 

diff --git a/rust/frameworks/tw_utxo/src/transaction/standard_transaction/builder/utxo.rs b/rust/frameworks/tw_utxo/src/transaction/standard_transaction/builder/utxo.rs
@@ -127,10 +127,14 @@ impl UtxoBuilder {
         let amount = self.finalize_amount()?;
         let sighash_ty = self.finalize_sighash_type()?;
 
+        // The scriptPubkey for signing is the same as declared at the unspent output.
+        let script_pubkey = conditions::new_p2pk(&pubkey.compressed());
+
         Ok((
             self.input,
             UtxoToSign {
-                script_pubkey: conditions::new_p2pk(&pubkey.compressed()),
+                prevout_script_pubkey: script_pubkey.clone(),
+                script_pubkey,
                 // P2PK output can be spent by a legacy address only.
                 signing_method: SigningMethod::Legacy,
                 // When the sighash is signed, build a P2PK script_sig.
@@ -157,10 +161,14 @@ impl UtxoBuilder {
         let amount = self.finalize_amount()?;
         let sighash_ty = self.finalize_sighash_type()?;
 
+        // The scriptPubkey for signing is the same as declared at the unspent output.
+        let script_pubkey = conditions::new_p2pkh(&pubkey_hash);
+
         Ok((
             self.input,
             UtxoToSign {
-                script_pubkey: conditions::new_p2pkh(&pubkey_hash),
+                prevout_script_pubkey: script_pubkey.clone(),
+                script_pubkey,
                 // P2PK output can be spent by a legacy address only.
                 signing_method: SigningMethod::Legacy,
                 // When the sighash is signed, build a P2PKH script_sig.
@@ -216,6 +224,8 @@ impl UtxoBuilder {
         Ok((
             self.input,
             UtxoToSign {
+                // Original P2WPKH scriptPubkey.
+                prevout_script_pubkey: conditions::new_p2wpkh(&pubkey_hash),
                 // To spend a P2WPKH UTXO, we need to sign the transaction with a corresponding P2PKH UTXO.
                 // Then the result script_sig will be published as a witness.
                 // Generating special scriptPubkey for P2WPKH.
@@ -253,13 +263,14 @@ impl UtxoBuilder {
         let amount = self.finalize_amount()?;
         let sighash_ty = self.finalize_sighash_type()?;
 
+        // The scriptPubkey for signing is the same as declared at the unspent output.
+        let script_pubkey = conditions::new_p2tr_dangerous_assume_tweaked(&tweaked_pubkey.bytes());
+
         Ok((
             self.input,
             UtxoToSign {
-                // Generating special scriptPubkey for P2WPKH.
-                script_pubkey: conditions::new_p2tr_dangerous_assume_tweaked(
-                    &tweaked_pubkey.bytes(),
-                ),
+                prevout_script_pubkey: script_pubkey.clone(),
+                script_pubkey,
                 // P2TR output can be spent by a Witness (eg "bc1") address only.
                 signing_method: SigningMethod::Taproot,
                 // When the sighash is signed, build a P2TR witness.
@@ -281,6 +292,7 @@ impl UtxoBuilder {
         internal_pubkey: &schnorr::PublicKey,
         payload: Script,
         control_block: Data,
+        merkle_root: &H256,
     ) -> SigningResult<(TransactionInput, UtxoToSign)> {
         // Construct the leaf hash.
         let script_buf = bitcoin::ScriptBuf::from_bytes(payload.to_vec());
@@ -296,9 +308,14 @@ impl UtxoBuilder {
         let amount = self.finalize_amount()?;
         let sighash_ty = self.finalize_sighash_type()?;
 
+        // Restore the original scriptPubkey declared at the unspent P2TR output.
+        let prevout_script_pubkey =
+            conditions::new_p2tr_script_path(&internal_pubkey.compressed(), merkle_root);
+
         Ok((
             self.input,
             UtxoToSign {
+                prevout_script_pubkey,
                 // We use the full (revealed) script as scriptPubkey here.
                 script_pubkey: payload.clone(),
                 signing_method: SigningMethod::Taproot,
@@ -338,8 +355,15 @@ impl UtxoBuilder {
             .or_tw_err(SigningErrorType::Error_internal)
             .context("'TaprootSpendInfo::control_block' is None")?;
 
+        let merkle_root = transfer.merkle_root()?;
         let transfer_payload = Script::from(transfer.script.to_bytes());
-        self.p2tr_script_path(pubkey, transfer_payload, control_block.serialize())
+
+        self.p2tr_script_path(
+            pubkey,
+            transfer_payload,
+            control_block.serialize(),
+            &merkle_root,
+        )
     }
 }
 

diff --git a/rust/tw_any_coin/tests/chains/bitcoin/bitcoin_address.rs b/rust/tw_any_coin/tests/chains/bitcoin/bitcoin_address.rs
@@ -82,11 +82,29 @@ fn test_bitcoin_address_invalid() {
 }
 
 #[test]
-fn test_bitcoin_address_get_data() {
+fn test_bitcoin_address_legacy_get_data() {
     test_address_get_data(
         CoinType::Bitcoin,
         "1Bp9U1ogV3A14FMvKbRJms7ctyso4Z4Tcx",
-        "00769bdff96a02f9135a1d19b749db6a78fe07dc90",
+        "769bdff96a02f9135a1d19b749db6a78fe07dc90",
+    );
+}
+
+#[test]
+fn test_bitcoin_address_segwit_get_data() {
+    test_address_get_data(
+        CoinType::Bitcoin,
+        "bc1qunq74p3h8425hr6wllevlvqqr6sezfxj262rff",
+        "e4c1ea86373d554b8f4efff2cfb0001ea19124d2",
+    );
+}
+
+#[test]
+fn test_bitcoin_address_taproot_get_data() {
+    test_address_get_data(
+        CoinType::Bitcoin,
+        "bc1pwse34zfpvt344rvlt7tw0ngjtfh9xasc4q03avf0lk74jzjpzjuqaz7ks5",
+        "74331a892162e35a8d9f5f96e7cd125a6e537618a81f1eb12ffdbd590a4114b8",
     );
 }
 

diff --git a/rust/tw_any_coin/tests/chains/bitcoin/bitcoin_sign/p2tr_key_path.rs b/rust/tw_any_coin/tests/chains/bitcoin/bitcoin_sign/p2tr_key_path.rs
@@ -4,7 +4,7 @@ use crate::chains::common::bitcoin::{
 };
 use tw_coin_registry::coin_type::CoinType;
 use tw_encoding::hex::DecodeHex;
-use tw_keypair::schnorr;
+use tw_keypair::{ecdsa, schnorr};
 use tw_misc::traits::ToBytesVec;
 use tw_proto::BitcoinV2::Proto;
 use tw_utxo::address::taproot::TaprootAddress;
@@ -287,3 +287,70 @@ fn test_bitcoin_sign_input_p2tr_key_path_with_max_amount_89c5d1() {
             fee: 600,
         });
 }
+
+#[test]
+fn test_bitcoin_sign_input_p2tr_and_p2wpkh() {
+    const P2TR_PRIVATE_KEY: &str =
+        "2481de1ce115aa2f0ae9066046baf37256db97958aa7a5ac26c6d8ed0a48e88c";
+    const P2WPKH_PRIVATE_KEY: &str =
+        "25834fabebf75c15d5852aed768894e6f49853023df586e3dd5e83b42a9e2e2b";
+    const SEND_TO: &str = "bc1q8jqgv4us4pluu8nql9ze66y6rs6kxwk5tq678a";
+
+    let p2tr_private_key = schnorr::PrivateKey::try_from(P2TR_PRIVATE_KEY).unwrap();
+    let p2wpkh_private_key = ecdsa::secp256k1::PrivateKey::try_from(P2WPKH_PRIVATE_KEY).unwrap();
+
+    let txid = "0200c9a11371465e5d02fdb86fd13f2fa76561fd9d7d14929f31b8f919217995";
+    let p2tr_utxo = Proto::Input {
+        out_point: input::out_point(txid, 0),
+        value: 5_817,
+        sighash_type: SIGHASH_ALL,
+        // Spend a P2TR UTXO.
+        claiming_script: input::p2tr_key_path(p2tr_private_key.public().to_vec()),
+        ..Default::default()
+    };
+
+    let txid = "aff21919411e1988755768c2f7d2c34b7451a12f81c333d7609f6f03f408f2f0";
+    let p2wpkh_utxo = Proto::Input {
+        out_point: input::out_point(txid, 1),
+        value: 4_500,
+        sighash_type: SIGHASH_ALL,
+        // Spend a P2WPKH UTXO.
+        claiming_script: input::p2wpkh(p2wpkh_private_key.public().to_vec()),
+        ..Default::default()
+    };
+
+    // Send max amount to a P2WPKH address.
+    let max_output = Proto::Output {
+        to_recipient: output::to_address(SEND_TO),
+        ..Proto::Output::default()
+    };
+
+    let signing = Proto::SigningInput {
+        version: Proto::TransactionVersion::V2,
+        private_keys: vec![
+            P2TR_PRIVATE_KEY.decode_hex().unwrap().into(),
+            P2WPKH_PRIVATE_KEY.decode_hex().unwrap().into(),
+        ],
+        inputs: vec![p2tr_utxo, p2wpkh_utxo],
+        input_selector: Proto::InputSelector::UseAll,
+        fee_per_vb: 4,
+        max_amount_output: Some(max_output),
+        chain_info: btc_info(),
+        dangerous_use_fixed_schnorr_rng: true,
+        dust_policy: dust_threshold(DUST),
+        ..Default::default()
+    };
+
+    // Successfully broadcasted: https://mempool.space/tx/bfc782da443774b9cf35e6d59b08312be311d791b4e802136fff88adc2312d28
+    sign::BitcoinSignHelper::new(&signing)
+        .coin(CoinType::Bitcoin)
+        .sign(sign::Expected {
+            encoded: "0200000000010295792119f9b8319f92147d9dfd6165a72f3fd16fb8fd025d5e467113a1c900020000000000fffffffff0f208f4036f9f60d733c3812fa151744bc3d2f7c268577588191e411919f2af0100000000ffffffff01ad250000000000001600143c80865790a87fce1e60f9459d689a1c35633ad40140d8cd111da08d7863366d4ff23f6b3be3ce7c7496ecabdbb6a275b1591270767e163d091b208190abae3075f24f11b1291656732369e3f9a14efdf131e481701302473044022021df9a043a886a9e7068fe06a19d8aa0a4a5a21c4b4e50abdbc6aeede32ca494022049da5e9082d83e6108027f4d63965cee50a2c9b619e603692748388150b3bf840121037ef29d31b889dfbae30cff4e996f742a49aae10a03fb6f992048c8d366b4b7c900000000",
+            txid: "bfc782da443774b9cf35e6d59b08312be311d791b4e802136fff88adc2312d28",
+            inputs: vec![5_817, 4_500],
+            outputs: vec![9_645],
+            vsize: 167,
+            weight: 667,
+            fee: 672,
+        });
+}
diff --git a/src/Bitcoin/Entry.cpp b/src/Bitcoin/Entry.cpp
@@ -171,27 +171,8 @@ Data Entry::preImageHashes([[maybe_unused]] TWCoinType coin, const Data& txInput
 void Entry::compile([[maybe_unused]] TWCoinType coin, const Data& txInputData, const std::vector<Data>& signatures,
                     const std::vector<PublicKey>& publicKeys, Data& dataOut) const {
     auto txCompilerFunctor = [&signatures, &publicKeys](auto&& input, auto&& output) noexcept {
-        if (signatures.empty() || publicKeys.empty()) {
-            output.set_error(Common::Proto::Error_invalid_params);
-            output.set_error_message("empty signatures or publickeys");
-            return;
-        }
-
-        if (signatures.size() != publicKeys.size()) {
-            output.set_error(Common::Proto::Error_invalid_params);
-            output.set_error_message("signatures size and publickeys size not equal");
-            return;
-        }
-
-        HashPubkeyList externalSignatures;
-        auto insertFunctor = [](auto&& signature, auto&& pubkey) noexcept {
-            return std::make_pair(signature, pubkey.bytes);
-        };
-        transform(begin(signatures), end(signatures), begin(publicKeys),
-                  back_inserter(externalSignatures), insertFunctor);
-        output = Signer::sign(input, externalSignatures);
+        output = Signer::compile(input, signatures, publicKeys);
     };
-
     dataOut = txCompilerTemplate<Proto::SigningInput, Proto::SigningOutput>(txInputData,
                                                                             txCompilerFunctor);
 }