diff --git a/firewood/src/db.rs b/firewood/src/db.rs index e5855a5d8..d89be0d37 100644 --- a/firewood/src/db.rs +++ b/firewood/src/db.rs @@ -1,7 +1,7 @@ // Copyright (C) 2023, Ava Labs, Inc. All rights reserved. // See the file LICENSE.md for licensing terms. -use crate::merkle::{Merkle, MerkleError}; +use crate::merkle::Merkle; use crate::proof::{Proof, ProofNode}; use crate::range_proof::RangeProof; use crate::stream::MerkleKeyValueStream; @@ -23,8 +23,6 @@ use typed_builder::TypedBuilder; #[non_exhaustive] /// Represents the different types of errors that can occur in the database. pub enum DbError { - /// Merkle error occurred. - Merkle(MerkleError), /// I/O error occurred. IO(std::io::Error), } @@ -32,7 +30,6 @@ pub enum DbError { impl fmt::Display for DbError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - DbError::Merkle(e) => write!(f, "merkle error: {e:?}"), DbError::IO(e) => write!(f, "I/O error: {e:?}"), } } @@ -69,7 +66,7 @@ pub trait DbViewSync { impl DbViewSync for HistoricalRev { fn val_sync(&self, key: K) -> Result>, DbError> { let merkle = Merkle::from(self); - let value = merkle.get_value(key.as_ref()).map_err(DbError::Merkle)?; + let value = merkle.get_value(key.as_ref())?; Ok(value) } } @@ -312,12 +309,12 @@ impl Db { } /// Dump the Trie of the latest revision. - pub async fn dump(&self, w: &mut dyn Write) -> Result<(), DbError> { + pub async fn dump(&self, w: &mut dyn Write) -> Result<(), std::io::Error> { self.dump_sync(w) } /// Dump the Trie of the latest revision, synchronously. - pub fn dump_sync(&self, w: &mut dyn Write) -> Result<(), DbError> { + pub fn dump_sync(&self, w: &mut dyn Write) -> Result<(), std::io::Error> { let latest_rev_nodestore = self .manager .read() @@ -325,8 +322,8 @@ impl Db { .current_revision(); let merkle = Merkle::from(latest_rev_nodestore); // TODO: This should be a stream - let output = merkle.dump().map_err(DbError::Merkle)?; - write!(w, "{}", output).map_err(DbError::IO) + let output = merkle.dump()?; + write!(w, "{}", output) } /// Get a copy of the database metrics diff --git a/firewood/src/merkle.rs b/firewood/src/merkle.rs index 33616ddb3..d6d41533d 100644 --- a/firewood/src/merkle.rs +++ b/firewood/src/merkle.rs @@ -8,9 +8,9 @@ use crate::v2::api; use futures::{StreamExt, TryStreamExt}; use metrics::counter; use std::collections::HashSet; -use std::fmt::Debug; +use std::fmt::{Debug, Write}; use std::future::ready; -use std::io::Write; +use std::io::Error; use std::iter::once; use std::num::NonZeroUsize; use std::sync::Arc; @@ -20,8 +20,6 @@ use storage::{ TrieReader, ValueDigest, }; -use thiserror::Error; - /// Keys are boxed u8 slices pub type Key = Box<[u8]>; @@ -29,18 +27,6 @@ pub type Key = Box<[u8]>; /// TODO: change to Box<[u8]> pub type Value = Vec; -#[derive(Debug, Error)] -/// Errors that can occur when interacting with the Merkle trie -pub enum MerkleError { - /// Can't generate proof for empty node - #[error("can't generate proof for empty node")] - Empty, - - /// IO error - #[error("IO error: {0:?}")] - IO(#[from] std::io::Error), -} - // convert a set of nibbles into a printable string // panics if there is a non-nibble byte in the set #[cfg(not(feature = "branch_factor_256"))] @@ -67,20 +53,21 @@ macro_rules! write_attributes { $writer, " pp={}", nibbles_formatter($node.partial_path.0.clone()) - )?; + ) + .map_err(|e| Error::other(e))?; } #[allow(clippy::unnecessary_to_owned)] if !$value.is_empty() { match std::str::from_utf8($value) { Ok(string) if string.chars().all(char::is_alphanumeric) => { - write!($writer, " val={}", string)? + write!($writer, " val={}", string).map_err(|e| Error::other(e))? } _ => { let hex = hex::encode($value); if hex.len() > 6 { - write!($writer, " val={:.6}...", hex)?; + write!($writer, " val={:.6}...", hex).map_err(|e| Error::other(e))?; } else { - write!($writer, " val={}", hex)?; + write!($writer, " val={}", hex).map_err(|e| Error::other(e))?; } } } @@ -93,7 +80,7 @@ fn get_helper( nodestore: &T, node: &Node, key: &[u8], -) -> Result, MerkleError> { +) -> Result, Error> { // 4 possibilities for the position of the `key` relative to `node`: // 1. The node is at `key` // 2. The key is above the node (i.e. its ancestor) @@ -161,15 +148,15 @@ impl Merkle { &self.nodestore } - fn read_node(&self, addr: LinearAddress) -> Result { - self.nodestore.read_node(addr).map_err(Into::into) + fn read_node(&self, addr: LinearAddress) -> Result { + self.nodestore.read_node(addr) } /// Returns a proof that the given key has a certain value, /// or that the key isn't in the trie. - pub fn prove(&self, key: &[u8]) -> Result, MerkleError> { + pub fn prove(&self, key: &[u8]) -> Result, ProofError> { let Some(root) = self.root() else { - return Err(MerkleError::Empty); + return Err(ProofError::Empty); }; // Get the path to the key @@ -218,10 +205,7 @@ impl Merkle { todo!() } - pub(crate) fn path_iter<'a>( - &self, - key: &'a [u8], - ) -> Result, MerkleError> { + pub(crate) fn path_iter<'a>(&self, key: &'a [u8]) -> Result, Error> { PathIterator::new(&self.nodestore, key) } @@ -328,14 +312,14 @@ impl Merkle { }) } - pub(crate) fn get_value(&self, key: &[u8]) -> Result>, MerkleError> { + pub(crate) fn get_value(&self, key: &[u8]) -> Result>, Error> { let Some(node) = self.get_node(key)? else { return Ok(None); }; Ok(node.value().map(|v| v.to_vec().into_boxed_slice())) } - pub(crate) fn get_node(&self, key: &[u8]) -> Result, MerkleError> { + pub(crate) fn get_node(&self, key: &[u8]) -> Result, Error> { let Some(root) = self.root() else { return Ok(None); }; @@ -352,16 +336,16 @@ impl Merkle { hash: Option<&TrieHash>, seen: &mut HashSet, writer: &mut dyn Write, - ) -> Result<(), MerkleError> { - write!(writer, " {addr}[label=\"addr:{addr:?}")?; + ) -> Result<(), Error> { + write!(writer, " {addr}[label=\"addr:{addr:?}").map_err(Error::other)?; if let Some(hash) = hash { - write!(writer, " hash:{hash:.6?}...")?; + write!(writer, " hash:{hash:.6?}...").map_err(Error::other)?; } match &*self.read_node(addr)? { Node::Branch(b) => { write_attributes!(writer, b, &b.value.clone().unwrap_or(Box::from([]))); - writeln!(writer, "\"]")?; + writeln!(writer, "\"]").map_err(Error::other)?; for (childidx, child) in b.children.iter().enumerate() { let (child_addr, child_hash) = match child { None => continue, @@ -376,32 +360,34 @@ impl Merkle { writeln!( writer, " {addr} -> {child_addr}[label=\"{childidx} (dup)\" color=red]" - )?; + ) + .map_err(Error::other)?; } else { - writeln!(writer, " {addr} -> {child_addr}[label=\"{childidx}\"]")?; + writeln!(writer, " {addr} -> {child_addr}[label=\"{childidx}\"]") + .map_err(Error::other)?; self.dump_node(child_addr, child_hash, seen, writer)?; } } } Node::Leaf(l) => { write_attributes!(writer, l, &l.value); - writeln!(writer, "\" shape=rect]")?; + writeln!(writer, "\" shape=rect]").map_err(Error::other)?; } }; Ok(()) } - pub(crate) fn dump(&self) -> Result { - let mut result = vec![]; - writeln!(result, "digraph Merkle {{\n rankdir=LR;")?; + pub(crate) fn dump(&self) -> Result { + let mut result = String::new(); + writeln!(result, "digraph Merkle {{\n rankdir=LR;").map_err(Error::other)?; if let Some((root_addr, root_hash)) = self.nodestore.root_address_and_hash()? { - writeln!(result, " root -> {root_addr}")?; + writeln!(result, " root -> {root_addr}").map_err(Error::other)?; let mut seen = HashSet::new(); self.dump_node(root_addr, Some(&root_hash), &mut seen, &mut result)?; } - write!(result, "}}")?; + write!(result, "}}").map_err(Error::other)?; - Ok(String::from_utf8_lossy(&result).to_string()) + Ok(result) } } @@ -424,7 +410,7 @@ impl Merkle> { /// Map `key` to `value` in the trie. /// Each element of key is 2 nibbles. - pub fn insert(&mut self, key: &[u8], value: Box<[u8]>) -> Result<(), MerkleError> { + pub fn insert(&mut self, key: &[u8], value: Box<[u8]>) -> Result<(), Error> { let key = Path::from_nibbles_iterator(NibblesIterator::new(key)); let root = self.nodestore.mut_root(); @@ -453,7 +439,7 @@ impl Merkle> { mut node: Node, key: &[u8], value: Box<[u8]>, - ) -> Result { + ) -> Result { // 4 possibilities for the position of the `key` relative to `node`: // 1. The node is at `key` // 2. The key is above the node (i.e. its ancestor) @@ -585,7 +571,7 @@ impl Merkle> { /// Returns the value that was removed, if any. /// Otherwise returns `None`. /// Each element of `key` is 2 nibbles. - pub fn remove(&mut self, key: &[u8]) -> Result>, MerkleError> { + pub fn remove(&mut self, key: &[u8]) -> Result>, Error> { let key = Path::from_nibbles_iterator(NibblesIterator::new(key)); let root = self.nodestore.mut_root(); @@ -615,7 +601,7 @@ impl Merkle> { &mut self, mut node: Node, key: &[u8], - ) -> Result<(Option, Option>), MerkleError> { + ) -> Result<(Option, Option>), Error> { // 4 possibilities for the position of the `key` relative to `node`: // 1. The node is at `key` // 2. The key is above the node (i.e. its ancestor) @@ -812,7 +798,7 @@ impl Merkle> { /// Removes any key-value pairs with keys that have the given `prefix`. /// Returns the number of key-value pairs removed. - pub fn remove_prefix(&mut self, prefix: &[u8]) -> Result { + pub fn remove_prefix(&mut self, prefix: &[u8]) -> Result { let prefix = Path::from_nibbles_iterator(NibblesIterator::new(prefix)); let root = self.nodestore.mut_root(); @@ -835,7 +821,7 @@ impl Merkle> { mut node: Node, key: &[u8], deleted: &mut usize, - ) -> Result, MerkleError> { + ) -> Result, Error> { // 4 possibilities for the position of the `key` relative to `node`: // 1. The node is at `key`, in which case we need to delete this node and all its children. // 2. The key is above the node (i.e. its ancestor), so the parent needs to be restructured (TODO). @@ -965,7 +951,7 @@ impl Merkle> { &mut self, branch: &mut BranchNode, deleted: &mut usize, - ) -> Result<(), std::io::Error> { + ) -> Result<(), Error> { if branch.value.is_some() { // a KV pair was in the branch itself *deleted += 1; @@ -1295,7 +1281,7 @@ mod tests { fn get_empty_proof() { let merkle = create_in_memory_merkle().hash(); let proof = merkle.prove(b"any-key"); - assert!(matches!(proof.unwrap_err(), MerkleError::Empty)); + assert!(matches!(proof.unwrap_err(), ProofError::Empty)); } #[test] @@ -1695,7 +1681,7 @@ mod tests { fn merkle_build_test, V: AsRef<[u8]>>( items: Vec<(K, V)>, - ) -> Result>, MerkleError> { + ) -> Result>, Error> { let nodestore = NodeStore::new_empty_proposal(MemStore::new(vec![]).into()); let mut merkle = Merkle::from(nodestore); for (k, v) in items.iter() { @@ -1706,7 +1692,7 @@ mod tests { } #[test] - fn test_root_hash_simple_insertions() -> Result<(), MerkleError> { + fn test_root_hash_simple_insertions() -> Result<(), Error> { let items = vec![ ("do", "verb"), ("doe", "reindeer"), @@ -1750,7 +1736,7 @@ mod tests { } #[test] - fn test_root_hash_fuzz_insertions() -> Result<(), MerkleError> { + fn test_root_hash_fuzz_insertions() -> Result<(), Error> { use rand::rngs::StdRng; use rand::{Rng, SeedableRng}; let rng = std::cell::RefCell::new(StdRng::seed_from_u64(42)); @@ -1815,79 +1801,86 @@ mod tests { } } - // #[test] - // #[allow(clippy::unwrap_used)] - // fn test_root_hash_reversed_deletions() -> Result<(), MerkleError> { - // use rand::rngs::StdRng; - // use rand::{Rng, SeedableRng}; - // let rng = std::cell::RefCell::new(StdRng::seed_from_u64(42)); - // let max_len0 = 8; - // let max_len1 = 4; - // let keygen = || { - // let (len0, len1): (usize, usize) = { - // let mut rng = rng.borrow_mut(); - // ( - // rng.random_range(1..max_len0 + 1), - // rng.random_range(1..max_len1 + 1), - // ) - // }; - // let key: Vec = (0..len0) - // .map(|_| rng.borrow_mut().random_range(0..2)) - // .chain((0..len1).map(|_| rng.borrow_mut().random())) - // .collect(); - // key - // }; + #[test] + #[allow(clippy::unwrap_used)] + fn test_root_hash_reversed_deletions() -> Result<(), Error> { + use rand::rngs::StdRng; + use rand::{Rng, SeedableRng}; + let rng = std::cell::RefCell::new(StdRng::seed_from_u64(42)); + let max_len0 = 8; + let max_len1 = 4; + let keygen = || { + let (len0, len1): (usize, usize) = { + let mut rng = rng.borrow_mut(); + ( + rng.random_range(1..max_len0 + 1), + rng.random_range(1..max_len1 + 1), + ) + }; + let key: Vec = (0..len0) + .map(|_| rng.borrow_mut().random_range(0..2)) + .chain((0..len1).map(|_| rng.borrow_mut().random())) + .collect(); + key + }; - // for _ in 0..10 { - // let mut items: Vec<_> = (0..10) - // .map(|_| keygen()) - // .map(|key| { - // let val: Box<[u8]> = (0..8).map(|_| rng.borrow_mut().random()).collect(); - // (key, val) - // }) - // .collect(); + for _ in 0..10 { + let mut items: Vec<_> = (0..10) + .map(|_| keygen()) + .map(|key| { + let val: Box<[u8]> = (0..8).map(|_| rng.borrow_mut().random()).collect(); + (key, val) + }) + .collect(); - // items.sort(); + items.sort(); - // let mut merkle = - // Merkle::new(HashedNodeStore::initialize(MemStore::new(vec![])).unwrap()); + let mut merkle = merkle_build_test(items.clone())?; - // let mut hashes = Vec::new(); + let mut hashes = Vec::new(); - // for (k, v) in items.iter() { - // hashes.push((merkle.root_hash()?, merkle.dump()?)); - // merkle.insert(k, v.clone())?; - // } + for (k, v) in items.iter() { + let root_hash = merkle + .nodestore + .root_address_and_hash()? + .map(|(_, hash)| hash); + hashes.push((root_hash, merkle.dump()?)); + merkle.insert(k, v.clone())?; + } - // let mut new_hashes = Vec::new(); + let mut new_hashes = Vec::new(); - // for (k, _) in items.iter().rev() { - // let before = merkle.dump()?; - // merkle.remove(k)?; - // new_hashes.push((merkle.root_hash()?, k, before, merkle.dump()?)); - // } + for (k, _) in items.iter().rev() { + let before = merkle.dump()?; + merkle.remove(k)?; + let root_hash = merkle + .nodestore + .root_address_and_hash()? + .map(|(_, hash)| hash); + new_hashes.push((root_hash, k, before, merkle.dump()?)); + } - // hashes.reverse(); - - // for i in 0..hashes.len() { - // #[allow(clippy::indexing_slicing)] - // let (new_hash, key, before_removal, after_removal) = &new_hashes[i]; - // #[allow(clippy::indexing_slicing)] - // let expected_hash = &hashes[i]; - // let key = key.iter().fold(String::new(), |mut s, b| { - // let _ = write!(s, "{:02x}", b); - // s - // }); - // // assert_eq!(new_hash, expected_hash, "\n\nkey: {key}\nbefore:\n{before_removal}\nafter:\n{after_removal}\n\nexpected:\n{expected_dump}\n"); - // } - // } + hashes.reverse(); - // Ok(()) - // } + for i in 0..hashes.len() { + #[allow(clippy::indexing_slicing)] + let (new_hash, key, before_removal, after_removal) = &new_hashes[i]; + #[allow(clippy::indexing_slicing)] + let expected_hash = &hashes[i]; + let key = key.iter().fold(String::new(), |mut s, b| { + let _ = write!(s, "{:02x}", b); + s + }); + assert_eq!(new_hash.clone(), expected_hash.0, "\n\nkey: {key}\nbefore:\n{before_removal}\nafter:\n{after_removal}\n\nexpected:\n{:?}\n", expected_hash.0); + } + } + + Ok(()) + } // #[test] // #[allow(clippy::unwrap_used)] - // fn test_root_hash_random_deletions() -> Result<(), MerkleError> { + // fn test_root_hash_random_deletions() -> Result<(), Error> { // use rand::rngs::StdRng; // use rand::seq::SliceRandom; // use rand::{Rng, SeedableRng}; @@ -1963,7 +1956,7 @@ mod tests { // #[test] // #[allow(clippy::unwrap_used, clippy::indexing_slicing)] - // fn test_proof() -> Result<(), MerkleError> { + // fn test_proof() -> Result<(), Error> { // let set = fixed_and_pseudorandom_data(500); // let mut items = Vec::from_iter(set.iter()); // items.sort(); @@ -1983,7 +1976,7 @@ mod tests { // #[test] // /// Verify the proofs that end with leaf node with the given key. - // fn test_proof_end_with_leaf() -> Result<(), MerkleError> { + // fn test_proof_end_with_leaf() -> Result<(), Error> { // let items = vec![ // ("do", "verb"), // ("doe", "reindeer"), @@ -2006,7 +1999,7 @@ mod tests { // #[test] // /// Verify the proofs that end with branch node with the given key. - // fn test_proof_end_with_branch() -> Result<(), MerkleError> { + // fn test_proof_end_with_branch() -> Result<(), Error> { // let items = vec![ // ("d", "verb"), // ("do", "verb"), @@ -2026,7 +2019,7 @@ mod tests { // } // #[test] - // fn test_bad_proof() -> Result<(), MerkleError> { + // fn test_bad_proof() -> Result<(), Error> { // let set = fixed_and_pseudorandom_data(800); // let mut items = Vec::from_iter(set.iter()); // items.sort(); @@ -2049,7 +2042,7 @@ mod tests { // #[test] // // Tests that missing keys can also be proven. The test explicitly uses a single // // entry trie and checks for missing keys both before and after the single entry. - // fn test_missing_key_proof() -> Result<(), MerkleError> { + // fn test_missing_key_proof() -> Result<(), Error> { // let items = vec![("k", "v")]; // let merkle = merkle_build_test(items)?; // for key in ["a", "j", "l", "z"] { @@ -2065,7 +2058,7 @@ mod tests { // } // #[test] - // fn test_empty_tree_proof() -> Result<(), MerkleError> { + // fn test_empty_tree_proof() -> Result<(), Error> { // let items: Vec<(&str, &str)> = Vec::new(); // let merkle = merkle_build_test(items)?; // let key = "x".as_ref(); @@ -2540,7 +2533,7 @@ mod tests { // #[test] // // Tests the element is not in the range covered by proofs. // #[allow(clippy::indexing_slicing)] - // fn test_same_side_proof() -> Result<(), MerkleError> { + // fn test_same_side_proof() -> Result<(), Error> { // let set = fixed_and_pseudorandom_data(4096); // let mut items = Vec::from_iter(set.iter()); // items.sort(); diff --git a/firewood/src/proof.rs b/firewood/src/proof.rs index 6168d0555..224057ba0 100644 --- a/firewood/src/proof.rs +++ b/firewood/src/proof.rs @@ -1,7 +1,6 @@ // Copyright (C) 2023, Ava Labs, Inc. All rights reserved. // See the file LICENSE.md for licensing terms. -use crate::merkle::MerkleError; use sha2::{Digest, Sha256}; use storage::{ BranchNode, Hashable, NibblesIterator, PathIterItem, Preimage, TrieHash, ValueDigest, @@ -57,7 +56,7 @@ pub enum ProofError { /// Error from the merkle package #[error("{0:?}")] - Merkle(#[from] MerkleError), + IO(#[from] std::io::Error), /// Empty range #[error("empty range")] diff --git a/firewood/src/stream.rs b/firewood/src/stream.rs index 0df5ed62f..1377f2449 100644 --- a/firewood/src/stream.rs +++ b/firewood/src/stream.rs @@ -1,7 +1,7 @@ // Copyright (C) 2023, Ava Labs, Inc. All rights reserved. // See the file LICENSE.md for licensing terms. -use crate::merkle::{Key, MerkleError, Value}; +use crate::merkle::{Key, Value}; use crate::v2::api; use futures::stream::FusedStream; @@ -390,7 +390,7 @@ pub struct PathIterator<'a, 'b, T> { } impl<'a, 'b, T: TrieReader> PathIterator<'a, 'b, T> { - pub(super) fn new(merkle: &'a T, key: &'b [u8]) -> Result { + pub(super) fn new(merkle: &'a T, key: &'b [u8]) -> Result { let Some(root) = merkle.root_node() else { return Ok(Self { state: PathIteratorState::Exhausted, @@ -410,7 +410,7 @@ impl<'a, 'b, T: TrieReader> PathIterator<'a, 'b, T> { } impl Iterator for PathIterator<'_, '_, T> { - type Item = Result; + type Item = Result; fn next(&mut self) -> Option { // destructuring is necessary here because we need mutable access to `state` @@ -483,7 +483,7 @@ impl Iterator for PathIterator<'_, '_, T> { Some(Child::AddressWithHash(child_addr, _)) => { let child = match merkle.read_node(*child_addr) { Ok(child) => child, - Err(e) => return Some(Err(e.into())), + Err(e) => return Some(Err(e)), }; let node_key = matched_key.clone().into_boxed_slice(); diff --git a/firewood/src/v2/api.rs b/firewood/src/v2/api.rs index d82b1ee36..f440f7242 100644 --- a/firewood/src/v2/api.rs +++ b/firewood/src/v2/api.rs @@ -2,8 +2,7 @@ // See the file LICENSE.md for licensing terms. use crate::manager::RevisionManagerError; -use crate::merkle::MerkleError; -use crate::proof::{Proof, ProofNode}; +use crate::proof::{Proof, ProofError, ProofNode}; pub use crate::range_proof::RangeProof; use async_trait::async_trait; use futures::Stream; @@ -140,9 +139,9 @@ pub enum Error { #[error("sibling already committed")] SiblingCommitted, - /// Generic merkle error - #[error("merkle error: {0}")] - Merkle(#[from] MerkleError), + /// Proof error + #[error("proof error")] + ProofError(#[from] ProofError), } impl From for Error { diff --git a/firewood/src/v2/db.rs b/firewood/src/v2/db.rs deleted file mode 100644 index ba6d1255e..000000000 --- a/firewood/src/v2/db.rs +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright (C) 2023, Ava Labs, Inc. All rights reserved. -// See the file LICENSE.md for licensing terms. - -use crate::db::DbError; -use crate::v2::api; - -impl From for api::Error { - fn from(value: DbError) -> Self { - match value { - DbError::Merkle(e) => api::Error::InternalError(Box::new(e)), - DbError::IO(e) => api::Error::IO(e), - } - } -} diff --git a/firewood/src/v2/mod.rs b/firewood/src/v2/mod.rs index b1a3a938b..39cf64e42 100644 --- a/firewood/src/v2/mod.rs +++ b/firewood/src/v2/mod.rs @@ -4,9 +4,6 @@ /// The public API pub mod api; -/// The database -pub mod db; - /// The proposal pub mod propose;