From 8a8be4ee886206634970acbc25baf5766daa71b1 Mon Sep 17 00:00:00 2001 From: Divyansh Gupta Date: Sat, 30 Mar 2024 22:27:13 +0530 Subject: [PATCH] refactor: move `all_zeroes` & `from_byte_array` to inherents & make them const Made these functions const fn & move them from the `Hash` trait to `hash_type` & `hash_newtype` macros fix #2377 --- bitcoin/examples/sign-tx-segwit-v0.rs | 1 - bitcoin/src/bip152.rs | 4 ++-- bitcoin/src/blockdata/constants.rs | 19 +++++++-------- bitcoin/src/blockdata/transaction.rs | 2 +- bitcoin/src/internal_macros.rs | 1 - bitcoin/src/merkle_tree/mod.rs | 2 +- bitcoin/src/p2p/message_blockdata.rs | 6 ++--- hashes/src/hmac.rs | 7 ------ hashes/src/internal_macros.rs | 24 +++++++++++-------- hashes/src/lib.rs | 10 -------- hashes/src/sha256.rs | 4 ++-- hashes/src/sha256t.rs | 12 ++++++++++ hashes/src/util.rs | 33 +++++++++++++++++---------- 13 files changed, 66 insertions(+), 59 deletions(-) diff --git a/bitcoin/examples/sign-tx-segwit-v0.rs b/bitcoin/examples/sign-tx-segwit-v0.rs index 6a8879d336..13f472eb84 100644 --- a/bitcoin/examples/sign-tx-segwit-v0.rs +++ b/bitcoin/examples/sign-tx-segwit-v0.rs @@ -4,7 +4,6 @@ use std::str::FromStr; -use bitcoin::hashes::Hash; use bitcoin::locktime::absolute; use bitcoin::secp256k1::{rand, Message, Secp256k1, SecretKey, Signing}; use bitcoin::sighash::{EcdsaSighashType, SighashCache}; diff --git a/bitcoin/src/bip152.rs b/bitcoin/src/bip152.rs index db871dfa5a..8918397cf5 100644 --- a/bitcoin/src/bip152.rs +++ b/bitcoin/src/bip152.rs @@ -471,7 +471,7 @@ mod test { { // test serialization let raw: Vec = serialize(&BlockTransactionsRequest { - block_hash: Hash::all_zeros(), + block_hash: BlockHash::all_zeros(), indexes: testcase.1, }); let mut expected_raw: Vec = [0u8; 32].to_vec(); @@ -494,7 +494,7 @@ mod test { #[should_panic] // 'attempt to add with overflow' in consensus_encode() fn test_getblocktx_panic_when_encoding_u64_max() { serialize(&BlockTransactionsRequest { - block_hash: Hash::all_zeros(), + block_hash: BlockHash::all_zeros(), indexes: vec![core::u64::MAX], }); } diff --git a/bitcoin/src/blockdata/constants.rs b/bitcoin/src/blockdata/constants.rs index 1f8de58b1f..7885bd9af3 100644 --- a/bitcoin/src/blockdata/constants.rs +++ b/bitcoin/src/blockdata/constants.rs @@ -20,7 +20,7 @@ use crate::blockdata::witness::Witness; use crate::internal_macros::impl_bytes_newtype; use crate::network::Network; use crate::pow::CompactTarget; -use crate::Amount; +use crate::{Amount, BlockHash}; /// How many seconds between blocks we expect on average. pub const TARGET_BLOCK_SPACING: u32 = 600; @@ -92,7 +92,7 @@ pub fn genesis_block(network: Network) -> Block { Network::Bitcoin => Block { header: block::Header { version: block::Version::ONE, - prev_blockhash: Hash::all_zeros(), + prev_blockhash: BlockHash::all_zeros(), merkle_root, time: 1231006505, bits: CompactTarget::from_consensus(0x1d00ffff), @@ -103,7 +103,7 @@ pub fn genesis_block(network: Network) -> Block { Network::Testnet => Block { header: block::Header { version: block::Version::ONE, - prev_blockhash: Hash::all_zeros(), + prev_blockhash: BlockHash::all_zeros(), merkle_root, time: 1296688602, bits: CompactTarget::from_consensus(0x1d00ffff), @@ -114,7 +114,7 @@ pub fn genesis_block(network: Network) -> Block { Network::Signet => Block { header: block::Header { version: block::Version::ONE, - prev_blockhash: Hash::all_zeros(), + prev_blockhash: BlockHash::all_zeros(), merkle_root, time: 1598918400, bits: CompactTarget::from_consensus(0x1e0377ae), @@ -125,7 +125,7 @@ pub fn genesis_block(network: Network) -> Block { Network::Regtest => Block { header: block::Header { version: block::Version::ONE, - prev_blockhash: Hash::all_zeros(), + prev_blockhash: BlockHash::all_zeros(), merkle_root, time: 1296688602, bits: CompactTarget::from_consensus(0x207fffff), @@ -188,6 +188,7 @@ mod test { use super::*; use crate::consensus::encode::serialize; + use crate::Txid; #[test] fn bitcoin_genesis_first_transaction() { @@ -195,7 +196,7 @@ mod test { assert_eq!(gen.version, transaction::Version::ONE); assert_eq!(gen.input.len(), 1); - assert_eq!(gen.input[0].previous_output.txid, Hash::all_zeros()); + assert_eq!(gen.input[0].previous_output.txid, Txid::all_zeros()); assert_eq!(gen.input[0].previous_output.vout, 0xFFFFFFFF); assert_eq!(serialize(&gen.input[0].script_sig), hex!("4d04ffff001d0104455468652054696d65732030332f4a616e2f32303039204368616e63656c6c6f72206f6e206272696e6b206f66207365636f6e64206261696c6f757420666f722062616e6b73")); @@ -218,7 +219,7 @@ mod test { let gen = genesis_block(Network::Bitcoin); assert_eq!(gen.header.version, block::Version::ONE); - assert_eq!(gen.header.prev_blockhash, Hash::all_zeros()); + assert_eq!(gen.header.prev_blockhash, BlockHash::all_zeros()); assert_eq!( gen.header.merkle_root.to_string(), "4a5e1e4baab89f3a32518a88c31bc87f618f76673e2cc77ab2127b7afdeda33b" @@ -237,7 +238,7 @@ mod test { fn testnet_genesis_full_block() { let gen = genesis_block(Network::Testnet); assert_eq!(gen.header.version, block::Version::ONE); - assert_eq!(gen.header.prev_blockhash, Hash::all_zeros()); + assert_eq!(gen.header.prev_blockhash, BlockHash::all_zeros()); assert_eq!( gen.header.merkle_root.to_string(), "4a5e1e4baab89f3a32518a88c31bc87f618f76673e2cc77ab2127b7afdeda33b" @@ -255,7 +256,7 @@ mod test { fn signet_genesis_full_block() { let gen = genesis_block(Network::Signet); assert_eq!(gen.header.version, block::Version::ONE); - assert_eq!(gen.header.prev_blockhash, Hash::all_zeros()); + assert_eq!(gen.header.prev_blockhash, BlockHash::all_zeros()); assert_eq!( gen.header.merkle_root.to_string(), "4a5e1e4baab89f3a32518a88c31bc87f618f76673e2cc77ab2127b7afdeda33b" diff --git a/bitcoin/src/blockdata/transaction.rs b/bitcoin/src/blockdata/transaction.rs index 58443b446f..66f405c725 100644 --- a/bitcoin/src/blockdata/transaction.rs +++ b/bitcoin/src/blockdata/transaction.rs @@ -84,7 +84,7 @@ impl OutPoint { /// /// This value is used for coinbase transactions because they don't have any previous outputs. #[inline] - pub fn null() -> OutPoint { OutPoint { txid: Hash::all_zeros(), vout: u32::MAX } } + pub fn null() -> OutPoint { OutPoint { txid: Txid::all_zeros(), vout: u32::MAX } } /// Checks if an `OutPoint` is "null". /// diff --git a/bitcoin/src/internal_macros.rs b/bitcoin/src/internal_macros.rs index 49c8cf5ed7..37c8b249c7 100644 --- a/bitcoin/src/internal_macros.rs +++ b/bitcoin/src/internal_macros.rs @@ -194,7 +194,6 @@ macro_rules! impl_hashencode { impl $crate::consensus::Decodable for $hashtype { fn consensus_decode(r: &mut R) -> core::result::Result { - use $crate::hashes::Hash; Ok(Self::from_byte_array(<<$hashtype as $crate::hashes::Hash>::Bytes>::consensus_decode(r)?)) } } diff --git a/bitcoin/src/merkle_tree/mod.rs b/bitcoin/src/merkle_tree/mod.rs index c3929d4206..5b738717b6 100644 --- a/bitcoin/src/merkle_tree/mod.rs +++ b/bitcoin/src/merkle_tree/mod.rs @@ -127,7 +127,7 @@ mod tests { let hashes_iter = block.txdata.iter().map(|obj| obj.compute_txid().to_raw_hash()); - let mut hashes_array: [sha256d::Hash; 15] = [Hash::all_zeros(); 15]; + let mut hashes_array: [sha256d::Hash; 15] = [sha256d::Hash::all_zeros(); 15]; for (i, hash) in hashes_iter.clone().enumerate() { hashes_array[i] = hash; } diff --git a/bitcoin/src/p2p/message_blockdata.rs b/bitcoin/src/p2p/message_blockdata.rs index 8719572aa3..c6cff13b72 100644 --- a/bitcoin/src/p2p/message_blockdata.rs +++ b/bitcoin/src/p2p/message_blockdata.rs @@ -145,11 +145,11 @@ impl_consensus_encoding!(GetHeadersMessage, version, locator_hashes, stop_hash); #[cfg(test)] mod tests { - use hashes::Hash; use hex::test_hex_unwrap as hex; use super::{GetBlocksMessage, GetHeadersMessage}; use crate::consensus::encode::{deserialize, serialize}; + use crate::BlockHash; #[test] fn getblocks_message_test() { @@ -162,7 +162,7 @@ mod tests { assert_eq!(real_decode.version, 70002); assert_eq!(real_decode.locator_hashes.len(), 1); assert_eq!(serialize(&real_decode.locator_hashes[0]), genhash); - assert_eq!(real_decode.stop_hash, Hash::all_zeros()); + assert_eq!(real_decode.stop_hash, BlockHash::all_zeros()); assert_eq!(serialize(&real_decode), from_sat); } @@ -178,7 +178,7 @@ mod tests { assert_eq!(real_decode.version, 70002); assert_eq!(real_decode.locator_hashes.len(), 1); assert_eq!(serialize(&real_decode.locator_hashes[0]), genhash); - assert_eq!(real_decode.stop_hash, Hash::all_zeros()); + assert_eq!(real_decode.stop_hash, BlockHash::all_zeros()); assert_eq!(serialize(&real_decode), from_sat); } diff --git a/hashes/src/hmac.rs b/hashes/src/hmac.rs index 32b967a5f5..372f2d3810 100644 --- a/hashes/src/hmac.rs +++ b/hashes/src/hmac.rs @@ -171,13 +171,6 @@ impl Hash for Hmac { fn to_byte_array(self) -> Self::Bytes { self.0.to_byte_array() } fn as_byte_array(&self) -> &Self::Bytes { self.0.as_byte_array() } - - fn from_byte_array(bytes: T::Bytes) -> Self { Hmac(T::from_byte_array(bytes)) } - - fn all_zeros() -> Self { - let zeros = T::all_zeros(); - Hmac(zeros) - } } #[cfg(feature = "serde")] diff --git a/hashes/src/internal_macros.rs b/hashes/src/internal_macros.rs index 9b3f9dde6f..98d31109e4 100644 --- a/hashes/src/internal_macros.rs +++ b/hashes/src/internal_macros.rs @@ -89,13 +89,13 @@ macro_rules! hash_trait_impls { impl<$($gen: $gent),*> str::FromStr for Hash<$($gen),*> { type Err = $crate::hex::HexToArrayError; fn from_str(s: &str) -> $crate::_export::_core::result::Result { - use $crate::{Hash, hex::{FromHex}}; + use $crate::{ hex::{FromHex}}; let mut bytes = <[u8; $bits / 8]>::from_hex(s)?; if $reverse { bytes.reverse(); } - Ok(Self::from_byte_array(bytes)) + Ok(Hash::from_byte_array(bytes)) } } @@ -150,14 +150,6 @@ macro_rules! hash_trait_impls { fn as_byte_array(&self) -> &Self::Bytes { &self.0 } - - fn from_byte_array(bytes: Self::Bytes) -> Self { - Self::internal_new(bytes) - } - - fn all_zeros() -> Self { - Hash::internal_new([0x00; $bits / 8]) - } } } } @@ -188,6 +180,18 @@ macro_rules! hash_type { fn internal_new(arr: [u8; $bits / 8]) -> Self { Hash(arr) } fn internal_engine() -> HashEngine { Default::default() } + + /// Constructs a hash from the underlying byte array. + pub const fn from_byte_array(bytes: ::Bytes) -> Self { + Hash(bytes) + } + + /// Returns an all zero hash. + /// + /// An all zeros hash is a made up construct because there is not a known input that can create + /// it. However,it is used in various places in Bitcoin e.g., the Bitcoin genesis block's + /// previous blockhash and the coinbase transaction's outpoint txid. + pub const fn all_zeros() -> Self { Hash([0x00; $bits / 8]) } } #[cfg(feature = "schemars")] diff --git a/hashes/src/lib.rs b/hashes/src/lib.rs index d0805e9a48..7952721773 100644 --- a/hashes/src/lib.rs +++ b/hashes/src/lib.rs @@ -221,16 +221,6 @@ pub trait Hash: /// Returns a reference to the underlying byte array. fn as_byte_array(&self) -> &Self::Bytes; - - /// Constructs a hash from the underlying byte array. - fn from_byte_array(bytes: Self::Bytes) -> Self; - - /// Returns an all zero hash. - /// - /// An all zeros hash is a made up construct because there is not a known input that can create - /// it, however it is used in various places in Bitcoin e.g., the Bitcoin genesis block's - /// previous blockhash and the coinbase transaction's outpoint txid. - fn all_zeros() -> Self; } /// Attempted to create a hash from an invalid length slice. diff --git a/hashes/src/sha256.rs b/hashes/src/sha256.rs index 63e214a91f..205f778afe 100644 --- a/hashes/src/sha256.rs +++ b/hashes/src/sha256.rs @@ -102,9 +102,9 @@ impl crate::HashEngine for HashEngine { impl Hash { /// Iterate the sha256 algorithm to turn a sha256 hash into a sha256d hash pub fn hash_again(&self) -> sha256d::Hash { - crate::Hash::from_byte_array(::hash(&self.0).0) + let sha256_hash = Hash::from_byte_array(::hash(&self.0).0); + sha256d::Hash::from_byte_array(sha256_hash.0) } - /// Computes hash from `bytes` in `const` context. /// /// Warning: this function is inefficient. It should be only used in `const` context. diff --git a/hashes/src/sha256t.rs b/hashes/src/sha256t.rs index 1a613cd396..942f826fa6 100644 --- a/hashes/src/sha256t.rs +++ b/hashes/src/sha256t.rs @@ -41,6 +41,18 @@ impl Hash { fn internal_new(arr: [u8; 32]) -> Self { Hash(arr, Default::default()) } fn internal_engine() -> HashEngine { T::engine() } + + /// Constructs a hash from the underlying byte array. + pub const fn from_byte_array(bytes: ::Bytes) -> Self { + Hash(bytes, PhantomData) + } + + /// Returns an all zero hash. + /// + /// An all zeros hash is a made up construct because there is not a known input that can create + /// it. However,it is used in various places in Bitcoin e.g., the Bitcoin genesis block's + /// previous blockhash and the coinbase transaction's outpoint txid. + pub const fn all_zeros() -> Self { Hash([0x00; 32], PhantomData) } } impl Copy for Hash {} diff --git a/hashes/src/util.rs b/hashes/src/util.rs index 79ac4d0526..cb17f4e868 100644 --- a/hashes/src/util.rs +++ b/hashes/src/util.rs @@ -210,8 +210,28 @@ macro_rules! hash_newtype { pub fn as_raw_hash(&self) -> &$hash { &self.0 } + + #[allow(unused)] + /// Constructs a hash from the underlying byte array. + pub const fn from_byte_array(bytes: ::Bytes) -> Self { + $newtype(<$hash>::from_byte_array(bytes)) + } + + + #[allow(unused)] + /// Returns an all zero hash. + /// + /// An all zeros hash is a made up construct because there is not a known input that can create + /// it. However,it is used in various places in Bitcoin e.g., the Bitcoin genesis block's + /// previous blockhash and the coinbase transaction's outpoint txid. + pub const fn all_zeros() -> Self { + let zeros = <$hash>::all_zeros(); + $newtype(zeros) + } } + + impl $crate::_export::_core::convert::From<$hash> for $newtype { fn from(inner: $hash) -> $newtype { // Due to rust 1.22 we have to use this instead of simple `Self(inner)` @@ -245,11 +265,6 @@ macro_rules! hash_newtype { Ok($newtype(<$hash as $crate::Hash>::from_slice(sl)?)) } - #[inline] - fn from_byte_array(bytes: Self::Bytes) -> Self { - $newtype(<$hash as $crate::Hash>::from_byte_array(bytes)) - } - #[inline] fn to_byte_array(self) -> Self::Bytes { self.0.to_byte_array() @@ -259,18 +274,12 @@ macro_rules! hash_newtype { fn as_byte_array(&self) -> &Self::Bytes { self.0.as_byte_array() } - - #[inline] - fn all_zeros() -> Self { - let zeros = <$hash>::all_zeros(); - $newtype(zeros) - } } impl $crate::_export::_core::str::FromStr for $newtype { type Err = $crate::hex::HexToArrayError; fn from_str(s: &str) -> $crate::_export::_core::result::Result<$newtype, Self::Err> { - use $crate::{Hash, hex::FromHex}; + use $crate::hex::FromHex; let mut bytes = <[u8; ::LEN]>::from_hex(s)?; if ::DISPLAY_BACKWARD {