From 0a7ae6c2336fc3bb9da179eb84d4e09ec368fca7 Mon Sep 17 00:00:00 2001 From: valued mammal Date: Fri, 24 May 2024 19:20:31 -0400 Subject: [PATCH] ref(signer): Use `Psbt::sighash_ecdsa` for computing sighashes - Change param `hash` to `&Message` in `sign_psbt_ecdsa` - Remove unused methods `compute_legacy_sighash`, and `compute_segwitv0_sighash`. - Match on `self.ctx` when signing for `SignerWrapper` --- crates/wallet/src/wallet/signer.rs | 307 ++++++++--------------------- 1 file changed, 84 insertions(+), 223 deletions(-) diff --git a/crates/wallet/src/wallet/signer.rs b/crates/wallet/src/wallet/signer.rs index ed0119faa6..283de5a6c9 100644 --- a/crates/wallet/src/wallet/signer.rs +++ b/crates/wallet/src/wallet/signer.rs @@ -91,7 +91,7 @@ use bitcoin::bip32::{ChildNumber, DerivationPath, Fingerprint, Xpriv}; use bitcoin::hashes::hash160; use bitcoin::secp256k1::Message; use bitcoin::sighash::{EcdsaSighashType, TapSighash, TapSighashType}; -use bitcoin::{ecdsa, psbt, sighash, taproot, transaction}; +use bitcoin::{ecdsa, psbt, sighash, taproot}; use bitcoin::{key::TapTweak, key::XOnlyPublicKey, secp256k1}; use bitcoin::{PrivateKey, Psbt, PublicKey}; @@ -159,12 +159,10 @@ pub enum SignerError { NonStandardSighash, /// Invalid SIGHASH for the signing context in use InvalidSighash, - /// Error while computing the hash to sign a P2WPKH input. - SighashP2wpkh(sighash::P2wpkhError), /// Error while computing the hash to sign a Taproot input. SighashTaproot(sighash::TaprootError), - /// Error while computing the hash, out of bounds access on the transaction inputs. - TxInputsIndexError(transaction::InputsIndexError), + /// PSBT sign error. + Psbt(psbt::SignError), /// Miniscript PSBT error MiniscriptPsbt(MiniscriptPsbtError), /// To be used only by external libraries implementing [`InputSigner`] or @@ -173,24 +171,6 @@ pub enum SignerError { External(String), } -impl From for SignerError { - fn from(v: transaction::InputsIndexError) -> Self { - Self::TxInputsIndexError(v) - } -} - -impl From for SignerError { - fn from(e: sighash::P2wpkhError) -> Self { - Self::SighashP2wpkh(e) - } -} - -impl From for SignerError { - fn from(e: sighash::TaprootError) -> Self { - Self::SighashTaproot(e) - } -} - impl fmt::Display for SignerError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { @@ -205,9 +185,8 @@ impl fmt::Display for SignerError { Self::MissingHdKeypath => write!(f, "Missing fingerprint and derivation path"), Self::NonStandardSighash => write!(f, "The psbt contains a non standard sighash"), Self::InvalidSighash => write!(f, "Invalid SIGHASH for the signing context in use"), - Self::SighashP2wpkh(err) => write!(f, "Error while computing the hash to sign a P2WPKH input: {}", err), Self::SighashTaproot(err) => write!(f, "Error while computing the hash to sign a Taproot input: {}", err), - Self::TxInputsIndexError(err) => write!(f, "Error while computing the hash, out of bounds access on the transaction inputs: {}", err), + Self::Psbt(err) => write!(f, "Error computing the sighash: {}", err), Self::MiniscriptPsbt(err) => write!(f, "Miniscript PSBT error: {}", err), Self::External(err) => write!(f, "{}", err), } @@ -472,92 +451,87 @@ impl InputSigner for SignerWrapper { } let pubkey = PublicKey::from_private_key(secp, self); - let x_only_pubkey = XOnlyPublicKey::from(pubkey.inner); - - 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 - && psbt.inputs[input_index].tap_key_sig.is_none() - && sign_options.sign_with_tap_internal_key - && x_only_pubkey == psbt_internal_key - { - let (hash, hash_ty) = compute_tap_sighash(psbt, input_index, None)?; - sign_psbt_schnorr( - &self.inner, - x_only_pubkey, - None, - &mut psbt.inputs[input_index], - hash, - hash_ty, - secp, - ); + + match self.ctx { + SignerContext::Tap { is_internal_key } => { + let x_only_pubkey = XOnlyPublicKey::from(pubkey.inner); + + if let Some(psbt_internal_key) = psbt.inputs[input_index].tap_internal_key { + if is_internal_key + && psbt.inputs[input_index].tap_key_sig.is_none() + && sign_options.sign_with_tap_internal_key + && x_only_pubkey == psbt_internal_key + { + let (sighash, sighash_type) = compute_tap_sighash(psbt, input_index, None)?; + sign_psbt_schnorr( + &self.inner, + x_only_pubkey, + None, + &mut psbt.inputs[input_index], + sighash, + sighash_type, + 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| { - // 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::>(); - for lh in leaf_hashes { - let (hash, hash_ty) = compute_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, - ); + if let Some((leaf_hashes, _)) = + psbt.inputs[input_index].tap_key_origins.get(&x_only_pubkey) + { + let leaf_hashes = leaf_hashes + .iter() + .filter(|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::>(); + for lh in leaf_hashes { + let (sighash, sighash_type) = + compute_tap_sighash(psbt, input_index, Some(lh))?; + sign_psbt_schnorr( + &self.inner, + x_only_pubkey, + Some(lh), + &mut psbt.inputs[input_index], + sighash, + sighash_type, + secp, + ); + } } } + SignerContext::Segwitv0 | SignerContext::Legacy => { + if psbt.inputs[input_index].partial_sigs.contains_key(&pubkey) { + return Ok(()); + } - return Ok(()); - } - - if psbt.inputs[input_index].partial_sigs.contains_key(&pubkey) { - return Ok(()); - } + let mut sighasher = sighash::SighashCache::new(psbt.unsigned_tx.clone()); + let (msg, sighash_type) = psbt + .sighash_ecdsa(input_index, &mut sighasher) + .map_err(SignerError::Psbt)?; - let (hash, hash_ty) = match self.ctx { - SignerContext::Segwitv0 => { - let (h, t) = compute_segwitv0_sighash(psbt, input_index)?; - let h = h.to_raw_hash(); - (h, t) - } - SignerContext::Legacy => { - let (h, t) = compute_legacy_sighash(psbt, input_index)?; - let h = h.to_raw_hash(); - (h, t) + sign_psbt_ecdsa( + &self.inner, + pubkey, + &mut psbt.inputs[input_index], + &msg, + sighash_type, + secp, + sign_options.allow_grinding, + ); } - _ => return Ok(()), // handled above - }; - sign_psbt_ecdsa( - &self.inner, - pubkey, - &mut psbt.inputs[input_index], - hash, - hash_ty, - secp, - sign_options.allow_grinding, - ); + } Ok(()) } @@ -567,12 +541,11 @@ fn sign_psbt_ecdsa( secret_key: &secp256k1::SecretKey, pubkey: PublicKey, psbt_input: &mut psbt::Input, - hash: impl bitcoin::hashes::Hash, + msg: &Message, sighash_type: EcdsaSighashType, secp: &SecpCtx, allow_grinding: bool, ) { - let msg = &Message::from_digest(hash.to_byte_array()); let signature = if allow_grinding { secp.sign_ecdsa_low_r(msg, secret_key) } else { @@ -594,7 +567,7 @@ fn sign_psbt_schnorr( pubkey: XOnlyPublicKey, leaf_hash: Option, psbt_input: &mut psbt::Input, - hash: TapSighash, + sighash: TapSighash, sighash_type: TapSighashType, secp: &SecpCtx, ) { @@ -606,7 +579,7 @@ fn sign_psbt_schnorr( Some(_) => keypair, // no tweak for script spend }; - let msg = &Message::from(hash); + let msg = &Message::from(sighash); let signature = secp.sign_schnorr(msg, &keypair); secp.verify_schnorr(&signature, msg, &XOnlyPublicKey::from_keypair(&keypair).0) .expect("invalid or corrupted schnorr signature"); @@ -870,120 +843,6 @@ impl Default for SignOptions { } } -/// Computes the legacy sighash. -fn compute_legacy_sighash( - psbt: &Psbt, - input_index: usize, -) -> Result<(sighash::LegacySighash, EcdsaSighashType), 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 tx_input = &psbt.unsigned_tx.input[input_index]; - - let sighash_type = psbt_input - .sighash_type - .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 => { - let non_witness_utxo = psbt_input - .non_witness_utxo - .as_ref() - .ok_or(SignerError::MissingNonWitnessUtxo)?; - let prev_out = non_witness_utxo - .output - .get(tx_input.previous_output.vout as usize) - .ok_or(SignerError::InvalidNonWitnessUtxo)?; - - prev_out.script_pubkey.clone() - } - }; - - Ok(( - sighash::SighashCache::new(&psbt.unsigned_tx).legacy_signature_hash( - input_index, - &script, - sighash_type.to_u32(), - )?, - sighash_type, - )) -} - -/// Computes the segwitv0 sighash. -fn compute_segwitv0_sighash( - psbt: &Psbt, - input_index: usize, -) -> Result<(sighash::SegwitV0Sighash, EcdsaSighashType), 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 tx_input = &psbt.unsigned_tx.input[input_index]; - - let sighash_type = psbt_input - .sighash_type - .unwrap_or_else(|| EcdsaSighashType::All.into()) - .ecdsa_hash_ty() - .map_err(|_| SignerError::InvalidSighash)?; - - // Always try first with the non-witness utxo - let utxo = if let Some(prev_tx) = &psbt_input.non_witness_utxo { - // Check the provided prev-tx - if prev_tx.compute_txid() != tx_input.previous_output.txid { - return Err(SignerError::InvalidNonWitnessUtxo); - } - - // The output should be present, if it's missing the `non_witness_utxo` is invalid - prev_tx - .output - .get(tx_input.previous_output.vout as usize) - .ok_or(SignerError::InvalidNonWitnessUtxo)? - } else if let Some(witness_utxo) = &psbt_input.witness_utxo { - // Fallback to the witness_utxo. If we aren't allowed to use it, signing should fail - // before we get to this point - witness_utxo - } else { - // Nothing has been provided - return Err(SignerError::MissingNonWitnessUtxo); - }; - let value = utxo.value; - - let mut sighasher = sighash::SighashCache::new(&psbt.unsigned_tx); - - let sighash = match psbt_input.witness_script { - Some(ref witness_script) => { - sighasher.p2wsh_signature_hash(input_index, witness_script, value, sighash_type)? - } - None => { - if utxo.script_pubkey.is_p2wpkh() { - sighasher.p2wpkh_signature_hash( - input_index, - &utxo.script_pubkey, - value, - sighash_type, - )? - } else if psbt_input - .redeem_script - .as_ref() - .map(|s| s.is_p2wpkh()) - .unwrap_or(false) - { - let script_pubkey = psbt_input.redeem_script.as_ref().unwrap(); - sighasher.p2wpkh_signature_hash(input_index, script_pubkey, value, sighash_type)? - } else { - return Err(SignerError::MissingWitnessScript); - } - } - }; - Ok((sighash, sighash_type)) -} - /// Computes the taproot sighash. fn compute_tap_sighash( psbt: &Psbt, @@ -1026,7 +885,9 @@ fn compute_tap_sighash( let extra = extra.map(|leaf_hash| (leaf_hash, 0xFFFFFFFF)); Ok(( - cache.taproot_signature_hash(input_index, &prevouts, None, extra, sighash_type)?, + cache + .taproot_signature_hash(input_index, &prevouts, None, extra, sighash_type) + .map_err(SignerError::SighashTaproot)?, sighash_type, )) }