From b8d85a1df0d93b2fd2ec33064b439663705a6d53 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 24 Jun 2024 12:54:07 +0000 Subject: [PATCH] bitcoin: remove all use of engine/from_engine on opaque hash types In the next commits we are going to stop exposing the ability to hash arbitrary data into wrapped hash types like Txid etc. In preparation for this, stop using these methods internally. This makes our internal code a little bit uglier and less DRY. An alternative approach would be to implement the from_engine and engine methods, but privately (and maybe having a macro to provide this). But I think this approach is more straightforward. The one exception is for the Taproot hashes, which are tagged hashes and currently do not have their own engine type. I will address these in a later PR because this one is already too big. --- bitcoin/src/bip158.rs | 5 ++--- bitcoin/src/bip32.rs | 5 +---- bitcoin/src/blockdata/block.rs | 8 ++++---- bitcoin/src/blockdata/script/mod.rs | 12 ++++++++---- bitcoin/src/blockdata/transaction.rs | 8 ++++---- bitcoin/src/crypto/key.rs | 6 ++++-- bitcoin/src/crypto/sighash.rs | 15 +++++++++++++++ bitcoin/src/psbt/mod.rs | 4 ++-- 8 files changed, 40 insertions(+), 23 deletions(-) diff --git a/bitcoin/src/bip158.rs b/bitcoin/src/bip158.rs index 6aa0cda06..8c12a871c 100644 --- a/bitcoin/src/bip158.rs +++ b/bitcoin/src/bip158.rs @@ -118,7 +118,7 @@ impl FilterHash { let mut header_data = [0u8; 64]; header_data[0..32].copy_from_slice(&self[..]); header_data[32..64].copy_from_slice(&previous_filter_header[..]); - FilterHeader::hash(&header_data) + FilterHeader(sha256d::Hash::hash(&header_data)) } } @@ -146,8 +146,7 @@ impl BlockFilter { /// /// [BIP 157]: pub fn filter_header(&self, previous_filter_header: FilterHeader) -> FilterHeader { - let filter_hash = FilterHash::hash(self.content.as_slice()); - filter_hash.filter_header(previous_filter_header) + FilterHash(sha256d::Hash::hash(&self.content)).filter_header(previous_filter_header) } /// Returns true if any query matches against this [`BlockFilter`]. diff --git a/bitcoin/src/bip32.rs b/bitcoin/src/bip32.rs index b846e309a..f8ade73aa 100644 --- a/bitcoin/src/bip32.rs +++ b/bitcoin/src/bip32.rs @@ -11,7 +11,6 @@ use core::{fmt, slice}; use hashes::{hash160, hash_newtype, sha512, GeneralHash, HashEngine, Hmac, HmacEngine}; use internals::{impl_array_newtype, write_err}; -use io::Write; use secp256k1::{Secp256k1, XOnlyPublicKey}; use crate::crypto::key::{CompressedPublicKey, Keypair, PrivateKey}; @@ -820,9 +819,7 @@ impl Xpub { /// Returns the HASH160 of the chaincode pub fn identifier(&self) -> XKeyIdentifier { - let mut engine = XKeyIdentifier::engine(); - engine.write_all(&self.public_key.serialize()).expect("engines don't error"); - XKeyIdentifier::from_engine(engine) + XKeyIdentifier(hash160::Hash::hash(&self.public_key.serialize())) } /// Returns the first four bytes of the identifier diff --git a/bitcoin/src/blockdata/block.rs b/bitcoin/src/blockdata/block.rs index 3980934cb..ca83df732 100644 --- a/bitcoin/src/blockdata/block.rs +++ b/bitcoin/src/blockdata/block.rs @@ -73,9 +73,9 @@ impl Header { /// Returns the block hash. pub fn block_hash(&self) -> BlockHash { - let mut engine = BlockHash::engine(); + let mut engine = sha256d::Hash::engine(); self.consensus_encode(&mut engine).expect("engines don't error"); - BlockHash::from_engine(engine) + BlockHash(sha256d::Hash::from_engine(engine)) } /// Computes the target (range [0, T] inclusive) that a blockhash must land in to be valid. @@ -295,10 +295,10 @@ impl Block { witness_root: WitnessMerkleNode, witness_reserved_value: &[u8], ) -> WitnessCommitment { - let mut encoder = WitnessCommitment::engine(); + let mut encoder = sha256d::Hash::engine(); witness_root.consensus_encode(&mut encoder).expect("engines don't error"); encoder.input(witness_reserved_value); - WitnessCommitment::from_engine(encoder) + WitnessCommitment(sha256d::Hash::from_engine(encoder)) } /// Computes the merkle root of transactions hashed for witness. diff --git a/bitcoin/src/blockdata/script/mod.rs b/bitcoin/src/blockdata/script/mod.rs index b50f9204c..4cb001f30 100644 --- a/bitcoin/src/blockdata/script/mod.rs +++ b/bitcoin/src/blockdata/script/mod.rs @@ -109,7 +109,7 @@ impl ScriptHash { return Err(RedeemScriptSizeError { size: redeem_script.len() }); } - Ok(ScriptHash::hash(redeem_script.as_bytes())) + Ok(ScriptHash(hash160::Hash::hash(redeem_script.as_bytes()))) } /// Creates a `ScriptHash` from any script irrespective of script size. @@ -118,7 +118,9 @@ impl ScriptHash { /// then the output will be unspendable (see [BIP-16]). /// /// [BIP-16]: - pub fn from_script_unchecked(script: &Script) -> Self { ScriptHash::hash(script.as_bytes()) } + pub fn from_script_unchecked(script: &Script) -> Self { + ScriptHash(hash160::Hash::hash(script.as_bytes())) + } } impl WScriptHash { @@ -135,7 +137,7 @@ impl WScriptHash { return Err(WitnessScriptSizeError { size: witness_script.len() }); } - Ok(WScriptHash::hash(witness_script.as_bytes())) + Ok(WScriptHash(sha256::Hash::hash(witness_script.as_bytes()))) } /// Creates a `WScriptHash` from any script irrespective of script size. @@ -144,7 +146,9 @@ impl WScriptHash { /// output then the output will be unspendable (see [BIP-141]). /// /// ref: [BIP-141](https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki) - pub fn from_script_unchecked(script: &Script) -> Self { WScriptHash::hash(script.as_bytes()) } + pub fn from_script_unchecked(script: &Script) -> Self { + WScriptHash(sha256::Hash::hash(script.as_bytes())) + } } impl TryFrom for ScriptHash { diff --git a/bitcoin/src/blockdata/transaction.rs b/bitcoin/src/blockdata/transaction.rs index b11f9c903..3e9db5b4b 100644 --- a/bitcoin/src/blockdata/transaction.rs +++ b/bitcoin/src/blockdata/transaction.rs @@ -754,12 +754,12 @@ impl Transaction { /// this will be equal to [`Transaction::compute_wtxid()`]. #[doc(alias = "txid")] pub fn compute_txid(&self) -> Txid { - let mut enc = Txid::engine(); + let mut enc = sha256d::Hash::engine(); self.version.consensus_encode(&mut enc).expect("engines don't error"); self.input.consensus_encode(&mut enc).expect("engines don't error"); self.output.consensus_encode(&mut enc).expect("engines don't error"); self.lock_time.consensus_encode(&mut enc).expect("engines don't error"); - Txid::from_engine(enc) + Txid(sha256d::Hash::from_engine(enc)) } /// Computes the segwit version of the transaction id. @@ -778,9 +778,9 @@ impl Transaction { /// this will be equal to [`Transaction::txid()`]. #[doc(alias = "wtxid")] pub fn compute_wtxid(&self) -> Wtxid { - let mut enc = Wtxid::engine(); + let mut enc = sha256d::Hash::engine(); self.consensus_encode(&mut enc).expect("engines don't error"); - Wtxid::from_engine(enc) + Wtxid(sha256d::Hash::from_engine(enc)) } /// Returns the weight of this transaction, as defined by BIP-141. diff --git a/bitcoin/src/crypto/key.rs b/bitcoin/src/crypto/key.rs index 11e0dbc54..b483ceeeb 100644 --- a/bitcoin/src/crypto/key.rs +++ b/bitcoin/src/crypto/key.rs @@ -58,7 +58,9 @@ impl PublicKey { } /// Returns bitcoin 160-bit hash of the public key. - pub fn pubkey_hash(&self) -> PubkeyHash { self.with_serialized(PubkeyHash::hash) } + pub fn pubkey_hash(&self) -> PubkeyHash { + PubkeyHash(self.with_serialized(hash160::Hash::hash)) + } /// Returns bitcoin 160-bit hash of the public key for witness program pub fn wpubkey_hash(&self) -> Result { @@ -275,7 +277,7 @@ pub struct CompressedPublicKey(pub secp256k1::PublicKey); impl CompressedPublicKey { /// Returns bitcoin 160-bit hash of the public key. - pub fn pubkey_hash(&self) -> PubkeyHash { PubkeyHash::hash(&self.to_bytes()) } + pub fn pubkey_hash(&self) -> PubkeyHash { PubkeyHash(hash160::Hash::hash(&self.to_bytes())) } /// Returns bitcoin 160-bit hash of the public key for witness program. pub fn wpubkey_hash(&self) -> WPubkeyHash { diff --git a/bitcoin/src/crypto/sighash.rs b/bitcoin/src/crypto/sighash.rs index 124a025fc..1919e24ad 100644 --- a/bitcoin/src/crypto/sighash.rs +++ b/bitcoin/src/crypto/sighash.rs @@ -55,6 +55,21 @@ hash_newtype! { impl_message_from_hash!(LegacySighash); impl_message_from_hash!(SegwitV0Sighash); +// Implement private engine/from_engine methods for use within this module; +// but outside of it, it should not be possible to construct these hash +// types from arbitrary data (except by casting via to/from_byte_array). +// +// These will be uncommented in the next commit; in this one they cause +// "duplicate definition" errors against the `hash_newtype!` macro. +impl LegacySighash { + //fn engine() -> sha256::HashEngine { sha256d::Hash::engine() } + //fn from_engine(e: sha256::HashEngine) -> Self { Self(sha256d::Hash::from_engine(e)) } +} +impl SegwitV0Sighash { + //fn engine() -> sha256::HashEngine { sha256d::Hash::engine() } + //fn from_engine(e: sha256::HashEngine) -> Self { Self(sha256d::Hash::from_engine(e)) } +} + sha256t_hash_newtype! { pub struct TapSighashTag = hash_str("TapSighash"); diff --git a/bitcoin/src/psbt/mod.rs b/bitcoin/src/psbt/mod.rs index a4531c7e9..eca18faa2 100644 --- a/bitcoin/src/psbt/mod.rs +++ b/bitcoin/src/psbt/mod.rs @@ -2251,7 +2251,7 @@ mod tests { fn sign_psbt() { use crate::bip32::{DerivationPath, Fingerprint}; use crate::witness_version::WitnessVersion; - use crate::{WPubkeyHash, WitnessProgram}; + use crate::WitnessProgram; let unsigned_tx = Transaction { version: transaction::Version::TWO, @@ -2271,7 +2271,7 @@ mod tests { // First input we can spend. See comment above on key_map for why we use defaults here. let txout_wpkh = TxOut { value: Amount::from_sat(10), - script_pubkey: ScriptBuf::new_p2wpkh(WPubkeyHash::hash(&pk.to_bytes())), + script_pubkey: ScriptBuf::new_p2wpkh(pk.wpubkey_hash().unwrap()), }; psbt.inputs[0].witness_utxo = Some(txout_wpkh);