From 353cf3e9ff9621f64e2b6fabb709f4afeca487c4 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Wed, 11 Dec 2024 21:29:45 -0700 Subject: [PATCH] Update `shardtree` to depend upon `incrementalmerkletree 0.8.0` --- Cargo.lock | 16 +------- Cargo.toml | 2 +- incrementalmerkletree-testing/Cargo.toml | 2 +- shardtree/Cargo.toml | 4 +- shardtree/src/legacy.rs | 6 +-- shardtree/src/prunable.rs | 52 +++++++++++++++++++----- shardtree/src/tree.rs | 20 +++++---- 7 files changed, 62 insertions(+), 40 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c2fde58..59c4448 100644 --- a/Cargo.lock +++ b/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", diff --git a/Cargo.toml b/Cargo.toml index 3ea6230..36d4208 100644 --- a/Cargo.toml +++ b/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 diff --git a/incrementalmerkletree-testing/Cargo.toml b/incrementalmerkletree-testing/Cargo.toml index e883418..48592f0 100644 --- a/incrementalmerkletree-testing/Cargo.toml +++ b/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 diff --git a/shardtree/Cargo.toml b/shardtree/Cargo.toml index 3dda594..b832ae0 100644 --- a/shardtree/Cargo.toml +++ b/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 diff --git a/shardtree/src/legacy.rs b/shardtree/src/legacy.rs index 86cbd45..2abf1be 100644 --- a/shardtree/src/legacy.rs +++ b/shardtree/src/legacy.rs @@ -187,7 +187,7 @@ impl LocatedPrunableTree { .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); diff --git a/shardtree/src/prunable.rs b/shardtree/src/prunable.rs index 6bd1f4d..5eb85c4 100644 --- a/shardtree/src/prunable.rs +++ b/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 From> for RetentionFlags { /// A [`Tree`] annotated with Merkle hashes. pub type PrunableTree = Tree>, (H, RetentionFlags)>; +/// Errors that can occur when merging trees. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum MergeError { + Conflict(Address), + TreeMalformed(Address), +} + +impl From 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 PrunableTree { /// Returns the the value if this is a leaf. pub fn leaf_value(&self) -> Option<&H> { @@ -242,14 +274,14 @@ impl PrunableTree { /// 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 { + pub fn merge_checked(self, root_addr: Address, other: Self) -> Result { /// Pre-condition: `root_addr` must be the address of `t0` and `t1`. #[allow(clippy::type_complexity)] fn go( addr: Address, t0: PrunableTree, t1: PrunableTree, - ) -> Result, Address> { + ) -> Result, 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 PrunableTree { 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 PrunableTree { 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 PrunableTree { }), ) = (lparent, rparent) { - let (l_addr, r_addr) = addr - .children() - .expect("The root address of a parent node must have children."); + let (l_addr, r_addr) = + addr.children().ok_or(MergeError::TreeMalformed(addr))?; Ok(Tree::unite( addr.level() - 1, lann.or(rann), @@ -310,7 +341,7 @@ impl PrunableTree { } } else { trace!(left = ?lroot, right = ?rroot, "Merge conflict for nodes"); - Err(addr) + Err(MergeError::Conflict(addr)) } } } @@ -708,7 +739,7 @@ impl LocatedPrunableTree { 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 = parent(t0, t2); diff --git a/shardtree/src/tree.rs b/shardtree/src/tree.rs index aec92f7..97ecbb2 100644 --- a/shardtree/src/tree.rs +++ b/shardtree/src/tree.rs @@ -199,33 +199,35 @@ pub struct LocatedTree { impl LocatedTree { /// Constructs a new LocatedTree from its constituent parts. /// - /// Returns `None` if `root_addr` is inconsistent with `root` (in particular, if the - /// level of `root_addr` is too small to contain `tree`). - pub fn from_parts(root_addr: Address, root: Tree) -> Option { + /// Returns the newly constructed error, or the address at which the provided tree extends + /// beyond the position range of the provided root address. + pub fn from_parts(root_addr: Address, root: Tree) -> Result { // 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(addr: Address, root: &Tree) -> bool { + fn check(addr: Address, root: &Tree) -> 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.