Skip to content

Commit

Permalink
Merge #645: Allow signing only specific leaf hashes
Browse files Browse the repository at this point in the history 
a713a5a Better customize signing in taproot transactions (Daniela Brozzoni)

Pull request description:

  Fixes #616

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### New Features:

  * [x] I've added tests for the new feature
  * [x] I've added docs for the new feature
  * [x] I've updated `CHANGELOG.md`

ACKs for top commit:
  afilini:
    ACK a713a5a

Tree-SHA512: 1100d43cb394b429450fc34f49dd815a024701987c0e6dd163865bd5c4c6f7102127b1ea6e10ced5fdb319874be97baeeb0deea66b4138410871a1d68b4def10
  • Loading branch information
@afilini
afilini committed Aug 2, 2022
2 parents 8e0d00a + a713a5a commit 5a41597
Show file tree
Hide file tree
Showing 3 changed files with 228 additions and 8 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Add `Excess` enum to handle remaining amount after coin selection.
- Move change creation from `Wallet::create_tx` to `CoinSelectionAlgorithm::coin_select`.
- Change the interface of `SqliteDatabase::new` to accept any type that implement AsRef<Path>
- Add the ability to specify which leaves to sign in a taproot transaction through `TapLeavesOptions` in `SignOptions`
- Add the ability to specify whether a taproot transaction should be signed using the internal key or not, using `sign_with_tap_internal_key` in `SignOptions`

## [v0.20.0] - [v0.19.0]

Expand Down
169 changes: 167 additions & 2 deletions src/wallet/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1086,7 +1086,7 @@ where
.iter()
.chain(self.change_signers.signers().iter())
{
signer.sign_transaction(psbt, &self.secp)?;
signer.sign_transaction(psbt, &sign_options, &self.secp)?;
}

// attempt to finalize
Expand Down Expand Up @@ -1975,6 +1975,10 @@ pub(crate) mod test {
"tr(b511bd5771e47ee27558b1765e87b541668304ec567721c7b880edc0a010da55,{pk(cPZzKuNmpuUjD1e8jUU4PVzy2b5LngbSip8mBsxf4e7rSFZVb4Uh),pk(8aee2b8120a5f157f1223f72b5e62b825831a27a9fdf427db7cc697494d4a642)})"
}

pub(crate) fn get_test_tr_with_taptree_both_priv() -> &'static str {
"tr(b511bd5771e47ee27558b1765e87b541668304ec567721c7b880edc0a010da55,{pk(cPZzKuNmpuUjD1e8jUU4PVzy2b5LngbSip8mBsxf4e7rSFZVb4Uh),pk(cNaQCDwmmh4dS9LzCgVtyy1e1xjCJ21GUDHe9K98nzb689JvinGV)})"
}

pub(crate) fn get_test_tr_repeated_key() -> &'static str {
"tr(b511bd5771e47ee27558b1765e87b541668304ec567721c7b880edc0a010da55,{and_v(v:pk(cVpPVruEDdmutPzisEsYvtST1usBR3ntr8pXSyt6D2YYqXRyPcFW),after(100)),and_v(v:pk(cVpPVruEDdmutPzisEsYvtST1usBR3ntr8pXSyt6D2YYqXRyPcFW),after(200))})"
}
Expand All @@ -1984,7 +1988,7 @@ pub(crate) mod test {
}

pub(crate) fn get_test_tr_with_taptree_xprv() -> &'static str {
"tr(b511bd5771e47ee27558b1765e87b541668304ec567721c7b880edc0a010da55,{pk(tprv8ZgxMBicQKsPdDArR4xSAECuVxeX1jwwSXR4ApKbkYgZiziDc4LdBy2WvJeGDfUSE4UT4hHhbgEwbdq8ajjUHiKDegkwrNU6V55CxcxonVN/*),pk(8aee2b8120a5f157f1223f72b5e62b825831a27a9fdf427db7cc697494d4a642)})"
"tr(cNJmN3fH9DDbDt131fQNkVakkpzawJBSeybCUNmP1BovpmGQ45xG,{pk(tprv8ZgxMBicQKsPdDArR4xSAECuVxeX1jwwSXR4ApKbkYgZiziDc4LdBy2WvJeGDfUSE4UT4hHhbgEwbdq8ajjUHiKDegkwrNU6V55CxcxonVN/*),pk(8aee2b8120a5f157f1223f72b5e62b825831a27a9fdf427db7cc697494d4a642)})"
}

macro_rules! assert_fee_rate {
Expand Down Expand Up @@ -4830,6 +4834,31 @@ pub(crate) mod test {
test_spend_from_wallet(wallet);
}

#[test]
fn test_taproot_no_key_spend() {
let (wallet, _, _) = get_funded_wallet(get_test_tr_with_taptree_both_priv());
let addr = wallet.get_address(AddressIndex::New).unwrap();

let mut builder = wallet.build_tx();
builder.add_recipient(addr.script_pubkey(), 25_000);
let (mut psbt, _) = builder.finish().unwrap();

assert!(
wallet
.sign(
&mut psbt,
SignOptions {
sign_with_tap_internal_key: false,
..Default::default()
},
)
.unwrap(),
"Unable to finalize tx"
);

assert!(psbt.inputs.iter().all(|i| i.tap_key_sig.is_none()));
}

#[test]
fn test_taproot_script_spend() {
let (wallet, _, _) = get_funded_wallet(get_test_tr_with_taptree());
Expand All @@ -4839,6 +4868,142 @@ pub(crate) mod test {
test_spend_from_wallet(wallet);
}

#[test]
fn test_taproot_script_spend_sign_all_leaves() {
use crate::signer::TapLeavesOptions;
let (wallet, _, _) = get_funded_wallet(get_test_tr_with_taptree_both_priv());
let addr = wallet.get_address(AddressIndex::New).unwrap();

let mut builder = wallet.build_tx();
builder.add_recipient(addr.script_pubkey(), 25_000);
let (mut psbt, _) = builder.finish().unwrap();

assert!(
wallet
.sign(
&mut psbt,
SignOptions {
tap_leaves_options: TapLeavesOptions::All,
..Default::default()
},
)
.unwrap(),
"Unable to finalize tx"
);

assert!(psbt
.inputs
.iter()
.all(|i| i.tap_script_sigs.len() == i.tap_scripts.len()));
}

#[test]
fn test_taproot_script_spend_sign_include_some_leaves() {
use crate::signer::TapLeavesOptions;
use crate::wallet::taproot::TapLeafHash;

let (wallet, _, _) = get_funded_wallet(get_test_tr_with_taptree_both_priv());
let addr = wallet.get_address(AddressIndex::New).unwrap();

let mut builder = wallet.build_tx();
builder.add_recipient(addr.script_pubkey(), 25_000);
let (mut psbt, _) = builder.finish().unwrap();
let mut script_leaves: Vec<_> = psbt.inputs[0]
.tap_scripts
.clone()
.values()
.map(|(script, version)| TapLeafHash::from_script(script, *version))
.collect();
let included_script_leaves = vec![script_leaves.pop().unwrap()];
let excluded_script_leaves = script_leaves;

assert!(
wallet
.sign(
&mut psbt,
SignOptions {
tap_leaves_options: TapLeavesOptions::Include(
included_script_leaves.clone()
),
..Default::default()
},
)
.unwrap(),
"Unable to finalize tx"
);

assert!(psbt.inputs[0]
.tap_script_sigs
.iter()
.all(|s| included_script_leaves.contains(&s.0 .1)
&& !excluded_script_leaves.contains(&s.0 .1)));
}

#[test]
fn test_taproot_script_spend_sign_exclude_some_leaves() {
use crate::signer::TapLeavesOptions;
use crate::wallet::taproot::TapLeafHash;

let (wallet, _, _) = get_funded_wallet(get_test_tr_with_taptree_both_priv());
let addr = wallet.get_address(AddressIndex::New).unwrap();

let mut builder = wallet.build_tx();
builder.add_recipient(addr.script_pubkey(), 25_000);
let (mut psbt, _) = builder.finish().unwrap();
let mut script_leaves: Vec<_> = psbt.inputs[0]
.tap_scripts
.clone()
.values()
.map(|(script, version)| TapLeafHash::from_script(script, *version))
.collect();
let included_script_leaves = vec![script_leaves.pop().unwrap()];
let excluded_script_leaves = script_leaves;

assert!(
wallet
.sign(
&mut psbt,
SignOptions {
tap_leaves_options: TapLeavesOptions::Exclude(
excluded_script_leaves.clone()
),
..Default::default()
},
)
.unwrap(),
"Unable to finalize tx"
);

assert!(psbt.inputs[0]
.tap_script_sigs
.iter()
.all(|s| included_script_leaves.contains(&s.0 .1)
&& !excluded_script_leaves.contains(&s.0 .1)));
}

#[test]
fn test_taproot_script_spend_sign_no_leaves() {
use crate::signer::TapLeavesOptions;
let (wallet, _, _) = get_funded_wallet(get_test_tr_with_taptree_both_priv());
let addr = wallet.get_address(AddressIndex::New).unwrap();

let mut builder = wallet.build_tx();
builder.add_recipient(addr.script_pubkey(), 25_000);
let (mut psbt, _) = builder.finish().unwrap();

wallet
.sign(
&mut psbt,
SignOptions {
tap_leaves_options: TapLeavesOptions::None,
..Default::default()
},
)
.unwrap();

assert!(psbt.inputs.iter().all(|i| i.tap_script_sigs.is_empty()));
}

#[test]
fn test_taproot_sign_derive_index_from_psbt() {
let (wallet, _, _) = get_funded_wallet(get_test_tr_single_sig_xprv());
Expand Down
65 changes: 59 additions & 6 deletions src/wallet/signer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
//! &self,
//! psbt: &mut psbt::PartiallySignedTransaction,
//! input_index: usize,
//! _sign_options: &SignOptions,
//! _secp: &Secp256k1<All>,
//! ) -> Result<(), SignerError> {
//! self.device.hsm_sign_input(psbt, input_index)?;
Expand Down Expand Up @@ -241,6 +242,7 @@ pub trait InputSigner: SignerCommon {
&self,
psbt: &mut psbt::PartiallySignedTransaction,
input_index: usize,
sign_options: &SignOptions,
secp: &SecpCtx,
) -> Result<(), SignerError>;
}
Expand All @@ -254,6 +256,7 @@ pub trait TransactionSigner: SignerCommon {
fn sign_transaction(
&self,
psbt: &mut psbt::PartiallySignedTransaction,
sign_options: &SignOptions,
secp: &SecpCtx,
) -> Result<(), SignerError>;
}
Expand All @@ -262,10 +265,11 @@ impl<T: InputSigner> TransactionSigner for T {
fn sign_transaction(
&self,
psbt: &mut psbt::PartiallySignedTransaction,
sign_options: &SignOptions,
secp: &SecpCtx,
) -> Result<(), SignerError> {
for input_index in 0..psbt.inputs.len() {
self.sign_input(psbt, input_index, secp)?;
self.sign_input(psbt, input_index, sign_options, secp)?;
}

Ok(())
Expand All @@ -287,6 +291,7 @@ impl InputSigner for SignerWrapper<DescriptorXKey<ExtendedPrivKey>> {
&self,
psbt: &mut psbt::PartiallySignedTransaction,
input_index: usize,
sign_options: &SignOptions,
secp: &SecpCtx,
) -> Result<(), SignerError> {
if input_index >= psbt.inputs.len() {
Expand Down Expand Up @@ -346,7 +351,7 @@ impl InputSigner for SignerWrapper<DescriptorXKey<ExtendedPrivKey>> {
inner: derived_key.private_key,
};

SignerWrapper::new(priv_key, self.ctx).sign_input(psbt, input_index, secp)
SignerWrapper::new(priv_key, self.ctx).sign_input(psbt, input_index, sign_options, secp)
}
}
}
Expand All @@ -369,6 +374,7 @@ impl InputSigner for SignerWrapper<PrivateKey> {
&self,
psbt: &mut psbt::PartiallySignedTransaction,
input_index: usize,
sign_options: &SignOptions,
secp: &SecpCtx,
) -> Result<(), SignerError> {
if input_index >= psbt.inputs.len() || input_index >= psbt.unsigned_tx.input.len() {
Expand All @@ -385,7 +391,10 @@ 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() {
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,
Expand All @@ -404,9 +413,18 @@ impl InputSigner for SignerWrapper<PrivateKey> {
let leaf_hashes = leaf_hashes
.iter()
.filter(|lh| {
!psbt.inputs[input_index]
.tap_script_sigs
.contains_key(&(x_only_pubkey, **lh))
// Removing the leaves we shouldn't sign for
let should_sign = match &sign_options.tap_leaves_options {
TapLeavesOptions::All => true,
TapLeavesOptions::Include(v) => v.contains(lh),
TapLeavesOptions::Exclude(v) => !v.contains(lh),
TapLeavesOptions::None => false,
};
// Filtering out the leaves without our key
should_sign
&& !psbt.inputs[input_index]
.tap_script_sigs
.contains_key(&(x_only_pubkey, **lh))
})
.cloned()
.collect::<Vec<_>>();
Expand Down Expand Up @@ -677,6 +695,38 @@ pub struct SignOptions {
///
/// Defaults to `true` which will try fianlizing psbt after inputs are signed.
pub try_finalize: bool,

/// Specifies which Taproot script-spend leaves we should sign for. This option is
/// ignored if we're signing a non-taproot PSBT.
///
/// Defaults to All, i.e., the wallet will sign all the leaves it has a key for.
pub tap_leaves_options: TapLeavesOptions,

/// Whether we should try to sign a taproot transaction with the taproot internal key
/// or not. This option is ignored if we're signing a non-taproot PSBT.
///
/// Defaults to `true`, i.e., we always try to sign with the taproot internal key.
pub sign_with_tap_internal_key: bool,
}

/// Customize which taproot script-path leaves the signer should sign.
#[derive(Debug, Clone, PartialEq)]
pub enum TapLeavesOptions {
/// The signer will sign all the leaves it has a key for.
All,
/// The signer won't sign leaves other than the ones specified. Note that it could still ignore
/// some of the specified leaves, if it doesn't have the right key to sign them.
Include(Vec<taproot::TapLeafHash>),
/// The signer won't sign the specified leaves.
Exclude(Vec<taproot::TapLeafHash>),
/// The signer won't sign any leaf.
None,
}

impl Default for TapLeavesOptions {
fn default() -> Self {
TapLeavesOptions::All
}
}

#[allow(clippy::derivable_impls)]
Expand All @@ -688,6 +738,8 @@ impl Default for SignOptions {
allow_all_sighashes: false,
remove_partial_sigs: true,
try_finalize: true,
tap_leaves_options: TapLeavesOptions::default(),
sign_with_tap_internal_key: true,
}
}
}
Expand Down Expand Up @@ -1018,6 +1070,7 @@ mod signers_container_tests {
fn sign_transaction(
&self,
_psbt: &mut psbt::PartiallySignedTransaction,
_sign_options: &SignOptions,
_secp: &SecpCtx,
) -> Result<(), SignerError> {
Ok(())
Expand Down

0 comments on commit 5a41597

Please sign in to comment.