Merge rust-bitcoin#5346: primitives: Move MerkleNode and TxIdentifier into primitives

a874123 primitives: Move MerkleNode and TxIdentifier into primitives (Mitchell Bagot)

Pull request description:

  The MerkleNode trait in bitcoin provides convenience functions for the TxMerkleNode and WitnessMerkleNode hash types to work with the nodes as a tree. Moving the functionality of this trait into primitives acts as a precursor to allow other functionality to move also.
  
  Move the MerkleNode trait to primitives, making it private. Provide the same interface for users through pub functions that wrap the private trait. Further, move TxIdentifier to primitives and re-export from bitcoin.


ACKs for top commit:
  tcharding:
    ACK a874123
  apoelstra:
    ACK a874123; successfully ran local tests


Tree-SHA512: 41fef68ebfb01e04b4258d62f35ad078ac4853537d3c27e3ca6f9121b5454285179ee91b10512e22ab310833a88b6be2759b202c878e2d2dab362496f51dfbca

Author: apoelstra

.cargo/mutants.toml

@@ -50,6 +50,7 @@ exclude_re = [
     "primitives/.* <impl Encoder for .*Encoder<'_>>::advance", # Replacing the return with true causes an infinite loop.
     "primitives/.* <impl Decoder for WitnessDecoder>::push_bytes", # Replacing == with != causes an infinite loop
     "primitives/.* WitnessDecoder::resize_if_needed", # Replacing *= with += still resizes the buffer making the mutant untestable.
+    "primitives/.* replace \\+ with \\* in MerkleNode::calculate_root", # Replacing + with * causes an infinite loop
 
     # consensus_encoding - most of these are for mutations in the logic used to determine when to stop encoding or decoding.
     "consensus_encoding/.* <impl Decoder for ArrayDecoder<N>>::push_bytes", # Mutations cause an infinite loop

api/primitives/all-features.txt

@@ -1330,13 +1330,16 @@ pub fn bitcoin_primitives::TxMerkleNode::as_ref(&self) -> &[u8; 32]
 pub fn bitcoin_primitives::TxMerkleNode::as_ref(&self) -> &[u8]
 pub fn bitcoin_primitives::TxMerkleNode::borrow(&self) -> &[u8; 32]
 pub fn bitcoin_primitives::TxMerkleNode::borrow(&self) -> &[u8]
+pub fn bitcoin_primitives::TxMerkleNode::calculate_root<I: core::iter::traits::iterator::Iterator<Item = bitcoin_primitives::Txid>>(iter: I) -> core::option::Option<Self>
 pub fn bitcoin_primitives::TxMerkleNode::clone(&self) -> bitcoin_primitives::TxMerkleNode
 pub fn bitcoin_primitives::TxMerkleNode::cmp(&self, other: &bitcoin_primitives::TxMerkleNode) -> core::cmp::Ordering
+pub fn bitcoin_primitives::TxMerkleNode::combine(&self, other: &Self) -> Self
 pub fn bitcoin_primitives::TxMerkleNode::decoder() -> Self::Decoder
 pub fn bitcoin_primitives::TxMerkleNode::deserialize<D: serde::de::Deserializer<'de>>(d: D) -> core::result::Result<bitcoin_primitives::TxMerkleNode, <D as serde::de::Deserializer>::Error>
 pub fn bitcoin_primitives::TxMerkleNode::encoder(&self) -> Self::Encoder
 pub fn bitcoin_primitives::TxMerkleNode::eq(&self, other: &bitcoin_primitives::TxMerkleNode) -> bool
 pub fn bitcoin_primitives::TxMerkleNode::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result
+pub fn bitcoin_primitives::TxMerkleNode::from_leaf(leaf: bitcoin_primitives::Txid) -> Self
 pub fn bitcoin_primitives::TxMerkleNode::from_str(s: &str) -> core::result::Result<Self, Self::Err>
 pub fn bitcoin_primitives::TxMerkleNode::hash<__H: core::hash::Hasher>(&self, state: &mut __H)
 pub fn bitcoin_primitives::TxMerkleNode::partial_cmp(&self, other: &bitcoin_primitives::TxMerkleNode) -> core::option::Option<core::cmp::Ordering>
@@ -1376,11 +1379,14 @@ pub fn bitcoin_primitives::WitnessMerkleNode::as_ref(&self) -> &[u8; 32]
 pub fn bitcoin_primitives::WitnessMerkleNode::as_ref(&self) -> &[u8]
 pub fn bitcoin_primitives::WitnessMerkleNode::borrow(&self) -> &[u8; 32]
 pub fn bitcoin_primitives::WitnessMerkleNode::borrow(&self) -> &[u8]
+pub fn bitcoin_primitives::WitnessMerkleNode::calculate_root<I: core::iter::traits::iterator::Iterator<Item = bitcoin_primitives::Wtxid>>(iter: I) -> core::option::Option<Self>
 pub fn bitcoin_primitives::WitnessMerkleNode::clone(&self) -> bitcoin_primitives::WitnessMerkleNode
 pub fn bitcoin_primitives::WitnessMerkleNode::cmp(&self, other: &bitcoin_primitives::WitnessMerkleNode) -> core::cmp::Ordering
+pub fn bitcoin_primitives::WitnessMerkleNode::combine(&self, other: &Self) -> Self
 pub fn bitcoin_primitives::WitnessMerkleNode::deserialize<D: serde::de::Deserializer<'de>>(d: D) -> core::result::Result<bitcoin_primitives::WitnessMerkleNode, <D as serde::de::Deserializer>::Error>
 pub fn bitcoin_primitives::WitnessMerkleNode::eq(&self, other: &bitcoin_primitives::WitnessMerkleNode) -> bool
 pub fn bitcoin_primitives::WitnessMerkleNode::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result
+pub fn bitcoin_primitives::WitnessMerkleNode::from_leaf(leaf: bitcoin_primitives::Wtxid) -> Self
 pub fn bitcoin_primitives::WitnessMerkleNode::from_str(s: &str) -> core::result::Result<Self, Self::Err>
 pub fn bitcoin_primitives::WitnessMerkleNode::hash<__H: core::hash::Hasher>(&self, state: &mut __H)
 pub fn bitcoin_primitives::WitnessMerkleNode::partial_cmp(&self, other: &bitcoin_primitives::WitnessMerkleNode) -> core::option::Option<core::cmp::Ordering>

api/primitives/alloc-only.txt

@@ -1187,12 +1187,15 @@ pub fn bitcoin_primitives::TxMerkleNode::as_ref(&self) -> &[u8; 32]
 pub fn bitcoin_primitives::TxMerkleNode::as_ref(&self) -> &[u8]
 pub fn bitcoin_primitives::TxMerkleNode::borrow(&self) -> &[u8; 32]
 pub fn bitcoin_primitives::TxMerkleNode::borrow(&self) -> &[u8]
+pub fn bitcoin_primitives::TxMerkleNode::calculate_root<I: core::iter::traits::iterator::Iterator<Item = bitcoin_primitives::Txid>>(iter: I) -> core::option::Option<Self>
 pub fn bitcoin_primitives::TxMerkleNode::clone(&self) -> bitcoin_primitives::TxMerkleNode
 pub fn bitcoin_primitives::TxMerkleNode::cmp(&self, other: &bitcoin_primitives::TxMerkleNode) -> core::cmp::Ordering
+pub fn bitcoin_primitives::TxMerkleNode::combine(&self, other: &Self) -> Self
 pub fn bitcoin_primitives::TxMerkleNode::decoder() -> Self::Decoder
 pub fn bitcoin_primitives::TxMerkleNode::encoder(&self) -> Self::Encoder
 pub fn bitcoin_primitives::TxMerkleNode::eq(&self, other: &bitcoin_primitives::TxMerkleNode) -> bool
 pub fn bitcoin_primitives::TxMerkleNode::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result
+pub fn bitcoin_primitives::TxMerkleNode::from_leaf(leaf: bitcoin_primitives::Txid) -> Self
 pub fn bitcoin_primitives::TxMerkleNode::hash<__H: core::hash::Hasher>(&self, state: &mut __H)
 pub fn bitcoin_primitives::TxMerkleNode::partial_cmp(&self, other: &bitcoin_primitives::TxMerkleNode) -> core::option::Option<core::cmp::Ordering>
 pub fn bitcoin_primitives::Txid::as_ref(&self) -> &[u8; 32]
@@ -1221,10 +1224,13 @@ pub fn bitcoin_primitives::WitnessMerkleNode::as_ref(&self) -> &[u8; 32]
 pub fn bitcoin_primitives::WitnessMerkleNode::as_ref(&self) -> &[u8]
 pub fn bitcoin_primitives::WitnessMerkleNode::borrow(&self) -> &[u8; 32]
 pub fn bitcoin_primitives::WitnessMerkleNode::borrow(&self) -> &[u8]
+pub fn bitcoin_primitives::WitnessMerkleNode::calculate_root<I: core::iter::traits::iterator::Iterator<Item = bitcoin_primitives::Wtxid>>(iter: I) -> core::option::Option<Self>
 pub fn bitcoin_primitives::WitnessMerkleNode::clone(&self) -> bitcoin_primitives::WitnessMerkleNode
 pub fn bitcoin_primitives::WitnessMerkleNode::cmp(&self, other: &bitcoin_primitives::WitnessMerkleNode) -> core::cmp::Ordering
+pub fn bitcoin_primitives::WitnessMerkleNode::combine(&self, other: &Self) -> Self
 pub fn bitcoin_primitives::WitnessMerkleNode::eq(&self, other: &bitcoin_primitives::WitnessMerkleNode) -> bool
 pub fn bitcoin_primitives::WitnessMerkleNode::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result
+pub fn bitcoin_primitives::WitnessMerkleNode::from_leaf(leaf: bitcoin_primitives::Wtxid) -> Self
 pub fn bitcoin_primitives::WitnessMerkleNode::hash<__H: core::hash::Hasher>(&self, state: &mut __H)
 pub fn bitcoin_primitives::WitnessMerkleNode::partial_cmp(&self, other: &bitcoin_primitives::WitnessMerkleNode) -> core::option::Option<core::cmp::Ordering>
 pub fn bitcoin_primitives::Wtxid::as_ref(&self) -> &[u8; 32]

bitcoin/src/blockdata/block.rs

@@ -15,7 +15,7 @@ use internals::{compact_size, ToU64};
 use io::{BufRead, Write};
 
 use crate::consensus::encode::{self, Decodable, Encodable, WriteExt as _};
-use crate::merkle_tree::{MerkleNode as _, TxMerkleNode, WitnessMerkleNode};
+use crate::merkle_tree::{TxMerkleNode, WitnessMerkleNode};
 use crate::network::Params;
 use crate::prelude::Vec;
 use crate::script::{self, ScriptIntError, ScriptExt as _};

bitcoin/src/merkle_tree/block.rs

@@ -19,7 +19,7 @@ use io::{BufRead, Write};
 
 use crate::block::{self, Block, Checked};
 use crate::consensus::encode::{self, Decodable, Encodable, ReadExt, WriteExt, MAX_VEC_SIZE};
-use crate::merkle_tree::{MerkleNode as _, TxMerkleNode};
+use crate::merkle_tree::TxMerkleNode;
 use crate::prelude::Vec;
 use crate::transaction::{Transaction, Txid};
 use crate::Weight;

bitcoin/src/merkle_tree/mod.rs

@@ -6,7 +6,7 @@
 //!
 //! ```
 //! # use bitcoin::Txid;
-//! # use bitcoin::merkle_tree::{MerkleNode as _, TxMerkleNode};
+//! # use bitcoin::merkle_tree::TxMerkleNode;
 //! # let tx1 = Txid::from_byte_array([0xAA; 32]);  // Arbitrary dummy hash values.
 //! # let tx2 = Txid::from_byte_array([0xFF; 32]);
 //! let tx_hashes = [tx1, tx2]; // All the hashes we wish to merkelize.
@@ -16,13 +16,8 @@
 
 mod block;
 
-use hashes::{sha256d, HashEngine as _};
 use io::{BufRead, Write};
 
-use crate::prelude::Vec;
-use crate::transaction::TxIdentifier;
-use crate::{Txid, Wtxid};
-
 #[rustfmt::skip]
 #[doc(inline)]
 pub use self::block::{MerkleBlock, MerkleBlockError, PartialMerkleTree};
@@ -53,105 +48,3 @@ impl Decodable for WitnessMerkleNode {
         Ok(Self::from_byte_array(<[u8; 32]>::consensus_decode(r)?))
     }
 }
-
-/// A node in a Merkle tree of transactions or witness data within a block.
-///
-/// This trait is used to compute the transaction Merkle root contained in
-/// a block header. This is a particularly weird algorithm -- it interprets
-/// the list of transactions as a balanced binary tree, duplicating branches
-/// as needed to fill out the tree to a power of two size.
-///
-/// Other Merkle trees in Bitcoin, such as those used in Taproot commitments,
-/// do not use this algorithm and cannot use this trait.
-pub trait MerkleNode: Copy + PartialEq {
-    /// The hash (TXID or WTXID) of a transaction in the tree.
-    type Leaf: TxIdentifier;
-
-    /// Convert a hash to a leaf node of the tree.
-    fn from_leaf(leaf: Self::Leaf) -> Self;
-    /// Combine two nodes to get a single node. The final node of a tree is called the "root".
-    fn combine(&self, other: &Self) -> Self;
-
-    /// Given an iterator of leaves, compute the Merkle root.
-    ///
-    /// Returns `None` if the iterator was empty, or if the transaction list contains
-    /// consecutive duplicates which would trigger CVE 2012-2459. Blocks with duplicate
-    /// transactions will always be invalid, so there is no harm in us refusing to
-    /// compute their merkle roots.
-    ///
-    /// Unless you are certain your transaction list is nonempty and has no duplicates,
-    /// you should not unwrap the `Option` returned by this method!
-    fn calculate_root<I: Iterator<Item = Self::Leaf>>(iter: I) -> Option<Self> {
-        let mut stack = Vec::<(usize, Self)>::with_capacity(32);
-        // Start with a standard Merkle tree root computation...
-        for (mut n, leaf) in iter.enumerate() {
-            stack.push((0, Self::from_leaf(leaf)));
-
-            while n & 1 == 1 {
-                let right = stack.pop().unwrap();
-                let left = stack.pop().unwrap();
-                if left.1 == right.1 {
-                    // Reject duplicate trees since they are guaranteed-invalid (Bitcoin does
-                    // not allow duplicate transactions in block) but can be used to confuse
-                    // nodes about legitimate blocks. See CVE 2012-2459 and the block comment
-                    // below.
-                    return None;
-                }
-                debug_assert_eq!(left.0, right.0);
-                stack.push((left.0 + 1, left.1.combine(&right.1)));
-                n >>= 1;
-            }
-        }
-        // ...then, deal with incomplete trees. Bitcoin does a weird thing in
-        // which it doubles-up nodes of the tree to fill out the tree, rather
-        // than treating incomplete branches specially. This makes this tree
-        // construction vulnerable to collisions (see CVE 2012-2459).
-        //
-        // (It is also vulnerable to collisions because it does not distinguish
-        // between internal nodes and transactions, but this collisions of this
-        // form are probably impractical. It is likely that 64-byte transactions
-        // will be forbidden in the future which will close this for good.)
-        //
-        // This is consensus logic so we cannot fix the Merkle tree construction.
-        // Instead we just have to reject the clearly-invalid half of the collision
-        // (see previous comment).
-        while stack.len() > 1 {
-            let mut right = stack.pop().unwrap();
-            let left = stack.pop().unwrap();
-            while right.0 != left.0 {
-                assert!(right.0 < left.0);
-                right = (right.0 + 1, right.1.combine(&right.1)); // combine with self
-            }
-            stack.push((left.0 + 1, left.1.combine(&right.1)));
-        }
-
-        stack.pop().map(|(_, h)| h)
-    }
-}
-
-// These two impl blocks are identical. FIXME once we have nailed down
-// our hash traits, it should be possible to put bounds on `MerkleNode`
-// and `MerkleNode::Leaf` which are sufficient to turn both methods into
-// provided methods in the trait definition.
-impl MerkleNode for TxMerkleNode {
-    type Leaf = Txid;
-    fn from_leaf(leaf: Self::Leaf) -> Self { Self::from_byte_array(leaf.to_byte_array()) }
-
-    fn combine(&self, other: &Self) -> Self {
-        let mut encoder = sha256d::Hash::engine();
-        encoder.input(self.as_byte_array());
-        encoder.input(other.as_byte_array());
-        Self::from_byte_array(sha256d::Hash::from_engine(encoder).to_byte_array())
-    }
-}
-impl MerkleNode for WitnessMerkleNode {
-    type Leaf = Wtxid;
-    fn from_leaf(leaf: Self::Leaf) -> Self { Self::from_byte_array(leaf.to_byte_array()) }
-
-    fn combine(&self, other: &Self) -> Self {
-        let mut encoder = sha256d::Hash::engine();
-        encoder.input(self.as_byte_array());
-        encoder.input(other.as_byte_array());
-        Self::from_byte_array(sha256d::Hash::from_engine(encoder).to_byte_array())
-    }
-}

primitives/src/hash_types/transaction_merkle_node.rs

@@ -12,6 +12,11 @@ use arbitrary::{Arbitrary, Unstructured};
 use hashes::sha256d;
 use internals::write_err;
 
+#[cfg(feature = "alloc")]
+use crate::merkle_tree::MerkleNode;
+#[cfg(feature = "alloc")]
+use crate::Txid;
+
 /// A hash of the Merkle tree branch or root for transactions.
 #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct TxMerkleNode(sha256d::Hash);
@@ -23,6 +28,27 @@ type Inner = sha256d::Hash;
 
 include!("./generic.rs");
 
+#[cfg(feature = "alloc")]
+impl TxMerkleNode {
+    /// Convert a [`Txid`] hash to a leaf node of the tree.
+    pub fn from_leaf(leaf: Txid) -> Self { MerkleNode::from_leaf(leaf) }
+
+    /// Combine two nodes to get a single node. The final node of a tree is called the "root".
+    #[must_use]
+    pub fn combine(&self, other: &Self) -> Self { MerkleNode::combine(self, other) }
+
+    /// Given an iterator of leaves, compute the Merkle root.
+    ///
+    /// Returns `None` if the iterator was empty, or if the transaction list contains
+    /// consecutive duplicates which would trigger CVE 2012-2459. Blocks with duplicate
+    /// transactions will always be invalid, so there is no harm in us refusing to
+    /// compute their merkle roots.
+    ///
+    /// Unless you are certain your transaction list is nonempty and has no duplicates,
+    /// you should not unwrap the `Option` returned by this method!
+    pub fn calculate_root<I: Iterator<Item = Txid>>(iter: I) -> Option<Self> { MerkleNode::calculate_root(iter) }
+}
+
 encoding::encoder_newtype! {
     /// The encoder for the [`TxMerkleNode`] type.
     pub struct TxMerkleNodeEncoder(encoding::ArrayEncoder<32>);

primitives/src/hash_types/witness_merkle_node.rs

@@ -11,6 +11,11 @@ use core::str;
 use arbitrary::{Arbitrary, Unstructured};
 use hashes::sha256d;
 
+#[cfg(feature = "alloc")]
+use crate::merkle_tree::MerkleNode;
+#[cfg(feature = "alloc")]
+use crate::Wtxid;
+
 /// A hash corresponding to the Merkle tree root for witness data.
 #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct WitnessMerkleNode(sha256d::Hash);
@@ -21,3 +26,24 @@ type HashType = WitnessMerkleNode;
 type Inner = sha256d::Hash;
 
 include!("./generic.rs");
+
+#[cfg(feature = "alloc")]
+impl WitnessMerkleNode {
+    /// Convert a [`Wtxid`] hash to a leaf node of the tree.
+    pub fn from_leaf(leaf: Wtxid) -> Self { MerkleNode::from_leaf(leaf) }
+
+    /// Combine two nodes to get a single node. The final node of a tree is called the "root".
+    #[must_use]
+    pub fn combine(&self, other: &Self) -> Self { MerkleNode::combine(self, other) }
+
+    /// Given an iterator of leaves, compute the Merkle root.
+    ///
+    /// Returns `None` if the iterator was empty, or if the transaction list contains
+    /// consecutive duplicates which would trigger CVE 2012-2459. Blocks with duplicate
+    /// transactions will always be invalid, so there is no harm in us refusing to
+    /// compute their merkle roots.
+    ///
+    /// Unless you are certain your transaction list is nonempty and has no duplicates,
+    /// you should not unwrap the `Option` returned by this method!
+    pub fn calculate_root<I: Iterator<Item = Wtxid>>(iter: I) -> Option<Self> { MerkleNode::calculate_root(iter) }
+}