From 21f572b407481257a81e6e0bc016ad4a97acc9bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Awirski?= Date: Wed, 16 Apr 2025 11:02:52 +0200 Subject: [PATCH] make from_iter_with_depth private, make sure iterators in tests have the same length --- Cargo.lock | 14 ++++- miden-crypto/Cargo.toml | 1 + miden-crypto/src/merkle/sparse_path.rs | 87 +++++++++++++------------- 3 files changed, 58 insertions(+), 44 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8190bc4..10795fd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -254,7 +254,7 @@ dependencies = [ "clap", "criterion-plot", "is-terminal", - "itertools", + "itertools 0.10.5", "num-traits", "once_cell", "oorandom", @@ -275,7 +275,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6b50826342786a51a89e2da3a28f1c32b06e387201bc2d19791f622c673706b1" dependencies = [ "cast", - "itertools", + "itertools 0.10.5", ] [[package]] @@ -453,6 +453,15 @@ dependencies = [ "either", ] +[[package]] +name = "itertools" +version = "0.14.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2b192c782037fadd9cfa75548310488aabdbf3d2da73885b31bd0abd03351285" +dependencies = [ + "either", +] + [[package]] name = "itoa" version = "1.0.15" @@ -524,6 +533,7 @@ dependencies = [ "glob", "hashbrown", "hex", + "itertools 0.14.0", "num", "num-complex", "proptest", diff --git a/miden-crypto/Cargo.toml b/miden-crypto/Cargo.toml index b6ea545..fcf7db7 100644 --- a/miden-crypto/Cargo.toml +++ b/miden-crypto/Cargo.toml @@ -83,6 +83,7 @@ assert_matches = { version = "1.5", default-features = false } criterion = { version = "0.5", features = ["html_reports"] } getrandom = { version = "0.3", default-features = false } hex = { version = "0.4", default-features = false, features = ["alloc"] } +itertools = { version = "0.14.0" } proptest = { version = "1.6", default-features = false, features = ["alloc"]} rand_chacha = { version = "0.9", default-features = false } rand-utils = { version = "0.12", package = "winter-rand-utils" } diff --git a/miden-crypto/src/merkle/sparse_path.rs b/miden-crypto/src/merkle/sparse_path.rs index 22d5a9d..305221c 100644 --- a/miden-crypto/src/merkle/sparse_path.rs +++ b/miden-crypto/src/merkle/sparse_path.rs @@ -36,8 +36,7 @@ impl SparseMerklePath { /// of deepest to shallowest. /// /// Knowing the size is necessary to calculate the depth of the tree, which is needed to detect - /// which nodes are empty nodes. If you know the size but your iterator type is not - /// [ExactSizeIterator], use [`SparseMerklePath::from_iter_with_depth()`]. + /// which nodes are empty nodes. /// /// # Errors /// Returns [MerkleError::DepthTooBig] if `tree_depth` is greater than [SMT_MAX_DEPTH]. @@ -51,35 +50,6 @@ impl SparseMerklePath { Self::from_iter_with_depth(iterator.len() as u8, iterator) } - /// Constructs a sparse Merkle path from a manually specified tree depth, and an iterator over - /// Merkle nodes from deepest to shallowest. - /// - /// Knowing the size is necessary to calculate the depth of the tree, which is needed to detect - /// which nodes are empty nodes. - /// - /// # Errors - /// Returns [MerkleError::DepthTooBig] if `tree_depth` is greater than [SMT_MAX_DEPTH]. - pub fn from_iter_with_depth( - tree_depth: u8, - iter: impl IntoIterator, - ) -> Result { - if tree_depth > SMT_MAX_DEPTH { - return Err(MerkleError::DepthTooBig(tree_depth as u64)); - } - - let path: Self = iter::zip(path_depth_iter(tree_depth), iter) - .map(|(depth, node)| { - let &equivalent_empty_node = EmptySubtreeRoots::entry(tree_depth, depth.get()); - let is_empty = node == equivalent_empty_node; - let node = if is_empty { None } else { Some(node) }; - - (depth, node) - }) - .collect(); - - Ok(path) - } - /// Returns the total depth of this path, i.e., the number of nodes this path represents. pub fn depth(&self) -> u8 { (self.nodes.len() + self.empty_nodes_mask.count_ones() as usize) as u8 @@ -159,6 +129,37 @@ impl SparseMerklePath { // PRIVATE HELPERS // ============================================================================================ + /// Constructs a sparse Merkle path from a manually specified tree depth, and an iterator over + /// Merkle nodes from deepest to shallowest. + /// + /// Knowing the size is necessary to calculate the depth of the tree, which is needed to detect + /// which nodes are empty nodes. + /// + /// Warning: this method does not check if the iterator contained more elements. + /// + /// # Errors + /// Returns [MerkleError::DepthTooBig] if `tree_depth` is greater than [SMT_MAX_DEPTH]. + fn from_iter_with_depth( + tree_depth: u8, + iter: impl IntoIterator, + ) -> Result { + if tree_depth > SMT_MAX_DEPTH { + return Err(MerkleError::DepthTooBig(tree_depth as u64)); + } + + let path: Self = iter::zip(path_depth_iter(tree_depth), iter) + .map(|(depth, node)| { + let &equivalent_empty_node = EmptySubtreeRoots::entry(tree_depth, depth.get()); + let is_empty = node == equivalent_empty_node; + let node = if is_empty { None } else { Some(node) }; + + (depth, node) + }) + .collect(); + + Ok(path) + } + const fn bitmask_for_depth(node_depth: NonZero) -> u64 { // - 1 because paths do not include the root. 1 << (node_depth.get() - 1) @@ -477,7 +478,7 @@ fn path_depth_iter(tree_depth: u8) -> impl ExactSizeIterator> #[cfg(test)] mod tests { use alloc::vec::Vec; - use core::{iter, num::NonZero}; + use core::num::NonZero; use assert_matches::assert_matches; @@ -725,25 +726,25 @@ mod tests { let index = NodeIndex::from(Smt::key_to_leaf_index(key)); let control_path = tree.get_path(key); - for (i, (&control_node, proof_index)) in - iter::zip(&*control_path, index.proof_indices()).enumerate() + for (&control_node, proof_index) in + itertools::zip_eq(&*control_path, index.proof_indices()) { let proof_node = tree.get_hash(proof_index); - assert_eq!(control_node, proof_node, "on iteration {i}"); + assert_eq!(control_node, proof_node); } let sparse_path = SparseMerklePath::from_sized_iter(control_path.clone().into_iter()).unwrap(); - for (i, (sparse_node, proof_idx)) in - iter::zip(sparse_path.clone(), index.proof_indices()).enumerate() + for (sparse_node, proof_idx) in + itertools::zip_eq(sparse_path.clone(), index.proof_indices()) { let proof_node = tree.get_hash(proof_idx); - assert_eq!(sparse_node, proof_node, "on iteration {i}"); + assert_eq!(sparse_node, proof_node); } assert_eq!(control_path.depth(), sparse_path.depth()); - for (i, (control, sparse)) in iter::zip(control_path, sparse_path).enumerate() { - assert_eq!(control, sparse, "on iteration {i}"); + for (control, sparse) in itertools::zip_eq(control_path, sparse_path) { + assert_eq!(control, sparse); } } } @@ -788,7 +789,9 @@ mod tests { // Test that both iterators yield the same amount of the same values. let mut count: u64 = 0; - for (&control_node, sparse_node) in iter::zip(control_path.iter(), sparse_path.iter()) { + for (&control_node, sparse_node) in + itertools::zip_eq(control_path.iter(), sparse_path.iter()) + { count += 1; assert_eq!(control_node, sparse_node); } @@ -809,7 +812,7 @@ mod tests { // Test that both iterators yield the same amount of the same values. let mut count: u64 = 0; - for (control_node, sparse_node) in iter::zip(control_path, sparse_path) { + for (control_node, sparse_node) in itertools::zip_eq(control_path, sparse_path) { count += 1; assert_eq!(control_node, sparse_node); }