diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index 883e954a7..2588e7d50 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/crates/wallet/src/wallet/mod.rs @@ -40,6 +40,7 @@ use bitcoin::{ use bitcoin::{consensus::encode::serialize, transaction, BlockHash, Psbt}; use bitcoin::{constants::genesis_block, Amount}; use core::fmt; +use core::mem; use core::ops::Deref; use descriptor::error::Error as DescriptorError; use miniscript::psbt::{PsbtExt, PsbtInputExt, PsbtInputSatisfier}; @@ -1821,7 +1822,8 @@ impl Wallet { /// Finalize a PSBT, i.e., for each input determine if sufficient data is available to pass /// validation and construct the respective `scriptSig` or `scriptWitness`. Please refer to - /// [BIP174](https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki#Input_Finalizer) + /// [BIP174](https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki#Input_Finalizer), + /// and [BIP371](https://github.com/bitcoin/bips/blob/master/bip-0371.mediawiki) /// for further information. /// /// Returns `true` if the PSBT could be finalized, and `false` otherwise. @@ -1884,20 +1886,17 @@ impl Wallet { ), ) { Ok(_) => { + // Set the UTXO fields, final script_sig and witness + // and clear everything else. + let original = mem::take(&mut psbt.inputs[n]); let psbt_input = &mut psbt.inputs[n]; - psbt_input.final_script_sig = Some(tmp_input.script_sig); - psbt_input.final_script_witness = Some(tmp_input.witness); - if sign_options.remove_partial_sigs { - psbt_input.partial_sigs.clear(); + psbt_input.non_witness_utxo = original.non_witness_utxo; + psbt_input.witness_utxo = original.witness_utxo; + if !tmp_input.script_sig.is_empty() { + psbt_input.final_script_sig = Some(tmp_input.script_sig); } - if sign_options.remove_taproot_extras { - // We just constructed the final witness, clear these fields. - psbt_input.tap_key_sig = None; - psbt_input.tap_script_sigs.clear(); - psbt_input.tap_scripts.clear(); - psbt_input.tap_key_origins.clear(); - psbt_input.tap_internal_key = None; - psbt_input.tap_merkle_root = None; + if !tmp_input.witness.is_empty() { + psbt_input.final_script_witness = Some(tmp_input.witness); } } Err(_) => finished = false, @@ -1907,8 +1906,10 @@ impl Wallet { } } - if finished && sign_options.remove_taproot_extras { + // Clear derivation paths from outputs + if finished { for output in &mut psbt.outputs { + output.bip32_derivation.clear(); output.tap_key_origins.clear(); } } diff --git a/crates/wallet/src/wallet/signer.rs b/crates/wallet/src/wallet/signer.rs index 16194961a..0ac7fb984 100644 --- a/crates/wallet/src/wallet/signer.rs +++ b/crates/wallet/src/wallet/signer.rs @@ -801,21 +801,6 @@ pub struct SignOptions { /// Defaults to `false` which will only allow signing using `SIGHASH_ALL`. pub allow_all_sighashes: bool, - /// Whether to remove partial signatures from the PSBT inputs while finalizing PSBT. - /// - /// Defaults to `true` which will remove partial signatures during finalization. - pub remove_partial_sigs: bool, - - /// Whether to remove taproot specific fields from the PSBT on finalization. - /// - /// For inputs this includes the taproot internal key, merkle root, and individual - /// scripts and signatures. For both inputs and outputs it includes key origin info. - /// - /// Defaults to `true` which will remove all of the above mentioned fields when finalizing. - /// - /// See [`BIP371`](https://github.com/bitcoin/bips/blob/master/bip-0371.mediawiki) for details. - pub remove_taproot_extras: bool, - /// Whether to try finalizing the PSBT after the inputs are signed. /// /// Defaults to `true` which will try finalizing PSBT after inputs are signed. @@ -860,8 +845,6 @@ impl Default for SignOptions { trust_witness_utxo: false, assume_height: None, allow_all_sighashes: false, - remove_partial_sigs: true, - remove_taproot_extras: true, try_finalize: true, tap_leaves_options: TapLeavesOptions::default(), sign_with_tap_internal_key: true, diff --git a/crates/wallet/tests/wallet.rs b/crates/wallet/tests/wallet.rs index 7303bdcd8..c895fe27d 100644 --- a/crates/wallet/tests/wallet.rs +++ b/crates/wallet/tests/wallet.rs @@ -2781,38 +2781,42 @@ fn test_signing_only_one_of_multiple_inputs() { } #[test] -fn test_remove_partial_sigs_after_finalize_sign_option() { +fn test_try_finalize_sign_option() { let (mut wallet, _) = get_funded_wallet("wpkh(tprv8ZgxMBicQKsPd3EupYiPRhaMooHKUHJxNsTfYuScep13go8QFfHdtkG9nRkFGb7busX4isf6X9dURGCoKgitaApQ6MupRhZMcELAxTBRJgS/*)"); - for remove_partial_sigs in &[true, false] { + for try_finalize in &[true, false] { let addr = wallet.next_unused_address(KeychainKind::External); let mut builder = wallet.build_tx(); builder.drain_to(addr.script_pubkey()).drain_wallet(); let mut psbt = builder.finish().unwrap(); - assert!(wallet + let finalized = wallet .sign( &mut psbt, SignOptions { - remove_partial_sigs: *remove_partial_sigs, + try_finalize: *try_finalize, ..Default::default() }, ) - .unwrap()); + .unwrap(); psbt.inputs.iter().for_each(|input| { - if *remove_partial_sigs { - assert!(input.partial_sigs.is_empty()) + if *try_finalize { + assert!(finalized); + assert!(input.final_script_sig.is_none()); + assert!(input.final_script_witness.is_some()); } else { - assert!(!input.partial_sigs.is_empty()) + assert!(!finalized); + assert!(input.final_script_sig.is_none()); + assert!(input.final_script_witness.is_none()); } }); } } #[test] -fn test_try_finalize_sign_option() { - let (mut wallet, _) = get_funded_wallet("wpkh(tprv8ZgxMBicQKsPd3EupYiPRhaMooHKUHJxNsTfYuScep13go8QFfHdtkG9nRkFGb7busX4isf6X9dURGCoKgitaApQ6MupRhZMcELAxTBRJgS/*)"); +fn test_taproot_try_finalize_sign_option() { + let (mut wallet, _) = get_funded_wallet(get_test_tr_with_taptree()); for try_finalize in &[true, false] { let addr = wallet.next_unused_address(KeychainKind::External); @@ -2833,14 +2837,29 @@ fn test_try_finalize_sign_option() { psbt.inputs.iter().for_each(|input| { if *try_finalize { assert!(finalized); - assert!(input.final_script_sig.is_some()); + assert!(input.final_script_sig.is_none()); assert!(input.final_script_witness.is_some()); + assert!(input.tap_key_sig.is_none()); + assert!(input.tap_script_sigs.is_empty()); + assert!(input.tap_scripts.is_empty()); + assert!(input.tap_key_origins.is_empty()); + assert!(input.tap_internal_key.is_none()); + assert!(input.tap_merkle_root.is_none()); } else { assert!(!finalized); assert!(input.final_script_sig.is_none()); assert!(input.final_script_witness.is_none()); } }); + psbt.outputs.iter().for_each(|output| { + if *try_finalize { + assert!(finalized); + assert!(output.tap_key_origins.is_empty()); + } else { + assert!(!finalized); + assert!(!output.tap_key_origins.is_empty()); + } + }); } } @@ -3164,32 +3183,6 @@ fn test_get_address_no_reuse() { }); } -#[test] -fn test_taproot_remove_tapfields_after_finalize_sign_option() { - let (mut wallet, _) = get_funded_wallet(get_test_tr_with_taptree()); - - let addr = wallet.next_unused_address(KeychainKind::External); - let mut builder = wallet.build_tx(); - builder.drain_to(addr.script_pubkey()).drain_wallet(); - let mut psbt = builder.finish().unwrap(); - let finalized = wallet.sign(&mut psbt, SignOptions::default()).unwrap(); - assert!(finalized); - - // removes tap_* from inputs - for input in &psbt.inputs { - assert!(input.tap_key_sig.is_none()); - assert!(input.tap_script_sigs.is_empty()); - assert!(input.tap_scripts.is_empty()); - assert!(input.tap_key_origins.is_empty()); - assert!(input.tap_internal_key.is_none()); - assert!(input.tap_merkle_root.is_none()); - } - // removes key origins from outputs - for output in &psbt.outputs { - assert!(output.tap_key_origins.is_empty()); - } -} - #[test] fn test_taproot_psbt_populate_tap_key_origins() { let (desc, change_desc) = get_test_tr_single_sig_xprv_with_change_desc(); @@ -3922,7 +3915,6 @@ fn test_fee_rate_sign_no_grinding_high_r() { .sign( &mut psbt, SignOptions { - remove_partial_sigs: false, try_finalize: false, allow_grinding: false, ..Default::default() @@ -3941,7 +3933,6 @@ fn test_fee_rate_sign_no_grinding_high_r() { .sign( &mut psbt, SignOptions { - remove_partial_sigs: false, allow_grinding: false, ..Default::default() }, @@ -3972,7 +3963,7 @@ fn test_fee_rate_sign_grinding_low_r() { .sign( &mut psbt, SignOptions { - remove_partial_sigs: false, + try_finalize: false, allow_grinding: true, ..Default::default() },