Update `shardtree` to depend upon `incrementalmerkletree 0.8.0`

Cargo.lock

@@ -101,18 +101,6 @@ version = "0.3.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "d77f7ec81a6d05a3abb01ab6eb7590f6083d08449fe5a1c8b1e620283546ccb7"
 
-[[package]]
-name = "incrementalmerkletree"
-version = "0.7.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "d45063fbc4b0a37837f6bfe0445f269d13d730ad0aa3b5a7f74aa7bf27a0f4df"
-dependencies = [
- "either",
- "proptest",
- "rand",
- "rand_core",
-]
-
 [[package]]
 name = "incrementalmerkletree"
 version = "0.8.0"
@@ -128,7 +116,7 @@ dependencies = [
 name = "incrementalmerkletree-testing"
 version = "0.2.0"
 dependencies = [
- "incrementalmerkletree 0.7.0",
+ "incrementalmerkletree",
  "proptest",
 ]
 
@@ -335,7 +323,7 @@ dependencies = [
  "assert_matches",
  "bitflags 2.4.1",
  "either",
- "incrementalmerkletree 0.7.0",
+ "incrementalmerkletree",
  "incrementalmerkletree-testing",
  "proptest",
  "tempfile",

Cargo.toml

@@ -18,7 +18,7 @@ categories = ["algorithms", "data-structures"]
 
 [workspace.dependencies]
 # Intra-workspace dependencies
-incrementalmerkletree = { version = "0.7", path = "incrementalmerkletree" }
+incrementalmerkletree = { version = "0.8", path = "incrementalmerkletree" }
 incrementalmerkletree-testing = { version = "0.2", path = "incrementalmerkletree-testing" }
 
 # Testing

incrementalmerkletree-testing/Cargo.toml

@@ -18,6 +18,6 @@ all-features = true
 rustdoc-args = ["--cfg", "docsrs"]
 
 [dependencies]
-incrementalmerkletree = { version = "0.7", features = ["test-dependencies"] }
+incrementalmerkletree = { workspace = true, features = ["test-dependencies"] }
 
 proptest.workspace = true

shardtree/Cargo.toml

@@ -20,14 +20,14 @@ rustdoc-args = ["--cfg", "docsrs"]
 assert_matches = { version = "1.5", optional = true }
 bitflags = "2"
 either = "1.8"
-incrementalmerkletree = "0.7"
+incrementalmerkletree.workspace = true
 proptest = { workspace = true, optional = true }
 incrementalmerkletree-testing = { workspace = true, optional = true }
 tracing = "0.1"
 
 [dev-dependencies]
 assert_matches = "1.5"
-incrementalmerkletree = { version = "0.7", features = ["test-dependencies"] }
+incrementalmerkletree = { workspace = true, features = ["test-dependencies"] }
 incrementalmerkletree-testing.workspace = true
 proptest.workspace = true
 

shardtree/src/legacy.rs

@@ -187,7 +187,7 @@ impl<H: Hashable + Clone + PartialEq> LocatedPrunableTree<H> {
                     .tree()
                     .to_frontier()
                     .take()
-                    .expect("IncrementalWitness must not be created from the empty tree."),
+                    .expect("IncrementalWitness cannot be constructed for the empty tree."),
                 &Retention::Marked,
             )?;
 
@@ -261,7 +261,7 @@ mod tests {
         for c in 'a'..'h' {
             base_tree.append(c.to_string()).unwrap();
         }
-        let mut witness = IncrementalWitness::from_tree(base_tree);
+        let mut witness = IncrementalWitness::from_tree(base_tree).unwrap();
         for c in 'h'..'z' {
             witness.append(c.to_string()).unwrap();
         }
@@ -309,7 +309,7 @@ mod tests {
         for c in 'a'..='c' {
             base_tree.append(c.to_string()).unwrap();
         }
-        let mut witness = IncrementalWitness::from_tree(base_tree);
+        let mut witness = IncrementalWitness::from_tree(base_tree).unwrap();
         witness.append("d".to_string()).unwrap();
 
         let root_addr = Address::from_parts(Level::from(3), 0);

shardtree/src/prunable.rs

@@ -1,6 +1,7 @@
 //! Helpers for working with trees that support pruning unneeded leaves and branches.
 
 use std::collections::{BTreeMap, BTreeSet};
+use std::fmt;
 use std::sync::Arc;
 
 use bitflags::bitflags;
@@ -78,6 +79,37 @@ impl<C> From<Retention<C>> for RetentionFlags {
 /// A [`Tree`] annotated with Merkle hashes.
 pub type PrunableTree<H> = Tree<Option<Arc<H>>, (H, RetentionFlags)>;
 
+/// Errors that can occur when merging trees.
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub enum MergeError {
+    Conflict(Address),
+    TreeMalformed(Address),
+}
+
+impl From<MergeError> for InsertionError {
+    fn from(value: MergeError) -> Self {
+        match value {
+            MergeError::Conflict(addr) => InsertionError::Conflict(addr),
+            MergeError::TreeMalformed(addr) => InsertionError::InputMalformed(addr),
+        }
+    }
+}
+
+impl fmt::Display for MergeError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match &self {
+            MergeError::Conflict(addr) => write!(
+                f,
+                "Inserted root conflicts with existing root at address {:?}",
+                addr
+            ),
+            MergeError::TreeMalformed(addr) => {
+                write!(f, "Merge input malformed at address {:?}", addr)
+            }
+        }
+    }
+}
+
 impl<H: Hashable + Clone + PartialEq> PrunableTree<H> {
     /// Returns the the value if this is a leaf.
     pub fn leaf_value(&self) -> Option<&H> {
@@ -242,14 +274,14 @@ impl<H: Hashable + Clone + PartialEq> PrunableTree<H> {
     /// would cause information loss or if a conflict between root hashes occurs at a node. The
     /// returned error contains the address of the node where such a conflict occurred.
     #[tracing::instrument()]
-    pub fn merge_checked(self, root_addr: Address, other: Self) -> Result<Self, Address> {
+    pub fn merge_checked(self, root_addr: Address, other: Self) -> Result<Self, MergeError> {
         /// Pre-condition: `root_addr` must be the address of `t0` and `t1`.
         #[allow(clippy::type_complexity)]
         fn go<H: Hashable + Clone + PartialEq>(
             addr: Address,
             t0: PrunableTree<H>,
             t1: PrunableTree<H>,
-        ) -> Result<PrunableTree<H>, Address> {
+        ) -> Result<PrunableTree<H>, MergeError> {
             // Require that any roots the we compute will not be default-filled by picking
             // a starting valid fill point that is outside the range of leaf positions.
             let no_default_fill = addr.position_range_end();
@@ -261,7 +293,7 @@ impl<H: Hashable + Clone + PartialEq> PrunableTree<H> {
                         Ok(Tree::leaf((vl.0, vl.1 | vr.1)))
                     } else {
                         trace!(left = ?vl.0, right = ?vr.0, "Merge conflict for leaves");
-                        Err(addr)
+                        Err(MergeError::Conflict(addr))
                     }
                 }
                 (Tree(Node::Leaf { value }), parent @ Tree(Node::Parent { .. }))
@@ -271,7 +303,7 @@ impl<H: Hashable + Clone + PartialEq> PrunableTree<H> {
                         Ok(parent.reannotate_root(Some(Arc::new(value.0))))
                     } else {
                         trace!(leaf = ?value, node = ?parent_hash, "Merge conflict for leaf into node");
-                        Err(addr)
+                        Err(MergeError::Conflict(addr))
                     }
                 }
                 (lparent, rparent) => {
@@ -296,9 +328,8 @@ impl<H: Hashable + Clone + PartialEq> PrunableTree<H> {
                             }),
                         ) = (lparent, rparent)
                         {
-                            let (l_addr, r_addr) = addr
+                            let (l_addr, r_addr) =
-                                .children()
+                                addr.children().ok_or(MergeError::TreeMalformed(addr))?;
-                                .expect("The root address of a parent node must have children.");
                             Ok(Tree::unite(
                                 addr.level() - 1,
                                 lann.or(rann),
@@ -310,7 +341,7 @@ impl<H: Hashable + Clone + PartialEq> PrunableTree<H> {
                         }
                     } else {
                         trace!(left = ?lroot, right = ?rroot, "Merge conflict for nodes");
-                        Err(addr)
+                        Err(MergeError::Conflict(addr))
                     }
                 }
             }
@@ -708,7 +739,7 @@ impl<H: Hashable + Clone + PartialEq> LocatedPrunableTree<H> {
                         parent
                             .clone()
                             .merge_checked(root_addr, subtree.root)
-                            .map_err(InsertionError::Conflict)
+                            .map_err(InsertionError::from)
                             .map(|tree| (tree, vec![]))
                     }
                     Tree(Node::Parent { ann, left, right }) => {
@@ -1026,6 +1057,7 @@ mod tests {
     use super::{LocatedPrunableTree, PrunableTree, RetentionFlags};
     use crate::{
         error::{InsertionError, QueryError},
+        prunable::MergeError,
         testing::{arb_char_str, arb_prunable_tree},
         tree::{
             tests::{leaf, nil, parent},
@@ -1122,7 +1154,7 @@ mod tests {
         assert_eq!(
             t0.clone()
                 .merge_checked(Address::from_parts(1.into(), 0), t2.clone()),
-            Err(Address::from_parts(0.into(), 0))
+            Err(MergeError::Conflict(Address::from_parts(0.into(), 0)))
         );
 
         let t3: PrunableTree<String> = parent(t0, t2);

shardtree/src/tree.rs

@@ -199,33 +199,35 @@ pub struct LocatedTree<A, V> {
 impl<A, V> LocatedTree<A, V> {
     /// Constructs a new LocatedTree from its constituent parts.
     ///
-    /// Returns `None` if `root_addr` is inconsistent with `root` (in particular, if the
+    /// Returns the newly constructed error, or the address at which the provided tree extends
-    /// level of `root_addr` is too small to contain `tree`).
+    /// beyond the position range of the provided root address.
-    pub fn from_parts(root_addr: Address, root: Tree<A, V>) -> Option<Self> {
+    pub fn from_parts(root_addr: Address, root: Tree<A, V>) -> Result<Self, Address> {
         // In order to meet various pre-conditions throughout the crate, we require that
         // no `Node::Parent` in `root` has a level of 0 relative to `root_addr`.
-        fn is_consistent<A, V>(addr: Address, root: &Tree<A, V>) -> bool {
+        fn check<A, V>(addr: Address, root: &Tree<A, V>) -> Result<(), Address> {
             match (&root.0, addr.children()) {
                 // Found an inconsistency!
-                (Node::Parent { .. }, None) => false,
+                (Node::Parent { .. }, None) => Err(addr),
                 // Check consistency of children recursively.
                 (Node::Parent { left, right, .. }, Some((l_addr, r_addr))) => {
-                    is_consistent(l_addr, left) && is_consistent(r_addr, right)
+                    check(l_addr, left)?;
+                    check(r_addr, right)?;
+                    Ok(())
                 }
 
                 // Leaves are technically allowed to occur at any level, so we do not
                 // require `addr` to have no children.
-                (Node::Leaf { .. }, _) => true,
+                (Node::Leaf { .. }, _) => Ok(()),
 
                 // Nil nodes have no information, so we cannot verify that the data it
                 // represents is consistent with `root_addr`. Instead we rely on methods
                 // that mutate `LocatedTree` to verify that the insertion address is not
                 // inconsistent with `root_addr`.
-                (Node::Nil, _) => true,
+                (Node::Nil, _) => Ok(()),
             }
         }
 
-        is_consistent(root_addr, &root).then_some(LocatedTree { root_addr, root })
+        check(root_addr, &root).map(|_| LocatedTree { root_addr, root })
     }
 
     /// Returns the root address of this tree.