Skip to content

Commit

Permalink
fix(bdk): remove Psbt tap_* fields after final_script_witness constru…
Browse files Browse the repository at this point in the history
…cted

We currently allow removing `partial_sigs` from a finalized `Psbt`,
which is relevant to non-taproot inputs, however taproot related `Psbt`
fields were left in place despite the recommendation of BIP371 to remove
them once the `final_script_witness` is constructed. This can cause
confusion for parsers that encounter extra taproot metadata in an
already satisfied input.

Fix this by introducing a new memeber to SignOptions
`remove_taproot_extras`, which when true will remove extra taproot
related data from a `Psbt` upon successful finalization. This change
makes removal of all taproot extras the default but configurable.

fixes #1243
  • Loading branch information
ValuedMammal committed Jan 31, 2024
1 parent c6b9ed3 commit fa46980
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 1 deletion.
4 changes: 4 additions & 0 deletions crates/bdk/src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1972,6 +1972,10 @@ impl<D> Wallet<D> {
if sign_options.remove_partial_sigs {
psbt_input.partial_sigs.clear();
}
if sign_options.remove_taproot_extras {
psbt_input.tap_key_sig = None;
// TODO: remove extra tap fields
}
}
Err(_) => finished = false,
}
Expand Down
12 changes: 12 additions & 0 deletions crates/bdk/src/wallet/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -782,6 +782,17 @@ pub struct SignOptions {
/// 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 fields such as the taproot internal key, key origin, and merkle
/// root, as well as individual scripts and signatures. For both inputs and outputs it also
/// includes the BIP32 derivation.
///
/// 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.
Expand Down Expand Up @@ -827,6 +838,7 @@ impl Default for SignOptions {
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,
Expand Down
10 changes: 9 additions & 1 deletion crates/bdk/tests/psbt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,15 @@ fn test_psbt_multiple_internalkey_signers() {
},
)),
);
let _ = wallet.sign(&mut psbt, SignOptions::default()).unwrap();
assert!(wallet
.sign(
&mut psbt,
SignOptions {
remove_taproot_extras: false,
..Default::default()
}
)
.unwrap());
// Checks that we signed using the right key
assert!(
psbt.finalize_mut(&secp).is_ok(),
Expand Down

0 comments on commit fa46980

Please sign in to comment.