fix(wallet)!: Simplify SignOptions and improve finalization logic
Rather than comingle various `SignOptions` with the finalization step, we simply clear all fields when finalizing as per the PSBT spec in BIPs 174 and 371 which is more in line with user expectations.
This commit is contained in:
valued mammal
parent
0543801787
commit
996605f2bf
@ -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();
|
||||
}
|
||||
}
|
||||
|
||||
@ -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,
|
||||
|
||||
@ -2780,36 +2780,6 @@ fn test_signing_only_one_of_multiple_inputs() {
|
||||
)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_remove_partial_sigs_after_finalize_sign_option() {
|
||||
let (mut wallet, _) = get_funded_wallet("wpkh(tprv8ZgxMBicQKsPd3EupYiPRhaMooHKUHJxNsTfYuScep13go8QFfHdtkG9nRkFGb7busX4isf6X9dURGCoKgitaApQ6MupRhZMcELAxTBRJgS/*)");
|
||||
|
||||
for remove_partial_sigs 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
|
||||
.sign(
|
||||
&mut psbt,
|
||||
SignOptions {
|
||||
remove_partial_sigs: *remove_partial_sigs,
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.unwrap());
|
||||
|
||||
psbt.inputs.iter().for_each(|input| {
|
||||
if *remove_partial_sigs {
|
||||
assert!(input.partial_sigs.is_empty())
|
||||
} else {
|
||||
assert!(!input.partial_sigs.is_empty())
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_try_finalize_sign_option() {
|
||||
let (mut wallet, _) = get_funded_wallet("wpkh(tprv8ZgxMBicQKsPd3EupYiPRhaMooHKUHJxNsTfYuScep13go8QFfHdtkG9nRkFGb7busX4isf6X9dURGCoKgitaApQ6MupRhZMcELAxTBRJgS/*)");
|
||||
@ -2833,7 +2803,7 @@ 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());
|
||||
} else {
|
||||
assert!(!finalized);
|
||||
@ -2844,6 +2814,55 @@ fn test_try_finalize_sign_option() {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
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);
|
||||
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 {
|
||||
try_finalize: *try_finalize,
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
psbt.inputs.iter().for_each(|input| {
|
||||
if *try_finalize {
|
||||
assert!(finalized);
|
||||
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());
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_sign_nonstandard_sighash() {
|
||||
let sighash = EcdsaSighashType::NonePlusAnyoneCanPay;
|
||||
@ -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()
|
||||
},
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user