cleanup(merkle-tree): reject non-power-of-two leaves, dedupe helpers

crypto/crypto/src/merkle_tree/backends/field_element.rs

@@ -84,7 +84,7 @@ mod tests {
 
     #[test]
     fn hash_data_field_element_backend_works_with_keccak_256() {
-        let values: Vec<FE> = (1..6).map(FE::from).collect();
+        let values: Vec<FE> = (1..9).map(FE::from).collect();
         let merkle_tree =
             MerkleTree::<FieldElementBackend<F, Keccak256, 32>>::build(&values).unwrap();
         let proof = merkle_tree.get_proof_by_pos(0).unwrap();
@@ -97,7 +97,7 @@ mod tests {
 
     #[test]
     fn hash_data_field_element_backend_works_with_sha3_256() {
-        let values: Vec<FE> = (1..6).map(FE::from).collect();
+        let values: Vec<FE> = (1..9).map(FE::from).collect();
         let merkle_tree =
             MerkleTree::<FieldElementBackend<F, Sha3_256, 32>>::build(&values).unwrap();
         let proof = merkle_tree.get_proof_by_pos(0).unwrap();
@@ -110,7 +110,7 @@ mod tests {
 
     #[test]
     fn hash_data_field_element_backend_works_with_keccak_512() {
-        let values: Vec<FE> = (1..6).map(FE::from).collect();
+        let values: Vec<FE> = (1..9).map(FE::from).collect();
         let merkle_tree =
             MerkleTree::<FieldElementBackend<F, Keccak512, 64>>::build(&values).unwrap();
         let proof = merkle_tree.get_proof_by_pos(0).unwrap();
@@ -123,7 +123,7 @@ mod tests {
 
     #[test]
     fn hash_data_field_element_backend_works_with_sha3_512() {
-        let values: Vec<FE> = (1..6).map(FE::from).collect();
+        let values: Vec<FE> = (1..9).map(FE::from).collect();
         let merkle_tree =
             MerkleTree::<FieldElementBackend<F, Sha3_512, 64>>::build(&values).unwrap();
         let proof = merkle_tree.get_proof_by_pos(0).unwrap();

crypto/crypto/src/merkle_tree/backends/field_element_vector.rs

@@ -41,18 +41,14 @@ where
         let mut hasher = D::new();
         hasher.update(input[0].as_bytes());
         hasher.update(input[1].as_bytes());
-        let mut result_hash = [0_u8; NUM_BYTES];
-        result_hash.copy_from_slice(&hasher.finalize());
-        result_hash
+        hasher.finalize().into()
     }
 
     fn hash_new_parent(left: &[u8; NUM_BYTES], right: &[u8; NUM_BYTES]) -> [u8; NUM_BYTES] {
         let mut hasher = D::new();
         hasher.update(left);
         hasher.update(right);
-        let mut result_hash = [0_u8; NUM_BYTES];
-        result_hash.copy_from_slice(&hasher.finalize());
-        result_hash
+        hasher.finalize().into()
     }
 }
 
@@ -82,9 +78,7 @@ where
     pub fn hash_bytes(data: &[u8]) -> [u8; NUM_BYTES] {
         let mut hasher = D::new();
         hasher.update(data);
-        let mut result = [0u8; NUM_BYTES];
-        result.copy_from_slice(&hasher.finalize());
-        result
+        hasher.finalize().into()
     }
 }
 
@@ -104,18 +98,14 @@ where
         for element in input.iter() {
             hasher.update(element.as_bytes());
         }
-        let mut result_hash = [0_u8; NUM_BYTES];
-        result_hash.copy_from_slice(&hasher.finalize());
-        result_hash
+        hasher.finalize().into()
     }
 
     fn hash_new_parent(left: &[u8; NUM_BYTES], right: &[u8; NUM_BYTES]) -> [u8; NUM_BYTES] {
         let mut hasher = D::new();
         hasher.update(left);
         hasher.update(right);
-        let mut result_hash = [0_u8; NUM_BYTES];
-        result_hash.copy_from_slice(&hasher.finalize());
-        result_hash
+        hasher.finalize().into()
     }
 }
 

crypto/crypto/src/merkle_tree/merkle.rs

@@ -127,21 +127,23 @@ where
     ///
     /// This skips the `hash_leaves` step, useful when leaves have already been
     /// hashed externally (e.g., to avoid materializing large intermediate data).
+    ///
+    /// Returns `None` unless the leaf count is a non-zero power of two. The tree
+    /// is intentionally not padded: padding by repeating the last leaf would
+    /// make the root of `[.., x]` collide with the root of `[.., x, x]`, so the
+    /// root would no longer bind the leaf count. Every prover commitment is
+    /// already over a power-of-two domain, so this is a caller-side invariant.
     pub fn build_from_hashed_leaves(hashed_leaves: Vec<B::Node>) -> Option<Self> {
-        if hashed_leaves.is_empty() {
+        if hashed_leaves.is_empty() || !hashed_leaves.len().is_power_of_two() {
             return None;
         }
-
-        //The leaf must be a power of 2 set
-        let hashed_leaves = complete_until_power_of_two(hashed_leaves);
         let leaves_len = hashed_leaves.len();
 
-        //The length of leaves minus one inner node in the merkle tree
-        //The first elements are overwritten by build function, it doesn't matter what it's there
+        // `nodes` holds (leaves_len - 1) inner nodes followed by the leaves.
+        // The inner-node entries are placeholders, overwritten by `build`.
         let mut nodes = vec![hashed_leaves[0].clone(); leaves_len - 1];
         nodes.extend(hashed_leaves);
 
-        //Build the inner nodes of the tree
         build::<B>(&mut nodes, leaves_len);
 
         Some(MerkleTree {
@@ -192,38 +194,30 @@ where
         &self.nodes
     }
 
-    /// Returns a Merkle proof for the element/s at position pos
-    /// For example, give me an inclusion proof for the 3rd element in the
-    /// Merkle tree
+    /// Returns a Merkle inclusion proof for the leaf at position `pos`.
     pub fn get_proof_by_pos(&self, pos: usize) -> Option<Proof<B::Node>> {
         let pos = pos + self.node_count() / 2;
-        let Ok(merkle_path) = self.build_merkle_path(pos) else {
-            return None;
-        };
-
-        self.create_proof(merkle_path)
-    }
-
-    /// Creates a proof from a Merkle pasth
-    fn create_proof(&self, merkle_path: Vec<B::Node>) -> Option<Proof<B::Node>> {
+        let merkle_path = self.build_merkle_path(pos).ok()?;
         Some(Proof { merkle_path })
     }
 
-    /// Returns the Merkle path for the element/s for the leaf at position pos
+    /// Returns the Merkle path (sibling hashes, leaf to root) for the node at `pos`.
     fn build_merkle_path(&self, pos: usize) -> Result<Vec<B::Node>, Error> {
         // Pre-allocate based on tree depth (log2 of tree size)
         let tree_depth = (self.node_count() + 1).ilog2() as usize;
         let mut merkle_path = Vec::with_capacity(tree_depth);
         let mut pos = pos;
 
         while pos != ROOT {
-            let Some(node) = self.node_get(sibling_index(pos)) else {
+            // `pos != ROOT` guarantees a sibling exists.
+            let sibling = get_sibling_pos(pos).expect("non-root node has a sibling");
+            let Some(node) = self.node_get(sibling) else {
                 // out of bounds, exit returning the current merkle_path
                 return Err(Error::OutOfBounds);
             };
             merkle_path.push(node.clone());
 
-            pos = parent_index(pos);
+            pos = get_parent_pos(pos);
         }
 
         Ok(merkle_path)
@@ -305,7 +299,7 @@ where
         // Number of levels in tree
         let num_levels = (self.node_count() + 1).ilog2();
 
-        // Iter lefevel-by-level from leaves to root.
+        // Iterate level-by-level from leaves to root.
         for _ in 0..num_levels - 1 {
             let mut next_obtainable = BTreeSet::new();
 
@@ -329,7 +323,7 @@ where
         }
 
         // Reverse to get descending order (larger indices first).
-        // This makes the proof ordered from bottom (nodes closer to leaves) to top (nodes loser to root).
+        // This makes the proof ordered from bottom (nodes closer to leaves) to top (nodes closer to root).
         auth_path_set.into_iter().rev().collect()
     }
 

crypto/crypto/src/merkle_tree/mod.rs

@@ -2,4 +2,5 @@ pub mod backends;
 pub mod merkle;
 pub mod proof;
 pub mod traits;
-pub mod utils;
+// Internal index/build helpers — not part of the public Merkle API.
+pub(crate) mod utils;

crypto/crypto/src/merkle_tree/proof.rs

@@ -1,7 +1,4 @@
 use alloc::{collections::BTreeMap, vec::Vec};
-#[cfg(feature = "alloc")]
-use math::traits::Serializable;
-use math::{errors::DeserializationError, traits::Deserializable};
 
 use super::{
     traits::IsMerkleTreeBackend,
@@ -41,36 +38,6 @@ impl<T: PartialEq + Eq> Proof<T> {
     }
 }
 
-#[cfg(feature = "alloc")]
-impl<T> Serializable for Proof<T>
-where
-    T: Serializable + PartialEq + Eq,
-{
-    fn serialize(&self) -> Vec<u8> {
-        self.merkle_path
-            .iter()
-            .flat_map(|node| node.serialize())
-            .collect()
-    }
-}
-
-impl<T> Deserializable for Proof<T>
-where
-    T: Deserializable + PartialEq + Eq,
-{
-    fn deserialize(bytes: &[u8]) -> Result<Self, DeserializationError>
-    where
-        Self: Sized,
-    {
-        let mut merkle_path = Vec::new();
-        for elem in bytes[0..].chunks(8) {
-            let node = T::deserialize(elem)?;
-            merkle_path.push(node);
-        }
-        Ok(Self { merkle_path })
-    }
-}
-
 /// Stores all the nodes needed to prove the inclusion of multiple leaves.
 ///
 /// # Proof Ordering

crypto/crypto/src/merkle_tree/utils.rs

@@ -1,27 +1,8 @@
-use alloc::vec::Vec;
-
 use super::traits::IsMerkleTreeBackend;
 #[cfg(feature = "parallel")]
 use rayon::prelude::*;
 
-pub fn sibling_index(node_index: usize) -> usize {
-    if node_index.is_multiple_of(2) {
-        node_index - 1
-    } else {
-        node_index + 1
-    }
-}
-
-pub fn parent_index(node_index: usize) -> usize {
-    if node_index.is_multiple_of(2) {
-        (node_index - 1) / 2
-    } else {
-        node_index / 2
-    }
-}
-
-/// Returns the sibling position for a given node index.
-/// Returns `None` for the root node (index 0) since it has no sibling.
+/// Sibling of `node_index` in the flat node array, or `None` for the root (0).
 pub fn get_sibling_pos(node_index: usize) -> Option<usize> {
     if node_index == 0 {
         return None;
@@ -33,8 +14,8 @@ pub fn get_sibling_pos(node_index: usize) -> Option<usize> {
     }
 }
 
+/// Parent of `node_index`. The root (0) has no parent and maps to itself.
 pub fn get_parent_pos(node_index: usize) -> usize {
-    // Root node (index 0) has no parent, return itself to avoid underflow
     if node_index == 0 {
         return node_index;
     }
@@ -45,26 +26,11 @@ pub fn get_parent_pos(node_index: usize) -> usize {
     }
 }
 
-// The list of values is completed repeating the last value to a power of two length
-pub fn complete_until_power_of_two<T: Clone>(mut values: Vec<T>) -> Vec<T> {
-    while !is_power_of_two(values.len()) {
-        values.push(values[values.len() - 1].clone());
-    }
-    values
-}
-
-// ! NOTE !
-// In this function we say 2^0 = 1 is a power of two.
-// In turn, this makes the smallest tree of one leaf, possible.
-// The function is private and is only used to ensure the tree
-// has a power of 2 number of leaves.
-fn is_power_of_two(x: usize) -> bool {
-    (x & (x - 1)) == 0
-}
-
-// ! CAUTION !
-// Make sure n=nodes.len()+1 is a power of two, and the last n/2 elements (leaves) are populated with hashes.
-// This function takes no precautions for other cases.
+/// Fills the inner nodes of a Merkle tree bottom-up.
+///
+/// Precondition (caller-enforced, not checked): `nodes.len() + 1` is a power of
+/// two and the last `leaves_len` entries of `nodes` already hold the leaf
+/// hashes. Behaviour is unspecified for any other input.
 pub fn build<B: IsMerkleTreeBackend>(nodes: &mut [B::Node], leaves_len: usize)
 where
     B::Node: Clone,

crypto/crypto/src/tests/merkle_proof_tests.rs

@@ -18,16 +18,17 @@ type FE = FieldElement<U64PF>;
 #[test]
 // expected | 8 | 7 | 1 | 6 | 1 | 7 | 7 | 2 | 4 | 6 | 8 | 10 | 10 | 10 | 10 |
 fn create_a_proof_over_value_that_belongs_to_a_given_merkle_tree_when_given_the_leaf_position() {
-    let values: Vec<FE> = (1..6).map(FE::new).collect();
+    // 8 leaves (power of two) — the same set the old non-power-of-two padding produced.
+    let values: Vec<FE> = [1, 2, 3, 4, 5, 5, 5, 5].into_iter().map(FE::new).collect();
     let merkle_tree = MerkleTree::<TestBackend<U64PF>>::build(&values).unwrap();
     let proof = &merkle_tree.get_proof_by_pos(1).unwrap();
     assert_merkle_path(&proof.merkle_path, &[FE::new(2), FE::new(1), FE::new(1)]);
     assert!(proof.verify::<TestBackend<U64PF>>(&merkle_tree.root, 1, &FE::new(2)));
 }
 
 #[test]
-fn create_a_merkle_tree_with_10000_elements_and_verify_that_an_element_is_part_of_it() {
-    let values: Vec<Ecgfp5FE> = (1..10000).map(Ecgfp5FE::new).collect();
+fn create_a_merkle_tree_with_16384_elements_and_verify_that_an_element_is_part_of_it() {
+    let values: Vec<Ecgfp5FE> = (1..=16384).map(Ecgfp5FE::new).collect();
     let merkle_tree = TestMerkleTreeEcgfp::build(&values).unwrap();
     let proof = merkle_tree.get_proof_by_pos(9349).unwrap();
     assert!(proof.verify::<TestBackend<Ecgfp5>>(&merkle_tree.root, 9349, &Ecgfp5FE::new(9350)));

crypto/crypto/src/tests/merkle_tests.rs

@@ -50,15 +50,15 @@ fn build_merkle_tree_from_a_power_of_two_list_of_values() {
 }
 
 #[test]
-// expected | 8 | 7 | 1 | 6 | 1 | 7 | 7 | 2 | 4 | 6 | 8 | 10 | 10 | 10 | 10 |
-fn build_merkle_tree_from_an_odd_set_of_leaves() {
+fn build_merkle_tree_from_a_non_power_of_two_set_returns_none() {
     const MODULUS: u64 = 13;
     type U64PF = U64Field<MODULUS>;
     type FE = FieldElement<U64PF>;
 
+    // A non-power-of-two leaf count is rejected rather than padded, so the
+    // root unambiguously binds the leaf count.
     let values: Vec<FE> = (1..6).map(FE::new).collect();
-    let merkle_tree = MerkleTree::<TestBackend<U64PF>>::build(&values).unwrap();
-    assert_eq!(merkle_tree.root, FE::new(8)); // Adjusted expected value
+    assert!(MerkleTree::<TestBackend<U64PF>>::build(&values).is_none());
 }
 
 #[test]

crypto/crypto/src/tests/merkle_utils_tests.rs

@@ -1,10 +1,7 @@
 use alloc::vec::Vec;
 use math::field::{element::FieldElement, test_fields::u64_test_field::U64Field};
 
-use crate::merkle_tree::{
-    traits::IsMerkleTreeBackend,
-    utils::{build, complete_until_power_of_two},
-};
+use crate::merkle_tree::{traits::IsMerkleTreeBackend, utils::build};
 use crate::tests::merkle_tests::TestBackend;
 
 const MODULUS: u64 = 13;
@@ -29,32 +26,6 @@ fn hash_leaves_from_a_list_of_field_elemnts() {
     }
 }
 
-#[test]
-// expected |1|2|3|4|5|5|5|5|
-fn complete_the_length_of_a_list_of_fields_elements_to_be_a_power_of_two() {
-    let values: Vec<FE> = (1..6).map(FE::new).collect();
-    let hashed_leaves = complete_until_power_of_two(values);
-
-    let mut expected_leaves = (1..6).map(FE::new).collect::<Vec<FE>>();
-    expected_leaves.extend([FE::new(5); 3]);
-
-    for (leaf, expected_leaves) in hashed_leaves.iter().zip(expected_leaves) {
-        assert_eq!(*leaf, expected_leaves);
-    }
-}
-
-#[test]
-// expected |2|2|
-fn complete_the_length_of_one_field_element_to_be_a_power_of_two() {
-    let values: Vec<FE> = vec![FE::new(2)];
-    let hashed_leaves = complete_until_power_of_two(values);
-
-    let mut expected_leaves = vec![FE::new(2)];
-    expected_leaves.extend([FE::new(2)]);
-    assert_eq!(hashed_leaves.len(), 1);
-    assert_eq!(hashed_leaves[0], expected_leaves[0]);
-}
-
 const ROOT: usize = 0;
 
 #[test]