From d9533523ac5e7c87a61d10c12c1c0536e804f741 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 16 Aug 2023 15:30:04 +1000 Subject: [PATCH] Remove usage of ThirtyTwoByteHash The `ThirtyTwoByteHash` trait is defined in `secp256k1` and used in `hashes` as well as `bitcoin`. This means that we must use the same version of `hashes` in both `bitcoin` and `secp256k1`. This makes doing release difficult. Remove usage of `ThirtyTwoByteHash` and use `Message::from_slice`. Include TODO above each usage because as soon as we release the new version of secp we can use the new `Message::from_digest`. This is step backwards as far as type safety goes and it makes the code more ugly as well because it uses `expect` but thems the breaks. --- bitcoin/examples/sighash.rs | 5 ++++- bitcoin/examples/taproot-psbt.rs | 6 +++++- bitcoin/src/crypto/sighash.rs | 4 +++- bitcoin/src/psbt/mod.rs | 31 ++++++++++++++++++++++++++----- bitcoin/src/sign_message.rs | 11 ++++++++--- 5 files changed, 46 insertions(+), 11 deletions(-) diff --git a/bitcoin/examples/sighash.rs b/bitcoin/examples/sighash.rs index 0230f379..746c0df9 100644 --- a/bitcoin/examples/sighash.rs +++ b/bitcoin/examples/sighash.rs @@ -1,3 +1,4 @@ +use bitcoin::hashes::Hash; use bitcoin::{consensus, ecdsa, sighash, Amount, PublicKey, Script, ScriptBuf, Transaction}; use hex_lit::hex; @@ -44,7 +45,9 @@ fn compute_sighash_p2wpkh(raw_tx: &[u8], inp_idx: usize, value: u64) { .p2wpkh_signature_hash(inp_idx, &spk, Amount::from_sat(value), sig.hash_ty) .expect("failed to compute sighash"); println!("Segwit p2wpkh sighash: {:x}", sighash); - let msg = secp256k1::Message::from(sighash); + // TODO: After upgrade of secp change this to Message::from_digest(sighash.to_byte_array()). + let msg = + secp256k1::Message::from_slice(sighash.as_byte_array()).expect("sighash is 32 bytes long"); println!("Message is {:x}", msg); let secp = secp256k1::Secp256k1::verification_only(); secp.verify_ecdsa(&msg, &sig.sig, &pk.inner).unwrap(); diff --git a/bitcoin/examples/taproot-psbt.rs b/bitcoin/examples/taproot-psbt.rs index 3794688d..367d69aa 100644 --- a/bitcoin/examples/taproot-psbt.rs +++ b/bitcoin/examples/taproot-psbt.rs @@ -80,6 +80,7 @@ use std::str::FromStr; use bitcoin::bip32::{ChildNumber, DerivationPath, ExtendedPrivKey, ExtendedPubKey, Fingerprint}; use bitcoin::consensus::encode; +use bitcoin::hashes::Hash; use bitcoin::key::{TapTweak, XOnlyPublicKey}; use bitcoin::opcodes::all::{OP_CHECKSIG, OP_CLTV, OP_DROP}; use bitcoin::psbt::{self, Input, Output, Psbt, PsbtSighashType}; @@ -737,7 +738,10 @@ fn sign_psbt_taproot( Some(_) => keypair, // no tweak for script spend }; - let sig = secp.sign_schnorr(&hash.into(), &keypair); + // TODO: After upgrade of secp change this to Message::from_digest(sighash.to_byte_array()). + let msg = + secp256k1::Message::from_slice(hash.as_byte_array()).expect("tap sighash is 32 bytes long"); + let sig = secp.sign_schnorr(&msg, &keypair); let final_signature = taproot::Signature { sig, hash_ty }; diff --git a/bitcoin/src/crypto/sighash.rs b/bitcoin/src/crypto/sighash.rs index ddc827c4..a24d9319 100644 --- a/bitcoin/src/crypto/sighash.rs +++ b/bitcoin/src/crypto/sighash.rs @@ -1751,7 +1751,9 @@ mod tests { .taproot_signature_hash(tx_ind, &Prevouts::All(&utxos), None, None, hash_ty) .unwrap(); - let msg = secp256k1::Message::from(sighash); + // TODO: After upgrade of secp change this to Message::from_digest(sighash.to_byte_array()). + let msg = secp256k1::Message::from_slice(sighash.as_byte_array()) + .expect("sighash is 32 bytes long"); let key_spend_sig = secp.sign_schnorr_with_aux_rand(&msg, &tweaked_keypair, &[0u8; 32]); assert_eq!(expected.internal_pubkey, internal_key); diff --git a/bitcoin/src/psbt/mod.rs b/bitcoin/src/psbt/mod.rs index 70143433..a8a98e23 100644 --- a/bitcoin/src/psbt/mod.rs +++ b/bitcoin/src/psbt/mod.rs @@ -11,6 +11,7 @@ use core::{cmp, fmt}; #[cfg(feature = "std")] use std::collections::{HashMap, HashSet}; +use hashes::Hash; use internals::write_err; use secp256k1::{Message, Secp256k1, Signing}; @@ -325,31 +326,51 @@ impl Psbt { match self.output_type(input_index)? { Bare => { let sighash = cache.legacy_signature_hash(input_index, spk, hash_ty.to_u32())?; - Ok((Message::from(sighash), hash_ty)) + // TODO: After upgrade of secp change this to Message::from_digest(sighash.to_byte_array()). + Ok(( + Message::from_slice(sighash.as_byte_array()).expect("sighash is 32 bytes long"), + hash_ty, + )) } Sh => { let script_code = input.redeem_script.as_ref().ok_or(SignError::MissingRedeemScript)?; let sighash = cache.legacy_signature_hash(input_index, script_code, hash_ty.to_u32())?; - Ok((Message::from(sighash), hash_ty)) + // TODO: After upgrade of secp change this to Message::from_digest(sighash.to_byte_array()). + Ok(( + Message::from_slice(sighash.as_byte_array()).expect("sighash is 32 bytes long"), + hash_ty, + )) } Wpkh => { let sighash = cache.p2wpkh_signature_hash(input_index, spk, utxo.value, hash_ty)?; - Ok((Message::from(sighash), hash_ty)) + // TODO: After upgrade of secp change this to Message::from_digest(sighash.to_byte_array()). + Ok(( + Message::from_slice(sighash.as_byte_array()).expect("sighash is 32 bytes long"), + hash_ty, + )) } ShWpkh => { let redeem_script = input.redeem_script.as_ref().expect("checked above"); let sighash = cache.p2wpkh_signature_hash(input_index, redeem_script, utxo.value, hash_ty)?; - Ok((Message::from(sighash), hash_ty)) + // TODO: After upgrade of secp change this to Message::from_digest(sighash.to_byte_array()). + Ok(( + Message::from_slice(sighash.as_byte_array()).expect("sighash is 32 bytes long"), + hash_ty, + )) } Wsh | ShWsh => { let witness_script = input.witness_script.as_ref().ok_or(SignError::MissingWitnessScript)?; let sighash = cache.p2wsh_signature_hash(input_index, witness_script, utxo.value, hash_ty)?; - Ok((Message::from(sighash), hash_ty)) + // TODO: After upgrade of secp change this to Message::from_digest(sighash.to_byte_array()). + Ok(( + Message::from_slice(sighash.as_byte_array()).expect("sighash is 32 bytes long"), + hash_ty, + )) } Tr => { // This PSBT signing API is WIP, taproot to come shortly. diff --git a/bitcoin/src/sign_message.rs b/bitcoin/src/sign_message.rs index d4a4a6da..abaf4651 100644 --- a/bitcoin/src/sign_message.rs +++ b/bitcoin/src/sign_message.rs @@ -19,7 +19,7 @@ pub const BITCOIN_SIGNED_MSG_PREFIX: &[u8] = b"\x18Bitcoin Signed Message:\n"; mod message_signing { use core::fmt; - use hashes::sha256d; + use hashes::{sha256d, Hash}; use internals::write_err; use secp256k1; use secp256k1::ecdsa::{RecoverableSignature, RecoveryId}; @@ -132,7 +132,10 @@ mod message_signing { secp_ctx: &secp256k1::Secp256k1, msg_hash: sha256d::Hash, ) -> Result { - let msg = secp256k1::Message::from(msg_hash); + // TODO: After upgrade of secp change this to Message::from_digest(sighash.to_byte_array()). + let msg = secp256k1::Message::from_slice(msg_hash.as_byte_array()) + .expect("sh256d hash is 32 bytes long"); + let pubkey = secp_ctx.recover_ecdsa(&msg, &self.signature)?; Ok(PublicKey { inner: pubkey, compressed: self.compressed }) } @@ -226,7 +229,9 @@ mod tests { let secp = secp256k1::Secp256k1::new(); let message = "rust-bitcoin MessageSignature test"; let msg_hash = super::signed_msg_hash(message); - let msg = secp256k1::Message::from(msg_hash); + // TODO: After upgrade of secp change this to Message::from_digest(sighash.to_byte_array()). + let msg = secp256k1::Message::from_slice(msg_hash.as_byte_array()) + .expect("sh256d hash is 32 bytes long"); let privkey = secp256k1::SecretKey::new(&mut secp256k1::rand::thread_rng()); let secp_sig = secp.sign_ecdsa_recoverable(&msg, &privkey);