Merge rust-bitcoin/rust-bitcoin#4128: Remove `From<hash>` for not-general-hash types

db9ec3bed8 Remove From<newtype> for $hash (Tobin C. Harding)
6b2b89c2f7 Remove From<hash> for not-general-hash types (Tobin C. Harding)
200ff47327 Use compute_merkle_root (Tobin C. Harding)

Pull request description:

  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.

ACKs for top commit:
  Kixunil:
    ACK db9ec3bed8
  apoelstra:
    ACK db9ec3bed8d6164a0345ba8db1e2162626db7cc5; successfully ran local tests

Tree-SHA512: 90bc325821cd2d72bbaef5b3cfef2d299192d1e7999cd4f96b6b69b8872e419964e431e91674c59bfdd2e9a5959dbc13ee89d5f87d03e96785044c616db19d72

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<Params>) -> Block<Checked> {
     let params = params.as_ref();
     let transactions = vec![bitcoin_genesis_tx(params)];
-    let hash: sha256d::Hash = transactions[0].compute_txid().into();
+    let merkle_root = block::compute_merkle_root(&transactions).expect("transactions is not empty");
-    let merkle_root: crate::TxMerkleNode = hash.into();
     let witness_root = block::compute_witness_root(&transactions);
 
     match params.network {

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::Inv(vec![Inventory::Block(BlockHash::from_byte_array(hash([8u8; 32]).to_byte_array()))]),
-            NetworkMessage::GetData(vec![Inventory::Transaction(hash([45u8; 32]).into())]),
+            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()],
+                vec![
-                hash([5u8; 32]).into(),
+                    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()],
+                vec![
-                hash([50u8; 32]).into(),
+                    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(),
+                stop_hash: BlockHash::from_byte_array(hash([53u8; 32]).to_byte_array()),
-                previous_filter_header: hash([12u8; 32]).into(),
+                previous_filter_header: FilterHeader::from_byte_array(hash([12u8; 32]).to_byte_array()),
-                filter_hashes: vec![hash([4u8; 32]).into(), hash([12u8; 32]).into()],
+                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(),
+                stop_hash: BlockHash::from_byte_array(hash([77u8; 32]).to_byte_array()),
-                filter_headers: vec![hash([3u8; 32]).into(), hash([99u8; 32]).into()],
+                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],
                 },
             }),

hashes/src/macros.rs

@@ -158,19 +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
-            }
-        }
-
         impl $crate::Hash for $newtype {
             type Bytes = <$hash as $crate::Hash>::Bytes;
 

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`].