From 8869f35a69b606527c04aba10f0105e9c23ed8b8 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sun, 16 Jun 2024 15:34:18 +0000 Subject: [PATCH] hashes: drop the `all_zeros` method on arbitrary hashes Manually implement it for Wtxid, Txid and BlockHash, where the all-zero "hash" has a consensus meaning. But in general we should not be implementing this method unless we have a good reason to do so. It can be emulated or implemeted in terms of from_byte_array. The use of Wtxid::all_zeros is obscure and specific enough that I am tempted to drop it. But for txid and blockhash, the 0 hash appears in actual blockdata and we should keep it. All other uses of all_zeros were either in test code or in places where the specific hash was not important and [u8; 32] was a more appropriate type. --- bitcoin/src/blockdata/block.rs | 7 +++++++ bitcoin/src/blockdata/transaction.rs | 18 ++++++++++++++++++ bitcoin/src/crypto/sighash.rs | 4 ++-- bitcoin/src/merkle_tree/block.rs | 2 +- bitcoin/src/p2p/message_blockdata.rs | 3 +-- bitcoin/src/pow.rs | 6 +++--- bitcoin/src/psbt/serialize.rs | 3 ++- hashes/src/hmac.rs | 5 ----- hashes/src/internal_macros.rs | 9 --------- hashes/src/lib.rs | 7 ------- hashes/src/sha256t.rs | 7 ------- hashes/src/util.rs | 17 ++++------------- 12 files changed, 38 insertions(+), 50 deletions(-) diff --git a/bitcoin/src/blockdata/block.rs b/bitcoin/src/blockdata/block.rs index 082a0fdf1..39ca71652 100644 --- a/bitcoin/src/blockdata/block.rs +++ b/bitcoin/src/blockdata/block.rs @@ -28,6 +28,13 @@ hashes::hash_newtype! { pub struct WitnessCommitment(sha256d::Hash); } impl_hashencode!(BlockHash); +impl BlockHash { + /// The "all zeros" blockhash. + /// + /// This is not the hash of a real block. It is used as the previous blockhash + /// of the genesis block and in other placeholder contexts. + pub fn all_zeros() -> Self { Self::from_byte_array([0; 32]) } +} /// Bitcoin block header. /// diff --git a/bitcoin/src/blockdata/transaction.rs b/bitcoin/src/blockdata/transaction.rs index 6cd5619ae..b11f9c903 100644 --- a/bitcoin/src/blockdata/transaction.rs +++ b/bitcoin/src/blockdata/transaction.rs @@ -49,6 +49,24 @@ hashes::hash_newtype! { impl_hashencode!(Txid); impl_hashencode!(Wtxid); +impl Txid { + /// The "all zeros" TXID. + /// + /// This is used as the "txid" of the dummy input of a coinbase transaction. It is + /// not a real TXID and should not be used in other contexts. + pub fn all_zeros() -> Self { Self::from_byte_array([0; 32]) } +} + +impl Wtxid { + /// The "all zeros" wTXID. + /// + /// This is used as the wTXID for the coinbase transaction when constructing blocks, + /// since the coinbase transaction contains a commitment to all transactions' wTXIDs + /// but naturally cannot commit to its own. It is not a real wTXID and should not be + /// used in other contexts. + pub fn all_zeros() -> Self { Self::from_byte_array([0; 32]) } +} + /// The marker MUST be a 1-byte zero value: 0x00. (BIP-141) const SEGWIT_MARKER: u8 = 0x00; /// The flag MUST be a 1-byte non-zero value. Currently, 0x01 MUST be used. (BIP-141) diff --git a/bitcoin/src/crypto/sighash.rs b/bitcoin/src/crypto/sighash.rs index 51ef6184a..124a025fc 100644 --- a/bitcoin/src/crypto/sighash.rs +++ b/bitcoin/src/crypto/sighash.rs @@ -781,7 +781,7 @@ impl> SighashCache { value: Amount, sighash_type: EcdsaSighashType, ) -> Result<(), SigningDataError> { - let zero_hash = sha256d::Hash::all_zeros(); + let zero_hash = [0; 32]; let (sighash, anyone_can_pay) = sighash_type.split_anyonecanpay_flag(); @@ -819,7 +819,7 @@ impl> SighashCache { let hash = LegacySighash::from_engine(single_enc); writer.write_all(hash.as_byte_array())?; } else { - writer.write_all(zero_hash.as_byte_array())?; + writer.write_all(&zero_hash)?; } self.tx.borrow().lock_time.consensus_encode(writer)?; diff --git a/bitcoin/src/merkle_tree/block.rs b/bitcoin/src/merkle_tree/block.rs index 20bc1d1ed..f791f70f3 100644 --- a/bitcoin/src/merkle_tree/block.rs +++ b/bitcoin/src/merkle_tree/block.rs @@ -603,7 +603,7 @@ mod tests { // Check that it has the same merkle root as the original, and a valid one assert_eq!(merkle_root_1, merkle_root_2); - assert_ne!(merkle_root_2, TxMerkleNode::all_zeros()); + assert_ne!(merkle_root_2, TxMerkleNode::from_byte_array([0; 32])); // check that it contains the matched transactions (in the same order!) assert_eq!(match_txid1, match_txid2); diff --git a/bitcoin/src/p2p/message_blockdata.rs b/bitcoin/src/p2p/message_blockdata.rs index d786f8274..44ae0fa68 100644 --- a/bitcoin/src/p2p/message_blockdata.rs +++ b/bitcoin/src/p2p/message_blockdata.rs @@ -5,7 +5,6 @@ //! This module describes network messages which are used for passing //! Bitcoin data (blocks and transactions) around. -use hashes::sha256d; use io::{BufRead, Write}; use crate::block::BlockHash; @@ -67,7 +66,7 @@ impl Encodable for Inventory { }; } Ok(match *self { - Inventory::Error => encode_inv!(0, sha256d::Hash::all_zeros()), + Inventory::Error => encode_inv!(0, [0; 32]), Inventory::Transaction(ref t) => encode_inv!(1, t), Inventory::Block(ref b) => encode_inv!(2, b), Inventory::CompactBlock(ref b) => encode_inv!(4, b), diff --git a/bitcoin/src/pow.rs b/bitcoin/src/pow.rs index bc5fb076c..0c9dec5cf 100644 --- a/bitcoin/src/pow.rs +++ b/bitcoin/src/pow.rs @@ -1795,7 +1795,7 @@ mod tests { let current = Header { version: Version::ONE, prev_blockhash: BlockHash::all_zeros(), - merkle_root: TxMerkleNode::all_zeros(), + merkle_root: TxMerkleNode::from_byte_array([0; 32]), time: 1599332177, bits: epoch_start.bits, nonce: epoch_start.nonce, @@ -1817,7 +1817,7 @@ mod tests { let epoch_start = Header { version: Version::ONE, prev_blockhash: BlockHash::all_zeros(), - merkle_root: TxMerkleNode::all_zeros(), + merkle_root: TxMerkleNode::from_byte_array([0; 32]), time: 1599332844, bits: starting_bits, nonce: 0, @@ -1827,7 +1827,7 @@ mod tests { let current = Header { version: Version::ONE, prev_blockhash: BlockHash::all_zeros(), - merkle_root: TxMerkleNode::all_zeros(), + merkle_root: TxMerkleNode::from_byte_array([0; 32]), time: 1600591200, bits: starting_bits, nonce: 0, diff --git a/bitcoin/src/psbt/serialize.rs b/bitcoin/src/psbt/serialize.rs index 4ad15fa5f..d3659aed6 100644 --- a/bitcoin/src/psbt/serialize.rs +++ b/bitcoin/src/psbt/serialize.rs @@ -404,6 +404,7 @@ mod tests { #[test] fn taptree_hidden() { + let dummy_hash = TapNodeHash::from_byte_array([0x12; 32]); let mut builder = compose_taproot_builder(0x51, &[2, 2, 2]); builder = builder .add_leaf_with_ver( @@ -412,7 +413,7 @@ mod tests { LeafVersion::from_consensus(0xC2).unwrap(), ) .unwrap(); - builder = builder.add_hidden_node(3, TapNodeHash::all_zeros()).unwrap(); + builder = builder.add_hidden_node(3, dummy_hash).unwrap(); assert!(TapTree::try_from(builder).is_err()); } diff --git a/hashes/src/hmac.rs b/hashes/src/hmac.rs index 90745c6a6..80268f489 100644 --- a/hashes/src/hmac.rs +++ b/hashes/src/hmac.rs @@ -147,11 +147,6 @@ impl Hash for Hmac { 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 912ace246..41006c4df 100644 --- a/hashes/src/internal_macros.rs +++ b/hashes/src/internal_macros.rs @@ -119,8 +119,6 @@ macro_rules! hash_trait_impls { fn as_byte_array(&self) -> &Self::Bytes { self.as_byte_array() } fn from_byte_array(bytes: Self::Bytes) -> Self { Self::from_byte_array(bytes) } - - fn all_zeros() -> Self { Self::all_zeros() } } } } @@ -218,13 +216,6 @@ macro_rules! hash_type { pub const fn from_byte_array(bytes: [u8; $bits / 8]) -> Self { Self::internal_new(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::internal_new([0x00; $bits / 8]) } } #[cfg(feature = "schemars")] diff --git a/hashes/src/lib.rs b/hashes/src/lib.rs index df3325bd1..f12f7de8c 100644 --- a/hashes/src/lib.rs +++ b/hashes/src/lib.rs @@ -214,13 +214,6 @@ pub trait Hash: /// 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/sha256t.rs b/hashes/src/sha256t.rs index a315f7f32..20924d633 100644 --- a/hashes/src/sha256t.rs +++ b/hashes/src/sha256t.rs @@ -117,13 +117,6 @@ where /// Constructs a hash from the underlying byte array. pub const fn from_byte_array(bytes: [u8; 32]) -> Self { Self::internal_new(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 fn all_zeros() -> Self { Hash::internal_new([0x00; 32]) } } impl Copy for Hash {} diff --git a/hashes/src/util.rs b/hashes/src/util.rs index fd9589e32..51f25b4a2 100644 --- a/hashes/src/util.rs +++ b/hashes/src/util.rs @@ -263,17 +263,6 @@ macro_rules! hash_newtype { pub const fn from_byte_array(bytes: <$hash as $crate::Hash>::Bytes) -> Self { $newtype(<$hash>::from_byte_array(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 fn all_zeros() -> Self { - let zeros = <$hash>::all_zeros(); - $newtype(zeros) - } - } impl $crate::_export::_core::convert::From<$hash> for $newtype { @@ -309,8 +298,6 @@ macro_rules! hash_newtype { fn as_byte_array(&self) -> &Self::Bytes { self.as_byte_array() } fn from_byte_array(bytes: Self::Bytes) -> Self { Self::from_byte_array(bytes) } - - fn all_zeros() -> Self { Self::all_zeros() } } impl $crate::_export::_core::str::FromStr for $newtype { @@ -462,6 +449,10 @@ mod test { struct TestHash(crate::sha256d::Hash); } + impl TestHash { + fn all_zeros() -> Self { Self::from_byte_array([0; 32]) } + } + #[test] fn display() { let want = "0000000000000000000000000000000000000000000000000000000000000000";