Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove extra taproot fields when finalizing PSBT #1310

Merged
merged 2 commits into from
Mar 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions crates/bdk/src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1972,6 +1972,15 @@ impl<D> Wallet<D> {
if sign_options.remove_partial_sigs {
psbt_input.partial_sigs.clear();
}
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;
}
}
Err(_) => finished = false,
}
Expand All @@ -1980,6 +1989,12 @@ impl<D> Wallet<D> {
}
}

if finished && sign_options.remove_taproot_extras {
for output in &mut psbt.outputs {
output.tap_key_origins.clear();
}
}

Ok(finished)
}

Expand Down
11 changes: 11 additions & 0 deletions crates/bdk/src/wallet/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -782,6 +782,16 @@ 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 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.
Expand Down Expand Up @@ -827,6 +837,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
52 changes: 42 additions & 10 deletions crates/bdk/tests/psbt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,16 +161,26 @@ fn test_psbt_fee_rate_with_missing_txout() {
fn test_psbt_multiple_internalkey_signers() {
use bdk::signer::{SignerContext, SignerOrdering, SignerWrapper};
use bdk::KeychainKind;
use bitcoin::{secp256k1::Secp256k1, PrivateKey};
use miniscript::psbt::PsbtExt;
use bitcoin::key::TapTweak;
use bitcoin::secp256k1::{schnorr, KeyPair, Message, Secp256k1, XOnlyPublicKey};
use bitcoin::sighash::{Prevouts, SighashCache, TapSighashType};
use bitcoin::{PrivateKey, TxOut};
use std::sync::Arc;

let secp = Secp256k1::new();
let (mut wallet, _) = get_funded_wallet(get_test_tr_single_sig());
let wif = "cNJmN3fH9DDbDt131fQNkVakkpzawJBSeybCUNmP1BovpmGQ45xG";
let desc = format!("tr({})", wif);
let prv = PrivateKey::from_wif(wif).unwrap();
let keypair = KeyPair::from_secret_key(&secp, &prv.inner);

let (mut wallet, _) = get_funded_wallet(&desc);
let to_spend = wallet.get_balance().total();
let send_to = wallet.get_address(AddressIndex::New);
let mut builder = wallet.build_tx();
builder.add_recipient(send_to.script_pubkey(), 10_000);
builder.drain_to(send_to.script_pubkey()).drain_wallet();
let mut psbt = builder.finish().unwrap();
let unsigned_tx = psbt.unsigned_tx.clone();

// Adds a signer for the wrong internal key, bdk should not use this key to sign
wallet.add_signer(
KeychainKind::External,
Expand All @@ -183,10 +193,32 @@ fn test_psbt_multiple_internalkey_signers() {
},
)),
);
let _ = wallet.sign(&mut psbt, SignOptions::default()).unwrap();
// Checks that we signed using the right key
assert!(
psbt.finalize_mut(&secp).is_ok(),
"The wrong internal key was used"
);
let finalized = wallet.sign(&mut psbt, SignOptions::default()).unwrap();
assert!(finalized);

// To verify, we need the signature, message, and pubkey
let witness = psbt.inputs[0].final_script_witness.as_ref().unwrap();
assert!(!witness.is_empty());
let signature = schnorr::Signature::from_slice(witness.iter().next().unwrap()).unwrap();

// the prevout we're spending
let prevouts = &[TxOut {
script_pubkey: send_to.script_pubkey(),
value: to_spend,
}];
let prevouts = Prevouts::All(prevouts);
let input_index = 0;
let mut sighash_cache = SighashCache::new(unsigned_tx);
let sighash = sighash_cache
.taproot_key_spend_signature_hash(input_index, &prevouts, TapSighashType::Default)
.unwrap();
let message = Message::from(sighash);

// add tweak. this was taken from `signer::sign_psbt_schnorr`
let keypair = keypair.tap_tweak(&secp, None).to_inner();
let (xonlykey, _parity) = XOnlyPublicKey::from_keypair(&keypair);

// Must verify if we used the correct key to sign
let verify_res = secp.verify_schnorr(&signature, &message, &xonlykey);
assert!(verify_res.is_ok(), "The wrong internal key was used");
}
26 changes: 26 additions & 0 deletions crates/bdk/tests/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2813,6 +2813,32 @@ fn test_get_address_no_reuse_single_descriptor() {
});
}

#[test]
fn test_taproot_remove_tapfields_after_finalize_sign_option() {
let (mut wallet, _) = get_funded_wallet(get_test_tr_with_taptree());

let addr = wallet.get_address(New);
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 (mut wallet, _) = get_funded_wallet(get_test_tr_single_sig_xprv());
Expand Down