From 8374e07c4ae66a48e65cc13351024bbff4d5b158 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Awirski?= Date: Sat, 3 May 2025 20:33:30 +0200 Subject: [PATCH 1/5] combine at_depth and at_depth_nonempty methods --- miden-crypto/src/merkle/sparse_path.rs | 29 ++++++-------------------- 1 file changed, 6 insertions(+), 23 deletions(-) diff --git a/miden-crypto/src/merkle/sparse_path.rs b/miden-crypto/src/merkle/sparse_path.rs index 4ab24f3..59dd828 100644 --- a/miden-crypto/src/merkle/sparse_path.rs +++ b/miden-crypto/src/merkle/sparse_path.rs @@ -71,33 +71,16 @@ impl SparseMerklePath { return Err(MerkleError::DepthTooBig(node_depth.get().into())); } - let node = self - .at_depth_nonempty(node_depth) - .unwrap_or_else(|| *EmptySubtreeRoots::entry(self.depth(), node_depth.get())); + let node = if self.is_depth_empty(node_depth) { + *EmptySubtreeRoots::entry(self.depth(), node_depth.get()) + } else { + let nonempty_index = self.get_nonempty_index(node_depth); + self.nodes[nonempty_index] + }; Ok(node) } - /// Get a specific non-empty node in this path at a given depth, or `None` if the specified - /// node is an empty node. - fn at_depth_nonempty(&self, node_depth: NonZero) -> Option { - assert!( - node_depth.get() <= self.depth(), - "node depth {} cannot be greater than tree depth {}", - node_depth.get(), - self.depth(), - ); - - if self.is_depth_empty(node_depth) { - return None; - } - - // Our index needs to account for all the empty nodes that aren't in `self.nodes`. - let nonempty_index = self.get_nonempty_index(node_depth); - - Some(self.nodes[nonempty_index]) - } - /// Returns the path node at the specified index, or [None] if the index is out of bounds. /// /// The node at index 0 is the deepest part of the path. From 5557a89538ccf712f23f63556abfacc97bce017e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Awirski?= Date: Sat, 3 May 2025 20:36:22 +0200 Subject: [PATCH 2/5] combine from_sized_iter and from_iter_with_depth methods --- miden-crypto/src/merkle/sparse_path.rs | 59 +++++++++----------------- 1 file changed, 21 insertions(+), 38 deletions(-) diff --git a/miden-crypto/src/merkle/sparse_path.rs b/miden-crypto/src/merkle/sparse_path.rs index 59dd828..f045646 100644 --- a/miden-crypto/src/merkle/sparse_path.rs +++ b/miden-crypto/src/merkle/sparse_path.rs @@ -47,9 +47,27 @@ impl SparseMerklePath { I: IntoIterator, { let iterator = iterator.into_iter(); - // `iterator.len() as u8` will truncate, but not below `SMT_MAX_DEPTH`, which - // `from_iter_with_depth` checks for. - Self::from_iter_with_depth(iterator.len() as u8, iterator) + let tree_depth = iterator.len() as u8; + + if tree_depth > SMT_MAX_DEPTH { + return Err(MerkleError::DepthTooBig(tree_depth as u64)); + } + + let mut empty_nodes_mask: u64 = 0; + let mut nodes: Vec = Default::default(); + + for (depth, node) in iter::zip(path_depth_iter(tree_depth), iterator) { + 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) }; + + match node { + Some(node) => nodes.push(node), + None => empty_nodes_mask |= Self::bitmask_for_depth(depth), + } + } + + Ok(SparseMerklePath { nodes, empty_nodes_mask }) } /// Returns the total depth of this path, i.e., the number of nodes this path represents. @@ -114,41 +132,6 @@ 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 mut empty_nodes_mask: u64 = 0; - let mut nodes: Vec = Default::default(); - - 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) }; - - 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 { // - 1 because paths do not include the root. 1 << (node_depth.get() - 1) From d3e70cf58bc2e5d485e6e5904b3fec0413039eef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Awirski?= Date: Sat, 3 May 2025 20:47:10 +0200 Subject: [PATCH 3/5] remove at_idx method --- miden-crypto/src/merkle/path.rs | 10 -------- miden-crypto/src/merkle/sparse_path.rs | 34 +------------------------- 2 files changed, 1 insertion(+), 43 deletions(-) diff --git a/miden-crypto/src/merkle/path.rs b/miden-crypto/src/merkle/path.rs index 0005029..28e78bf 100644 --- a/miden-crypto/src/merkle/path.rs +++ b/miden-crypto/src/merkle/path.rs @@ -48,16 +48,6 @@ impl MerklePath { 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 - /// of bounds. - /// - /// 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 { - self.nodes.get(index).copied() - } - /// Returns the depth in which this Merkle path proof is valid. pub fn depth(&self) -> u8 { self.nodes.len() as u8 diff --git a/miden-crypto/src/merkle/sparse_path.rs b/miden-crypto/src/merkle/sparse_path.rs index f045646..fa064bb 100644 --- a/miden-crypto/src/merkle/sparse_path.rs +++ b/miden-crypto/src/merkle/sparse_path.rs @@ -17,7 +17,7 @@ use super::{ /// 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. +/// [`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. @@ -99,28 +99,6 @@ impl SparseMerklePath { Ok(node) } - /// Returns the path node at the specified index, or [None] if the index is out of bounds. - /// - /// The node at index 0 is the deepest part of the path. - /// - /// ``` - /// # use core::num::NonZero; - /// # use miden_crypto::{ZERO, ONE, hash::rpo::RpoDigest, merkle::SparseMerklePath}; - /// # let zero = RpoDigest::new([ZERO; 4]); - /// # let one = RpoDigest::new([ONE; 4]); - /// # let sparse_path = SparseMerklePath::from_sized_iter(vec![zero, one, one, zero]).unwrap(); - /// let depth = NonZero::new(sparse_path.depth()).unwrap(); - /// assert_eq!( - /// sparse_path.at_idx(0).unwrap(), - /// sparse_path.at_depth(depth).unwrap(), - /// ); - /// ``` - pub fn at_idx(&self, index: usize) -> Option { - // If this overflows *or* if the depth is zero then the index was out of bounds. - let depth = NonZero::new(u8::checked_sub(self.depth(), index as u8)?)?; - self.at_depth(depth).ok() - } - // PROVIDERS // ============================================================================================ @@ -570,15 +548,6 @@ mod tests { let sparse_node = sparse_path.at_depth(depth).unwrap(); assert_eq!(control_node, sparse_node, "at depth {depth} for entry {i}"); } - - // Test random access by index. - // 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); - let sparse_node = sparse_path.at_idx(index); - assert_eq!(control_node, sparse_node); - } } } @@ -636,7 +605,6 @@ mod tests { sparse_path.at_depth(NonZero::new(1).unwrap()), Err(MerkleError::DepthTooBig(1)) ); - assert_eq!(sparse_path.at_idx(0), None); assert_eq!(sparse_path.iter().next(), None); assert_eq!(sparse_path.into_iter().next(), None); } From e405f58ef527a2cd4aa2164a7bc1c14fcc9d7fb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Awirski?= Date: Sat, 3 May 2025 20:54:00 +0200 Subject: [PATCH 4/5] document the direction of iteration --- miden-crypto/src/merkle/sparse_path.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/miden-crypto/src/merkle/sparse_path.rs b/miden-crypto/src/merkle/sparse_path.rs index fa064bb..8d3ed95 100644 --- a/miden-crypto/src/merkle/sparse_path.rs +++ b/miden-crypto/src/merkle/sparse_path.rs @@ -103,6 +103,7 @@ impl SparseMerklePath { // ============================================================================================ /// Constructs a borrowing iterator over the nodes in this path. + /// Starts from the leaf and iterates toward the root (excluding root). pub fn iter(&self) -> impl ExactSizeIterator { self.into_iter() } @@ -196,7 +197,8 @@ impl From for Vec { // ITERATORS // ================================================================================================ -/// Iterator for [`SparseMerklePath`]. +/// Iterator for [`SparseMerklePath`]. Starts from the leaf and iterates toward the root (excluding +/// root). pub struct SparseMerklePathIter<'p> { /// The "inner" value we're iterating over. path: Cow<'p, SparseMerklePath>, From f84a2e5556ddaf682d57d5ec3f2a9497ee071e79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20=C5=9Awirski?= Date: Sat, 3 May 2025 21:12:57 +0200 Subject: [PATCH 5/5] style --- miden-crypto/src/merkle/sparse_path.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/miden-crypto/src/merkle/sparse_path.rs b/miden-crypto/src/merkle/sparse_path.rs index 8d3ed95..5bdaa80 100644 --- a/miden-crypto/src/merkle/sparse_path.rs +++ b/miden-crypto/src/merkle/sparse_path.rs @@ -89,11 +89,10 @@ impl SparseMerklePath { return Err(MerkleError::DepthTooBig(node_depth.get().into())); } - let node = if self.is_depth_empty(node_depth) { - *EmptySubtreeRoots::entry(self.depth(), node_depth.get()) - } else { - let nonempty_index = self.get_nonempty_index(node_depth); + let node = if let Some(nonempty_index) = self.get_nonempty_index(node_depth) { self.nodes[nonempty_index] + } else { + *EmptySubtreeRoots::entry(self.depth(), node_depth.get()) }; Ok(node) @@ -103,7 +102,7 @@ impl SparseMerklePath { // ============================================================================================ /// Constructs a borrowing iterator over the nodes in this path. - /// Starts from the leaf and iterates toward the root (excluding root). + /// Starts from the leaf and iterates toward the root (excluding the root). pub fn iter(&self) -> impl ExactSizeIterator { self.into_iter() } @@ -120,14 +119,20 @@ impl SparseMerklePath { (self.empty_nodes_mask & Self::bitmask_for_depth(node_depth)) != 0 } - fn get_nonempty_index(&self, node_depth: NonZero) -> usize { + /// Index of the non-empty node in the `self.nodes` vector. If the specified depth is + /// empty, None is returned. + fn get_nonempty_index(&self, node_depth: NonZero) -> Option { + if self.is_depth_empty(node_depth) { + return None; + } + let bit_index = node_depth.get() - 1; let without_shallower = self.empty_nodes_mask >> bit_index; let empty_deeper = without_shallower.count_ones() as usize; // The vec index we would use if we didn't have any empty nodes to account for... let normal_index = (self.depth() - node_depth.get()) as usize; // subtracted by the number of empty nodes that are deeper than us. - normal_index - empty_deeper + Some(normal_index - empty_deeper) } } @@ -198,7 +203,7 @@ impl From for Vec { // ================================================================================================ /// Iterator for [`SparseMerklePath`]. Starts from the leaf and iterates toward the root (excluding -/// root). +/// the root). pub struct SparseMerklePathIter<'p> { /// The "inner" value we're iterating over. path: Cow<'p, SparseMerklePath>, @@ -492,7 +497,7 @@ mod tests { // 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()), + sparse_path.get_nonempty_index(NonZero::new(depth).unwrap()).unwrap(), nonempty_idx ); let test_node = sparse_path.nodes.get(nonempty_idx).unwrap();