From f79f20d4e6d66a195e82baab3d3e304516bf5e8b Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 3 Apr 2024 12:35:00 +1100 Subject: [PATCH 1/3] Remove stale rustdoc We recently added support for signing taproot inputs but forgot to update the docs to reflect this. Remove stale rustdoc from `Psbt::sign` function. --- bitcoin/src/psbt/mod.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/bitcoin/src/psbt/mod.rs b/bitcoin/src/psbt/mod.rs index 05843d8d..274fa225 100644 --- a/bitcoin/src/psbt/mod.rs +++ b/bitcoin/src/psbt/mod.rs @@ -283,9 +283,6 @@ impl Psbt { /// Attempts to create _all_ the required signatures for this PSBT using `k`. /// - /// **NOTE**: Taproot inputs are, as yet, not supported by this function. We currently only - /// attempt to sign ECDSA inputs. - /// /// If you just want to sign an input with one specific key consider using `sighash_ecdsa`. This /// function does not support scripts that contain `OP_CODESEPARATOR`. /// From ffd5664c080613757b2069f7c182df3e7fb29f58 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 3 Apr 2024 12:33:30 +1100 Subject: [PATCH 2/3] Do not panic if input_index is out of bounds There is no need to panic if input index is out of bounds because we have a function to check the validity of the `input_index` argument and use it in other places already. --- bitcoin/src/psbt/mod.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/bitcoin/src/psbt/mod.rs b/bitcoin/src/psbt/mod.rs index 274fa225..b6028c32 100644 --- a/bitcoin/src/psbt/mod.rs +++ b/bitcoin/src/psbt/mod.rs @@ -402,8 +402,7 @@ impl Psbt { /// # Returns /// /// - Ok: A list of the public keys used in signing. - /// - Err: Error encountered trying to calculate the sighash AND we had the signing key. Also panics - /// if input_index is out of bounds. + /// - Err: Error encountered trying to calculate the sighash AND we had the signing key. fn bip32_sign_schnorr( &mut self, k: &K, @@ -416,7 +415,7 @@ impl Psbt { T: Borrow, K: GetKey, { - let mut input = self.inputs[input_index].clone(); + let mut input = self.checked_input(input_index)?.clone(); let mut used = vec![]; // List of pubkeys used to sign the input. @@ -487,7 +486,7 @@ impl Psbt { } } - self.inputs[input_index] = input; + self.inputs[input_index] = input; // input_index is checked above. Ok(used) } From 14040e2ff570cb24b63610c415601cbdbca775e6 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 3 Apr 2024 12:38:48 +1100 Subject: [PATCH 3/3] psbt: Return the internal key for key path spend When signing a Taproot input (in a PSBT) using a key path spend we currently return the pubkey associated with key that signs. However it is common to think of the internal key as being the one that signs even though this is not technically true. We also have the internal key in the PSBT so matching against it is less surprising. When using the `Psbt` type to sign a Taproot input using a key path spend return the internal key. --- bitcoin/src/psbt/mod.rs | 43 ++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/bitcoin/src/psbt/mod.rs b/bitcoin/src/psbt/mod.rs index b6028c32..3d428fda 100644 --- a/bitcoin/src/psbt/mod.rs +++ b/bitcoin/src/psbt/mod.rs @@ -25,7 +25,7 @@ use crate::bip32::{self, KeySource, Xpriv, Xpub}; use crate::blockdata::transaction::{self, Transaction, TxOut}; use crate::crypto::key::{PrivateKey, PublicKey}; use crate::crypto::{ecdsa, taproot}; -use crate::key::TapTweak; +use crate::key::{TapTweak, XOnlyPublicKey}; use crate::prelude::*; use crate::sighash::{self, EcdsaSighashType, Prevouts, SighashCache}; use crate::{Amount, FeeRate, TapLeafHash, TapSighashType}; @@ -283,14 +283,12 @@ impl Psbt { /// Attempts to create _all_ the required signatures for this PSBT using `k`. /// - /// If you just want to sign an input with one specific key consider using `sighash_ecdsa`. This - /// function does not support scripts that contain `OP_CODESEPARATOR`. + /// If you just want to sign an input with one specific key consider using `sighash_ecdsa` or + /// `sighash_taproot`. This function does not support scripts that contain `OP_CODESEPARATOR`. /// /// # Returns /// - /// Either Ok(SigningKeys) or Err((SigningKeys, SigningErrors)), where - /// - SigningKeys: A map of input index -> pubkey associated with secret key used to sign. - /// - SigningKeys: A map of input index -> the error encountered while attempting to sign. + /// A map of input index -> keys used to sign, for Taproot specifics please see [`SigningKeys`]. /// /// If an error is returned some signatures may already have been added to the PSBT. Since /// `partial_sigs` is a [`BTreeMap`] it is safe to retry, previous sigs will be overwritten. @@ -298,7 +296,7 @@ impl Psbt { &mut self, k: &K, secp: &Secp256k1, - ) -> Result + ) -> Result where C: Signing + Verification, K: GetKey, @@ -314,7 +312,7 @@ impl Psbt { Ok(SigningAlgorithm::Ecdsa) => match self.bip32_sign_ecdsa(k, i, &mut cache, secp) { Ok(v) => { - used.insert(i, v); + used.insert(i, SigningKeys::Ecdsa(v)); } Err(e) => { errors.insert(i, e); @@ -323,7 +321,7 @@ impl Psbt { Ok(SigningAlgorithm::Schnorr) => { match self.bip32_sign_schnorr(k, i, &mut cache, secp) { Ok(v) => { - used.insert(i, v); + used.insert(i, SigningKeys::Schnorr(v)); } Err(e) => { errors.insert(i, e); @@ -401,7 +399,8 @@ impl Psbt { /// /// # Returns /// - /// - Ok: A list of the public keys used in signing. + /// - Ok: A list of the xonly public keys used in signing. When signing a key path spend we + /// return the internal key. /// - Err: Error encountered trying to calculate the sighash AND we had the signing key. fn bip32_sign_schnorr( &mut self, @@ -409,7 +408,7 @@ impl Psbt { input_index: usize, cache: &mut SighashCache, secp: &Secp256k1, - ) -> Result, SignError> + ) -> Result, SignError> where C: Signing + Verification, T: Borrow, @@ -453,7 +452,7 @@ impl Psbt { let signature = taproot::Signature { signature, sighash_type }; input.tap_key_sig = Some(signature); - used.push(sk.public_key(secp)); + used.push(internal_key); } } @@ -481,7 +480,7 @@ impl Psbt { input.tap_script_sigs.insert((xonly, lh), signature); } - used.push(sk.public_key(secp)); + used.push(sk.public_key(secp).into()); } } } @@ -774,8 +773,20 @@ impl GetKey for Xpriv { } } -/// Map of input index -> pubkey associated with secret key used to create signature for that input. -pub type SigningKeys = BTreeMap>; +/// Map of input index -> signing key for that input (see [`SigningKeys`]). +pub type SigningKeysMap = BTreeMap; + +/// A list of keys used to sign an input. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] +pub enum SigningKeys { + /// Keys used to sign an ECDSA input. + Ecdsa(Vec), + /// Keys used to sign a Taproot input. + /// + /// - Key path spend: This is the internal key. + /// - Script path spend: This is the pubkey associated with the secret key that signed. + Schnorr(Vec), +} /// Map of input index -> the error encountered while attempting to sign that input. pub type SigningErrors = BTreeMap; @@ -2279,6 +2290,6 @@ mod tests { let (signing_keys, _) = psbt.sign(&key_map, &secp).unwrap_err(); assert_eq!(signing_keys.len(), 1); - assert_eq!(signing_keys[&0], vec![pk]); + assert_eq!(signing_keys[&0], SigningKeys::Ecdsa(vec![pk])); } }