diff --git a/firewood/src/db.rs b/firewood/src/db.rs index df5489c1f..4564d925e 100644 --- a/firewood/src/db.rs +++ b/firewood/src/db.rs @@ -15,28 +15,26 @@ use std::error::Error; use std::fmt; use std::io::Write; use std::path::Path; -use std::sync::{Arc, RwLock}; +use std::sync::Arc; use storage::{Committed, FileBacked, HashedNodeReader, ImmutableProposal, NodeStore, TrieHash}; +use tokio::sync::RwLock; use typed_builder::TypedBuilder; #[derive(Debug)] #[non_exhaustive] +/// Represents the different types of errors that can occur in the database. pub enum DbError { - InvalidParams, + /// Merkle error occurred. Merkle(MerkleError), - CreateError, + /// I/O error occurred. IO(std::io::Error), - InvalidProposal, } impl fmt::Display for DbError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - DbError::InvalidParams => write!(f, "invalid parameters provided"), DbError::Merkle(e) => write!(f, "merkle error: {e:?}"), - DbError::CreateError => write!(f, "database create error"), DbError::IO(e) => write!(f, "I/O error: {e:?}"), - DbError::InvalidProposal => write!(f, "invalid proposal"), } } } @@ -51,6 +49,8 @@ impl Error for DbError {} type HistoricalRev = NodeStore; +/// Metrics for the database. +/// TODO: Add more metrics pub struct DbMetrics { proposals: metrics::Counter, } @@ -109,11 +109,13 @@ pub struct DbConfig { /// existing contents will be lost. #[builder(default = false)] pub truncate: bool, + /// Revision manager configuration. #[builder(default = RevisionManagerConfig::builder().build())] pub manager: RevisionManagerConfig, } #[derive(Debug)] +/// A database instance. pub struct Db { metrics: Arc, // TODO: consider using https://docs.rs/lock_api/latest/lock_api/struct.RwLock.html#method.upgradable_read @@ -134,20 +136,16 @@ where Self: 'p; async fn revision(&self, root_hash: TrieHash) -> Result, api::Error> { - let nodestore = self - .manager - .read() - .expect("poisoned lock") - .revision(root_hash)?; + let nodestore = self.manager.read().await.revision(root_hash)?; Ok(nodestore) } async fn root_hash(&self) -> Result, api::Error> { - Ok(self.manager.read().expect("poisoned lock").root_hash()?) + Ok(self.manager.read().await.root_hash()?) } async fn all_hashes(&self) -> Result, api::Error> { - Ok(self.manager.read().expect("poisoned lock").all_hashes()) + Ok(self.manager.read().await.all_hashes()) } async fn propose<'p, K: KeyType, V: ValueType>( @@ -157,11 +155,7 @@ where where Self: 'p, { - let parent = self - .manager - .read() - .expect("poisoned lock") - .current_revision(); + let parent = self.manager.read().await.current_revision(); let proposal = NodeStore::new(parent)?; let mut merkle = Merkle::from(proposal); for op in batch { @@ -177,10 +171,7 @@ where let nodestore = merkle.into_inner(); let immutable: Arc, FileBacked>> = Arc::new(nodestore.into()); - self.manager - .write() - .expect("poisoned lock") - .add_proposal(immutable.clone()); + self.manager.write().await.add_proposal(immutable.clone()); self.metrics.proposals.increment(1); @@ -193,6 +184,7 @@ where } impl Db { + /// Create a new database instance. pub async fn new>(db_path: P, cfg: DbConfig) -> Result { let metrics = Arc::new(DbMetrics { proposals: counter!("firewood.proposals"), @@ -211,24 +203,22 @@ impl Db { } /// Dump the Trie of the latest revision. - pub fn dump(&self, w: &mut dyn Write) -> Result<(), DbError> { - let latest_rev_nodestore = self - .manager - .read() - .expect("poisoned lock") - .current_revision(); + pub async fn dump(&self, w: &mut dyn Write) -> Result<(), DbError> { + let latest_rev_nodestore = self.manager.read().await.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) } + /// Get a copy of the database metrics pub fn metrics(&self) -> Arc { self.metrics.clone() } } #[derive(Debug)] +/// A user-visible database proposal pub struct Proposal<'p> { nodestore: Arc, FileBacked>>, db: &'p Db, @@ -299,7 +289,7 @@ impl<'a> api::Proposal for Proposal<'a> { self.db .manager .write() - .expect("poisoned lock") + .await .add_proposal(immutable.clone()); Ok(Self::Proposal { @@ -312,7 +302,7 @@ impl<'a> api::Proposal for Proposal<'a> { async fn commit(self: Arc) -> Result<(), api::Error> { match Arc::into_inner(self) { Some(proposal) => { - let mut manager = proposal.db.manager.write().expect("poisoned lock"); + let mut manager = proposal.db.manager.write().await; Ok(manager.commit(proposal.nodestore.clone())?) } None => Err(api::Error::CannotCommitClonedProposal), diff --git a/firewood/src/lib.rs b/firewood/src/lib.rs index 26037445a..2d0d15f45 100644 --- a/firewood/src/lib.rs +++ b/firewood/src/lib.rs @@ -105,13 +105,27 @@ //! A commit involves simply writing the nodes and the freelist to disk. If the proposal is //! abandoned, nothing has actually been written to disk. //! +#![warn(missing_debug_implementations, rust_2018_idioms, missing_docs)] +/// Database module for Firewood. pub mod db; + +/// Database manager module pub mod manager; + +/// Merkle module, containing merkle operations pub mod merkle; + +/// Proof module pub mod proof; + +/// Range proof module pub mod range_proof; + +/// Stream module, for both node and key-value streams pub mod stream; +/// Version 2 API pub mod v2; +/// Expose the storage logger pub use storage::logger; diff --git a/firewood/src/manager.rs b/firewood/src/manager.rs index 289ad5702..d5ba09ab7 100644 --- a/firewood/src/manager.rs +++ b/firewood/src/manager.rs @@ -17,11 +17,13 @@ use crate::v2::api::HashKey; use storage::{Committed, FileBacked, ImmutableProposal, NodeStore, Parentable, TrieHash}; #[derive(Clone, Debug, TypedBuilder)] +/// Revision manager configuratoin pub struct RevisionManagerConfig { /// The number of historical revisions to keep in memory. #[builder(default = 128)] max_revisions: usize, + /// The size of the node cache #[builder(default_code = "NonZero::new(1500000).expect(\"non-zero\")")] node_cache_size: NonZero, diff --git a/firewood/src/merkle.rs b/firewood/src/merkle.rs index 66812c31d..4382c750a 100644 --- a/firewood/src/merkle.rs +++ b/firewood/src/merkle.rs @@ -23,29 +23,23 @@ use storage::{ use thiserror::Error; +/// Keys are boxed u8 slices pub type Key = Box<[u8]>; + +/// Values are vectors +/// 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, - #[error("read only")] - ReadOnly, - #[error("node not a branch node")] - NotBranchNode, + + /// IO error #[error("IO error: {0:?}")] IO(#[from] std::io::Error), - #[error("parent should not be a leaf branch")] - ParentLeafBranch, - #[error("removing internal node references failed")] - UnsetInternal, - #[error("merkle serde error: {0}")] - BinarySerdeError(String), - #[error("invalid utf8")] - UTF8Error, - #[error("node not found")] - NodeNotFound, } // convert a set of nibbles into a printable string @@ -141,12 +135,13 @@ fn get_helper( } #[derive(Debug)] +/// Merkle operations against a nodestore pub struct Merkle { nodestore: T, } impl Merkle { - pub fn into_inner(self) -> T { + pub(crate) fn into_inner(self) -> T { self.nodestore } } @@ -158,11 +153,12 @@ impl From for Merkle { } impl Merkle { - pub fn root(&self) -> Option> { + pub(crate) fn root(&self) -> Option> { self.nodestore.root_node() } - pub const fn nodestore(&self) -> &T { + #[cfg(test)] + pub(crate) const fn nodestore(&self) -> &T { &self.nodestore } @@ -211,6 +207,7 @@ impl Merkle { Ok(Proof(proof.into_boxed_slice())) } + /// Verify a proof that a key has a certain value, or that the key isn't in the trie. pub fn verify_range_proof>( &self, _proof: &Proof, @@ -222,7 +219,10 @@ impl Merkle { todo!() } - pub fn path_iter<'a>(&self, key: &'a [u8]) -> Result, MerkleError> { + pub(crate) fn path_iter<'a>( + &self, + key: &'a [u8], + ) -> Result, MerkleError> { PathIterator::new(&self.nodestore, key) } @@ -329,14 +329,14 @@ impl Merkle { }) } - pub fn get_value(&self, key: &[u8]) -> Result>, MerkleError> { + pub(crate) fn get_value(&self, key: &[u8]) -> Result>, MerkleError> { let Some(node) = self.get_node(key)? else { return Ok(None); }; Ok(node.value().map(|v| v.to_vec().into_boxed_slice())) } - pub fn get_node(&self, key: &[u8]) -> Result>, MerkleError> { + pub(crate) fn get_node(&self, key: &[u8]) -> Result>, MerkleError> { let Some(root) = self.root() else { return Ok(None); }; @@ -347,7 +347,7 @@ impl Merkle { } impl Merkle { - pub fn dump_node( + pub(crate) fn dump_node( &self, addr: LinearAddress, hash: Option<&TrieHash>, @@ -392,7 +392,7 @@ impl Merkle { Ok(()) } - pub fn dump(&self) -> Result { + pub(crate) fn dump(&self) -> Result { let mut result = vec![]; writeln!(result, "digraph Merkle {{\n rankdir=LR;")?; if let Some((root_addr, root_hash)) = self.nodestore.root_address_and_hash()? { @@ -417,6 +417,8 @@ impl From>> } impl Merkle> { + /// Convert a merkle backed by an MutableProposal into an ImmutableProposal + /// TODO: We probably don't need this function pub fn hash(self) -> Merkle, S>> { self.into() } diff --git a/firewood/src/proof.rs b/firewood/src/proof.rs index 8e82ade79..6168d0555 100644 --- a/firewood/src/proof.rs +++ b/firewood/src/proof.rs @@ -9,36 +9,64 @@ use storage::{ use thiserror::Error; #[derive(Debug, Error)] +/// Reasons why a proof is invalid pub enum ProofError { + /// Non-monotonic range decrease #[error("non-monotonic range increase")] NonMonotonicIncreaseRange, + + /// Unexpected hash #[error("unexpected hash")] UnexpectedHash, + + /// Unexpected value #[error("unexpected value")] UnexpectedValue, + + /// Value mismatch #[error("value mismatch")] ValueMismatch, + + /// Expected value but got None #[error("expected value but got None")] ExpectedValue, + + /// Proof is empty #[error("proof can't be empty")] Empty, + + /// Each proof node key should be a prefix of the proven key #[error("each proof node key should be a prefix of the proven key")] ShouldBePrefixOfProvenKey, + + /// Each proof node key should be a prefix of the next key #[error("each proof node key should be a prefix of the next key")] ShouldBePrefixOfNextKey, + + /// Child index is out of bounds #[error("child index is out of bounds")] ChildIndexOutOfBounds, + + /// Only nodes with even length key can have values #[error("only nodes with even length key can have values")] ValueAtOddNibbleLength, + + /// Node not in trie #[error("node not in trie")] NodeNotInTrie, + + /// Error from the merkle package #[error("{0:?}")] Merkle(#[from] MerkleError), + + /// Empty range #[error("empty range")] EmptyRange, } #[derive(Clone, Debug)] + +/// A node in a proof. pub struct ProofNode { /// The key this node is at. Each byte is a nibble. pub key: Box<[u8]>, @@ -104,6 +132,7 @@ impl From<&ProofNode> for TrieHash { pub struct Proof(pub Box<[T]>); impl Proof { + /// Verify a proof pub fn verify, V: AsRef<[u8]>>( &self, key: K, diff --git a/firewood/src/stream.rs b/firewood/src/stream.rs index 6f305c4b0..097f02a2a 100644 --- a/firewood/src/stream.rs +++ b/firewood/src/stream.rs @@ -62,6 +62,7 @@ enum NodeStreamState { } #[derive(Debug)] +/// A stream of nodes in order starting from a specific point in the trie. pub struct MerkleNodeStream<'a, T> { state: NodeStreamState, merkle: &'a T, @@ -284,6 +285,7 @@ impl<'a, T: TrieReader> MerkleKeyValueStreamState<'a, T> { } #[derive(Debug)] +/// A stream of key-value pairs in order starting from a specific point in the trie. pub struct MerkleKeyValueStream<'a, T> { state: MerkleKeyValueStreamState<'a, T>, merkle: &'a T, @@ -305,6 +307,8 @@ impl<'a, T: TrieReader> FusedStream for MerkleKeyValueStream<'a, T> { } impl<'a, T: TrieReader> MerkleKeyValueStream<'a, T> { + /// Construct a [MerkleKeyValueStream] that will iterate over all the key-value pairs in `merkle` + /// starting from a particular key pub fn from_key>(merkle: &'a T, key: K) -> Self { Self { state: MerkleKeyValueStreamState::from(key.as_ref()), diff --git a/firewood/src/v2/api.rs b/firewood/src/v2/api.rs index a1570adde..b441fc3e0 100644 --- a/firewood/src/v2/api.rs +++ b/firewood/src/v2/api.rs @@ -39,8 +39,19 @@ pub type HashKey = storage::TrieHash; /// supported #[derive(Debug)] pub enum BatchOp { - Put { key: K, value: V }, - Delete { key: K }, + /// Upsert a key/value pair + Put { + /// the key + key: K, + /// the value + value: V, + }, + + /// Delete a key + Delete { + /// The key + key: K, + }, } /// A list of operations to consist of a batch that @@ -63,49 +74,66 @@ pub fn vec_into_batch(value: Vec<(K, V)>) -> Batch {end_key:?}")] InvalidRange { + /// The provided starting key start_key: Box<[u8]>, + /// The provided ending key end_key: Box<[u8]>, }, #[error("IO error: {0}")] + /// An IO error occurred IO(#[from] std::io::Error), - #[error("Invalid proposal")] - InvalidProposal, - - // Cloned proposals are problematic because if they are committed, then you could - // create another proposal from this committed proposal, so we error at commit time - // if there are outstanding clones + /// Cannot commit a cloned proposal + /// + /// Cloned proposals are problematic because if they are committed, then you could + /// create another proposal from this committed proposal, so we error at commit time + /// if there are outstanding clones #[error("Cannot commit a cloned proposal")] CannotCommitClonedProposal, + /// Internal error #[error("Internal error")] InternalError(Box), + /// Range too small #[error("Range too small")] RangeTooSmall, + /// Request RangeProof for empty trie #[error("request RangeProof for empty trie")] RangeProofOnEmptyTrie, + /// Request RangeProof for empty range #[error("the latest revision is empty and has no root hash")] LatestIsEmpty, + /// This is not the latest proposal #[error("commit the parents of this proposal first")] NotLatest, + /// Sibling already committed #[error("sibling already committed")] SiblingCommitted, + /// Generic merkle error #[error("merkle error: {0}")] Merkle(#[from] MerkleError), } @@ -125,8 +153,10 @@ impl From for Error { /// is the api::DbView trait defined next. #[async_trait] pub trait Db { + /// The type of a historical revision type Historical: DbView; + /// The type of a proposal type Proposal<'p>: DbView + Proposal where Self: 'p; @@ -173,6 +203,7 @@ pub trait Db { /// A [Proposal] requires implementing DbView #[async_trait] pub trait DbView { + /// The type of a stream of key/value pairs type Stream<'a>: Stream, Vec), Error>> where Self: 'a; @@ -238,6 +269,7 @@ pub trait DbView { /// obtain proofs. #[async_trait] pub trait Proposal: DbView + Send + Sync { + /// The type of a proposal type Proposal: DbView + Proposal; /// Commit this revision diff --git a/firewood/src/v2/db.rs b/firewood/src/v2/db.rs index 6caca6509..c544faca1 100644 --- a/firewood/src/v2/db.rs +++ b/firewood/src/v2/db.rs @@ -1,7 +1,8 @@ // Copyright (C) 2023, Ava Labs, Inc. All rights reserved. // See the file LICENSE.md for licensing terms. -use crate::{db::DbError, v2::api}; +use crate::db::DbError; +use crate::v2::api; #[cfg_attr(doc, aquamarine::aquamarine)] /// ```mermaid @@ -17,11 +18,8 @@ use crate::{db::DbError, v2::api}; impl From for api::Error { fn from(value: DbError) -> Self { match value { - DbError::InvalidParams => api::Error::InternalError(Box::new(value)), DbError::Merkle(e) => api::Error::InternalError(Box::new(e)), - DbError::CreateError => api::Error::InternalError(Box::new(value)), DbError::IO(e) => api::Error::IO(e), - DbError::InvalidProposal => api::Error::InvalidProposal, } } } diff --git a/firewood/src/v2/emptydb.rs b/firewood/src/v2/emptydb.rs index cd171cb65..77cb9cf1a 100644 --- a/firewood/src/v2/emptydb.rs +++ b/firewood/src/v2/emptydb.rs @@ -88,6 +88,8 @@ impl DbView for HistoricalImpl { } } +#[derive(Debug)] +/// An empty streamer that doesn't stream any data pub struct EmptyStreamer; impl Stream for EmptyStreamer { diff --git a/firewood/src/v2/mod.rs b/firewood/src/v2/mod.rs index 454b1d3cf..b1a3a938b 100644 --- a/firewood/src/v2/mod.rs +++ b/firewood/src/v2/mod.rs @@ -1,9 +1,14 @@ // Copyright (C) 2023, Ava Labs, Inc. All rights reserved. // See the file LICENSE.md for licensing terms. +/// The public API pub mod api; + +/// The database pub mod db; + +/// The proposal pub mod propose; -// #[cfg(test)] +/// An empty database implementation for testing pub mod emptydb; diff --git a/fwdctl/src/graph.rs b/fwdctl/src/graph.rs index 833c1e140..e070169e4 100644 --- a/fwdctl/src/graph.rs +++ b/fwdctl/src/graph.rs @@ -22,6 +22,6 @@ pub(super) async fn run(opts: &Options) -> Result<(), api::Error> { let cfg = DbConfig::builder().truncate(false); let db = Db::new(opts.db.clone(), cfg.build()).await?; - db.dump(&mut stdout())?; + db.dump(&mut stdout()).await?; Ok(()) } diff --git a/grpc-testtool/src/service.rs b/grpc-testtool/src/service.rs index 163cb006c..9d1570544 100644 --- a/grpc-testtool/src/service.rs +++ b/grpc-testtool/src/service.rs @@ -32,9 +32,7 @@ impl IntoStatusResultExt for Result { Error::IncorrectRootHash { .. } | Error::HashNotFound { .. } | Error::RangeTooSmall => { Status::invalid_argument(err.to_string()) } - Error::IO { .. } | Error::InternalError { .. } | Error::InvalidProposal => { - Status::internal(err.to_string()) - } + Error::IO { .. } | Error::InternalError { .. } => Status::internal(err.to_string()), _ => Status::internal(err.to_string()), }) }