From 2df454c85584c47209a681485447abdc4d4e9223 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Awirski?= Date: Wed, 16 Apr 2025 09:47:06 +0200 Subject: [PATCH 01/12] clearer assertions in sparse_path tests --- miden-crypto/src/merkle/sparse_path.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/miden-crypto/src/merkle/sparse_path.rs b/miden-crypto/src/merkle/sparse_path.rs index 6121920..9cffb6d 100644 --- a/miden-crypto/src/merkle/sparse_path.rs +++ b/miden-crypto/src/merkle/sparse_path.rs @@ -283,7 +283,10 @@ impl Iterator for SparseMerklePathIter<'_> { self.next_depth = this_depth.get() - 1; // `this_depth` is only ever decreasing, so it can't ever exceed `self.path.depth()`. - let node = self.path.at_depth(this_depth).expect("current depth should never exceed the path depth"); + let node = self + .path + .at_depth(this_depth) + .expect("current depth should never exceed the path depth"); Some(node) } @@ -709,16 +712,20 @@ mod tests { let index = NodeIndex::from(Smt::key_to_leaf_index(key)); let control_path = tree.get_path(key); - for (&control_node, proof_index) in iter::zip(&*control_path, index.proof_indices()) { + for (i, (&control_node, proof_index)) in + iter::zip(&*control_path, index.proof_indices()).enumerate() + { let proof_node = tree.get_hash(proof_index); - assert_eq!(control_node, proof_node, "WHat"); + assert_eq!(control_node, proof_node, "on iteration {i}"); } let sparse_path = SparseMerklePath::from_sized_iter(control_path.clone().into_iter()).unwrap(); - for (sparse_node, proof_idx) in iter::zip(sparse_path.clone(), index.proof_indices()) { + for (i, (sparse_node, proof_idx)) in + iter::zip(sparse_path.clone(), index.proof_indices()).enumerate() + { let proof_node = tree.get_hash(proof_idx); - assert_eq!(sparse_node, proof_node, "WHat"); + assert_eq!(sparse_node, proof_node, "on iteration {i}"); } assert_eq!(control_path.depth(), sparse_path.depth()); From a2a315d46df45ef10c3c9cd304352470bb613b49 Mon Sep 17 00:00:00 2001 From: Qyriad Date: Thu, 24 Apr 2025 13:19:48 +0200 Subject: [PATCH 02/12] sort crate:: imports after super:: imports --- miden-crypto/src/merkle/smt/simple/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/miden-crypto/src/merkle/smt/simple/mod.rs b/miden-crypto/src/merkle/smt/simple/mod.rs index cb5ba91..abdd80b 100644 --- a/miden-crypto/src/merkle/smt/simple/mod.rs +++ b/miden-crypto/src/merkle/smt/simple/mod.rs @@ -1,12 +1,11 @@ use alloc::collections::BTreeSet; -use crate::merkle::{SparseMerklePath, SparseValuePath}; - use super::{ super::ValuePath, EMPTY_WORD, EmptySubtreeRoots, InnerNode, InnerNodeInfo, InnerNodes, LeafIndex, MerkleError, MerklePath, MutationSet, NodeIndex, RpoDigest, SMT_MAX_DEPTH, SMT_MIN_DEPTH, SparseMerkleTree, Word, }; +use crate::merkle::{SparseMerklePath, SparseValuePath}; #[cfg(test)] mod tests; From 33e2d226a159647f0faeb47bbe8ece548309f32d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Awirski?= Date: Wed, 16 Apr 2025 09:50:20 +0200 Subject: [PATCH 03/12] move try_from doc to function level for better ide support --- miden-crypto/src/merkle/sparse_path.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/miden-crypto/src/merkle/sparse_path.rs b/miden-crypto/src/merkle/sparse_path.rs index 9cffb6d..2b25222 100644 --- a/miden-crypto/src/merkle/sparse_path.rs +++ b/miden-crypto/src/merkle/sparse_path.rs @@ -211,13 +211,13 @@ impl From for MerklePath { } } -/// # Errors -/// -/// This conversion returns [MerkleError::DepthTooBig] if the path length is greater than -/// [`SMT_MAX_DEPTH`]. impl TryFrom for SparseMerklePath { type Error = MerkleError; + /// # Errors + /// + /// This conversion returns [MerkleError::DepthTooBig] if the path length is greater than + /// [`SMT_MAX_DEPTH`]. fn try_from(path: MerklePath) -> Result { SparseMerklePath::from_sized_iter(path) } From 7daabb196415d24165f452010a08fd20e2eab99b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Awirski?= Date: Wed, 16 Apr 2025 10:02:39 +0200 Subject: [PATCH 04/12] return a deserialization error if too many empty nodes detected --- miden-crypto/src/merkle/sparse_path.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/miden-crypto/src/merkle/sparse_path.rs b/miden-crypto/src/merkle/sparse_path.rs index 2b25222..b59077e 100644 --- a/miden-crypto/src/merkle/sparse_path.rs +++ b/miden-crypto/src/merkle/sparse_path.rs @@ -195,8 +195,21 @@ impl Deserializable for SparseMerklePath { source: &mut R, ) -> Result { let depth = source.read_u8()?; + if depth > SMT_MAX_DEPTH { + return Err(DeserializationError::InvalidValue(format!( + "SparseMerklePath max depth exceeded ({} > {})", + depth, SMT_MAX_DEPTH + ))); + } let empty_nodes_mask = source.read_u64()?; - let count = depth as u32 - empty_nodes_mask.count_ones(); + let empty_nodes_count = empty_nodes_mask.count_ones(); + if empty_nodes_count > depth as u32 { + return Err(DeserializationError::InvalidValue(format!( + "SparseMerklePath has more empty nodes ({}) than its full length ({})", + empty_nodes_count, depth + ))); + } + let count = depth as u32 - empty_nodes_count; let nodes = source.read_many::(count as usize)?; Ok(Self { empty_nodes_mask, nodes }) } From 92085a75e030329113a7563f7ef53b753f6d8b16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Awirski?= Date: Wed, 16 Apr 2025 10:07:56 +0200 Subject: [PATCH 05/12] move comment --- miden-crypto/src/merkle/sparse_path.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/miden-crypto/src/merkle/sparse_path.rs b/miden-crypto/src/merkle/sparse_path.rs index b59077e..1763da4 100644 --- a/miden-crypto/src/merkle/sparse_path.rs +++ b/miden-crypto/src/merkle/sparse_path.rs @@ -422,13 +422,13 @@ impl From<(SparseMerklePath, Word)> for SparseValuePath { } } -/// # Errors -/// -/// This conversion returns [MerkleError::DepthTooBig] if the path length is greater than -/// [`SMT_MAX_DEPTH`]. impl TryFrom for SparseValuePath { type Error = MerkleError; + /// # Errors + /// + /// This conversion returns [MerkleError::DepthTooBig] if the path length is greater than + /// [`SMT_MAX_DEPTH`]. fn try_from(other: ValuePath) -> Result { let ValuePath { value, path } = other; let path = SparseMerklePath::try_from(path)?; From 6eb93aa3348695a8a08aac40a8f108c0c130c23c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Awirski?= Date: Wed, 16 Apr 2025 10:26:46 +0200 Subject: [PATCH 06/12] keep at_depth and at_idx consistent wrt ownership --- miden-crypto/src/merkle/path.rs | 8 ++++---- miden-crypto/src/merkle/sparse_path.rs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/miden-crypto/src/merkle/path.rs b/miden-crypto/src/merkle/path.rs index 564d0ae..0005029 100644 --- a/miden-crypto/src/merkle/path.rs +++ b/miden-crypto/src/merkle/path.rs @@ -43,9 +43,9 @@ impl MerklePath { /// The `depth` parameter is defined in terms of `self.depth()`. Merkle paths conventionally do /// not include the root, so the shallowest depth is `1`, and the deepest depth is /// `self.depth()`. - pub fn at_depth(&self, depth: NonZero) -> Option<&RpoDigest> { + pub fn at_depth(&self, depth: NonZero) -> Option { let index = u8::checked_sub(self.depth(), depth.get())?; - self.nodes.get(index as usize) + self.nodes.get(index as usize).copied() } /// Returns a reference to the path node at the specified index, or [None] if the index is out @@ -54,8 +54,8 @@ impl MerklePath { /// The node at index 0 is the deepest part of the path. /// /// This is a checked version of using the indexing operator `[]`. - pub fn at_idx(&self, index: usize) -> Option<&RpoDigest> { - self.nodes.get(index) + pub fn at_idx(&self, index: usize) -> Option { + self.nodes.get(index).copied() } /// Returns the depth in which this Merkle path proof is valid. diff --git a/miden-crypto/src/merkle/sparse_path.rs b/miden-crypto/src/merkle/sparse_path.rs index 1763da4..22d5a9d 100644 --- a/miden-crypto/src/merkle/sparse_path.rs +++ b/miden-crypto/src/merkle/sparse_path.rs @@ -760,7 +760,7 @@ mod tests { // Test random access by depth. for depth in path_depth_iter(control_path.depth()) { - let &control_node = control_path.at_depth(depth).unwrap(); + let control_node = control_path.at_depth(depth).unwrap(); let sparse_node = sparse_path.at_depth(depth).unwrap(); assert_eq!(control_node, sparse_node, "at depth {depth} for entry {i}"); } @@ -769,7 +769,7 @@ mod tests { // Letting index get to `control_path.len()` will test that both sides correctly return // `None` for out of bounds access. for index in 0..=(control_path.len()) { - let control_node = control_path.at_idx(index).copied(); + let control_node = control_path.at_idx(index); let sparse_node = sparse_path.at_idx(index); assert_eq!(control_node, sparse_node); } From 3362aeaddba355185aa4ab27c2f6fe7f785e0378 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 07/12] 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); } From 5d8e3f0cf86141468d0ea07848dc02323cd06bfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Awirski?= Date: Wed, 16 Apr 2025 11:27:12 +0200 Subject: [PATCH 08/12] remove unsafe FromIterator<(NonZero, Option)> --- miden-crypto/src/merkle/sparse_path.rs | 43 ++++++++------------------ 1 file changed, 13 insertions(+), 30 deletions(-) diff --git a/miden-crypto/src/merkle/sparse_path.rs b/miden-crypto/src/merkle/sparse_path.rs index 305221c..8095452 100644 --- a/miden-crypto/src/merkle/sparse_path.rs +++ b/miden-crypto/src/merkle/sparse_path.rs @@ -147,17 +147,21 @@ impl SparseMerklePath { 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) }; + let mut empty_nodes_mask: u64 = 0; + let mut nodes: Vec = Default::default(); - (depth, node) - }) - .collect(); + for (depth, node) in iter::zip(path_depth_iter(tree_depth), iter) { + 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) }; - Ok(path) + match node { + Some(node) => nodes.push(node), + None => empty_nodes_mask |= Self::bitmask_for_depth(depth), + } + } + + Ok(SparseMerklePath { nodes, empty_nodes_mask }) } const fn bitmask_for_depth(node_depth: NonZero) -> u64 { @@ -246,27 +250,6 @@ impl From for Vec { // ITERATORS // ================================================================================================ -/// Contructs a [SparseMerklePath] out of an iterator of optional nodes, where `None` indicates an -/// empty node. -impl FromIterator<(NonZero, Option)> for SparseMerklePath { - fn from_iter(iter: I) -> SparseMerklePath - where - I: IntoIterator, Option)>, - { - let mut empty_nodes_mask: u64 = 0; - let mut nodes: Vec = Default::default(); - - for (depth, node) in iter { - match node { - Some(node) => nodes.push(node), - None => empty_nodes_mask |= Self::bitmask_for_depth(depth), - } - } - - SparseMerklePath { nodes, empty_nodes_mask } - } -} - impl<'p> IntoIterator for &'p SparseMerklePath { type Item = as Iterator>::Item; type IntoIter = SparseMerklePathIter<'p>; From 8676326550e30b7f590473b6ee0b153392f1bac8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Awirski?= Date: Wed, 16 Apr 2025 11:33:16 +0200 Subject: [PATCH 09/12] clarify that SparseMerklePath can store up to SMT_MAX_DEPTH nodes total --- miden-crypto/src/merkle/sparse_path.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/miden-crypto/src/merkle/sparse_path.rs b/miden-crypto/src/merkle/sparse_path.rs index 8095452..cad3f04 100644 --- a/miden-crypto/src/merkle/sparse_path.rs +++ b/miden-crypto/src/merkle/sparse_path.rs @@ -14,10 +14,10 @@ use super::{ /// with empty nodes. /// /// Empty nodes in the path are stored only as their position, represented with a bitmask. A -/// maximum of 64 nodes in the path can be empty. The more nodes in a path are empty, the less -/// memory this struct will use. This type calculates empty nodes on-demand when iterated through, -/// converted to a [MerklePath], or an empty node is retrieved with [`SparseMerklePath::at_idx()`] -/// or [`SparseMerklePath::at_depth()`], which will incur overhead. +/// maximum of 64 nodes (`SMT_MAX_DEPTH`) can be stored (empty and non-empty). The more nodes in a +/// path are empty, the less memory this struct will use. This type calculates empty nodes on-demand +/// when iterated through, converted to a [MerklePath], or an empty node is retrieved with +/// [`SparseMerklePath::at_idx()`] or [`SparseMerklePath::at_depth()`], which will incur overhead. /// /// NOTE: This type assumes that Merkle paths always span from the root of the tree to a leaf. /// Partial paths are not supported. From 0cc404c66fb3babdccad5d37601606b9800d2a16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Awirski?= Date: Wed, 16 Apr 2025 11:45:09 +0200 Subject: [PATCH 10/12] clarify empty_nodes_mask documentation --- miden-crypto/src/merkle/sparse_path.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/miden-crypto/src/merkle/sparse_path.rs b/miden-crypto/src/merkle/sparse_path.rs index cad3f04..a86095b 100644 --- a/miden-crypto/src/merkle/sparse_path.rs +++ b/miden-crypto/src/merkle/sparse_path.rs @@ -25,6 +25,8 @@ use super::{ #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] pub struct SparseMerklePath { /// A bitmask representing empty nodes. The set bit corresponds to the depth of an empty node. + /// The least significant bit (bit 0) describes depth 1 node (root's children). + /// The `bit index + 1` is equal to node's depth. empty_nodes_mask: u64, /// The non-empty nodes, stored in depth-order, but not contiguous across depth. nodes: Vec, From 4356d11e594014b36dc96a8772a2bd79decfbffe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Awirski?= Date: Wed, 16 Apr 2025 11:58:18 +0200 Subject: [PATCH 11/12] loop over depths in tests --- miden-crypto/src/merkle/sparse_path.rs | 153 ++++--------------------- 1 file changed, 21 insertions(+), 132 deletions(-) diff --git a/miden-crypto/src/merkle/sparse_path.rs b/miden-crypto/src/merkle/sparse_path.rs index a86095b..a918510 100644 --- a/miden-crypto/src/merkle/sparse_path.rs +++ b/miden-crypto/src/merkle/sparse_path.rs @@ -561,145 +561,34 @@ mod tests { assert_eq!(sparse_path.empty_nodes_mask, EMPTY_BITS); - // Depth 8. - { - let depth: u8 = 8; + // Keep track of how many non-empty nodes we have seen + let mut nonempty_idx = 0; - // Check that the way we calculate these indices is correct. + // Test starting from the deepest nodes (depth 8) + for depth in (1..=8).rev() { let idx = (sparse_path.depth() - depth) as usize; - assert_eq!(idx, 0); + let bit = 1 << (depth - 1); - // Check that the way we calculate these bitmasks is correct. - let bit = 0b1000_0000; - assert_eq!(bit, 1 << (depth - 1)); - - // Check that the depth-8 bit is not set... - let is_set = (sparse_path.empty_nodes_mask & bit) != 0; - assert!(!is_set); - // ...which should match the status of the `sparse_nodes` element being `None`. - assert_eq!(is_set, sparse_nodes.get(idx).unwrap().is_none()); - - // And finally, check that we can calculate non-empty indices correctly. - let control_node = raw_nodes.get(idx).unwrap(); - let nonempty_idx: usize = 0; - assert_eq!(sparse_path.get_nonempty_index(NonZero::new(depth).unwrap()), nonempty_idx); - let test_node = sparse_path.nodes.get(nonempty_idx).unwrap(); - assert_eq!(test_node, control_node); - } - - // Rinse and repeat for each remaining depth. - - // Depth 7. - { - let depth: u8 = 7; - let idx = (sparse_path.depth() - depth) as usize; - assert_eq!(idx, 1); - let bit = 0b0100_0000; - assert_eq!(bit, 1 << (depth - 1)); - let is_set = (sparse_path.empty_nodes_mask & bit) != 0; - assert!(is_set); - assert_eq!(is_set, sparse_nodes.get(idx).unwrap().is_none()); - - let &test_node = sparse_nodes.get(idx).unwrap(); - assert_eq!(test_node, None); - } - - // Depth 6. - { - let depth: u8 = 6; - let idx = (sparse_path.depth() - depth) as usize; - assert_eq!(idx, 2); - let bit = 0b0010_0000; - assert_eq!(bit, 1 << (depth - 1)); + // Check that the depth bit is set correctly... let is_set = (sparse_path.empty_nodes_mask & bit) != 0; assert_eq!(is_set, sparse_nodes.get(idx).unwrap().is_none()); - assert!(is_set); - let &test_node = sparse_nodes.get(idx).unwrap(); - assert_eq!(test_node, None); - } + if is_set { + // Check that we don't return digests for empty nodes + let &test_node = sparse_nodes.get(idx).unwrap(); + assert_eq!(test_node, None); + } else { + // Check that we can calculate non-empty indices correctly. + let control_node = raw_nodes.get(idx).unwrap(); + assert_eq!( + sparse_path.get_nonempty_index(NonZero::new(depth).unwrap()), + nonempty_idx + ); + let test_node = sparse_path.nodes.get(nonempty_idx).unwrap(); + assert_eq!(test_node, control_node); - // Depth 5. - { - let depth: u8 = 5; - let idx = (sparse_path.depth() - depth) as usize; - assert_eq!(idx, 3); - let bit = 0b0001_0000; - assert_eq!(bit, 1 << (depth - 1)); - let is_set = (sparse_path.empty_nodes_mask & bit) != 0; - assert_eq!(is_set, sparse_nodes.get(idx).unwrap().is_none()); - assert!(!is_set); - - let control_node = raw_nodes.get(idx).unwrap(); - let nonempty_idx: usize = 1; - assert_eq!(sparse_path.nodes.get(nonempty_idx).unwrap(), control_node); - assert_eq!(sparse_path.get_nonempty_index(NonZero::new(depth).unwrap()), nonempty_idx,); - let test_node = sparse_path.nodes.get(nonempty_idx).unwrap(); - assert_eq!(test_node, control_node); - } - - // Depth 4. - { - let depth: u8 = 4; - let idx = (sparse_path.depth() - depth) as usize; - assert_eq!(idx, 4); - let bit = 0b0000_1000; - assert_eq!(bit, 1 << (depth - 1)); - let is_set = (sparse_path.empty_nodes_mask & bit) != 0; - assert_eq!(is_set, sparse_nodes.get(idx).unwrap().is_none()); - assert!(!is_set); - - let control_node = raw_nodes.get(idx).unwrap(); - let nonempty_idx: usize = 2; - assert_eq!(sparse_path.nodes.get(nonempty_idx).unwrap(), control_node); - assert_eq!(sparse_path.get_nonempty_index(NonZero::new(depth).unwrap()), nonempty_idx,); - let test_node = sparse_path.nodes.get(nonempty_idx).unwrap(); - assert_eq!(test_node, control_node); - } - - // Depth 3. - { - let depth: u8 = 3; - let idx = (sparse_path.depth() - depth) as usize; - assert_eq!(idx, 5); - let bit = 0b0000_0100; - assert_eq!(bit, 1 << (depth - 1)); - let is_set = (sparse_path.empty_nodes_mask & bit) != 0; - assert!(is_set); - assert_eq!(is_set, sparse_nodes.get(idx).unwrap().is_none()); - - let &test_node = sparse_nodes.get(idx).unwrap(); - assert_eq!(test_node, None); - } - - // Depth 2. - { - let depth: u8 = 2; - let idx = (sparse_path.depth() - depth) as usize; - assert_eq!(idx, 6); - let bit = 0b0000_0010; - assert_eq!(bit, 1 << (depth - 1)); - let is_set = (sparse_path.empty_nodes_mask & bit) != 0; - assert!(is_set); - assert_eq!(is_set, sparse_nodes.get(idx).unwrap().is_none()); - - let &test_node = sparse_nodes.get(idx).unwrap(); - assert_eq!(test_node, None); - } - - // Depth 1. - { - let depth: u8 = 1; - let idx = (sparse_path.depth() - depth) as usize; - assert_eq!(idx, 7); - let bit = 0b0000_0001; - assert_eq!(bit, 1 << (depth - 1)); - let is_set = (sparse_path.empty_nodes_mask & bit) != 0; - assert!(is_set); - assert_eq!(is_set, sparse_nodes.get(idx).unwrap().is_none()); - - let &test_node = sparse_nodes.get(idx).unwrap(); - assert_eq!(test_node, None); + nonempty_idx += 1; + } } } From 750ce32a715661e620b08795c1ac2a2af67ec491 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Awirski?= Date: Wed, 16 Apr 2025 17:47:33 +0200 Subject: [PATCH 12/12] use Cow instead of the additional IntoIter struct --- miden-crypto/src/merkle/sparse_path.rs | 71 +++++++------------------- 1 file changed, 18 insertions(+), 53 deletions(-) diff --git a/miden-crypto/src/merkle/sparse_path.rs b/miden-crypto/src/merkle/sparse_path.rs index a918510..32ba2d6 100644 --- a/miden-crypto/src/merkle/sparse_path.rs +++ b/miden-crypto/src/merkle/sparse_path.rs @@ -1,4 +1,4 @@ -use alloc::vec::Vec; +use alloc::{borrow::Cow, vec::Vec}; use core::{ iter::{self, FusedIterator}, num::NonZero, @@ -252,20 +252,10 @@ impl From for Vec { // ITERATORS // ================================================================================================ -impl<'p> IntoIterator for &'p SparseMerklePath { - type Item = as Iterator>::Item; - type IntoIter = SparseMerklePathIter<'p>; - - fn into_iter(self) -> SparseMerklePathIter<'p> { - let tree_depth = self.depth(); - SparseMerklePathIter { path: self, next_depth: tree_depth } - } -} - -/// Borrowing iterator for [`SparseMerklePath`]. +/// Iterator for [`SparseMerklePath`]. pub struct SparseMerklePathIter<'p> { /// The "inner" value we're iterating over. - path: &'p SparseMerklePath, + path: Cow<'p, SparseMerklePath>, /// The depth a `next()` call will get. `next_depth == 0` indicates that the iterator has been /// exhausted. @@ -306,57 +296,32 @@ impl FusedIterator for SparseMerklePathIter<'_> {} // TODO: impl DoubleEndedIterator. -/// Owning iterator for [SparseMerklePath]. -pub struct IntoIter { - /// The "inner" value we're iterating over. - path: SparseMerklePath, - - /// The depth a `next()` call will get. `next_depth == 0` indicates that the iterator has been - /// exhausted. - next_depth: u8, -} - impl IntoIterator for SparseMerklePath { - type IntoIter = IntoIter; + type IntoIter = SparseMerklePathIter<'static>; type Item = ::Item; - fn into_iter(self) -> IntoIter { + fn into_iter(self) -> SparseMerklePathIter<'static> { let tree_depth = self.depth(); - IntoIter { path: self, next_depth: tree_depth } + SparseMerklePathIter { + path: Cow::Owned(self), + next_depth: tree_depth, + } } } -impl Iterator for IntoIter { - type Item = RpoDigest; +impl<'p> IntoIterator for &'p SparseMerklePath { + type Item = as Iterator>::Item; + type IntoIter = SparseMerklePathIter<'p>; - fn next(&mut self) -> Option { - let this_depth = self.next_depth; - // Paths don't include the root, so if `this_depth` is 0 then we keep returning `None`. - let this_depth = NonZero::new(this_depth)?; - self.next_depth = this_depth.get() - 1; - - // `this_depth` is only ever decreasing, so it can't ever exceed `self.path.depth()`. - let node = self.path.at_depth(this_depth).unwrap(); - Some(node) - } - - // IntoIter always knows its exact size. - fn size_hint(&self) -> (usize, Option) { - let remaining = ExactSizeIterator::len(self); - (remaining, Some(remaining)) + fn into_iter(self) -> SparseMerklePathIter<'p> { + let tree_depth = self.depth(); + SparseMerklePathIter { + path: Cow::Borrowed(self), + next_depth: tree_depth, + } } } -impl ExactSizeIterator for IntoIter { - fn len(&self) -> usize { - self.next_depth as usize - } -} - -impl FusedIterator for IntoIter {} - -// TODO: impl DoubleEndedIterator. - // COMPARISONS // ================================================================================================ impl PartialEq for SparseMerklePath {