fix(bdk): Check if we're using the correct...

...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.
This commit is contained in:
Daniela Brozzoni 2023-11-08 18:16:21 +01:00
parent 2f2f138595
commit e553231eae
No known key found for this signature in database
GPG Key ID: 7DE4F1FDCED0AB87
2 changed files with 51 additions and 14 deletions

View File

@ -459,9 +459,11 @@ impl InputSigner for SignerWrapper<PrivateKey> {
let x_only_pubkey = XOnlyPublicKey::from(pubkey.inner); let x_only_pubkey = XOnlyPublicKey::from(pubkey.inner);
if let SignerContext::Tap { is_internal_key } = self.ctx { if let SignerContext::Tap { is_internal_key } = self.ctx {
if let Some(psbt_internal_key) = psbt.inputs[input_index].tap_internal_key {
if is_internal_key if is_internal_key
&& psbt.inputs[input_index].tap_key_sig.is_none() && psbt.inputs[input_index].tap_key_sig.is_none()
&& sign_options.sign_with_tap_internal_key && sign_options.sign_with_tap_internal_key
&& x_only_pubkey == psbt_internal_key
{ {
let (hash, hash_ty) = Tap::sighash(psbt, input_index, None)?; let (hash, hash_ty) = Tap::sighash(psbt, input_index, None)?;
sign_psbt_schnorr( sign_psbt_schnorr(
@ -474,6 +476,7 @@ impl InputSigner for SignerWrapper<PrivateKey> {
secp, secp,
); );
} }
}
if let Some((leaf_hashes, _)) = if let Some((leaf_hashes, _)) =
psbt.inputs[input_index].tap_key_origins.get(&x_only_pubkey) psbt.inputs[input_index].tap_key_origins.get(&x_only_pubkey)

View File

@ -156,3 +156,37 @@ fn test_psbt_fee_rate_with_missing_txout() {
assert!(pkh_psbt.fee_amount().is_none()); assert!(pkh_psbt.fee_amount().is_none());
assert!(pkh_psbt.fee_rate().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
assert!(
psbt.finalize_mut(&secp).is_ok(),
"The wrong internal key was used"
);
}