From aadd7df2660a59319c47dd8da24861bd804e20a3 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sun, 9 Feb 2025 20:11:46 +0000 Subject: [PATCH] hashes: move `from_engine` fn impl out of macro body This macro is pretty weird -- it requires that a freestanding `from_engine` method exists, which it uses to implement a `from_engine` method within an impl block, by just calling through to the freestanding method. To reduce indirection, at a very small cost in increased repeated code (we now need to add a `impl Hash {` and `}` and doccomment around each freestanding function, we just remove this from the macro entirely. This will allow us to implement `from_engine` for `sha256t::Hash` in a different way than for the non-generic hash types. To minimize the diff, we do not re-indent the freestanding `from_engine`. We will do that in the next commit. However, the diff is still a bit noisy because I had to replace `fn from_engine` with `pub fn from_engine` in every case. I took the opportunity to also change the return type from `Hash` to `Self` to make it clearer that these are constructors. --- hashes/src/hash160/mod.rs | 6 +++- hashes/src/internal_macros.rs | 13 +++---- hashes/src/ripemd160/mod.rs | 9 +++-- hashes/src/sha1/mod.rs | 5 ++- hashes/src/sha256/mod.rs | 64 ++++++++++++++++++----------------- hashes/src/sha256d/mod.rs | 5 ++- hashes/src/sha256t/mod.rs | 2 +- hashes/src/sha384/mod.rs | 7 ++-- hashes/src/sha512/mod.rs | 8 +++-- hashes/src/sha512_256/mod.rs | 7 ++-- hashes/src/siphash24/mod.rs | 18 +++++----- 11 files changed, 82 insertions(+), 62 deletions(-) diff --git a/hashes/src/hash160/mod.rs b/hashes/src/hash160/mod.rs index 3c457c1aa..406bdc5ab 100644 --- a/hashes/src/hash160/mod.rs +++ b/hashes/src/hash160/mod.rs @@ -15,7 +15,10 @@ crate::internal_macros::general_hash_type! { "Output of the Bitcoin HASH160 hash function. (RIPEMD160(SHA256))" } -fn from_engine(e: HashEngine) -> Hash { + +impl Hash { +/// Finalize a hash engine to produce a hash. +pub fn from_engine(e: HashEngine) -> Self { let sha2 = sha256::Hash::from_engine(e.0); let rmd = ripemd160::Hash::hash(sha2.as_byte_array()); @@ -23,6 +26,7 @@ fn from_engine(e: HashEngine) -> Hash { ret.copy_from_slice(rmd.as_byte_array()); Hash(ret) } +} /// Engine to compute HASH160 hash function. #[derive(Debug, Clone)] diff --git a/hashes/src/internal_macros.rs b/hashes/src/internal_macros.rs index d325d13b7..f0f410167 100644 --- a/hashes/src/internal_macros.rs +++ b/hashes/src/internal_macros.rs @@ -15,10 +15,7 @@ /// /// Restrictions on usage: /// -/// * There must be a free-standing `fn from_engine(HashEngine) -> Hash` in the scope -/// * `fn internal_new([u8; $bits / 8]) -> Self` must exist on `Hash` -/// -/// `from_engine` obviously implements the finalization algorithm. +/// * The hash type must implement the `GeneralHash` trait. macro_rules! hash_trait_impls { ($bits:expr, $reverse:expr $(, $gen:ident: $gent:ident)*) => { $crate::impl_bytelike_traits!(Hash, { $bits / 8 } $(, $gen: $gent)*); @@ -62,8 +59,9 @@ pub(crate) use hash_trait_impls; /// * `$reverse` - `true` if the hash should be displayed backwards, `false` otherwise /// * `$doc` - doc string to put on the type /// -/// The `from_engine` free-standing function is still required with this macro. See the doc of -/// [`hash_trait_impls`]. +/// Restrictions on usage: +/// +/// * The hash type must implement the `GeneralHash` trait. macro_rules! general_hash_type { ($bits:expr, $reverse:expr, $doc:literal) => { /// Hashes some bytes. @@ -93,9 +91,6 @@ macro_rules! general_hash_type { $crate::internal_macros::hash_type_no_default!($bits, $reverse, $doc); impl Hash { - /// Produces a hash from the current state of a given engine. - pub fn from_engine(e: HashEngine) -> Hash { from_engine(e) } - /// Constructs a new engine. pub fn engine() -> HashEngine { Default::default() } diff --git a/hashes/src/ripemd160/mod.rs b/hashes/src/ripemd160/mod.rs index bb2941455..7b0e5377f 100644 --- a/hashes/src/ripemd160/mod.rs +++ b/hashes/src/ripemd160/mod.rs @@ -20,8 +20,11 @@ crate::internal_macros::general_hash_type! { "Output of the RIPEMD160 hash function." } + +impl Hash { +/// Finalize a hash engine to produce a hash. #[cfg(not(hashes_fuzz))] -fn from_engine(mut e: HashEngine) -> Hash { +pub fn from_engine(mut e: HashEngine) -> Self { // pad buffer with a single 1-bit then all 0s, until there are exactly 8 bytes remaining let n_bytes_hashed = e.bytes_hashed; @@ -40,12 +43,14 @@ fn from_engine(mut e: HashEngine) -> Hash { Hash(e.midstate()) } +/// Finalize a hash engine to produce a hash. #[cfg(hashes_fuzz)] -fn from_engine(e: HashEngine) -> Hash { +pub fn from_engine(e: HashEngine) -> Self { let mut res = e.midstate(); res[0] ^= (e.bytes_hashed & 0xff) as u8; Hash(res) } +} const BLOCK_SIZE: usize = 64; diff --git a/hashes/src/sha1/mod.rs b/hashes/src/sha1/mod.rs index 69ca30897..ae49c364f 100644 --- a/hashes/src/sha1/mod.rs +++ b/hashes/src/sha1/mod.rs @@ -20,7 +20,9 @@ crate::internal_macros::general_hash_type! { "Output of the SHA1 hash function." } -fn from_engine(mut e: HashEngine) -> Hash { +impl Hash { +/// Finalize a hash engine to produce a hash. +pub fn from_engine(mut e: HashEngine) -> Self { // pad buffer with a single 1-bit then all 0s, until there are exactly 8 bytes remaining let n_bytes_hashed = e.bytes_hashed; @@ -38,6 +40,7 @@ fn from_engine(mut e: HashEngine) -> Hash { Hash(e.midstate()) } +} const BLOCK_SIZE: usize = 64; diff --git a/hashes/src/sha256/mod.rs b/hashes/src/sha256/mod.rs index a45eb0bfe..1def47bf5 100644 --- a/hashes/src/sha256/mod.rs +++ b/hashes/src/sha256/mod.rs @@ -22,37 +22,6 @@ crate::internal_macros::general_hash_type! { "Output of the SHA256 hash function." } -#[cfg(not(hashes_fuzz))] -fn from_engine(mut e: HashEngine) -> Hash { - // pad buffer with a single 1-bit then all 0s, until there are exactly 8 bytes remaining - let n_bytes_hashed = e.bytes_hashed; - - let zeroes = [0; BLOCK_SIZE - 8]; - e.input(&[0x80]); - if incomplete_block_len(&e) > zeroes.len() { - e.input(&zeroes); - } - let pad_length = zeroes.len() - incomplete_block_len(&e); - e.input(&zeroes[..pad_length]); - debug_assert_eq!(incomplete_block_len(&e), zeroes.len()); - - e.input(&(8 * n_bytes_hashed).to_be_bytes()); - debug_assert_eq!(incomplete_block_len(&e), 0); - - Hash(e.midstate_unchecked().bytes) -} - -#[cfg(hashes_fuzz)] -fn from_engine(e: HashEngine) -> Hash { - let mut hash = e.midstate_unchecked().bytes; - if hash == [0; 32] { - // Assume sha256 is secure and never generate 0-hashes (which represent invalid - // secp256k1 secret keys, causing downstream application breakage). - hash[0] = 1; - } - Hash(hash) -} - const BLOCK_SIZE: usize = 64; /// Engine to compute SHA256 hash function. @@ -141,6 +110,39 @@ impl crate::HashEngine for HashEngine { } impl Hash { +/// Finalize a hash engine to obtain a hash. +#[cfg(not(hashes_fuzz))] +pub fn from_engine(mut e: HashEngine) -> Self { + // pad buffer with a single 1-bit then all 0s, until there are exactly 8 bytes remaining + let n_bytes_hashed = e.bytes_hashed; + + let zeroes = [0; BLOCK_SIZE - 8]; + e.input(&[0x80]); + if incomplete_block_len(&e) > zeroes.len() { + e.input(&zeroes); + } + let pad_length = zeroes.len() - incomplete_block_len(&e); + e.input(&zeroes[..pad_length]); + debug_assert_eq!(incomplete_block_len(&e), zeroes.len()); + + e.input(&(8 * n_bytes_hashed).to_be_bytes()); + debug_assert_eq!(incomplete_block_len(&e), 0); + + Hash(e.midstate_unchecked().bytes) +} + +/// Finalize a hash engine to obtain a hash. +#[cfg(hashes_fuzz)] +pub fn from_engine(e: HashEngine) -> Self { + let mut hash = e.midstate_unchecked().bytes; + if hash == [0; 32] { + // Assume sha256 is secure and never generate 0-hashes (which represent invalid + // secp256k1 secret keys, causing downstream application breakage). + hash[0] = 1; + } + Hash(hash) +} + /// Iterate the sha256 algorithm to turn a sha256 hash into a sha256d hash #[must_use] pub fn hash_again(&self) -> sha256d::Hash { sha256d::Hash::from_byte_array(hash(&self.0).0) } diff --git a/hashes/src/sha256d/mod.rs b/hashes/src/sha256d/mod.rs index 69bb8a72a..8ec236e2c 100644 --- a/hashes/src/sha256d/mod.rs +++ b/hashes/src/sha256d/mod.rs @@ -10,7 +10,9 @@ crate::internal_macros::general_hash_type! { "Output of the SHA256d hash function." } -fn from_engine(e: HashEngine) -> Hash { +impl Hash { +/// Finalize a hash engine to produce a hash. +pub fn from_engine(e: HashEngine) -> Self { let sha2 = sha256::Hash::from_engine(e.0); let sha2d = sha256::Hash::hash(sha2.as_byte_array()); @@ -18,6 +20,7 @@ fn from_engine(e: HashEngine) -> Hash { ret.copy_from_slice(sha2d.as_byte_array()); Hash(ret) } +} /// Engine to compute SHA256d hash function. #[derive(Debug, Clone)] diff --git a/hashes/src/sha256t/mod.rs b/hashes/src/sha256t/mod.rs index d8f0a1ca0..6b38aaee4 100644 --- a/hashes/src/sha256t/mod.rs +++ b/hashes/src/sha256t/mod.rs @@ -83,7 +83,7 @@ where } /// Produces a hash from the current state of a given engine. - pub fn from_engine(e: HashEngine) -> Hash { + pub fn from_engine(e: HashEngine) -> Self { Hash::from_byte_array(sha256::Hash::from_engine(e.0).to_byte_array()) } diff --git a/hashes/src/sha384/mod.rs b/hashes/src/sha384/mod.rs index 3d6538af0..e127e1b93 100644 --- a/hashes/src/sha384/mod.rs +++ b/hashes/src/sha384/mod.rs @@ -10,11 +10,14 @@ crate::internal_macros::general_hash_type! { "Output of the SHA384 hash function." } -fn from_engine(e: HashEngine) -> Hash { +impl Hash { +/// Finalize a hash engine to produce a hash. +pub fn from_engine(e: HashEngine) -> Self { let mut ret = [0; 48]; - ret.copy_from_slice(&sha512::from_engine(e.0).as_byte_array()[..48]); + ret.copy_from_slice(&sha512::Hash::from_engine(e.0).as_byte_array()[..48]); Hash(ret) } +} /// Engine to compute SHA384 hash function. #[derive(Debug, Clone)] diff --git a/hashes/src/sha512/mod.rs b/hashes/src/sha512/mod.rs index 31203aba4..91814488e 100644 --- a/hashes/src/sha512/mod.rs +++ b/hashes/src/sha512/mod.rs @@ -20,8 +20,10 @@ crate::internal_macros::general_hash_type! { "Output of the SHA512 hash function." } +impl Hash { +/// Finalize a hash engine to produce a hash. #[cfg(not(hashes_fuzz))] -pub(crate) fn from_engine(mut e: HashEngine) -> Hash { +pub fn from_engine(mut e: HashEngine) -> Self { // pad buffer with a single 1-bit then all 0s, until there are exactly 16 bytes remaining let n_bytes_hashed = e.bytes_hashed; @@ -41,12 +43,14 @@ pub(crate) fn from_engine(mut e: HashEngine) -> Hash { Hash(e.midstate()) } +/// Finalize a hash engine to produce a hash. #[cfg(hashes_fuzz)] -pub(crate) fn from_engine(e: HashEngine) -> Hash { +pub fn from_engine(e: HashEngine) -> Self { let mut hash = e.midstate(); hash[0] ^= 0xff; // Make this distinct from SHA-256 Hash(hash) } +} pub(crate) const BLOCK_SIZE: usize = 128; diff --git a/hashes/src/sha512_256/mod.rs b/hashes/src/sha512_256/mod.rs index cf12d4604..7cd817820 100644 --- a/hashes/src/sha512_256/mod.rs +++ b/hashes/src/sha512_256/mod.rs @@ -15,11 +15,14 @@ crate::internal_macros::general_hash_type! { "Output of the SHA512/256 hash function.\n\nSHA512/256 is a hash function that uses the sha512 algorithm but it truncates the output to 256 bits. It has different initial constants than sha512 so it produces an entirely different hash compared to sha512. More information at ." } -fn from_engine(e: HashEngine) -> Hash { +impl Hash { +/// Finalize a hash engine to produce a hash. +pub fn from_engine(e: HashEngine) -> Self { let mut ret = [0; 32]; - ret.copy_from_slice(&sha512::from_engine(e.0).as_byte_array()[..32]); + ret.copy_from_slice(&sha512::Hash::from_engine(e.0).as_byte_array()[..32]); Hash(ret) } +} /// Engine to compute SHA512/256 hash function. /// diff --git a/hashes/src/siphash24/mod.rs b/hashes/src/siphash24/mod.rs index 50abe8575..61f3dcf05 100644 --- a/hashes/src/siphash24/mod.rs +++ b/hashes/src/siphash24/mod.rs @@ -12,15 +12,6 @@ crate::internal_macros::hash_type_no_default! { "Output of the SipHash24 hash function." } -#[cfg(not(hashes_fuzz))] -fn from_engine(e: HashEngine) -> Hash { Hash::from_u64(Hash::from_engine_to_u64(e)) } - -#[cfg(hashes_fuzz)] -fn from_engine(e: HashEngine) -> Hash { - let state = e.state.clone(); - Hash::from_u64(state.v0 ^ state.v1 ^ state.v2 ^ state.v3) -} - macro_rules! compress { ($state:expr) => {{ compress!($state.v0, $state.v1, $state.v2, $state.v3) @@ -176,7 +167,14 @@ impl Hash { pub fn engine(k0: u64, k1: u64) -> HashEngine { HashEngine::with_keys(k0, k1) } /// Produces a hash from the current state of a given engine. - pub fn from_engine(e: HashEngine) -> Hash { from_engine(e) } + #[cfg(not(hashes_fuzz))] + pub fn from_engine(e: HashEngine) -> Self { Hash::from_u64(Hash::from_engine_to_u64(e)) } + + #[cfg(hashes_fuzz)] + pub fn from_engine(e: HashEngine) -> Self { + let state = e.state.clone(); + Hash::from_u64(state.v0 ^ state.v1 ^ state.v2 ^ state.v3) + } /// Hashes the given data with an engine with the provided keys. pub fn hash_with_keys(k0: u64, k1: u64, data: &[u8]) -> Hash {