diff --git a/packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/mod.rs b/packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/mod.rs index 1354363..9612f1c 100644 --- a/packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/mod.rs +++ b/packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/mod.rs @@ -454,6 +454,52 @@ impl BitGoPsbt { )) } + /// Create a new empty PSBT with the same network parameters as an existing PSBT + /// + /// This is useful for reconstructing PSBTs - it copies: + /// - Network + /// - Transaction version and locktime + /// - For Zcash: consensus_branch_id, version_group_id, expiry_height + /// + /// # Arguments + /// * `template` - The existing PSBT to copy network parameters from + /// * `wallet_keys` - The wallet's root keys for the new PSBT + /// + /// # Returns + /// * `Ok(Self)` - A new empty PSBT with the same network parameters + /// * `Err(String)` - If the template is a Zcash PSBT missing the consensus branch ID + pub fn new_like( + template: &BitGoPsbt, + wallet_keys: &crate::fixed_script_wallet::RootWalletKeys, + ) -> Result { + let network = template.network(); + let version = template.psbt().unsigned_tx.version.0; + let lock_time = template.psbt().unsigned_tx.lock_time.to_consensus_u32(); + + match template { + BitGoPsbt::Zcash(zcash_psbt, _) => { + // For Zcash, extract all required parameters from the template + let consensus_branch_id = propkv::get_zec_consensus_branch_id(&zcash_psbt.psbt) + .ok_or("Template PSBT missing ZecConsensusBranchId")?; + Ok(Self::new_zcash( + network, + wallet_keys, + consensus_branch_id, + Some(version), + Some(lock_time), + zcash_psbt.version_group_id, + zcash_psbt.expiry_height, + )) + } + _ => Ok(Self::new( + network, + wallet_keys, + Some(version), + Some(lock_time), + )), + } + } + fn new_internal( network: Network, wallet_keys: &crate::fixed_script_wallet::RootWalletKeys, @@ -1075,6 +1121,37 @@ impl BitGoPsbt { } } + /// Extract the finalized transaction bytes with network-appropriate serialization + /// + /// This method extracts the fully-signed transaction from a finalized PSBT, + /// serializing it with the correct format for the network: + /// - For Zcash: includes version_group_id, expiry_height, and sapling fields + /// - For other networks: uses standard Bitcoin transaction serialization + /// + /// This method consumes the PSBT since the underlying `extract_tx()` requires ownership. + /// + /// # Requirements + /// All inputs must be finalized before calling this method. + /// + /// # Returns + /// * `Ok(Vec)` - The serialized transaction bytes + /// * `Err(String)` - If transaction extraction fails + pub fn extract_tx(self) -> Result, String> { + use miniscript::bitcoin::consensus::serialize; + + match self { + BitGoPsbt::Zcash(zcash_psbt, _) => zcash_psbt + .extract_tx() + .map_err(|e| format!("Failed to extract transaction: {}", e)), + BitGoPsbt::BitcoinLike(psbt, _) | BitGoPsbt::Dash(DashBitGoPsbt { psbt, .. }, _) => { + let tx = psbt + .extract_tx() + .map_err(|e| format!("Failed to extract transaction: {}", e))?; + Ok(serialize(&tx)) + } + } + } + pub fn into_psbt(self) -> Psbt { match self { BitGoPsbt::BitcoinLike(psbt, _network) => psbt, @@ -3212,27 +3289,29 @@ mod tests { format, ) .expect("Failed to load fixture"); - let bitgo_psbt = fixture + let mut bitgo_psbt = fixture .to_bitgo_psbt(network) .expect("Failed to convert to BitGo PSBT"); let fixture_extracted_transaction = fixture .extracted_transaction .expect("Failed to extract transaction"); - // // Use BitGoPsbt::finalize() which handles MuSig2 inputs + // Finalize and extract using the network-aware extract_tx() method let secp = crate::bitcoin::secp256k1::Secp256k1::new(); - let finalized_psbt = bitgo_psbt.finalize(&secp).expect("Failed to finalize PSBT"); - let extracted_transaction = finalized_psbt + bitgo_psbt + .finalize_mut(&secp) + .expect("Failed to finalize PSBT"); + + let extracted_tx_bytes = bitgo_psbt .extract_tx() .expect("Failed to extract transaction"); - use miniscript::bitcoin::consensus::serialize; - let extracted_transaction_hex = hex::encode(serialize(&extracted_transaction)); + let extracted_transaction_hex = hex::encode(extracted_tx_bytes); + assert_eq!( extracted_transaction_hex, fixture_extracted_transaction, "Extracted transaction should match" ); - // Zcash fixtures were created with legacy Bitcoin sighash; implementation uses ZIP-243 - }, ignore: [Zcash]); + }); #[test] fn test_add_paygo_attestation() { @@ -3418,32 +3497,37 @@ mod tests { format, ) .expect("Failed to load fixture"); - + let bitgo_psbt = fixture .to_bitgo_psbt(network) .expect("Failed to convert to BitGo PSBT"); - + // Get wallet keys from fixture let wallet_xprv = fixture .get_wallet_xprvs() .expect("Failed to get wallet keys"); let wallet_keys = wallet_xprv.to_root_wallet_keys(); - + // Create replay protection with the replay protection script from fixture let replay_protection = crate::fixed_script_wallet::ReplayProtection::new(vec![ - miniscript::bitcoin::ScriptBuf::from_hex("a91420b37094d82a513451ff0ccd9db23aba05bc5ef387") - .expect("Failed to parse replay protection output script"), + miniscript::bitcoin::ScriptBuf::from_hex( + "a91420b37094d82a513451ff0ccd9db23aba05bc5ef387", + ) + .expect("Failed to parse replay protection output script"), ]); - + // Parse the transaction (no PayGo verification in tests) let parsed = bitgo_psbt .parse_transaction_with_wallet_keys(&wallet_keys, &replay_protection, &[]) .expect("Failed to parse transaction"); - + // Basic validations assert!(!parsed.inputs.is_empty(), "Should have at least one input"); - assert!(!parsed.outputs.is_empty(), "Should have at least one output"); - + assert!( + !parsed.outputs.is_empty(), + "Should have at least one output" + ); + // Verify at least one replay protection input exists let replay_protection_inputs = parsed .inputs @@ -3454,18 +3538,15 @@ mod tests { replay_protection_inputs > 0, "Should have at least one replay protection input" ); - + // Verify at least one wallet input exists let wallet_inputs = parsed .inputs .iter() .filter(|i| i.script_id.is_some()) .count(); - assert!( - wallet_inputs > 0, - "Should have at least one wallet input" - ); - + assert!(wallet_inputs > 0, "Should have at least one wallet input"); + // Count internal (wallet) and external outputs let internal_outputs = parsed .outputs @@ -3477,13 +3558,13 @@ mod tests { .iter() .filter(|o| o.script_id.is_none()) .count(); - + assert_eq!( internal_outputs + external_outputs, parsed.outputs.len(), "All outputs should be either internal or external" ); - + // Verify spend amount only includes external outputs let calculated_spend_amount: u64 = parsed .outputs @@ -3495,23 +3576,23 @@ mod tests { parsed.spend_amount, calculated_spend_amount, "Spend amount should equal sum of external output values" ); - + // Verify total values let total_input_value: u64 = parsed.inputs.iter().map(|i| i.value).sum(); let total_output_value: u64 = parsed.outputs.iter().map(|o| o.value).sum(); - + assert_eq!( parsed.miner_fee, total_input_value - total_output_value, "Miner fee should equal inputs minus outputs" ); - + // Verify virtual size is reasonable assert!( parsed.virtual_size > 0, "Virtual size should be greater than 0" ); - + // Verify outputs (fixtures now have 3 external outputs) assert_eq!( external_outputs, 3, @@ -3526,7 +3607,7 @@ mod tests { parsed.spend_amount > 0, "Spend amount should be greater than 0 when there are external outputs" ); - }, ignore: []); + }); #[test] fn test_serialize_bitcoin_psbt() { @@ -3636,19 +3717,9 @@ mod tests { .parse_outputs(&other_wallet_keys, &[]) .expect("Failed to parse outputs with other wallet keys"); - // Create empty PSBT with same version and locktime as original - let original_version = original_psbt.psbt().unsigned_tx.version.0 as i32; - let original_locktime = original_psbt - .psbt() - .unsigned_tx - .lock_time - .to_consensus_u32(); - let mut reconstructed = BitGoPsbt::new( - network, - &wallet_keys, - Some(original_version), - Some(original_locktime), - ); + // Create empty PSBT with same network parameters as original (handles Zcash automatically) + let mut reconstructed = BitGoPsbt::new_like(&original_psbt, &wallet_keys) + .expect("Failed to create PSBT from template"); // Track which inputs are wallet inputs vs replay protection let mut wallet_input_indices = Vec::new(); @@ -3958,15 +4029,14 @@ mod tests { let reconstructed_bytes = reconstructed .serialize() .expect("Failed to serialize reconstructed"); - assert_equal_psbt(&original_bytes, &reconstructed_bytes); + assert_equal_psbt(&original_bytes, &reconstructed_bytes, network); } // Note: Only testing PsbtLite format for now because full PSBT format // uses non_witness_utxo instead of witness_utxo for non-segwit inputs - // Zcash: Transaction decoding fails because Zcash tx format differs from Bitcoin crate::test_psbt_fixtures!(test_psbt_reconstruction, network, format, { test_psbt_reconstruction_for_network(network, format); - }, ignore: [Zcash]); + }); #[test] fn test_dogecoin_single_input_single_output_large_amount() { diff --git a/packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/zcash_psbt.rs b/packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/zcash_psbt.rs index 1b7a483..f807a5f 100644 --- a/packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/zcash_psbt.rs +++ b/packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/zcash_psbt.rs @@ -65,10 +65,19 @@ impl ZcashBitGoPsbt { /// /// This extracts the fully-signed transaction with Zcash-specific fields. /// Must be called after all inputs have been finalized. - pub fn extract_zcash_transaction(&self) -> Result, super::DeserializeError> { + /// + /// This method consumes the PSBT to avoid cloning. + pub fn extract_tx(self) -> Result, super::DeserializeError> { use miniscript::bitcoin::psbt::ExtractTxError; - let tx = self.psbt.clone().extract_tx().map_err(|e| match e { + // Capture Zcash-specific fields before consuming psbt + let version_group_id = self + .version_group_id + .unwrap_or(ZCASH_SAPLING_VERSION_GROUP_ID); + let expiry_height = self.expiry_height.unwrap_or(0); + let sapling_fields = self.sapling_fields; + + let tx = self.psbt.extract_tx().map_err(|e| match e { ExtractTxError::AbsurdFeeRate { .. } => { super::DeserializeError::Network(format!("Absurd fee rate: {}", e)) } @@ -81,7 +90,15 @@ impl ZcashBitGoPsbt { _ => super::DeserializeError::Network(format!("Failed to extract transaction: {}", e)), })?; - self.serialize_as_zcash_transaction(&tx) + let parts = crate::zcash::transaction::ZcashTransactionParts { + transaction: tx, + is_overwintered: true, + version_group_id: Some(version_group_id), + expiry_height: Some(expiry_height), + sapling_fields, + }; + crate::zcash::transaction::encode_zcash_transaction_parts(&parts) + .map_err(super::DeserializeError::Network) } /// Compute the transaction ID for the unsigned Zcash transaction diff --git a/packages/wasm-utxo/src/fixed_script_wallet/test_utils/psbt_compare.rs b/packages/wasm-utxo/src/fixed_script_wallet/test_utils/psbt_compare.rs index 86681bf..c5299a5 100644 --- a/packages/wasm-utxo/src/fixed_script_wallet/test_utils/psbt_compare.rs +++ b/packages/wasm-utxo/src/fixed_script_wallet/test_utils/psbt_compare.rs @@ -8,13 +8,15 @@ //! //! ```ignore //! use crate::fixed_script_wallet::test_utils::psbt_compare::assert_equal_psbt; +//! use crate::Network; //! //! let original_bytes = original_psbt.serialize().unwrap(); //! let reconstructed_bytes = reconstructed_psbt.serialize().unwrap(); //! -//! assert_equal_psbt(&original_bytes, &reconstructed_bytes); +//! assert_equal_psbt(&original_bytes, &reconstructed_bytes, Network::Bitcoin); //! ``` +use crate::Network; use miniscript::bitcoin::consensus::Decodable; use miniscript::bitcoin::{Transaction, VarInt}; use std::collections::HashMap; @@ -130,12 +132,25 @@ fn decode_pair(bytes: &[u8], pos: usize) -> Result<(RawPair, usize), String> { } /// Extract transaction input/output counts from global map -fn extract_tx_counts(global_pairs: &[RawPair]) -> Result<(usize, usize), String> { +/// +/// Uses the network parameter to determine whether to decode as Bitcoin or Zcash. +fn extract_tx_counts(global_pairs: &[RawPair], network: Network) -> Result<(usize, usize), String> { for pair in global_pairs { if pair.type_value == 0x00 { - let tx = Transaction::consensus_decode(&mut &pair.value[..]) - .map_err(|e| format!("Failed to decode unsigned transaction: {}", e))?; - return Ok((tx.input.len(), tx.output.len())); + if matches!(network, Network::Zcash | Network::ZcashTestnet) { + // Zcash transaction decoding + let parts = crate::zcash::transaction::decode_zcash_transaction_parts(&pair.value) + .map_err(|e| format!("Failed to decode Zcash transaction: {}", e))?; + return Ok(( + parts.transaction.input.len(), + parts.transaction.output.len(), + )); + } else { + // Standard Bitcoin transaction decoding + let tx = Transaction::consensus_decode(&mut &pair.value[..]) + .map_err(|e| format!("Failed to decode transaction: {}", e))?; + return Ok((tx.input.len(), tx.output.len())); + } } } Err("No unsigned transaction found in global map".to_string()) @@ -175,7 +190,11 @@ fn decode_map_pairs(bytes: &[u8], start_pos: usize) -> Result<(Vec, usi } /// Parse PSBT bytes into a structured ParsedPsbt -pub fn parse_psbt_to_maps(bytes: &[u8]) -> Result { +/// +/// # Arguments +/// * `bytes` - The PSBT bytes to parse +/// * `network` - The network to use for transaction decoding (Zcash uses a different format) +pub fn parse_psbt_to_maps(bytes: &[u8], network: Network) -> Result { // Check magic bytes if bytes.len() < 5 { return Err("PSBT too short to contain magic bytes".to_string()); @@ -193,7 +212,7 @@ pub fn parse_psbt_to_maps(bytes: &[u8]) -> Result { pos = new_pos; // Extract input/output counts - let (input_count, output_count) = extract_tx_counts(&global_pairs)?; + let (input_count, output_count) = extract_tx_counts(&global_pairs, network)?; let global = ParsedMap { pairs: global_pairs, @@ -407,9 +426,18 @@ pub fn compare_psbts(left: &ParsedPsbt, right: &ParsedPsbt) -> Vec { } /// Compare two PSBTs and return Ok(()) if equal, or Err with detailed differences -pub fn compare_psbt_bytes(left_bytes: &[u8], right_bytes: &[u8]) -> Result<(), String> { - let left = parse_psbt_to_maps(left_bytes)?; - let right = parse_psbt_to_maps(right_bytes)?; +/// +/// # Arguments +/// * `left_bytes` - First PSBT bytes +/// * `right_bytes` - Second PSBT bytes +/// * `network` - Network for transaction decoding (Zcash uses a different format) +pub fn compare_psbt_bytes( + left_bytes: &[u8], + right_bytes: &[u8], + network: Network, +) -> Result<(), String> { + let left = parse_psbt_to_maps(left_bytes, network)?; + let right = parse_psbt_to_maps(right_bytes, network)?; let diffs = compare_psbts(&left, &right); @@ -429,12 +457,17 @@ pub fn compare_psbt_bytes(left_bytes: &[u8], right_bytes: &[u8]) -> Result<(), S /// This provides much more detailed error messages than simple byte comparison, /// showing exactly which fields differ between the two PSBTs. /// +/// # Arguments +/// * `left_bytes` - First PSBT bytes +/// * `right_bytes` - Second PSBT bytes +/// * `network` - Network for transaction decoding (Zcash uses a different format) +/// /// # Panics /// /// Panics if the PSBTs differ, with a detailed message showing which fields /// are different. -pub fn assert_equal_psbt(left_bytes: &[u8], right_bytes: &[u8]) { - if let Err(e) = compare_psbt_bytes(left_bytes, right_bytes) { +pub fn assert_equal_psbt(left_bytes: &[u8], right_bytes: &[u8], network: Network) { + if let Err(e) = compare_psbt_bytes(left_bytes, right_bytes, network) { panic!("{}", e); } } @@ -455,7 +488,8 @@ mod tests { .expect("Failed to load fixture"); let psbt_bytes = fixture.to_psbt_bytes().expect("Failed to serialize PSBT"); - let parsed = parse_psbt_to_maps(&psbt_bytes).expect("Failed to parse PSBT"); + let parsed = + parse_psbt_to_maps(&psbt_bytes, Network::Bitcoin).expect("Failed to parse PSBT"); // Should have global map with at least unsigned tx assert!(parsed.global.has_type(0x00), "Should have unsigned tx"); @@ -479,7 +513,7 @@ mod tests { let psbt_bytes = fixture.to_psbt_bytes().expect("Failed to serialize PSBT"); // Comparing identical PSBTs should succeed - assert_equal_psbt(&psbt_bytes, &psbt_bytes); + assert_equal_psbt(&psbt_bytes, &psbt_bytes, Network::Bitcoin); } #[test] @@ -504,7 +538,7 @@ mod tests { let fullsigned_bytes = fullsigned.to_psbt_bytes().expect("Failed to serialize"); // Different PSBTs should produce an error - let result = compare_psbt_bytes(&unsigned_bytes, &fullsigned_bytes); + let result = compare_psbt_bytes(&unsigned_bytes, &fullsigned_bytes, Network::Bitcoin); assert!(result.is_err(), "Different PSBTs should produce error"); let err = result.unwrap_err(); diff --git a/packages/wasm-utxo/src/wasm/fixed_script_wallet.rs b/packages/wasm-utxo/src/wasm/fixed_script_wallet.rs index 8d7021c..223cea1 100644 --- a/packages/wasm-utxo/src/wasm/fixed_script_wallet.rs +++ b/packages/wasm-utxo/src/wasm/fixed_script_wallet.rs @@ -939,39 +939,16 @@ impl BitGoPsbt { /// Extract the final transaction from a finalized PSBT /// /// This method should be called after all inputs have been finalized. - /// It extracts the fully signed transaction. + /// It extracts the fully signed transaction with network-appropriate serialization. /// /// # Returns /// - `Ok(Vec)` containing the serialized transaction bytes /// - `Err(WasmUtxoError)` if the PSBT is not fully finalized or extraction fails pub fn extract_transaction(&self) -> Result, WasmUtxoError> { - use crate::fixed_script_wallet::bitgo_psbt::BitGoPsbt as InnerBitGoPsbt; - - match &self.psbt { - InnerBitGoPsbt::Zcash(zcash_psbt, _) => { - // Use Zcash-specific serialization with version_group_id, expiry_height, etc. - zcash_psbt.extract_zcash_transaction().map_err(|e| { - WasmUtxoError::new(&format!("Failed to extract Zcash transaction: {}", e)) - }) - } - InnerBitGoPsbt::BitcoinLike(psbt, _) => { - let tx = psbt.clone().extract_tx().map_err(|e| { - WasmUtxoError::new(&format!("Failed to extract transaction: {}", e)) - })?; - - // Serialize the transaction - use miniscript::bitcoin::consensus::encode::serialize; - Ok(serialize(&tx)) - } - InnerBitGoPsbt::Dash(dash_psbt, _) => { - let tx = dash_psbt.psbt.clone().extract_tx().map_err(|e| { - WasmUtxoError::new(&format!("Failed to extract transaction: {}", e)) - })?; - - // Serialize the transaction - use miniscript::bitcoin::consensus::encode::serialize; - Ok(serialize(&tx)) - } - } + // Clone and use extract_tx() which handles all network-specific serialization + self.psbt + .clone() + .extract_tx() + .map_err(|e| WasmUtxoError::new(&e)) } }