From 6b2b89c2f71e9bc4c33f0a7385a2c1fab3bb59ec Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 26 Feb 2025 14:13:32 +1100 Subject: [PATCH] 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