Skip to content

Commit

Permalink
fix(bdk): Check if we're using the correct...
Browse files Browse the repository at this point in the history
...internal key before signing

Fixes #1142

We would previously sign with whatever x_only_pubkey we had in hand,
without first checking if it was the right key or not. This effectively
meant that adding multiple taproot PrivateKey signers would produce
unbroadcastable transactions.
  • Loading branch information
danielabrozzoni committed Nov 8, 2023
1 parent 2f2f138 commit c450499
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 14 deletions.
31 changes: 17 additions & 14 deletions crates/bdk/src/wallet/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -459,20 +459,23 @@ impl InputSigner for SignerWrapper<PrivateKey> {
let x_only_pubkey = XOnlyPublicKey::from(pubkey.inner);

if let SignerContext::Tap { is_internal_key } = self.ctx {
if is_internal_key
&& psbt.inputs[input_index].tap_key_sig.is_none()
&& sign_options.sign_with_tap_internal_key
{
let (hash, hash_ty) = Tap::sighash(psbt, input_index, None)?;
sign_psbt_schnorr(
&self.inner,
x_only_pubkey,
None,
&mut psbt.inputs[input_index],
hash,
hash_ty,
secp,
);
if let Some(psbt_internal_key) = psbt.inputs[input_index].tap_internal_key {
if is_internal_key
&& psbt.inputs[input_index].tap_key_sig.is_none()
&& sign_options.sign_with_tap_internal_key
&& x_only_pubkey == psbt_internal_key
{
let (hash, hash_ty) = Tap::sighash(psbt, input_index, None)?;
sign_psbt_schnorr(
&self.inner,
x_only_pubkey,
None,
&mut psbt.inputs[input_index],
hash,
hash_ty,
secp,
);
}
}

if let Some((leaf_hashes, _)) =
Expand Down
31 changes: 31 additions & 0 deletions crates/bdk/tests/psbt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,3 +156,34 @@ fn test_psbt_fee_rate_with_missing_txout() {
assert!(pkh_psbt.fee_amount().is_none());
assert!(pkh_psbt.fee_rate().is_none());
}

#[test]
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 std::sync::Arc;

let secp = Secp256k1::new();
let (mut wallet, _) = get_funded_wallet(get_test_tr_single_sig());
let send_to = wallet.get_address(AddressIndex::New);
let mut builder = wallet.build_tx();
builder.add_recipient(send_to.script_pubkey(), 10_000);
let mut psbt = builder.finish().unwrap();
// Adds a signer for the wrong internal key, bdk should not use this key to sign
wallet.add_signer(
KeychainKind::External,
// A signerordering lower than 100, bdk will use this signer first
SignerOrdering(0),
Arc::new(SignerWrapper::new(
PrivateKey::from_wif("5J5PZqvCe1uThJ3FZeUUFLCh2FuK9pZhtEK4MzhNmugqTmxCdwE").unwrap(),
SignerContext::Tap {
is_internal_key: true,
},
)),
);
let _ = wallet.sign(&mut psbt, SignOptions::default()).unwrap();
// Checks that we signed using the right key
psbt.finalize_mut(&secp).unwrap();
}

0 comments on commit c450499

Please sign in to comment.