remove concurrent SimpleSmt::with_leaves

CHANGELOG.md

@@ -1,7 +1,7 @@
 ## 0.11.0 (2024-10-30)
 
 - [BREAKING] Updated Winterfell dependency to v0.10 (#338).
-- Added `Smt::with_entries_par()`, a parallel version of `with_entries()` with significantly better performance (#341).
+- Added parallel implementation of `Smt::with_entries()` with significantly better performance when the `concurrent` feature is enabled (#341).
 
 ## 0.11.0 (2024-10-17)
 

benches/parallel-subtree.rs

@@ -33,10 +33,14 @@ fn smt_parallel_subtree(c: &mut Criterion) {
                             (key, value)
                         })
                         .collect();
-                    entries
+
+                    let control = Smt::with_entries_sequential(entries.clone()).unwrap();
+                    (entries, control)
                 },
-                |entries| {
-                    Smt::with_entries(hint::black_box(entries)).unwrap()
+                |(entries, control)| {
+                    // Benchmarked function.
+                    let tree = Smt::with_entries(hint::black_box(entries)).unwrap();
+                    assert_eq!(tree.root(), control.root());
                 },
                 BatchSize::SmallInput,
             );

src/main.rs

@@ -34,11 +34,6 @@ pub fn benchmark_smt() {
     }
 
     let mut tree = construction(entries.clone(), tree_size).unwrap();
-    #[cfg(feature = "concurrent")]
-    {
-        let parallel = parallel_construction(entries, tree_size).unwrap();
-        assert_eq!(tree, parallel);
-    }
     insertion(&mut tree, tree_size).unwrap();
     batched_insertion(&mut tree, tree_size).unwrap();
     proof_generation(&mut tree, tree_size).unwrap();
@@ -61,27 +56,6 @@ pub fn construction(entries: Vec<(RpoDigest, Word)>, size: u64) -> Result<Smt, M
     Ok(tree)
 }
 
-#[cfg(feature = "concurrent")]
-pub fn parallel_construction(
-    entries: Vec<(RpoDigest, Word)>,
-    size: u64,
-) -> Result<Smt, MerkleError> {
-    println!("Running a parallel construction benchmark:");
-    let now = Instant::now();
-
-    let tree = Smt::with_entries(entries).unwrap();
-
-    let elapsed = now.elapsed();
-    println!(
-        "Parallel-constructed an SMT with {} key-value pairs in {:.3} seconds",
-        size,
-        elapsed.as_secs_f32(),
-    );
-    println!("Number of leaf nodes: {}\n", tree.leaves().count());
-
-    Ok(tree)
-}
-
 /// Runs the insertion benchmark for the [`Smt`].
 pub fn insertion(tree: &mut Smt, size: u64) -> Result<(), MerkleError> {
     println!("Running an insertion benchmark:");

src/merkle/smt/full/mod.rs

@@ -71,8 +71,11 @@ impl Smt {
 
     /// Returns a new [Smt] instantiated with leaves set as specified by the provided entries.
     ///
-    /// All leaves omitted from the entries list are set to [Self::EMPTY_VALUE].
+    /// If the `concurrent` feature is enabled, this function uses a parallel implementation to 
+    /// process the entries efficiently, otherwise it defaults to the sequential implementation.
     ///
+    /// All leaves omitted from the entries list are set to [Self::EMPTY_VALUE].
+    /// 
     /// # Errors
     /// Returns an error if the provided entries contain multiple values for the same key.
     pub fn with_entries(
@@ -84,12 +87,12 @@ impl Smt {
             let entries: Vec<_> = entries
                 .into_iter()
                 .map(|(key, value)| {
-                    if !seen_keys.insert(key) {
+                    if seen_keys.insert(key) {
+                        Ok((key, value))
+                    } else {
                         Err(MerkleError::DuplicateValuesForIndex(
                             LeafIndex::<SMT_DEPTH>::from(key).value(),
                         ))
-                    } else {
-                        Ok((key, value))
                     }
                 })
                 .collect::<Result<_, _>>()?;
@@ -100,28 +103,41 @@ impl Smt {
         }
         #[cfg(not(feature="concurrent"))]
         {
-            // create an empty tree
-            let mut tree = Self::new();
+            Self::with_entries_sequential(entries)
+        }
+    }
 
-            // This being a sparse data structure, the EMPTY_WORD is not assigned to the `BTreeMap`, so
-            // entries with the empty value need additional tracking.
-            let mut key_set_to_zero = BTreeSet::new();
+    /// Returns a new [Smt] instantiated with leaves set as specified by the provided entries.
+    ///
+    /// This sequential implementation processes entries one at a time to build the tree.
+    /// All leaves omitted from the entries list are set to [Self::EMPTY_VALUE].
+    ///
+    /// # Errors
+    /// Returns an error if the provided entries contain multiple values for the same key.
+    pub fn with_entries_sequential(
+        entries: impl IntoIterator<Item = (RpoDigest, Word)>,
+    ) -> Result<Self, MerkleError> {
+        // create an empty tree
+        let mut tree = Self::new();
 
-            for (key, value) in entries {
-                let old_value = tree.insert(key, value);
+        // This being a sparse data structure, the EMPTY_WORD is not assigned to the `BTreeMap`, so
+        // entries with the empty value need additional tracking.
+        let mut key_set_to_zero = BTreeSet::new();
 
-                if old_value != EMPTY_WORD || key_set_to_zero.contains(&key) {
-                    return Err(MerkleError::DuplicateValuesForIndex(
-                        LeafIndex::<SMT_DEPTH>::from(key).value(),
-                    ));
-                }
+        for (key, value) in entries {
+            let old_value = tree.insert(key, value);
 
-                if value == EMPTY_WORD {
-                    key_set_to_zero.insert(key);
-                };
+            if old_value != EMPTY_WORD || key_set_to_zero.contains(&key) {
+                return Err(MerkleError::DuplicateValuesForIndex(
+                    LeafIndex::<SMT_DEPTH>::from(key).value(),
+                ));
             }
-            Ok(tree)
+
+            if value == EMPTY_WORD {
+                key_set_to_zero.insert(key);
+            };
         }
+        Ok(tree)
     }
 
     /// Returns a new [`Smt`] instantiated from already computed leaves and nodes.
@@ -310,7 +326,7 @@ impl Smt {
     pub fn build_subtree(
         leaves: Vec<SubtreeLeaf>,
         bottom_depth: u8,
-    ) -> (BTreeMap<NodeIndex, InnerNode>, Vec<SubtreeLeaf>) {
+    ) -> (BTreeMap<NodeIndex, InnerNode>, SubtreeLeaf) {
         <Self as SparseMerkleTree<SMT_DEPTH>>::build_subtree(leaves, bottom_depth)
     }
 }

src/merkle/smt/mod.rs

@@ -452,7 +452,7 @@ pub(crate) trait SparseMerkleTree<const DEPTH: u8> {
     fn build_subtree(
         mut leaves: Vec<SubtreeLeaf>,
         bottom_depth: u8,
-    ) -> (BTreeMap<NodeIndex, InnerNode>, Vec<SubtreeLeaf>) {
+    ) -> (BTreeMap<NodeIndex, InnerNode>, SubtreeLeaf) {
         debug_assert!(bottom_depth <= DEPTH);
         debug_assert!(Integer::is_multiple_of(&bottom_depth, &SUBTREE_DEPTH));
         debug_assert!(leaves.len() <= usize::pow(2, SUBTREE_DEPTH as u32));
@@ -534,8 +534,9 @@ pub(crate) trait SparseMerkleTree<const DEPTH: u8> {
             // other collection.
             mem::swap(&mut leaves, &mut next_leaves);
         }
-
-        (inner_nodes, leaves)
+        debug_assert_eq!(leaves.len(), 1);
+        let root = leaves.pop().unwrap();
+        (inner_nodes, root)
     }
 
     /// Computes the raw parts for a new sparse Merkle tree from a set of key-value pairs.
@@ -563,22 +564,18 @@ pub(crate) trait SparseMerkleTree<const DEPTH: u8> {
         } = Self::sorted_pairs_to_leaves(entries);
 
         for current_depth in (SUBTREE_DEPTH..=DEPTH).step_by(SUBTREE_DEPTH as usize).rev() {
-            let (nodes, subtrees): (Vec<BTreeMap<_, _>>, Vec<Vec<SubtreeLeaf>>) = leaf_subtrees
+            let (nodes, mut subtree_roots): (Vec<BTreeMap<_, _>>, Vec<SubtreeLeaf>) = leaf_subtrees
                 .into_par_iter()
                 .map(|subtree| {
                     debug_assert!(subtree.is_sorted());
                     debug_assert!(!subtree.is_empty());
 
-                    let (nodes, next_leaves) = Self::build_subtree(subtree, current_depth);
-
-                    debug_assert!(next_leaves.is_sorted());
-
-                    (nodes, next_leaves)
+                    let (nodes, subtree_root) = Self::build_subtree(subtree, current_depth);
+                    (nodes, subtree_root)
                 })
                 .unzip();
 
-            let mut all_leaves: Vec<SubtreeLeaf> = subtrees.into_iter().flatten().collect();
-            leaf_subtrees = SubtreeLeavesIter::from_leaves(&mut all_leaves).collect();
+            leaf_subtrees = SubtreeLeavesIter::from_leaves(&mut subtree_roots).collect();
             accumulated_nodes.extend(nodes.into_iter().flatten());
 
             debug_assert!(!leaf_subtrees.is_empty());

src/merkle/smt/simple/mod.rs

@@ -71,60 +71,33 @@ impl<const DEPTH: u8> SimpleSmt<DEPTH> {
     pub fn with_leaves(
         entries: impl IntoIterator<Item = (u64, Word)>,
     ) -> Result<Self, MerkleError> {
+        // create an empty tree
+        let mut tree = Self::new()?;
+
         // compute the max number of entries. We use an upper bound of depth 63 because we consider
         // passing in a vector of size 2^64 infeasible.
         let max_num_entries = 2_usize.pow(DEPTH.min(63).into());
 
-        #[cfg(feature = "concurrent")]
-        {
-            let mut seen_keys = BTreeSet::new();
-            let entries: Vec<_> = entries
-            .into_iter()
-            .map(|(col, value)| {
-                if col >= max_num_entries as u64 {
-                    Err(MerkleError::InvalidIndex {
-                        depth: DEPTH,
-                        value: col,
-                    })
-                } else if !seen_keys.insert(col) {
-                    Err(MerkleError::DuplicateValuesForIndex(col))
-                } else {
-                    Ok((LeafIndex::<DEPTH>::new(col).unwrap(), value))
-                }
-            })
-            .collect::<Result<_, _>>()?;
-
-            if entries.is_empty() {
-                return Self::new();
-            }
-            <Self as SparseMerkleTree<DEPTH>>::with_entries_par(entries)
-        }
-        #[cfg(not(feature = "concurrent"))]
-        {
-            // create an empty tree
-            let mut tree = Self::new()?;
-
-            // This being a sparse data structure, the EMPTY_WORD is not assigned to the `BTreeMap`, so
-            // entries with the empty value need additional tracking.
-            let mut key_set_to_zero = BTreeSet::new();
+        // This being a sparse data structure, the EMPTY_WORD is not assigned to the `BTreeMap`, so
+        // entries with the empty value need additional tracking.
+        let mut key_set_to_zero = BTreeSet::new();
 
-            for (idx, (key, value)) in entries.into_iter().enumerate() {
-                if idx >= max_num_entries {
-                    return Err(MerkleError::InvalidNumEntries(max_num_entries));
-                }
-
-                let old_value = tree.insert(LeafIndex::<DEPTH>::new(key)?, value);
+        for (idx, (key, value)) in entries.into_iter().enumerate() {
+            if idx >= max_num_entries {
+                return Err(MerkleError::InvalidNumEntries(max_num_entries));
+            }
 
-                if old_value != Self::EMPTY_VALUE || key_set_to_zero.contains(&key) {
-                    return Err(MerkleError::DuplicateValuesForIndex(key));
-                }
+            let old_value = tree.insert(LeafIndex::<DEPTH>::new(key)?, value);
 
-                if value == Self::EMPTY_VALUE {
-                    key_set_to_zero.insert(key);
-                };
+            if old_value != Self::EMPTY_VALUE || key_set_to_zero.contains(&key) {
+                return Err(MerkleError::DuplicateValuesForIndex(key));
             }
-            Ok(tree)
+
+            if value == Self::EMPTY_VALUE {
+                key_set_to_zero.insert(key);
+            };
         }
+        Ok(tree)
     }
 
     /// Returns a new [`SimpleSmt`] instantiated from already computed leaves and nodes.

src/merkle/smt/simple/tests.rs

@@ -1,4 +1,3 @@
-#[cfg(not(feature="concurrent"))]
 use alloc::vec::Vec;
 
 use super::{
@@ -87,17 +86,16 @@ fn build_sparse_tree() {
 #[test]
 fn build_contiguous_tree() {
     let tree_with_leaves =
-        SimpleSmt::<8>::with_leaves([0, 1, 2, 3].into_iter().zip(digests_to_words(&VALUES4)))
+        SimpleSmt::<2>::with_leaves([0, 1, 2, 3].into_iter().zip(digests_to_words(&VALUES4)))
             .unwrap();
 
     let tree_with_contiguous_leaves =
-        SimpleSmt::<8>::with_contiguous_leaves(digests_to_words(&VALUES4)).unwrap();
+        SimpleSmt::<2>::with_contiguous_leaves(digests_to_words(&VALUES4)).unwrap();
 
     assert_eq!(tree_with_leaves, tree_with_contiguous_leaves);
 }
 
 #[test]
-#[cfg(not(feature="concurrent"))]
 fn test_depth2_tree() {
     let tree =
         SimpleSmt::<2>::with_leaves(KEYS4.into_iter().zip(digests_to_words(&VALUES4))).unwrap();
@@ -122,7 +120,6 @@ fn test_depth2_tree() {
 }
 
 #[test]
-#[cfg(not(feature="concurrent"))]
 fn test_inner_node_iterator() -> Result<(), MerkleError> {
     let tree =
         SimpleSmt::<2>::with_leaves(KEYS4.into_iter().zip(digests_to_words(&VALUES4))).unwrap();
@@ -154,7 +151,6 @@ fn test_inner_node_iterator() -> Result<(), MerkleError> {
 }
 
 #[test]
-#[cfg(not(feature="concurrent"))]
 fn test_insert() {
     const DEPTH: u8 = 3;
     let mut tree =
@@ -197,7 +193,6 @@ fn test_insert() {
 }
 
 #[test]
-#[cfg(not(feature="concurrent"))]
 fn small_tree_opening_is_consistent() {
     //        ____k____
     //       /         \
@@ -319,7 +314,6 @@ fn test_simplesmt_with_leaves_nonexisting_leaf() {
 }
 
 #[test]
-#[cfg(not(feature="concurrent"))]
 fn test_simplesmt_set_subtree() {
     // Final Tree:
     //
@@ -375,7 +369,6 @@ fn test_simplesmt_set_subtree() {
 
 /// Ensures that an invalid input node index into `set_subtree()` incurs no mutation of the tree
 #[test]
-#[cfg(not(feature="concurrent"))]
 fn test_simplesmt_set_subtree_unchanged_for_wrong_index() {
     // Final Tree:
     //
@@ -417,7 +410,6 @@ fn test_simplesmt_set_subtree_unchanged_for_wrong_index() {
 
 /// We insert an empty subtree that has the same depth as the original tree
 #[test]
-#[cfg(not(feature="concurrent"))]
 fn test_simplesmt_set_subtree_entire_tree() {
     // Initial Tree:
     //
@@ -471,7 +463,7 @@ fn test_simplesmt_check_empty_root_constant() {
 
 // HELPER FUNCTIONS
 // --------------------------------------------------------------------------------------------
-#[cfg(not(feature="concurrent"))]
+
 fn compute_internal_nodes() -> (RpoDigest, RpoDigest, RpoDigest) {
     let node2 = Rpo256::merge(&[VALUES4[0], VALUES4[1]]);
     let node3 = Rpo256::merge(&[VALUES4[2], VALUES4[3]]);