Merge rust-bitcoin#5356: Remove alloc feature gate for MerkleNode

cc0da4c Remove alloc feature gate for MerkleNode (Mitchell Bagot)
4c65ce7 Add pop to ArrayVec (Mitchell Bagot)

Pull request description:

  The MerkleNode::calculate_root function currently relies on the alloc feature to be enabled in order to provide functionality. Since the calculate_root function generates a balanced tree, it only requires a stack of size log2(N), where N is the number of transactions in the input iterator. The ArrayVec struct in internals can replace the Vec for this function in the case where the transaction count is bounded.
  
  NOTE: This PR has `unsafe` logic in 7a5c157.
  
  Follow up to rust-bitcoin#5346


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


Tree-SHA512: e4042c350770f36adfe9d88f9c399ca950a0b055c62b148754368b67e859ee94496adca855cf5fca6482ccae678a105e0f8e883a02dc541f299e76b36d527c77

Author: apoelstra

.cargo/mutants.toml

@@ -51,6 +51,7 @@ exclude_re = [
     "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
+    "primitives/.* replace == with != in MerkleNode::calculate_root", # Replacing == with != isn't caught unless alloc is disabled.
 
     # 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/no-features.txt

@@ -514,12 +514,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]
@@ -546,12 +549,15 @@ 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::decoder() -> Self::Decoder
 pub fn bitcoin_primitives::WitnessMerkleNode::encoder(&self) -> Self::Encoder
 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]

internals/src/array_vec.rs

@@ -69,6 +69,23 @@ mod safety_boundary {
             self.len += 1;
         }
 
+        /// Removes the last element, returning it.
+        ///
+        /// # Returns
+        ///
+        /// None if the ArrayVec is empty.
+        pub fn pop(&mut self) -> Option<T> {
+            if self.len > 0 {
+                self.len -= 1;
+                // SAFETY: All elements in 0..len are initialized
+                let res = self.data[self.len];
+                self.data[self.len] = MaybeUninit::uninit();
+                Some(unsafe { res.assume_init() })
+            } else {
+                None
+            }
+        }
+
         /// Copies and appends all elements from `slice` into `self`.
         ///
         /// # Panics

primitives/src/hash_types/transaction_merkle_node.rs

@@ -12,9 +12,7 @@ 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.
@@ -28,7 +26,6 @@ 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) }

primitives/src/hash_types/witness_merkle_node.rs

@@ -12,9 +12,7 @@ use arbitrary::{Arbitrary, Unstructured};
 use hashes::sha256d;
 use internals::write_err;
 
-#[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.
@@ -28,7 +26,6 @@ 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) }

primitives/src/merkle_tree.rs

@@ -11,13 +11,12 @@
 // C'est la vie.
 #[cfg(feature = "alloc")]
 use alloc::vec::Vec;
+#[cfg(not(feature = "alloc"))]
+use internals::array_vec::ArrayVec;
 
-#[cfg(feature = "alloc")]
 use hashes::{HashEngine, sha256d};
 
-#[cfg(feature = "alloc")]
 use crate::hash_types::{Txid, Wtxid};
-#[cfg(feature = "alloc")]
 use crate::transaction::TxIdentifier;
 
 #[doc(inline)]
@@ -32,7 +31,6 @@ pub use crate::hash_types::{TxMerkleNode, TxMerkleNodeEncoder, WitnessMerkleNode
 ///
 /// Other Merkle trees in Bitcoin, such as those used in Taproot commitments,
 /// do not use this algorithm and cannot use this trait.
-#[cfg(feature = "alloc")]
 pub(crate) trait MerkleNode: Copy + PartialEq {
     /// The hash (TXID or WTXID) of a transaction in the tree.
     type Leaf: TxIdentifier;
@@ -50,13 +48,23 @@ pub(crate) trait MerkleNode: Copy + PartialEq {
     /// transactions will always be invalid, so there is no harm in us refusing to
     /// compute their merkle roots.
     ///
+    /// Also returns `None` if the `alloc` feature is disabled and `iter` has more than
+    /// 32,767 transactions.
+    ///
     /// 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> {
         {
+            #[cfg(feature = "alloc")]
             let mut stack = Vec::<(usize, Self)>::with_capacity(32);
+            #[cfg(not(feature = "alloc"))]
+            let mut stack = ArrayVec::<(usize, Self), 15>::new();
+
             // Start with a standard Merkle tree root computation...
             for (mut n, leaf) in iter.enumerate() {
+                #[cfg(not(feature = "alloc"))]
+                // This is the only time that the stack actually grows, rather than being combined.
+                if stack.len() == 15 { return None; }
                 stack.push((0, Self::from_leaf(leaf)));
 
                 while n & 1 == 1 {
@@ -106,7 +114,6 @@ pub(crate) trait MerkleNode: Copy + PartialEq {
 // 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.
-#[cfg(feature = "alloc")]
 impl MerkleNode for TxMerkleNode {
     type Leaf = Txid;
     fn from_leaf(leaf: Self::Leaf) -> Self { Self::from_byte_array(leaf.to_byte_array()) }
@@ -118,7 +125,6 @@ impl MerkleNode for TxMerkleNode {
         Self::from_byte_array(sha256d::Hash::from_engine(encoder).to_byte_array())
     }
 }
-#[cfg(feature = "alloc")]
 impl MerkleNode for WitnessMerkleNode {
     type Leaf = Wtxid;
     fn from_leaf(leaf: Self::Leaf) -> Self { Self::from_byte_array(leaf.to_byte_array()) }
@@ -133,19 +139,16 @@ impl MerkleNode for WitnessMerkleNode {
 
 #[cfg(test)]
 mod tests {
-    #[cfg(feature = "alloc")]
     use crate::hash_types::*;
 
     // Helper to make a Txid, TxMerkleNode pair with a single number byte array
-    #[cfg(feature = "alloc")]
     fn make_leaf_node(byte: u8) -> (Txid, TxMerkleNode) {
         let leaf = Txid::from_byte_array([byte; 32]);
         let node = TxMerkleNode::from_leaf(leaf);
         (leaf, node)
     }
 
     #[test]
-    #[cfg(feature = "alloc")]
     fn tx_merkle_node_single_leaf() {
         let (leaf, node) = make_leaf_node(1);
         let root = TxMerkleNode::calculate_root([leaf].into_iter());
@@ -154,7 +157,6 @@ mod tests {
     }
 
     #[test]
-    #[cfg(feature = "alloc")]
     fn tx_merkle_node_two_leaves() {
         let (leaf1, node1) = make_leaf_node(1);
         let (leaf2, node2) = make_leaf_node(2);
@@ -169,7 +171,6 @@ mod tests {
     }
 
     #[test]
-    #[cfg(feature = "alloc")]
     fn tx_merkle_node_duplicate_leaves() {
         let leaf = Txid::from_byte_array([3; 32]);
         // Duplicate transaction list should be rejected (CVE 2012‑2459).
@@ -178,13 +179,11 @@ mod tests {
     }
 
     #[test]
-    #[cfg(feature = "alloc")]
     fn tx_merkle_node_empty() {
         assert!(TxMerkleNode::calculate_root([].into_iter()).is_none(), "Empty iterator should return None");
     }
 
     #[test]
-    #[cfg(feature = "alloc")]
     fn tx_merkle_node_2n_minus_1_unbalanced_tree() {
         // Test a tree with 2^n - 1 unique nodes and at least 3 layers deep.
         let (leaf1, node1) = make_leaf_node(1);
@@ -243,7 +242,30 @@ mod tests {
     }
 
     #[test]
-    #[cfg(feature = "alloc")]
+    fn tx_merkle_node_oversize_tree() {
+        // Confirm that with no-alloc, we return None for iter length >= 32768
+        let root = TxMerkleNode::calculate_root((0..32768u32).map(|i| {
+            let mut buf = [0u8; 32];
+            buf[..4].copy_from_slice(&i.to_le_bytes());
+            Txid::from_byte_array(buf)
+        }));
+
+        // We just want to confirm that we return None at the 32768 element boundary.
+        #[cfg(feature = "alloc")]
+        assert_ne!(root, None);
+        #[cfg(not(feature = "alloc"))]
+        assert_eq!(root, None);
+
+        // Check just under the boundary
+        let root = TxMerkleNode::calculate_root((0..32767u32).map(|i| {
+            let mut buf = [0u8; 32];
+            buf[..4].copy_from_slice(&i.to_le_bytes());
+            Txid::from_byte_array(buf)
+        }));
+        assert_ne!(root, None);
+    }
+
+    #[test]
     fn witness_merkle_node_single_leaf() {
         let leaf = Wtxid::from_byte_array([1; 32]);
         let root = WitnessMerkleNode::calculate_root([leaf].into_iter());
@@ -253,7 +275,6 @@ mod tests {
     }
 
     #[test]
-    #[cfg(feature = "alloc")]
     fn witness_merkle_node_duplicate_leaves() {
         let leaf = Wtxid::from_byte_array([2; 32]);
         let root = WitnessMerkleNode::calculate_root([leaf, leaf].into_iter());

primitives/src/transaction.rs

@@ -250,12 +250,9 @@ impl From<&Transaction> for Wtxid {
 }
 
 /// Trait that abstracts over a transaction identifier i.e., `Txid` and `Wtxid`.
-#[cfg(feature = "alloc")]
 pub(crate) trait TxIdentifier: AsRef<[u8]> {}
 
-#[cfg(feature = "alloc")]
 impl TxIdentifier for Txid {}
-#[cfg(feature = "alloc")]
 impl TxIdentifier for Wtxid {}
 
 // Duplicated in `bitcoin`.