diff --git a/crypto/crypto/src/merkle_tree/backends/field_element.rs b/crypto/crypto/src/merkle_tree/backends/field_element.rs index 738ae349e..161ff1427 100644 --- a/crypto/crypto/src/merkle_tree/backends/field_element.rs +++ b/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 = (1..6).map(FE::from).collect(); + let values: Vec = (1..9).map(FE::from).collect(); let merkle_tree = MerkleTree::>::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 = (1..6).map(FE::from).collect(); + let values: Vec = (1..9).map(FE::from).collect(); let merkle_tree = MerkleTree::>::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 = (1..6).map(FE::from).collect(); + let values: Vec = (1..9).map(FE::from).collect(); let merkle_tree = MerkleTree::>::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 = (1..6).map(FE::from).collect(); + let values: Vec = (1..9).map(FE::from).collect(); let merkle_tree = MerkleTree::>::build(&values).unwrap(); let proof = merkle_tree.get_proof_by_pos(0).unwrap(); diff --git a/crypto/crypto/src/merkle_tree/backends/field_element_vector.rs b/crypto/crypto/src/merkle_tree/backends/field_element_vector.rs index 1023b3a03..19395f588 100644 --- a/crypto/crypto/src/merkle_tree/backends/field_element_vector.rs +++ b/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() } } diff --git a/crypto/crypto/src/merkle_tree/merkle.rs b/crypto/crypto/src/merkle_tree/merkle.rs index 4ea0e5411..6ec0ac053 100644 --- a/crypto/crypto/src/merkle_tree/merkle.rs +++ b/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) -> Option { - 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::(&mut nodes, leaves_len); Some(MerkleTree { @@ -192,24 +194,14 @@ 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> { 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) -> Option> { + 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, Error> { // Pre-allocate based on tree depth (log2 of tree size) let tree_depth = (self.node_count() + 1).ilog2() as usize; @@ -217,13 +209,15 @@ where 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() } diff --git a/crypto/crypto/src/merkle_tree/mod.rs b/crypto/crypto/src/merkle_tree/mod.rs index 99ea82dea..9a274500d 100644 --- a/crypto/crypto/src/merkle_tree/mod.rs +++ b/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; diff --git a/crypto/crypto/src/merkle_tree/proof.rs b/crypto/crypto/src/merkle_tree/proof.rs index 20d5452a2..3114a3436 100644 --- a/crypto/crypto/src/merkle_tree/proof.rs +++ b/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 Proof { } } -#[cfg(feature = "alloc")] -impl Serializable for Proof -where - T: Serializable + PartialEq + Eq, -{ - fn serialize(&self) -> Vec { - self.merkle_path - .iter() - .flat_map(|node| node.serialize()) - .collect() - } -} - -impl Deserializable for Proof -where - T: Deserializable + PartialEq + Eq, -{ - fn deserialize(bytes: &[u8]) -> Result - 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 diff --git a/crypto/crypto/src/merkle_tree/utils.rs b/crypto/crypto/src/merkle_tree/utils.rs index 7cc64166b..f3b19ad9b 100644 --- a/crypto/crypto/src/merkle_tree/utils.rs +++ b/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 { if node_index == 0 { return None; @@ -33,8 +14,8 @@ pub fn get_sibling_pos(node_index: usize) -> Option { } } +/// 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(mut values: Vec) -> Vec { - 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(nodes: &mut [B::Node], leaves_len: usize) where B::Node: Clone, diff --git a/crypto/crypto/src/tests/merkle_proof_tests.rs b/crypto/crypto/src/tests/merkle_proof_tests.rs index 458d33c0c..fa7ca1e6a 100644 --- a/crypto/crypto/src/tests/merkle_proof_tests.rs +++ b/crypto/crypto/src/tests/merkle_proof_tests.rs @@ -18,7 +18,8 @@ type FE = FieldElement; #[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 = (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 = [1, 2, 3, 4, 5, 5, 5, 5].into_iter().map(FE::new).collect(); let merkle_tree = MerkleTree::>::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)]); @@ -26,8 +27,8 @@ fn create_a_proof_over_value_that_belongs_to_a_given_merkle_tree_when_given_the_ } #[test] -fn create_a_merkle_tree_with_10000_elements_and_verify_that_an_element_is_part_of_it() { - let values: Vec = (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 = (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::>(&merkle_tree.root, 9349, &Ecgfp5FE::new(9350))); diff --git a/crypto/crypto/src/tests/merkle_tests.rs b/crypto/crypto/src/tests/merkle_tests.rs index 18f853083..915bb5f3d 100644 --- a/crypto/crypto/src/tests/merkle_tests.rs +++ b/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; type FE = FieldElement; + // A non-power-of-two leaf count is rejected rather than padded, so the + // root unambiguously binds the leaf count. let values: Vec = (1..6).map(FE::new).collect(); - let merkle_tree = MerkleTree::>::build(&values).unwrap(); - assert_eq!(merkle_tree.root, FE::new(8)); // Adjusted expected value + assert!(MerkleTree::>::build(&values).is_none()); } #[test] diff --git a/crypto/crypto/src/tests/merkle_utils_tests.rs b/crypto/crypto/src/tests/merkle_utils_tests.rs index 549da139d..f5c173c1b 100644 --- a/crypto/crypto/src/tests/merkle_utils_tests.rs +++ b/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 = (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::>(); - 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 = 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]