From 200ff473276f62866f0db130ecb485c905714aaa Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 21 Mar 2025 09:07:00 +1100 Subject: [PATCH 1/3] Use compute_merkle_root Remove manual implementation of merkle root calculation and just use the function we already have. Refactor only, no logic change. --- bitcoin/src/blockdata/constants.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/bitcoin/src/blockdata/constants.rs b/bitcoin/src/blockdata/constants.rs index 6e8ef036e..60d528862 100644 --- a/bitcoin/src/blockdata/constants.rs +++ b/bitcoin/src/blockdata/constants.rs @@ -6,8 +6,6 @@ //! consensus code. In particular, it defines the genesis block and its //! single transaction. -use hashes::sha256d; - use crate::block::{self, Block, Checked}; use crate::internal_macros::{impl_array_newtype, impl_array_newtype_stringify}; use crate::locktime::absolute; @@ -124,8 +122,7 @@ fn bitcoin_genesis_tx(params: &Params) -> Transaction { pub fn genesis_block(params: impl AsRef) -> Block { let params = params.as_ref(); let transactions = vec![bitcoin_genesis_tx(params)]; - let hash: sha256d::Hash = transactions[0].compute_txid().into(); - let merkle_root: crate::TxMerkleNode = hash.into(); + let merkle_root = block::compute_merkle_root(&transactions).expect("transactions is not empty"); let witness_root = block::compute_witness_root(&transactions); match params.network { From 6b2b89c2f71e9bc4c33f0a7385a2c1fab3bb59ec Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 26 Feb 2025 14:13:32 +1100 Subject: [PATCH 2/3] Remove From for not-general-hash types The `hash_newtype` macro is explicitly designed to produce a hash that is not a general purpose hash type to try and prevent users hashing arbitrary stuff with it. E.g., `Txid` isn't meant to be just hash arbitrary data. However we provide a `From` impl that will convert any instance of the inner hash type into the new type. This kind of defeats the purpose. We provide `from_byte_array` and `to_byte_array` to allow folk to 'cast' from one hash type to another if they really want to and its ugly on purpose. Also, it is becoming apparent that we may be able to remove the `hashes` crate from the public API of `primitives` allowing us to stabalise `primitives` without stabalising `hashes`. For both these reasons remove the `From` impl from the `hash_newtype` macro. Note that deprecating doesn't seem to work so we just delete it. --- bitcoin/src/p2p/message.rs | 41 +++++++++++++++++++++----------------- hashes/src/macros.rs | 7 ------- 2 files changed, 23 insertions(+), 25 deletions(-) diff --git a/bitcoin/src/p2p/message.rs b/bitcoin/src/p2p/message.rs index d02483a7f..e9522a7c5 100644 --- a/bitcoin/src/p2p/message.rs +++ b/bitcoin/src/p2p/message.rs @@ -702,7 +702,8 @@ mod test { use super::*; use crate::bip152::BlockTransactionsRequest; - use crate::block::Block; + use crate::bip158::{FilterHeader, FilterHash}; + use crate::block::{Block, BlockHash}; use crate::consensus::encode::{deserialize, deserialize_partial, serialize}; use crate::p2p::address::AddrV2; use crate::p2p::message_blockdata::{GetBlocksMessage, GetHeadersMessage, Inventory}; @@ -714,7 +715,7 @@ mod test { use crate::p2p::message_network::{Reject, RejectReason, VersionMessage}; use crate::p2p::ServiceFlags; use crate::script::ScriptBuf; - use crate::transaction::Transaction; + use crate::transaction::{Transaction, Txid}; fn hash(array: [u8; 32]) -> sha256d::Hash { sha256d::Hash::from_byte_array(array) } @@ -737,16 +738,20 @@ mod test { 45, Address::new(&([123, 255, 000, 100], 833).into(), ServiceFlags::NETWORK), )]), - NetworkMessage::Inv(vec![Inventory::Block(hash([8u8; 32]).into())]), - NetworkMessage::GetData(vec![Inventory::Transaction(hash([45u8; 32]).into())]), + NetworkMessage::Inv(vec![Inventory::Block(BlockHash::from_byte_array(hash([8u8; 32]).to_byte_array()))]), + NetworkMessage::GetData(vec![Inventory::Transaction(Txid::from_byte_array(hash([45u8; 32]).to_byte_array()))]), NetworkMessage::NotFound(vec![Inventory::Error([0u8; 32])]), NetworkMessage::GetBlocks(GetBlocksMessage::new( - vec![hash([1u8; 32]).into(), hash([4u8; 32]).into()], - hash([5u8; 32]).into(), + vec![ + BlockHash::from_byte_array(hash([1u8; 32]).to_byte_array()), + BlockHash::from_byte_array(hash([4u8; 32]).to_byte_array())], + BlockHash::from_byte_array(hash([5u8; 32]).to_byte_array()), )), NetworkMessage::GetHeaders(GetHeadersMessage::new( - vec![hash([10u8; 32]).into(), hash([40u8; 32]).into()], - hash([50u8; 32]).into(), + vec![ + BlockHash::from_byte_array(hash([10u8; 32]).to_byte_array()), + BlockHash::from_byte_array(hash([40u8; 32]).to_byte_array())], + BlockHash::from_byte_array(hash([50u8; 32]).to_byte_array()), )), NetworkMessage::MemPool, NetworkMessage::Tx(tx), @@ -771,32 +776,32 @@ mod test { NetworkMessage::GetCFilters(GetCFilters { filter_type: 2, start_height: BlockHeight::from(52), - stop_hash: hash([42u8; 32]).into(), + stop_hash: BlockHash::from_byte_array(hash([42u8; 32]).to_byte_array()), }), NetworkMessage::CFilter(CFilter { filter_type: 7, - block_hash: hash([25u8; 32]).into(), + block_hash: BlockHash::from_byte_array(hash([25u8; 32]).to_byte_array()), filter: vec![1, 2, 3], }), NetworkMessage::GetCFHeaders(GetCFHeaders { filter_type: 4, start_height: BlockHeight::from(102), - stop_hash: hash([47u8; 32]).into(), + stop_hash: BlockHash::from_byte_array(hash([47u8; 32]).to_byte_array()), }), NetworkMessage::CFHeaders(CFHeaders { filter_type: 13, - stop_hash: hash([53u8; 32]).into(), - previous_filter_header: hash([12u8; 32]).into(), - filter_hashes: vec![hash([4u8; 32]).into(), hash([12u8; 32]).into()], + stop_hash: BlockHash::from_byte_array(hash([53u8; 32]).to_byte_array()), + previous_filter_header: FilterHeader::from_byte_array(hash([12u8; 32]).to_byte_array()), + filter_hashes: vec![FilterHash::from_byte_array(hash([4u8; 32]).to_byte_array()), FilterHash::from_byte_array(hash([12u8; 32]).to_byte_array())], }), NetworkMessage::GetCFCheckpt(GetCFCheckpt { filter_type: 17, - stop_hash: hash([25u8; 32]).into(), + stop_hash: BlockHash::from_byte_array(hash([25u8; 32]).to_byte_array()), }), NetworkMessage::CFCheckpt(CFCheckpt { filter_type: 27, - stop_hash: hash([77u8; 32]).into(), - filter_headers: vec![hash([3u8; 32]).into(), hash([99u8; 32]).into()], + stop_hash: BlockHash::from_byte_array(hash([77u8; 32]).to_byte_array()), + filter_headers: vec![FilterHeader::from_byte_array(hash([3u8; 32]).to_byte_array()), FilterHeader::from_byte_array(hash([99u8; 32]).to_byte_array())], }), NetworkMessage::Alert(vec![45, 66, 3, 2, 6, 8, 9, 12, 3, 130]), NetworkMessage::Reject(Reject { @@ -817,7 +822,7 @@ mod test { NetworkMessage::CmpctBlock(cmptblock), NetworkMessage::GetBlockTxn(GetBlockTxn { txs_request: BlockTransactionsRequest { - block_hash: hash([11u8; 32]).into(), + block_hash: BlockHash::from_byte_array(hash([11u8; 32]).to_byte_array()), indexes: vec![0, 1, 2, 3, 10, 3002], }, }), diff --git a/hashes/src/macros.rs b/hashes/src/macros.rs index 070b72728..f1b06727c 100644 --- a/hashes/src/macros.rs +++ b/hashes/src/macros.rs @@ -158,13 +158,6 @@ macro_rules! hash_newtype { } } - 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)` - Self { 0: inner } - } - } - impl $crate::_export::_core::convert::From<$newtype> for $hash { fn from(hashtype: $newtype) -> $hash { hashtype.0 From db9ec3bed8d6164a0345ba8db1e2162626db7cc5 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Thu, 20 Mar 2025 14:34:59 +1100 Subject: [PATCH 3/3] Remove From for $hash We provide the from/to_byte_array functions for casting between arrays. We shouldn't be supporting calls to `into` to quickly do the cast. We already removed the other direction, now remove this one. --- hashes/src/macros.rs | 6 ------ primitives/src/transaction.rs | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/hashes/src/macros.rs b/hashes/src/macros.rs index f1b06727c..2c9aa1bcf 100644 --- a/hashes/src/macros.rs +++ b/hashes/src/macros.rs @@ -158,12 +158,6 @@ macro_rules! hash_newtype { } } - impl $crate::_export::_core::convert::From<$newtype> for $hash { - fn from(hashtype: $newtype) -> $hash { - hashtype.0 - } - } - impl $crate::Hash for $newtype { type Bytes = <$hash as $crate::Hash>::Bytes; diff --git a/primitives/src/transaction.rs b/primitives/src/transaction.rs index 764ed68c6..1c79a187d 100644 --- a/primitives/src/transaction.rs +++ b/primitives/src/transaction.rs @@ -151,7 +151,7 @@ impl Transaction { .collect(), output: self.output.clone(), }; - cloned_tx.compute_txid().into() + sha256d::Hash::from_byte_array(cloned_tx.compute_txid().to_byte_array()) } /// Computes the [`Txid`].