Merge rust-bitcoin/rust-bitcoin#2652: psbt: Return internal key for key path spend

14040e2ff5 psbt: Return the internal key for key path spend (Tobin C. Harding)
ffd5664c08 Do not panic if input_index is out of bounds (Tobin C. Harding)
f79f20d4e6 Remove stale rustdoc (Tobin C. Harding)

Pull request description:

  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.

  - Patch 1: Fix stale docs
  - Patch 2: Remove unnecessary panic
  - Patch 3: Change the key returned when signing Taproot input as key path spend

  Done as part of #2557, this is the release blocking part.

ACKs for top commit:
  sanket1729:
    ACK 14040e2ff5
  apoelstra:
    ACK 14040e2ff5

Tree-SHA512: 0b38b9009fd5dab682461dcb79f46c7540ec79cfd9c856fc5036ad8ecda061781c58b6fd157db9f034ab07dda6fa747417318f613092ee2017b8f2f44898dcd7
This commit is contained in:
Andrew Poelstra 2024-04-04 14:29:48 +00:00
commit df9b31ddcf
No known key found for this signature in database
GPG Key ID: C588D63CE41B97C1
1 changed files with 30 additions and 23 deletions

View File

@ -25,7 +25,7 @@ use crate::bip32::{self, KeySource, Xpriv, Xpub};
use crate::blockdata::transaction::{self, Transaction, TxOut}; use crate::blockdata::transaction::{self, Transaction, TxOut};
use crate::crypto::key::{PrivateKey, PublicKey}; use crate::crypto::key::{PrivateKey, PublicKey};
use crate::crypto::{ecdsa, taproot}; use crate::crypto::{ecdsa, taproot};
use crate::key::TapTweak; use crate::key::{TapTweak, XOnlyPublicKey};
use crate::prelude::*; use crate::prelude::*;
use crate::sighash::{self, EcdsaSighashType, Prevouts, SighashCache}; use crate::sighash::{self, EcdsaSighashType, Prevouts, SighashCache};
use crate::{Amount, FeeRate, TapLeafHash, TapSighashType}; use crate::{Amount, FeeRate, TapLeafHash, TapSighashType};
@ -283,17 +283,12 @@ impl Psbt {
/// Attempts to create _all_ the required signatures for this PSBT using `k`. /// 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 /// If you just want to sign an input with one specific key consider using `sighash_ecdsa` or
/// attempt to sign ECDSA inputs. /// `sighash_taproot`. 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`. This
/// function does not support scripts that contain `OP_CODESEPARATOR`.
/// ///
/// # Returns /// # Returns
/// ///
/// Either Ok(SigningKeys) or Err((SigningKeys, SigningErrors)), where /// A map of input index -> keys used to sign, for Taproot specifics please see [`SigningKeys`].
/// - 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.
/// ///
/// If an error is returned some signatures may already have been added to the PSBT. Since /// 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. /// `partial_sigs` is a [`BTreeMap`] it is safe to retry, previous sigs will be overwritten.
@ -301,7 +296,7 @@ impl Psbt {
&mut self, &mut self,
k: &K, k: &K,
secp: &Secp256k1<C>, secp: &Secp256k1<C>,
) -> Result<SigningKeys, (SigningKeys, SigningErrors)> ) -> Result<SigningKeysMap, (SigningKeysMap, SigningErrors)>
where where
C: Signing + Verification, C: Signing + Verification,
K: GetKey, K: GetKey,
@ -317,7 +312,7 @@ impl Psbt {
Ok(SigningAlgorithm::Ecdsa) => Ok(SigningAlgorithm::Ecdsa) =>
match self.bip32_sign_ecdsa(k, i, &mut cache, secp) { match self.bip32_sign_ecdsa(k, i, &mut cache, secp) {
Ok(v) => { Ok(v) => {
used.insert(i, v); used.insert(i, SigningKeys::Ecdsa(v));
} }
Err(e) => { Err(e) => {
errors.insert(i, e); errors.insert(i, e);
@ -326,7 +321,7 @@ impl Psbt {
Ok(SigningAlgorithm::Schnorr) => { Ok(SigningAlgorithm::Schnorr) => {
match self.bip32_sign_schnorr(k, i, &mut cache, secp) { match self.bip32_sign_schnorr(k, i, &mut cache, secp) {
Ok(v) => { Ok(v) => {
used.insert(i, v); used.insert(i, SigningKeys::Schnorr(v));
} }
Err(e) => { Err(e) => {
errors.insert(i, e); errors.insert(i, e);
@ -404,22 +399,22 @@ impl Psbt {
/// ///
/// # Returns /// # 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
/// - Err: Error encountered trying to calculate the sighash AND we had the signing key. Also panics /// return the internal key.
/// 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<C, K, T>( fn bip32_sign_schnorr<C, K, T>(
&mut self, &mut self,
k: &K, k: &K,
input_index: usize, input_index: usize,
cache: &mut SighashCache<T>, cache: &mut SighashCache<T>,
secp: &Secp256k1<C>, secp: &Secp256k1<C>,
) -> Result<Vec<PublicKey>, SignError> ) -> Result<Vec<XOnlyPublicKey>, SignError>
where where
C: Signing + Verification, C: Signing + Verification,
T: Borrow<Transaction>, T: Borrow<Transaction>,
K: GetKey, 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. let mut used = vec![]; // List of pubkeys used to sign the input.
@ -457,7 +452,7 @@ impl Psbt {
let signature = taproot::Signature { signature, sighash_type }; let signature = taproot::Signature { signature, sighash_type };
input.tap_key_sig = Some(signature); input.tap_key_sig = Some(signature);
used.push(sk.public_key(secp)); used.push(internal_key);
} }
} }
@ -485,12 +480,12 @@ impl Psbt {
input.tap_script_sigs.insert((xonly, lh), signature); input.tap_script_sigs.insert((xonly, lh), signature);
} }
used.push(sk.public_key(secp)); used.push(sk.public_key(secp).into());
} }
} }
} }
self.inputs[input_index] = input; self.inputs[input_index] = input; // input_index is checked above.
Ok(used) Ok(used)
} }
@ -778,8 +773,20 @@ impl GetKey for Xpriv {
} }
} }
/// Map of input index -> pubkey associated with secret key used to create signature for that input. /// Map of input index -> signing key for that input (see [`SigningKeys`]).
pub type SigningKeys = BTreeMap<usize, Vec<PublicKey>>; pub type SigningKeysMap = BTreeMap<usize, SigningKeys>;
/// 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<PublicKey>),
/// 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<XOnlyPublicKey>),
}
/// Map of input index -> the error encountered while attempting to sign that input. /// Map of input index -> the error encountered while attempting to sign that input.
pub type SigningErrors = BTreeMap<usize, SignError>; pub type SigningErrors = BTreeMap<usize, SignError>;
@ -2283,6 +2290,6 @@ mod tests {
let (signing_keys, _) = psbt.sign(&key_map, &secp).unwrap_err(); let (signing_keys, _) = psbt.sign(&key_map, &secp).unwrap_err();
assert_eq!(signing_keys.len(), 1); assert_eq!(signing_keys.len(), 1);
assert_eq!(signing_keys[&0], vec![pk]); assert_eq!(signing_keys[&0], SigningKeys::Ecdsa(vec![pk]));
} }
} }