From 906598ad9254643a204df7711693dcc0a73a0332 Mon Sep 17 00:00:00 2001 From: Alekos Filini Date: Tue, 26 Apr 2022 16:54:10 +0200 Subject: [PATCH] Refactor signer traits, add support for taproot signatures --- Cargo.toml | 2 +- src/wallet/mod.rs | 16 +- src/wallet/signer.rs | 420 ++++++++++++++++++++++++++++++------------- 3 files changed, 306 insertions(+), 132 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 14d21c3e..e7e1fd70 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,7 +15,7 @@ license = "MIT OR Apache-2.0" bdk-macros = "^0.6" log = "^0.4" miniscript = { version = "7.0", features = ["use-serde"] } -bitcoin = { version = "0.28", features = ["use-serde", "base64"] } +bitcoin = { version = "0.28", features = ["use-serde", "base64", "rand"] } serde = { version = "^1.0", features = ["derive"] } serde_json = { version = "^1.0" } rand = "^0.7" diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 55ef3ba4..b04921bd 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -50,7 +50,7 @@ pub use utils::IsDust; use address_validator::AddressValidator; use coin_selection::DefaultCoinSelectionAlgorithm; -use signer::{SignOptions, Signer, SignerOrdering, SignersContainer}; +use signer::{SignOptions, SignerOrdering, SignersContainer, TransactionSigner}; use tx_builder::{BumpFee, CreateTx, FeePolicy, TxBuilder, TxParams}; use utils::{check_nlocktime, check_nsequence_rbf, After, Older, SecpCtx}; @@ -79,10 +79,10 @@ const CACHE_ADDR_BATCH_SIZE: u32 = 100; /// /// 1. output *descriptors* from which it can derive addresses. /// 2. A [`Database`] where it tracks transactions and utxos related to the descriptors. -/// 3. [`Signer`]s that can contribute signatures to addresses instantiated from the descriptors. +/// 3. [`signer`]s that can contribute signatures to addresses instantiated from the descriptors. /// /// [`Database`]: crate::database::Database -/// [`Signer`]: crate::signer::Signer +/// [`signer`]: crate::signer #[derive(Debug)] pub struct Wallet { descriptor: ExtendedDescriptor, @@ -457,7 +457,7 @@ where &mut self, keychain: KeychainKind, ordering: SignerOrdering, - signer: Arc, + signer: Arc, ) { let signers = match keychain { KeychainKind::External => Arc::make_mut(&mut self.signers), @@ -1036,13 +1036,7 @@ where .iter() .chain(self.change_signers.signers().iter()) { - if signer.sign_whole_tx() { - signer.sign(psbt, None, &self.secp)?; - } else { - for index in 0..psbt.inputs.len() { - signer.sign(psbt, Some(index), &self.secp)?; - } - } + signer.sign_transaction(psbt, &self.secp)?; } // attempt to finalize diff --git a/src/wallet/signer.rs b/src/wallet/signer.rs index ebb9217b..b6371035 100644 --- a/src/wallet/signer.rs +++ b/src/wallet/signer.rs @@ -26,7 +26,7 @@ //! # #[derive(Debug)] //! # struct CustomHSM; //! # impl CustomHSM { -//! # fn sign_input(&self, _psbt: &mut psbt::PartiallySignedTransaction, _input: usize) -> Result<(), SignerError> { +//! # fn hsm_sign_input(&self, _psbt: &mut psbt::PartiallySignedTransaction, _input: usize) -> Result<(), SignerError> { //! # Ok(()) //! # } //! # fn connect() -> Self { @@ -47,25 +47,22 @@ //! } //! } //! -//! impl Signer for CustomSigner { -//! fn sign( -//! &self, -//! psbt: &mut psbt::PartiallySignedTransaction, -//! input_index: Option, -//! _secp: &Secp256k1, -//! ) -> Result<(), SignerError> { -//! let input_index = input_index.ok_or(SignerError::InputIndexOutOfRange)?; -//! self.device.sign_input(psbt, input_index)?; -//! -//! Ok(()) -//! } -//! +//! impl SignerCommon for CustomSigner { //! fn id(&self, _secp: &Secp256k1) -> SignerId { //! self.device.get_id() //! } +//! } //! -//! fn sign_whole_tx(&self) -> bool { -//! false +//! impl InputSigner for CustomSigner { +//! fn sign_input( +//! &self, +//! psbt: &mut psbt::PartiallySignedTransaction, +//! input_index: usize, +//! _secp: &Secp256k1, +//! ) -> Result<(), SignerError> { +//! self.device.hsm_sign_input(psbt, input_index)?; +//! +//! Ok(()) //! } //! } //! @@ -91,14 +88,14 @@ use std::sync::Arc; use bitcoin::blockdata::opcodes; use bitcoin::blockdata::script::Builder as ScriptBuilder; use bitcoin::hashes::{hash160, Hash}; -use bitcoin::secp256k1; use bitcoin::secp256k1::{Message, Secp256k1}; use bitcoin::util::bip32::{ChildNumber, DerivationPath, ExtendedPrivKey, Fingerprint}; -use bitcoin::util::{ecdsa, psbt, sighash}; -use bitcoin::{EcdsaSighashType, PrivateKey, PublicKey, Script, Sighash}; +use bitcoin::util::{ecdsa, psbt, schnorr, sighash, taproot}; +use bitcoin::{secp256k1, XOnlyPublicKey}; +use bitcoin::{EcdsaSighashType, PrivateKey, PublicKey, SchnorrSighashType, Script}; use miniscript::descriptor::{DescriptorSecretKey, DescriptorSinglePriv, DescriptorXKey, KeyMap}; -use miniscript::{Legacy, MiniscriptKey, Segwitv0}; +use miniscript::{Legacy, MiniscriptKey, Segwitv0, Tap}; use super::utils::SecpCtx; use crate::descriptor::XKeyUtils; @@ -174,27 +171,8 @@ impl fmt::Display for SignerError { impl std::error::Error for SignerError {} -/// Trait for signers -/// -/// This trait can be implemented to provide customized signers to the wallet. For an example see -/// [`this module`](crate::wallet::signer)'s documentation. -pub trait Signer: fmt::Debug + Send + Sync { - /// Sign a PSBT - /// - /// The `input_index` argument is only provided if the wallet doesn't declare to sign the whole - /// transaction in one go (see [`Signer::sign_whole_tx`]). Otherwise its value is `None` and - /// can be ignored. - fn sign( - &self, - psbt: &mut psbt::PartiallySignedTransaction, - input_index: Option, - secp: &SecpCtx, - ) -> Result<(), SignerError>; - - /// Return whether or not the signer signs the whole transaction in one go instead of every - /// input individually - fn sign_whole_tx(&self) -> bool; - +/// Common signer methods +pub trait SignerCommon: fmt::Debug + Send + Sync { /// Return the [`SignerId`] for this signer /// /// The [`SignerId`] can be used to lookup a signer in the [`Wallet`](crate::Wallet)'s signers map or to @@ -211,14 +189,65 @@ pub trait Signer: fmt::Debug + Send + Sync { } } -impl Signer for DescriptorXKey { - fn sign( +/// PSBT Input signer +/// +/// This trait can be implemented to provide custom signers to the wallet. If the signer supports signing +/// individual inputs, this trait should be implemented and BDK will provide automatically an implementation +/// for [`TransactionSigner`]. +pub trait InputSigner: SignerCommon { + /// Sign a single psbt input + fn sign_input( + &self, + psbt: &mut psbt::PartiallySignedTransaction, + input_index: usize, + secp: &SecpCtx, + ) -> Result<(), SignerError>; +} + +/// PSBT signer +/// +/// This trait can be implemented when the signer can't sign inputs individually, but signs the whole transaction +/// at once. +pub trait TransactionSigner: SignerCommon { + /// Sign all the inputs of the psbt + fn sign_transaction( + &self, + psbt: &mut psbt::PartiallySignedTransaction, + secp: &SecpCtx, + ) -> Result<(), SignerError>; +} + +impl TransactionSigner for T { + fn sign_transaction( &self, psbt: &mut psbt::PartiallySignedTransaction, - input_index: Option, secp: &SecpCtx, ) -> Result<(), SignerError> { - let input_index = input_index.unwrap(); + for input_index in 0..psbt.inputs.len() { + self.sign_input(psbt, input_index, secp)?; + } + + Ok(()) + } +} + +impl SignerCommon for DescriptorXKey { + fn id(&self, secp: &SecpCtx) -> SignerId { + SignerId::from(self.root_fingerprint(secp)) + } + + fn descriptor_secret_key(&self) -> Option { + Some(DescriptorSecretKey::XPrv(self.clone())) + } +} + +impl InputSigner for DescriptorXKey { + fn sign_input( + &self, + psbt: &mut psbt::PartiallySignedTransaction, + input_index: usize, + secp: &SecpCtx, + ) -> Result<(), SignerError> { if input_index >= psbt.inputs.len() { return Err(SignerError::InputIndexOutOfRange); } @@ -265,31 +294,31 @@ impl Signer for DescriptorXKey { network: self.xkey.network, inner: derived_key.private_key, } - .sign(psbt, Some(input_index), secp) + .sign_input(psbt, input_index, secp) } } - - fn sign_whole_tx(&self) -> bool { - false - } - - fn id(&self, secp: &SecpCtx) -> SignerId { - SignerId::from(self.root_fingerprint(secp)) - } - - fn descriptor_secret_key(&self) -> Option { - Some(DescriptorSecretKey::XPrv(self.clone())) - } } -impl Signer for PrivateKey { - fn sign( +impl SignerCommon for PrivateKey { + fn id(&self, secp: &SecpCtx) -> SignerId { + SignerId::from(self.public_key(secp).to_pubkeyhash()) + } + + fn descriptor_secret_key(&self) -> Option { + Some(DescriptorSecretKey::SinglePriv(DescriptorSinglePriv { + key: *self, + origin: None, + })) + } +} + +impl InputSigner for PrivateKey { + fn sign_input( &self, psbt: &mut psbt::PartiallySignedTransaction, - input_index: Option, + input_index: usize, secp: &SecpCtx, ) -> Result<(), SignerError> { - let input_index = input_index.unwrap(); if input_index >= psbt.inputs.len() || input_index >= psbt.unsigned_tx.input.len() { return Err(SignerError::InputIndexOutOfRange); } @@ -301,48 +330,127 @@ impl Signer for PrivateKey { } let pubkey = PublicKey::from_private_key(secp, self); - if psbt.inputs[input_index].partial_sigs.contains_key(&pubkey) { - return Ok(()); + let x_only_pubkey = XOnlyPublicKey::from(pubkey.inner); + let is_taproot = psbt.inputs[input_index].tap_internal_key.is_some() + || psbt.inputs[input_index].tap_merkle_root.is_some(); + + match psbt.inputs[input_index].tap_internal_key { + Some(k) if k == x_only_pubkey && psbt.inputs[input_index].tap_key_sig.is_none() => { + 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, _)) = psbt.inputs[input_index].tap_key_origins.get(&x_only_pubkey) + { + let leaf_hashes = leaf_hashes + .iter() + .filter(|lh| { + !psbt.inputs[input_index] + .tap_script_sigs + .contains_key(&(x_only_pubkey, **lh)) + }) + .cloned() + .collect::>(); + for lh in leaf_hashes { + let (hash, hash_ty) = Tap::sighash(psbt, input_index, Some(lh))?; + sign_psbt_schnorr( + &self.inner, + x_only_pubkey, + Some(lh), + &mut psbt.inputs[input_index], + hash, + hash_ty, + secp, + ); + } } - // FIXME: use the presence of `witness_utxo` as an indication that we should make a bip143 - // sig. Does this make sense? Should we add an extra argument to explicitly switch between - // these? The original idea was to declare sign() as sign() and use Ctx, - // but that violates the rules for trait-objects, so we can't do it. - let (hash, sighash) = match psbt.inputs[input_index].witness_utxo { - Some(_) => Segwitv0::sighash(psbt, input_index)?, - None => Legacy::sighash(psbt, input_index)?, - }; + if !is_taproot { + if psbt.inputs[input_index].partial_sigs.contains_key(&pubkey) { + return Ok(()); + } - let sig = secp.sign_ecdsa( - &Message::from_slice(&hash.into_inner()[..]).unwrap(), - &self.inner, - ); - - let final_signature = ecdsa::EcdsaSig { - sig, - hash_ty: sighash.ecdsa_hash_ty().unwrap(), // FIXME - }; - psbt.inputs[input_index] - .partial_sigs - .insert(pubkey, final_signature); + // FIXME: use the presence of `witness_utxo` as an indication that we should make a bip143 + // sig. Does this make sense? Should we add an extra argument to explicitly switch between + // these? The original idea was to declare sign() as sign() and use Ctx, + // but that violates the rules for trait-objects, so we can't do it. + let (hash, hash_ty) = match psbt.inputs[input_index].witness_utxo { + Some(_) => Segwitv0::sighash(psbt, input_index, ())?, + None => Legacy::sighash(psbt, input_index, ())?, + }; + sign_psbt_ecdsa( + &self.inner, + pubkey, + &mut psbt.inputs[input_index], + hash, + hash_ty, + secp, + ); + } Ok(()) } +} - fn sign_whole_tx(&self) -> bool { - false - } +fn sign_psbt_ecdsa( + secret_key: &secp256k1::SecretKey, + pubkey: PublicKey, + psbt_input: &mut psbt::Input, + hash: bitcoin::Sighash, + hash_ty: EcdsaSighashType, + secp: &SecpCtx, +) { + let sig = secp.sign_ecdsa( + &Message::from_slice(&hash.into_inner()[..]).unwrap(), + secret_key, + ); - fn id(&self, secp: &SecpCtx) -> SignerId { - SignerId::from(self.public_key(secp).to_pubkeyhash()) - } + let final_signature = ecdsa::EcdsaSig { sig, hash_ty }; + psbt_input.partial_sigs.insert(pubkey, final_signature); +} - fn descriptor_secret_key(&self) -> Option { - Some(DescriptorSecretKey::SinglePriv(DescriptorSinglePriv { - key: *self, - origin: None, - })) +// Calling this with `leaf_hash` = `None` will sign for key-spend +fn sign_psbt_schnorr( + secret_key: &secp256k1::SecretKey, + pubkey: XOnlyPublicKey, + leaf_hash: Option, + psbt_input: &mut psbt::Input, + hash: taproot::TapSighashHash, + hash_ty: SchnorrSighashType, + secp: &SecpCtx, +) { + use schnorr::TapTweak; + + let keypair = secp256k1::KeyPair::from_seckey_slice(secp, secret_key.as_ref()).unwrap(); + let keypair = match leaf_hash { + None => keypair + .tap_tweak(secp, psbt_input.tap_merkle_root) + .into_inner(), + Some(_) => keypair, // no tweak for script spend + }; + + let sig = secp.sign_schnorr( + &Message::from_slice(&hash.into_inner()[..]).unwrap(), + &keypair, + ); + + let final_signature = schnorr::SchnorrSig { sig, hash_ty }; + + if let Some(lh) = leaf_hash { + psbt_input + .tap_script_sigs + .insert((pubkey, lh), final_signature); + } else { + psbt_input.tap_key_sig = Some(final_signature) } } @@ -377,7 +485,7 @@ impl From<(SignerId, SignerOrdering)> for SignersContainerKey { /// Container for multiple signers #[derive(Debug, Default, Clone)] -pub struct SignersContainer(BTreeMap>); +pub struct SignersContainer(BTreeMap>); impl SignersContainer { /// Create a map of public keys to secret keys @@ -426,13 +534,17 @@ impl SignersContainer { &mut self, id: SignerId, ordering: SignerOrdering, - signer: Arc, - ) -> Option> { + signer: Arc, + ) -> Option> { self.0.insert((id, ordering).into(), signer) } /// Removes a signer from the container and returns it - pub fn remove(&mut self, id: SignerId, ordering: SignerOrdering) -> Option> { + pub fn remove( + &mut self, + id: SignerId, + ordering: SignerOrdering, + ) -> Option> { self.0.remove(&(id, ordering).into()) } @@ -445,12 +557,12 @@ impl SignersContainer { } /// Returns the list of signers in the container, sorted by lowest to highest `ordering` - pub fn signers(&self) -> Vec<&Arc> { + pub fn signers(&self) -> Vec<&Arc> { self.0.values().collect() } /// Finds the signer with lowest ordering for a given id in the container. - pub fn find(&self, id: SignerId) -> Option<&Arc> { + pub fn find(&self, id: SignerId) -> Option<&Arc> { self.0 .range(( Included(&(id.clone(), SignerOrdering(0)).into()), @@ -508,17 +620,27 @@ impl Default for SignOptions { } pub(crate) trait ComputeSighash { + type Extra; + type Sighash; + type SighashType; + fn sighash( psbt: &psbt::PartiallySignedTransaction, input_index: usize, - ) -> Result<(Sighash, psbt::PsbtSighashType), SignerError>; + extra: Self::Extra, + ) -> Result<(Self::Sighash, Self::SighashType), SignerError>; } impl ComputeSighash for Legacy { + type Extra = (); + type Sighash = bitcoin::Sighash; + type SighashType = EcdsaSighashType; + fn sighash( psbt: &psbt::PartiallySignedTransaction, input_index: usize, - ) -> Result<(Sighash, psbt::PsbtSighashType), SignerError> { + _extra: (), + ) -> Result<(Self::Sighash, Self::SighashType), SignerError> { if input_index >= psbt.inputs.len() || input_index >= psbt.unsigned_tx.input.len() { return Err(SignerError::InputIndexOutOfRange); } @@ -528,7 +650,9 @@ impl ComputeSighash for Legacy { let sighash = psbt_input .sighash_type - .unwrap_or_else(|| EcdsaSighashType::All.into()); + .unwrap_or_else(|| EcdsaSighashType::All.into()) + .ecdsa_hash_ty() + .map_err(|_| SignerError::InvalidSighash)?; let script = match psbt_input.redeem_script { Some(ref redeem_script) => redeem_script.clone(), None => { @@ -567,10 +691,15 @@ fn p2wpkh_script_code(script: &Script) -> Script { } impl ComputeSighash for Segwitv0 { + type Extra = (); + type Sighash = bitcoin::Sighash; + type SighashType = EcdsaSighashType; + fn sighash( psbt: &psbt::PartiallySignedTransaction, input_index: usize, - ) -> Result<(Sighash, psbt::PsbtSighashType), SignerError> { + _extra: (), + ) -> Result<(Self::Sighash, Self::SighashType), SignerError> { if input_index >= psbt.inputs.len() || input_index >= psbt.unsigned_tx.input.len() { return Err(SignerError::InputIndexOutOfRange); } @@ -631,7 +760,62 @@ impl ComputeSighash for Segwitv0 { value, sighash, )?, - sighash.into(), + sighash, + )) + } +} + +impl ComputeSighash for Tap { + type Extra = Option; + type Sighash = taproot::TapSighashHash; + type SighashType = SchnorrSighashType; + + fn sighash( + psbt: &psbt::PartiallySignedTransaction, + input_index: usize, + extra: Self::Extra, + ) -> Result<(Self::Sighash, SchnorrSighashType), SignerError> { + if input_index >= psbt.inputs.len() || input_index >= psbt.unsigned_tx.input.len() { + return Err(SignerError::InputIndexOutOfRange); + } + + let psbt_input = &psbt.inputs[input_index]; + + let sighash_type = psbt_input + .sighash_type + .unwrap_or_else(|| SchnorrSighashType::Default.into()) + .schnorr_hash_ty() + .map_err(|_| SignerError::InvalidSighash)?; + let witness_utxos = psbt + .inputs + .iter() + .cloned() + .map(|i| i.witness_utxo) + .collect::>(); + let mut all_witness_utxos = vec![]; + + let mut cache = sighash::SighashCache::new(&psbt.unsigned_tx); + let is_anyone_can_pay = psbt::PsbtSighashType::from(sighash_type).to_u32() & 0x80 != 0; + let prevouts = if is_anyone_can_pay { + sighash::Prevouts::One( + input_index, + witness_utxos[input_index] + .as_ref() + .ok_or(SignerError::MissingWitnessUtxo)?, + ) + } else if witness_utxos.iter().all(Option::is_some) { + all_witness_utxos.extend(witness_utxos.iter().filter_map(|x| x.as_ref())); + sighash::Prevouts::All(&all_witness_utxos) + } else { + return Err(SignerError::MissingWitnessUtxo); + }; + + // Assume no OP_CODESEPARATOR + let extra = extra.map(|leaf_hash| (leaf_hash, 0xFFFFFFFF)); + + Ok(( + cache.taproot_signature_hash(input_index, &prevouts, None, extra, sighash_type)?, + sighash_type, )) } } @@ -666,12 +850,11 @@ mod signers_container_tests { use crate::keys::{DescriptorKey, IntoDescriptorKey}; use bitcoin::secp256k1::{All, Secp256k1}; use bitcoin::util::bip32; - use bitcoin::util::psbt::PartiallySignedTransaction; use bitcoin::Network; use miniscript::ScriptContext; use std::str::FromStr; - fn is_equal(this: &Arc, that: &Arc) -> bool { + fn is_equal(this: &Arc, that: &Arc) -> bool { let secp = Secp256k1::new(); this.id(&secp) == that.id(&secp) } @@ -752,22 +935,19 @@ mod signers_container_tests { number: u64, } - impl Signer for DummySigner { - fn sign( - &self, - _psbt: &mut PartiallySignedTransaction, - _input_index: Option, - _secp: &SecpCtx, - ) -> Result<(), SignerError> { - Ok(()) - } - + impl SignerCommon for DummySigner { fn id(&self, _secp: &SecpCtx) -> SignerId { SignerId::Dummy(self.number) } + } - fn sign_whole_tx(&self) -> bool { - true + impl TransactionSigner for DummySigner { + fn sign_transaction( + &self, + _psbt: &mut psbt::PartiallySignedTransaction, + _secp: &SecpCtx, + ) -> Result<(), SignerError> { + Ok(()) } }