From 172893791e91814632c1c9c5559c5334b1ffc6b9 Mon Sep 17 00:00:00 2001 From: Evgeny Fomin Date: Wed, 1 Dec 2021 01:14:49 +0300 Subject: [PATCH 1/4] feat: abstract Merk storage this includes abstracting from rocksdb as well as making merk unaware of prefixes and other details --- grovedb/src/lib.rs | 2 +- merk/src/merk/chunks.rs | 1 + storage/Cargo.toml | 4 +--- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/grovedb/src/lib.rs b/grovedb/src/lib.rs index 5c7c769b3..d77139daa 100644 --- a/grovedb/src/lib.rs +++ b/grovedb/src/lib.rs @@ -10,11 +10,11 @@ use std::{ use merk::{self, Merk}; use rs_merkle::{algorithms::Sha256, MerkleTree}; -pub use subtree::Element; use storage::{ rocksdb_storage::{PrefixedRocksDbStorage, PrefixedRocksDbStorageError}, Storage, }; +pub use subtree::Element; /// Limit of possible indirections const MAX_REFERENCE_HOPS: usize = 10; diff --git a/merk/src/merk/chunks.rs b/merk/src/merk/chunks.rs index b2fa3c5d5..f185bbd7c 100644 --- a/merk/src/merk/chunks.rs +++ b/merk/src/merk/chunks.rs @@ -183,6 +183,7 @@ where #[cfg(test)] mod tests { use storage::rocksdb_storage::{default_rocksdb, PrefixedRocksDbStorage}; + use tempdir::TempDir; use super::*; diff --git a/storage/Cargo.toml b/storage/Cargo.toml index 73fddd70b..a289a4d27 100644 --- a/storage/Cargo.toml +++ b/storage/Cargo.toml @@ -8,7 +8,5 @@ edition = "2021" [dependencies] num_cpus = "1.13.0" rocksdb = "0.17.0" -thiserror = "1.0.30" - -[dev-dependencies] tempdir = "0.3.7" +thiserror = "1.0.30" From 3d8b2e217abc80b14612eb46b819bbc59703133b Mon Sep 17 00:00:00 2001 From: Evgeny Fomin Date: Tue, 7 Dec 2021 14:01:39 +0300 Subject: [PATCH 2/4] add rocksdb prefixed storage tests --- merk/src/merk/chunks.rs | 1 - storage/Cargo.toml | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/merk/src/merk/chunks.rs b/merk/src/merk/chunks.rs index f185bbd7c..b2fa3c5d5 100644 --- a/merk/src/merk/chunks.rs +++ b/merk/src/merk/chunks.rs @@ -183,7 +183,6 @@ where #[cfg(test)] mod tests { use storage::rocksdb_storage::{default_rocksdb, PrefixedRocksDbStorage}; - use tempdir::TempDir; use super::*; diff --git a/storage/Cargo.toml b/storage/Cargo.toml index a289a4d27..1eddc5da1 100644 --- a/storage/Cargo.toml +++ b/storage/Cargo.toml @@ -10,3 +10,6 @@ num_cpus = "1.13.0" rocksdb = "0.17.0" tempdir = "0.3.7" thiserror = "1.0.30" + +[dev-dependencies] +tempdir = "0.3.7" From e0f38da5e2592d7767cc746444ffb93c03d970e8 Mon Sep 17 00:00:00 2001 From: Evgeny Fomin Date: Wed, 8 Dec 2021 15:47:29 +0300 Subject: [PATCH 3/4] feat: add simple checkpointing, better error handling --- grovedb/src/lib.rs | 55 ++++++++++------ grovedb/src/subtree.rs | 17 +++-- grovedb/src/tests.rs | 59 +++++++++++++++++ merk/Cargo.toml | 3 +- merk/src/lib.rs | 3 - merk/src/merk/chunks.rs | 37 +++++------ merk/src/merk/mod.rs | 104 ++++-------------------------- merk/src/proofs/chunk.rs | 8 +-- merk/src/proofs/encoding.rs | 9 ++- merk/src/proofs/query/map.rs | 5 +- merk/src/proofs/query/mod.rs | 7 +- merk/src/proofs/tree.rs | 21 +++--- merk/src/test_utils/crash_merk.rs | 7 +- merk/src/test_utils/temp_merk.rs | 7 +- merk/src/tree/commit.rs | 3 +- merk/src/tree/encoding.rs | 4 +- merk/src/tree/link.rs | 44 ++++++------- merk/src/tree/mod.rs | 3 +- merk/src/tree/ops.rs | 2 +- merk/src/tree/walk/fetch.rs | 3 +- merk/src/tree/walk/mod.rs | 3 +- merk/src/tree/walk/ref_walker.rs | 3 +- node-grove/src/converter.rs | 50 +++++++------- node-grove/src/lib.rs | 43 ++++++------ storage/src/rocksdb_storage.rs | 2 +- 25 files changed, 251 insertions(+), 251 deletions(-) diff --git a/grovedb/src/lib.rs b/grovedb/src/lib.rs index d77139daa..41d980832 100644 --- a/grovedb/src/lib.rs +++ b/grovedb/src/lib.rs @@ -26,24 +26,18 @@ const ROOT_LEAFS_SERIALIZED_KEY: &[u8] = b"rootLeafsSerialized"; #[derive(Debug, thiserror::Error)] pub enum Error { - #[error("rocksdb error")] - RocksDBError(#[from] PrefixedRocksDbStorageError), - #[error("unable to open Merk db")] - MerkError(merk::Error), - #[error("invalid path: {0}")] - InvalidPath(&'static str), - #[error("unable to decode")] - BincodeError(#[from] bincode::Error), + // Input data errors #[error("cyclic reference path")] CyclicReference, #[error("reference hops limit exceeded")] ReferenceLimit, -} - -impl From for Error { - fn from(e: merk::Error) -> Self { - Error::MerkError(e) - } + #[error("invalid path: {0}")] + InvalidPath(&'static str), + // Irrecoverable errors + #[error("storage error: {0}")] + StorageError(#[from] PrefixedRocksDbStorageError), + #[error("data corruption error: {0}")] + CorruptedData(String), } pub struct GroveDb { @@ -69,10 +63,14 @@ impl GroveDb { let mut subtrees = HashMap::new(); // TODO: owned `get` is not required for deserialization if let Some(prefixes_serialized) = meta_storage.get_meta(SUBTRESS_SERIALIZED_KEY)? { - let subtrees_prefixes: Vec> = bincode::deserialize(&prefixes_serialized)?; + let subtrees_prefixes: Vec> = bincode::deserialize(&prefixes_serialized) + .map_err(|_| { + Error::CorruptedData(String::from("unable to deserialize prefixes")) + })?; for prefix in subtrees_prefixes { let subtree_merk = - Merk::open(PrefixedRocksDbStorage::new(db.clone(), prefix.to_vec())?)?; + Merk::open(PrefixedRocksDbStorage::new(db.clone(), prefix.to_vec())?) + .map_err(|e| Error::CorruptedData(e.to_string()))?; subtrees.insert(prefix.to_vec(), subtree_merk); } } @@ -81,7 +79,9 @@ impl GroveDb { let root_leaf_keys: HashMap, usize> = if let Some(root_leaf_keys_serialized) = meta_storage.get_meta(ROOT_LEAFS_SERIALIZED_KEY)? { - bincode::deserialize(&root_leaf_keys_serialized)? + bincode::deserialize(&root_leaf_keys_serialized).map_err(|_| { + Error::CorruptedData(String::from("unable to deserialize root leafs")) + })? } else { HashMap::new() }; @@ -95,13 +95,25 @@ impl GroveDb { }) } + pub fn checkpoint>(&self, path: P) -> Result { + storage::rocksdb_storage::Checkpoint::new(&self.db) + .and_then(|x| x.create_checkpoint(&path)) + .map_err(PrefixedRocksDbStorageError::RocksDbError)?; + GroveDb::open(path) + } + fn store_subtrees_keys_data(&self) -> Result<(), Error> { let prefixes: Vec> = self.subtrees.keys().map(|x| x.clone()).collect(); - self.meta_storage - .put_meta(SUBTRESS_SERIALIZED_KEY, &bincode::serialize(&prefixes)?)?; + self.meta_storage.put_meta( + SUBTRESS_SERIALIZED_KEY, + &bincode::serialize(&prefixes) + .map_err(|_| Error::CorruptedData(String::from("unable to serialize prefixes")))?, + )?; self.meta_storage.put_meta( ROOT_LEAFS_SERIALIZED_KEY, - &bincode::serialize(&self.root_leaf_keys)?, + &bincode::serialize(&self.root_leaf_keys).map_err(|_| { + Error::CorruptedData(String::from("unable to serialize root leafs")) + })?, )?; Ok(()) } @@ -140,7 +152,8 @@ impl GroveDb { Merk::open(PrefixedRocksDbStorage::new( self.db.clone(), compressed_path_subtree, - )?)?, + )?) + .map_err(|e| Error::CorruptedData(e.to_string()))?, )) }; if path.is_empty() { diff --git a/grovedb/src/subtree.rs b/grovedb/src/subtree.rs index f70fc6aef..5e479be32 100644 --- a/grovedb/src/subtree.rs +++ b/grovedb/src/subtree.rs @@ -30,10 +30,12 @@ impl Element { /// Merk should be loaded by this moment pub fn get(merk: &Merk, key: &[u8]) -> Result { let element = bincode::deserialize( - merk.get(&key)? + merk.get(&key) + .map_err(|e| Error::CorruptedData(e.to_string()))? .ok_or(Error::InvalidPath("key not found in Merk"))? .as_slice(), - )?; + ) + .map_err(|_| Error::CorruptedData(String::from("unable to deserialize element")))?; Ok(element) } @@ -44,8 +46,15 @@ impl Element { merk: &mut Merk, key: Vec, ) -> Result<(), Error> { - let batch = [(key, Op::Put(bincode::serialize(self)?))]; - merk.apply(&batch, &[]).map_err(|e| e.into()) + let batch = + [( + key, + Op::Put(bincode::serialize(self).map_err(|_| { + Error::CorruptedData(String::from("unable to serialize element")) + })?), + )]; + merk.apply(&batch, &[]) + .map_err(|e| Error::CorruptedData(e.to_string())) } } diff --git a/grovedb/src/tests.rs b/grovedb/src/tests.rs index df8e6e29b..0b0f18247 100644 --- a/grovedb/src/tests.rs +++ b/grovedb/src/tests.rs @@ -239,3 +239,62 @@ fn test_root_tree_leafs_are_noted() { assert_eq!(db.root_leaf_keys, hm); assert_eq!(db.root_tree.leaves_len(), 2); } + +#[test] +fn test_checkpoint() { + let mut db = make_grovedb(); + let element1 = Element::Item(b"ayy".to_vec()); + + db.insert(&[], b"key1".to_vec(), Element::empty_tree()) + .expect("cannot insert a subtree 1 into GroveDB"); + db.insert(&[b"key1"], b"key2".to_vec(), Element::empty_tree()) + .expect("cannot insert a subtree 2 into GroveDB"); + db.insert(&[b"key1", b"key2"], b"key3".to_vec(), element1.clone()) + .expect("cannot insert an item into GroveDB"); + + assert_eq!( + db.get(&[b"key1", b"key2"], b"key3") + .expect("cannot get from grovedb"), + element1 + ); + + let checkpoint_tempdir = TempDir::new("checkpoint").expect("cannot open tempdir"); + let mut checkpoint = db + .checkpoint(checkpoint_tempdir.path().join("checkpoint")) + .expect("cannot create a checkpoint"); + + assert_eq!( + db.get(&[b"key1", b"key2"], b"key3") + .expect("cannot get from grovedb"), + element1 + ); + assert_eq!( + checkpoint + .get(&[b"key1", b"key2"], b"key3") + .expect("cannot get from checkpoint"), + element1 + ); + + let element2 = Element::Item(b"ayy2".to_vec()); + let element3 = Element::Item(b"ayy3".to_vec()); + + checkpoint + .insert(&[b"key1"], b"key4".to_vec(), element2.clone()) + .expect("cannot insert into checkpoint"); + + db.insert(&[b"key1"], b"key4".to_vec(), element3.clone()) + .expect("cannot insert into GroveDB"); + + assert_eq!( + checkpoint + .get(&[b"key1"], b"key4") + .expect("cannot get from checkpoint"), + element2, + ); + + assert_eq!( + db.get(&[b"key1"], b"key4") + .expect("cannot get from GroveDB"), + element3 + ); +} diff --git a/merk/Cargo.toml b/merk/Cargo.toml index ab4d7f762..493fad326 100644 --- a/merk/Cargo.toml +++ b/merk/Cargo.toml @@ -10,8 +10,9 @@ license = "MIT" tempdir = "0.3.7" storage = { path = "../storage" } thiserror = "1.0.30" -failure = "0.1.8" rocksdb = "0.17.0" +anyhow = "1.0.51" +failure = "0.1.8" [dependencies.time] version = "0.1.42" diff --git a/merk/src/lib.rs b/merk/src/lib.rs index 8c3c1cbea..0da006f5b 100644 --- a/merk/src/lib.rs +++ b/merk/src/lib.rs @@ -1,7 +1,5 @@ #![feature(map_first_last)] -/// Error and Result types. -mod error; /// The top-level store API. #[cfg(feature = "full")] mod merk; @@ -18,7 +16,6 @@ pub mod test_utils; /// The core tree data structure. pub mod tree; -pub use error::{Error, Result}; #[allow(deprecated)] pub use proofs::query::verify_query; pub use proofs::query::{execute_proof, verify}; diff --git a/merk/src/merk/chunks.rs b/merk/src/merk/chunks.rs index b2fa3c5d5..55a9aaa21 100644 --- a/merk/src/merk/chunks.rs +++ b/merk/src/merk/chunks.rs @@ -1,17 +1,13 @@ //! Provides `ChunkProducer`, which creates chunk proofs for full replication of //! a Merk. +use std::error::Error; -use std::marker::PhantomData; - +use anyhow::{anyhow, bail, Result}; use ed::Encode; -use failure::bail; use storage::{RawIterator, Storage}; use super::Merk; -use crate::{ - proofs::{chunk::get_next_chunk, Node, Op}, - Result, -}; +use crate::proofs::{chunk::get_next_chunk, Node, Op}; /// A `ChunkProducer` allows the creation of chunk proofs, used for trustlessly /// replicating entire Merk trees. Chunks can be generated on the fly in a @@ -19,8 +15,7 @@ use crate::{ pub struct ChunkProducer<'a, S: Storage + 'a> where S: Storage, - // crate::error::Error: From, - ::Error: std::error::Error + Sync + Send + 'static, + ::Error: Error + Sync + Send + 'static, { trunk: Vec, chunk_boundaries: Vec>, @@ -31,8 +26,7 @@ where impl<'a, S> ChunkProducer<'a, S> where S: Storage, - // crate::error::Error: From, - ::Error: std::error::Error + Sync + Send + 'static, + ::Error: Error + Sync + Send + 'static, { /// Creates a new `ChunkProducer` for the given `Merk` instance. In the /// constructor, the first chunk (the "trunk") will be created. @@ -106,7 +100,10 @@ where bail!("Attempted to fetch chunk on empty tree"); } self.index += 1; - return self.trunk.encode(); + return self + .trunk + .encode() + .map_err(|e| anyhow!("cannot get next chunk: {}", e)); } if self.index >= self.len() { @@ -119,15 +116,16 @@ where self.index += 1; let chunk = get_next_chunk(&mut self.raw_iter, end_key_slice)?; - chunk.encode() + chunk + .encode() + .map_err(|e| anyhow!("cannot get next chunk: {}", e)) } } impl<'a, S> IntoIterator for ChunkProducer<'a, S> where S: Storage, - // crate::error::Error: From, - ::Error: std::error::Error + Sync + Send + 'static, + ::Error: Error + Sync + Send + 'static, { type IntoIter = ChunkIter<'a, S>; type Item = as Iterator>::Item; @@ -143,14 +141,12 @@ where pub struct ChunkIter<'a, S>(ChunkProducer<'a, S>) where S: Storage, - // crate::error::Error: From, - ::Error: std::error::Error + Sync + Send + 'static; + ::Error: Error + Sync + Send + 'static; impl<'a, S> Iterator for ChunkIter<'a, S> where S: Storage, - // crate::error::Error: From, - ::Error: std::error::Error + Sync + Send + 'static, + ::Error: Error + Sync + Send + 'static, { type Item = Result>; @@ -170,8 +166,7 @@ where impl Merk where S: Storage, - // crate::error::Error: From, - ::Error: std::error::Error + Sync + Send + 'static, + ::Error: Error + Sync + Send + 'static, { /// Creates a `ChunkProducer` which can return chunk proofs for replicating /// the entire Merk tree. diff --git a/merk/src/merk/mod.rs b/merk/src/merk/mod.rs index beddb79d1..e77c422ee 100644 --- a/merk/src/merk/mod.rs +++ b/merk/src/merk/mod.rs @@ -1,14 +1,12 @@ pub mod chunks; // TODO // pub mod restore; - use std::{cell::Cell, cmp::Ordering, collections::LinkedList}; -use failure::format_err; +use anyhow::{anyhow, bail, Result}; use storage::{self, Batch, Storage, Store}; use crate::{ - error::Result, proofs::{encode_into, query::QueryItem, Query}, tree::{Commit, Fetch, Hash, Link, MerkBatch, Op, RefWalker, Tree, Walker, NULL_HASH}, }; @@ -19,7 +17,7 @@ const ROOT_KEY_KEY: &[u8] = b"root"; pub struct Merk where S: Storage, - crate::error::Error: From, + // crate::error::Error: From, { pub(crate) tree: Cell>, pub(crate) storage: S, @@ -29,7 +27,7 @@ pub type UseTreeMutResult = Result, Option>)>>; impl Merk where - crate::error::Error: From<::Error>, + // crate::error::Error: From<::Error>, ::Error: std::error::Error, { pub fn open(storage: S) -> Result> { @@ -130,8 +128,8 @@ where for (key, _) in batch.iter() { if let Some(prev_key) = maybe_prev_key { match prev_key.cmp(key) { - Ordering::Greater => return Err(format_err!("Keys in batch must be sorted")), - Ordering::Equal => return Err(format_err!("Keys in batch must be unique")), + Ordering::Greater => bail!("Keys in batch must be sorted"), + Ordering::Equal => bail!("Keys in batch must be unique"), _ => (), } } @@ -212,7 +210,7 @@ where let query_vec: Vec = query.into_iter().map(Into::into).collect(); self.use_tree_mut(|maybe_tree| { - let tree = maybe_tree.ok_or(format_err!("Cannot create proof for empty tree"))?; + let tree = maybe_tree.ok_or(anyhow!("Cannot create proof for empty tree"))?; let mut ref_walker = RefWalker::new(tree, self.source()); let (proof, _) = ref_walker.create_proof(query_vec.as_slice())?; @@ -279,15 +277,10 @@ where res } - pub fn raw_iter<'a>(&'a self) -> S::RawIterator<'a> { + pub fn raw_iter(&self) -> S::RawIterator<'_> { self.storage.raw_iter() } - // // pub fn checkpoint>(&self, path: P, prefix: &[u8]) -> - // // Result { Checkpoint::new(&self.db)?.create_checkpoint(&path)?; - // // Merk::open(path, prefix) - // // } - fn source(&self) -> MerkSource { MerkSource { storage: &self.storage, @@ -308,9 +301,9 @@ where res } - pub(crate) fn set_root_key(&mut self, key: &[u8]) -> Result<()> { - Ok(self.storage.put_root(ROOT_KEY_KEY, key)?) - } + // pub(crate) fn set_root_key(&mut self, key: &[u8]) -> Result<()> { + // Ok(self.storage.put_root(ROOT_KEY_KEY, key)?) + // } pub(crate) fn load_root(&mut self) -> Result<()> { if let Some(tree_root_key) = self.storage.get_root(ROOT_KEY_KEY)? { @@ -337,11 +330,12 @@ impl<'a, S: Storage> Clone for MerkSource<'a, S> { impl<'a, S: Storage> Fetch for MerkSource<'a, S> where - crate::error::Error: From<::Error>, + // crate::error::Error: From<::Error>, ::Error: std::error::Error, { fn fetch(&self, link: &Link) -> Result { - Ok(Tree::get(&self.storage, link.key())?.ok_or(format_err!("Key not found"))?) + Tree::get(&self.storage, link.key())?.ok_or(anyhow!("Key not found")) } } @@ -572,76 +566,4 @@ mod test { assert_eq!(reopen_nodes, original_nodes); } - - // #[test] - // fn checkpoint() { - // let mut merk = TempMerk::new(); - - // merk.apply(&[(vec![1], Op::Put(vec![0]))], &[]) - // .expect("apply failed"); - - // let mut checkpoint = - // merk.inner.checkpoint(merk.path.path().join("checkpoint")).unwrap(); - - // assert_eq!(merk.get(&[1]).unwrap(), Some(vec![0])); - // assert_eq!(checkpoint.get(&[1]).unwrap(), Some(vec![0])); - - // merk.apply( - // &[(vec![1], Op::Put(vec![1])), (vec![2], Op::Put(vec![0]))], - // &[], - // ) - // .expect("apply failed"); - - // assert_eq!(merk.get(&[1]).unwrap(), Some(vec![1])); - // assert_eq!(merk.get(&[2]).unwrap(), Some(vec![0])); - // assert_eq!(checkpoint.get(&[1]).unwrap(), Some(vec![0])); - // assert_eq!(checkpoint.get(&[2]).unwrap(), None); - - // checkpoint - // .apply(&[(vec![2], Op::Put(vec![123]))], &[]) - // .expect("apply failed"); - - // assert_eq!(merk.get(&[1]).unwrap(), Some(vec![1])); - // assert_eq!(merk.get(&[2]).unwrap(), Some(vec![0])); - // assert_eq!(checkpoint.get(&[1]).unwrap(), Some(vec![0])); - // assert_eq!(checkpoint.get(&[2]).unwrap(), Some(vec![123])); - - // checkpoint.destroy().unwrap(); - - // assert_eq!(merk.get(&[1]).unwrap(), Some(vec![1])); - // assert_eq!(merk.get(&[2]).unwrap(), Some(vec![0])); - // } - - // #[test] - // fn checkpoint_iterator() { - // let path = thread::current().name().unwrap().to_owned(); - // let mut merk = TempMerk::open(&path).expect("failed to open merk"); - - // merk.apply(&make_batch_seq(1..100), &[]) - // .expect("apply failed"); - - // let path: std::path::PathBuf = (path + ".checkpoint").into(); - // if path.exists() { - // std::fs::remove_dir_all(&path).unwrap(); - // } - // let checkpoint = merk.checkpoint(&path).unwrap(); - - // let mut merk_iter = merk.raw_iter(); - // let mut checkpoint_iter = checkpoint.raw_iter(); - - // loop { - // assert_eq!(merk_iter.valid(), checkpoint_iter.valid()); - // if !merk_iter.valid() { - // break; - // } - - // assert_eq!(merk_iter.key(), checkpoint_iter.key()); - // assert_eq!(merk_iter.value(), checkpoint_iter.value()); - - // merk_iter.next(); - // checkpoint_iter.next(); - // } - - // std::fs::remove_dir_all(&path).unwrap(); - // } } diff --git a/merk/src/proofs/chunk.rs b/merk/src/proofs/chunk.rs index 35d8e937d..f0495878c 100644 --- a/merk/src/proofs/chunk.rs +++ b/merk/src/proofs/chunk.rs @@ -1,18 +1,14 @@ +use anyhow::{bail, Result}; use storage::RawIterator; #[cfg(feature = "full")] use { super::tree::{execute, Tree as ProofTree}, crate::tree::Hash, crate::tree::Tree, - failure::bail, - rocksdb::DBRawIterator, }; use super::{Node, Op}; -use crate::{ - error::Result, - tree::{Fetch, RefWalker}, -}; +use crate::tree::{Fetch, RefWalker}; /// The minimum number of layers the trunk will be guaranteed to have before /// splitting into multiple chunks. If the tree's height is less than double diff --git a/merk/src/proofs/encoding.rs b/merk/src/proofs/encoding.rs index e7ef87599..442edb7df 100644 --- a/merk/src/proofs/encoding.rs +++ b/merk/src/proofs/encoding.rs @@ -1,10 +1,10 @@ use std::io::{Read, Write}; +use anyhow::{anyhow, Result}; use ed::{Decode, Encode, Terminated}; -use failure::bail; use super::{Node, Op}; -use crate::{error::Result, tree::HASH_LENGTH}; +use crate::tree::HASH_LENGTH; impl Encode for Op { fn encode_into(&self, dest: &mut W) -> ed::Result<()> { @@ -71,7 +71,8 @@ impl Decode for Op { } 0x10 => Op::Parent, 0x11 => Op::Child, - _ => bail!("Proof has unexpected value"), + // TODO: get rid of `failure` with improvements to ed API (or removing dependency on ed) + _ => failure::bail!("Proof has unexpected value"), }) } } @@ -81,6 +82,7 @@ impl Terminated for Op {} impl Op { fn encode_into(&self, dest: &mut W) -> Result<()> { Encode::encode_into(self, dest) + .map_err(|e| anyhow!("failed to encode an proofs::Op structure ({})", e)) } fn encoding_length(&self) -> usize { @@ -89,6 +91,7 @@ impl Op { pub fn decode(bytes: &[u8]) -> Result { Decode::decode(bytes) + .map_err(|e| anyhow!("failed to decode an proofs::Op structure ({})", e)) } } diff --git a/merk/src/proofs/query/map.rs b/merk/src/proofs/query/map.rs index cac838cb8..9bfebcabb 100644 --- a/merk/src/proofs/query/map.rs +++ b/merk/src/proofs/query/map.rs @@ -3,10 +3,9 @@ use std::{ ops::{Bound, RangeBounds}, }; -use failure::{bail, ensure, format_err}; +use anyhow::{anyhow, bail, ensure, Result}; use super::super::Node; -use crate::Result; /// `MapBuilder` allows a consumer to construct a `Map` by inserting the nodes /// contained in a proof, in key-order. @@ -200,7 +199,7 @@ impl<'a> Iterator for Range<'a> { // if nodes weren't contiguous, we cannot verify that we have all values // in the desired range if !skip_exclusion_check && !contiguous { - return Some(Err(format_err!("Proof is missing data for query"))); + return Some(Err(anyhow!("Proof is missing data for query"))); } // passed checks, return entry diff --git a/merk/src/proofs/query/mod.rs b/merk/src/proofs/query/mod.rs index 191e85161..07dcdfa31 100644 --- a/merk/src/proofs/query/mod.rs +++ b/merk/src/proofs/query/mod.rs @@ -6,16 +6,13 @@ use std::{ ops::{Range, RangeInclusive}, }; -use failure::bail; +use anyhow::{bail, Result}; pub use map::*; #[cfg(feature = "full")] use {super::Op, std::collections::LinkedList}; use super::{tree::execute, Decoder, Node}; -use crate::{ - error::Result, - tree::{Fetch, Hash, Link, RefWalker}, -}; +use crate::tree::{Fetch, Hash, Link, RefWalker}; /// `Query` represents one or more keys or ranges of keys, which can be used to /// resolve a proof which will include all of the requested values. diff --git a/merk/src/proofs/tree.rs b/merk/src/proofs/tree.rs index 24da1e13f..898079d49 100644 --- a/merk/src/proofs/tree.rs +++ b/merk/src/proofs/tree.rs @@ -1,10 +1,7 @@ -use failure::bail; +use anyhow::{bail, Result}; use super::{Node, Op}; -use crate::{ - error::Result, - tree::{kv_hash, node_hash, Hash, NULL_HASH}, -}; +use crate::tree::{kv_hash, node_hash, Hash, NULL_HASH}; /// Contains a tree's child node and its hash. The hash can always be assumed to /// be up-to-date. @@ -143,13 +140,13 @@ impl Tree { Node::Hash(self.hash()).into() } - #[cfg(feature = "full")] - pub(crate) fn key(&self) -> &[u8] { - match self.node { - Node::KV(ref key, _) => key, - _ => panic!("Expected node to be type KV"), - } - } + // #[cfg(feature = "full")] + // pub(crate) fn key(&self) -> &[u8] { + // match self.node { + // Node::KV(ref key, _) => key, + // _ => panic!("Expected node to be type KV"), + // } + // } } /// `LayerIter` iterates over the nodes in a `Tree` at a given depth. Nodes are diff --git a/merk/src/test_utils/crash_merk.rs b/merk/src/test_utils/crash_merk.rs index 6d39bfca6..df007831b 100644 --- a/merk/src/test_utils/crash_merk.rs +++ b/merk/src/test_utils/crash_merk.rs @@ -3,10 +3,11 @@ use std::{ rc::Rc, }; +use anyhow::Result; use storage::rocksdb_storage::{default_rocksdb, PrefixedRocksDbStorage}; use tempdir::TempDir; -use crate::{Merk, Result}; +use crate::Merk; /// Wraps a Merk instance and drops it without flushing once it goes out of /// scope. @@ -31,7 +32,9 @@ impl CrashMerk { } pub fn crash(&mut self) { - self.path.take().map(|x| drop(x)); + if let Some(a) = self.path.take() { + drop(a) + } } } diff --git a/merk/src/test_utils/temp_merk.rs b/merk/src/test_utils/temp_merk.rs index 147ca0cc2..fc49a27e5 100644 --- a/merk/src/test_utils/temp_merk.rs +++ b/merk/src/test_utils/temp_merk.rs @@ -1,6 +1,5 @@ use std::{ ops::{Deref, DerefMut}, - path::Path, rc::Rc, }; @@ -31,6 +30,12 @@ impl TempMerk { } } +impl Default for TempMerk { + fn default() -> Self { + Self::new() + } +} + impl Deref for TempMerk { type Target = Merk; diff --git a/merk/src/tree/commit.rs b/merk/src/tree/commit.rs index f366556db..8e62560b4 100644 --- a/merk/src/tree/commit.rs +++ b/merk/src/tree/commit.rs @@ -1,5 +1,6 @@ +use anyhow::Result; + use super::Tree; -use crate::error::Result; /// To be used when committing a tree (writing it to a store after applying the /// changes). diff --git a/merk/src/tree/encoding.rs b/merk/src/tree/encoding.rs index 095a3d370..161af2039 100644 --- a/merk/src/tree/encoding.rs +++ b/merk/src/tree/encoding.rs @@ -1,8 +1,8 @@ +use anyhow::{anyhow, Error}; use ed::{Decode, Encode}; use storage::{Storage, Store}; use super::Tree; -use crate::error::Error; impl Store for Tree { type Error = Error; @@ -12,7 +12,7 @@ impl Store for Tree { } fn decode(bytes: &[u8]) -> Result { - Decode::decode(bytes) + Decode::decode(bytes).map_err(|e| anyhow!("failed to decode a Tree structure ({})", e)) } fn get(storage: S, key: &[u8]) -> Result, Self::Error> diff --git a/merk/src/tree/link.rs b/merk/src/tree/link.rs index eb8b4f22c..d02820ea8 100644 --- a/merk/src/tree/link.rs +++ b/merk/src/tree/link.rs @@ -175,28 +175,28 @@ impl Link { } } - #[inline] - #[cfg(feature = "full")] - pub(crate) fn child_heights_mut(&mut self) -> &mut (u8, u8) { - match self { - Link::Reference { - ref mut child_heights, - .. - } => child_heights, - Link::Modified { - ref mut child_heights, - .. - } => child_heights, - Link::Uncommitted { - ref mut child_heights, - .. - } => child_heights, - Link::Loaded { - ref mut child_heights, - .. - } => child_heights, - } - } + // #[inline] + // #[cfg(feature = "full")] + // pub(crate) fn child_heights_mut(&mut self) -> &mut (u8, u8) { + // match self { + // Link::Reference { + // ref mut child_heights, + // .. + // } => child_heights, + // Link::Modified { + // ref mut child_heights, + // .. + // } => child_heights, + // Link::Uncommitted { + // ref mut child_heights, + // .. + // } => child_heights, + // Link::Loaded { + // ref mut child_heights, + // .. + // } => child_heights, + // } + // } } impl Encode for Link { diff --git a/merk/src/tree/mod.rs b/merk/src/tree/mod.rs index a09c0af54..a6dbad230 100644 --- a/merk/src/tree/mod.rs +++ b/merk/src/tree/mod.rs @@ -12,6 +12,7 @@ mod walk; use std::cmp::max; +use anyhow::Result; pub use commit::{Commit, NoopCommit}; use ed::{Decode, Encode}; pub use hash::{kv_hash, node_hash, Hash, HASH_LENGTH, NULL_HASH}; @@ -20,8 +21,6 @@ pub use link::Link; pub use ops::{BatchEntry, MerkBatch, Op, PanicSource}; pub use walk::{Fetch, RefWalker, Walker}; -use super::error::Result; - // TODO: remove need for `TreeInner`, and just use `Box` receiver for // relevant methods diff --git a/merk/src/tree/ops.rs b/merk/src/tree/ops.rs index 57f71efcd..7df0301ae 100644 --- a/merk/src/tree/ops.rs +++ b/merk/src/tree/ops.rs @@ -1,9 +1,9 @@ use std::{collections::LinkedList, fmt}; +use anyhow::Result; use Op::*; use super::{Fetch, Link, Tree, Walker}; -use crate::error::Result; /// An operation to be applied to a key in the store. pub enum Op { diff --git a/merk/src/tree/walk/fetch.rs b/merk/src/tree/walk/fetch.rs index 6b4c576ed..505b9a90d 100644 --- a/merk/src/tree/walk/fetch.rs +++ b/merk/src/tree/walk/fetch.rs @@ -1,5 +1,6 @@ +use anyhow::Result; + use super::super::{Link, Tree}; -use crate::error::Result; /// A source of data to be used by the tree when encountering a pruned node. /// This typcially means fetching the tree node from a backing store by its key, diff --git a/merk/src/tree/walk/mod.rs b/merk/src/tree/walk/mod.rs index 22ed18602..00c504674 100644 --- a/merk/src/tree/walk/mod.rs +++ b/merk/src/tree/walk/mod.rs @@ -1,11 +1,12 @@ mod fetch; mod ref_walker; +use anyhow::Result; pub use fetch::Fetch; pub use ref_walker::RefWalker; use super::{Link, Tree}; -use crate::{error::Result, owner::Owner}; +use crate::owner::Owner; /// Allows traversal of a `Tree`, fetching from the given source when traversing /// to a pruned node, detaching children as they are traversed. diff --git a/merk/src/tree/walk/ref_walker.rs b/merk/src/tree/walk/ref_walker.rs index a937e4bfa..f7e1e8d9d 100644 --- a/merk/src/tree/walk/ref_walker.rs +++ b/merk/src/tree/walk/ref_walker.rs @@ -1,8 +1,9 @@ +use anyhow::Result; + use super::{ super::{Link, Tree}, Fetch, }; -use crate::error::Result; /// Allows read-only traversal of a `Tree`, fetching from the given source when /// traversing to a pruned node. The fetched nodes are then retained in memory diff --git a/node-grove/src/converter.rs b/node-grove/src/converter.rs index 4eccd54b1..8c6f61077 100644 --- a/node-grove/src/converter.rs +++ b/node-grove/src/converter.rs @@ -1,15 +1,18 @@ -use grovedb::{Element}; -use neon::{prelude::*, borrow::Borrow}; +use grovedb::Element; +use neon::{borrow::Borrow, prelude::*}; fn element_to_string(element: Element) -> String { match element { - Element::Item(_) => { "item".to_string() } - Element::Reference(_) => { "reference".to_string() } - Element::Tree(_) => { "tree".to_string() } + Element::Item(_) => "item".to_string(), + Element::Reference(_) => "reference".to_string(), + Element::Tree(_) => "tree".to_string(), } } -pub fn js_object_to_element<'a, C: Context<'a>>(js_object: Handle, cx: &mut C) -> NeonResult { +pub fn js_object_to_element<'a, C: Context<'a>>( + js_object: Handle, + cx: &mut C, +) -> NeonResult { let js_element_string = js_object.get(cx, "type")?.to_string(cx)?; let value = js_object.get(cx, "value")?; @@ -20,32 +23,32 @@ pub fn js_object_to_element<'a, C: Context<'a>>(js_object: Handle, cx: let js_buffer = value.downcast_or_throw::(cx)?; let item = js_buffer_to_vec_u8(js_buffer, cx); Ok(Element::Item(item)) - }, + } "reference" => { let js_array = value.downcast_or_throw::(cx)?; let reference = js_array_of_buffers_to_vec(js_array, cx)?; Ok(Element::Reference(reference)) - }, + } "tree" => { let js_buffer = value.downcast_or_throw::(cx)?; let tree_vec = js_buffer_to_vec_u8(js_buffer, cx); - Ok(Element::Tree( - tree_vec - .try_into() - .or_else(|v: Vec| { - cx.throw_error( - format!("Tree buffer is expected to be 32 bytes long, but got {}", v.len()) - ) - })? - )) - } - _ => { - cx.throw_error(format!("Unexpected element type {}", element_string)) + Ok(Element::Tree(tree_vec.try_into().or_else( + |v: Vec| { + cx.throw_error(format!( + "Tree buffer is expected to be 32 bytes long, but got {}", + v.len() + )) + }, + )?)) } + _ => cx.throw_error(format!("Unexpected element type {}", element_string)), } } -pub fn element_to_js_object<'a, C: Context<'a>>(element: Element, cx: &mut C) -> NeonResult> { +pub fn element_to_js_object<'a, C: Context<'a>>( + element: Element, + cx: &mut C, +) -> NeonResult> { let js_object = cx.empty_object(); let js_type_string = cx.string(element_to_string(element.clone())); js_object.set(cx, "type", js_type_string)?; @@ -84,7 +87,10 @@ pub fn js_buffer_to_vec_u8<'a, C: Context<'a>>(js_buffer: Handle, cx: key_slice.to_vec() } -pub fn js_array_of_buffers_to_vec<'a, C: Context<'a>>(js_array: Handle, cx: &mut C) -> NeonResult>> { +pub fn js_array_of_buffers_to_vec<'a, C: Context<'a>>( + js_array: Handle, + cx: &mut C, +) -> NeonResult>> { let buf_vec = js_array.to_vec(cx)?; let mut vec: Vec> = Vec::new(); diff --git a/node-grove/src/lib.rs b/node-grove/src/lib.rs index f1130295a..48bf89285 100644 --- a/node-grove/src/lib.rs +++ b/node-grove/src/lib.rs @@ -1,10 +1,8 @@ mod converter; -use std::path::Path; -use std::sync::mpsc; -use std::thread; +use std::{path::Path, sync::mpsc, thread}; -use grovedb::{GroveDb}; +use grovedb::GroveDb; use neon::prelude::*; type DbCallback = Box; @@ -22,18 +20,17 @@ struct GroveDbWrapper { tx: mpsc::Sender, } -// Internal wrapper logic. Needed to avoid issues with passing threads to node.js. -// Avoiding thread conflicts by having a dedicated thread for the groveDB instance -// and uses events to communicate with it +// Internal wrapper logic. Needed to avoid issues with passing threads to +// node.js. Avoiding thread conflicts by having a dedicated thread for the +// groveDB instance and uses events to communicate with it impl GroveDbWrapper { // Creates a new instance of `GroveDbWrapper` // // 1. Creates a connection and a channel // 2. Spawns a thread and moves the channel receiver and connection to it - // 3. On a separate thread, read closures off the channel and execute with access - // to the connection. - fn new(cx: &mut FunctionContext) -> NeonResult - { + // 3. On a separate thread, read closures off the channel and execute with + // access to the connection. + fn new(cx: &mut FunctionContext) -> NeonResult { let path_string = cx.argument::(0)?.value(cx); // Channel for sending callbacks to execute on the GroveDb connection thread @@ -79,7 +76,8 @@ impl GroveDbWrapper { } // Idiomatic rust would take an owned `self` to prevent use after close - // However, it's not possible to prevent JavaScript from continuing to hold a closed database + // However, it's not possible to prevent JavaScript from continuing to hold a + // closed database fn close( &self, callback: impl FnOnce(&Channel) + Send + 'static, @@ -95,8 +93,8 @@ impl GroveDbWrapper { } } -// Ensures that GroveDbWrapper is properly disposed when the corresponding JS object -// gets garbage collected +// Ensures that GroveDbWrapper is properly disposed when the corresponding JS +// object gets garbage collected impl Finalize for GroveDbWrapper {} // External wrapper logic @@ -135,14 +133,12 @@ impl GroveDbWrapper { // First parameter of JS callbacks is error, which is null in this case vec![ task_context.null().upcast(), - converter::element_to_js_object(element, &mut task_context)? + converter::element_to_js_object(element, &mut task_context)?, ] - }, + } // Convert the error to a JavaScript exception on failure - Err(err) => vec![ - task_context.error(err.to_string())?.upcast() - ], + Err(err) => vec![task_context.error(err.to_string())?.upcast()], }; callback.call(&mut task_context, this, callback_arguments)?; @@ -187,7 +183,7 @@ impl GroveDbWrapper { Ok(()) }); }) - .or_else(|err| cx.throw_error(err.to_string()))?; + .or_else(|err| cx.throw_error(err.to_string()))?; Ok(cx.undefined()) } @@ -211,15 +207,14 @@ impl GroveDbWrapper { channel.send(move |mut task_context| { let callback = js_callback.into_inner(&mut task_context); let this = task_context.undefined(); - let callback_arguments: Vec> = vec![ - task_context.null().upcast(), - ]; + let callback_arguments: Vec> = vec![task_context.null().upcast()]; callback.call(&mut task_context, this, callback_arguments)?; Ok(()) }); - }).or_else(|err| cx.throw_error(err.to_string()))?; + }) + .or_else(|err| cx.throw_error(err.to_string()))?; Ok(cx.undefined()) } diff --git a/storage/src/rocksdb_storage.rs b/storage/src/rocksdb_storage.rs index 818c94e34..541e4cb8a 100644 --- a/storage/src/rocksdb_storage.rs +++ b/storage/src/rocksdb_storage.rs @@ -1,8 +1,8 @@ //! Storage implementation using RocksDB use std::{path::Path, rc::Rc}; +pub use rocksdb::{checkpoint::Checkpoint, Error, DB}; use rocksdb::{ColumnFamily, ColumnFamilyDescriptor, DBRawIterator, WriteBatch}; -pub use rocksdb::{Error, DB}; use crate::{Batch, RawIterator, Storage}; From 4e5906f4fc0f9b3d29854e28a54551f281cc45b2 Mon Sep 17 00:00:00 2001 From: Evgeny Fomin Date: Mon, 20 Dec 2021 15:05:46 +0300 Subject: [PATCH 4/4] add checkpoint key non presence test --- grovedb/src/tests.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/grovedb/src/tests.rs b/grovedb/src/tests.rs index 0b0f18247..4b92ce4d8 100644 --- a/grovedb/src/tests.rs +++ b/grovedb/src/tests.rs @@ -297,4 +297,21 @@ fn test_checkpoint() { .expect("cannot get from GroveDB"), element3 ); + + checkpoint + .insert(&[b"key1"], b"key5".to_vec(), element3.clone()) + .expect("cannot insert into checkpoint"); + + db.insert(&[b"key1"], b"key6".to_vec(), element3.clone()) + .expect("cannot insert into GroveDB"); + + assert!(matches!( + checkpoint.get(&[b"key1"], b"key6"), + Err(Error::InvalidPath(_)) + )); + + assert!(matches!( + db.get(&[b"key1"], b"key5"), + Err(Error::InvalidPath(_)) + )); }