From 7481e403537bb3c51c040fa6b8217fdf4b1ea811 Mon Sep 17 00:00:00 2001 From: Evgeny Fomin Date: Wed, 14 Aug 2024 10:47:49 +0200 Subject: [PATCH 1/4] add tests --- grovedb/src/tests/mod.rs | 128 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) diff --git a/grovedb/src/tests/mod.rs b/grovedb/src/tests/mod.rs index f98fcb64..fa48a900 100644 --- a/grovedb/src/tests/mod.rs +++ b/grovedb/src/tests/mod.rs @@ -758,6 +758,7 @@ pub fn make_deep_tree_with_sum_trees(grove_version: &GroveVersion) -> TempGroveD } mod tests { + use batch::GroveDbOp; use grovedb_merk::proofs::query::SubqueryBranch; use super::*; @@ -3995,4 +3996,131 @@ mod tests { // `verify_grovedb` must identify issues assert!(db.verify_grovedb(None, true, grove_version).unwrap().len() > 0); } + + #[test] + fn test_verify_corrupted_long_reference() { + let grove_version = GroveVersion::latest(); + let db = make_test_grovedb(grove_version); + + db.insert( + &[TEST_LEAF], + b"value", + Element::new_item(b"hello".to_vec()), + None, + None, + grove_version, + ) + .unwrap() + .unwrap(); + + db.insert( + &[TEST_LEAF], + b"refc", + Element::new_reference(ReferencePathType::SiblingReference(b"value".to_vec())), + None, + None, + grove_version, + ) + .unwrap() + .unwrap(); + + db.insert( + &[TEST_LEAF], + b"refb", + Element::new_reference(ReferencePathType::SiblingReference(b"refc".to_vec())), + None, + None, + grove_version, + ) + .unwrap() + .unwrap(); + + db.insert( + &[TEST_LEAF], + b"refa", + Element::new_reference(ReferencePathType::SiblingReference(b"refb".to_vec())), + None, + None, + grove_version, + ) + .unwrap() + .unwrap(); + + assert!(db + .verify_grovedb(None, true, grove_version) + .unwrap() + .is_empty()); + + // Breaking things there: + db.insert( + &[TEST_LEAF], + b"value", + Element::new_item(b"not hello >:(".to_vec()), + None, + None, + grove_version, + ) + .unwrap() + .unwrap(); + + assert!(!db + .verify_grovedb(None, true, grove_version) + .unwrap() + .is_empty()); + } + + #[test] + fn test_verify_corrupted_long_reference_batch() { + let grove_version = GroveVersion::latest(); + let db = make_test_grovedb(grove_version); + + let ops = vec![ + GroveDbOp::insert_op( + vec![TEST_LEAF.to_vec()], + b"value".to_vec(), + Element::new_item(b"hello".to_vec()), + ), + GroveDbOp::insert_op( + vec![TEST_LEAF.to_vec()], + b"refc".to_vec(), + Element::new_reference(ReferencePathType::SiblingReference(b"value".to_vec())), + ), + GroveDbOp::insert_op( + vec![TEST_LEAF.to_vec()], + b"refb".to_vec(), + Element::new_reference(ReferencePathType::SiblingReference(b"refc".to_vec())), + ), + GroveDbOp::insert_op( + vec![TEST_LEAF.to_vec()], + b"refa".to_vec(), + Element::new_reference(ReferencePathType::SiblingReference(b"refb".to_vec())), + ), + ]; + + db.apply_batch(ops, None, None, grove_version) + .unwrap() + .unwrap(); + + assert!(db + .verify_grovedb(None, true, grove_version) + .unwrap() + .is_empty()); + + // Breaking things there: + db.insert( + &[TEST_LEAF], + b"value", + Element::new_item(b"not hello >:(".to_vec()), + None, + None, + grove_version, + ) + .unwrap() + .unwrap(); + + assert!(!db + .verify_grovedb(None, true, grove_version) + .unwrap() + .is_empty()); + } } From 35a97e11b451ed021d29a421167a50dd3f1999c3 Mon Sep 17 00:00:00 2001 From: Evgeny Fomin Date: Wed, 14 Aug 2024 15:30:49 +0200 Subject: [PATCH 2/4] fix verify function --- grovedb/src/lib.rs | 80 +++++++++++----------------------------------- 1 file changed, 19 insertions(+), 61 deletions(-) diff --git a/grovedb/src/lib.rs b/grovedb/src/lib.rs index 0d28e0a7..0c15d0c9 100644 --- a/grovedb/src/lib.rs +++ b/grovedb/src/lib.rs @@ -1063,46 +1063,26 @@ impl GroveDb { "expected merk to contain value at key".to_string(), ))?; - // Absolute path to the referenced merk and the key in that merk: - let (referenced_path, referenced_key) = { + let referenced_value_hash = { let mut full_path = path_from_reference_path_type( reference_path.clone(), &path.to_vec(), Some(&key), )?; - let key = full_path.pop().ok_or_else(|| { - Error::MissingReference( - "can't resolve reference path to absolute path".to_owned(), + let item = self + .follow_reference( + (full_path.as_slice()).into(), + true, + None, + grove_version, ) - })?; - (full_path, key) + .unwrap()?; + value_hash(&item.serialize(grove_version)?).unwrap() }; - // Open another subtree, one that is referenced: - let referenced_merk = self - .open_non_transactional_merk_at_path( - (referenced_path.as_slice()).into(), - None, - grove_version, - ) - .unwrap()?; - - // Get value hash of the referenced item - let (_, referenced_value_hash) = referenced_merk - .get_value_and_value_hash( - &referenced_key, - true, - None::<&fn(&[u8], &GroveVersion) -> Option>, - grove_version, - ) - .unwrap() - .map_err(MerkError)? - .ok_or(Error::CorruptedData( - "expected merk to contain value at key".to_string(), - ))?; - // Take the current item (reference) hash and combine it with referenced value's // hash + let self_actual_value_hash = value_hash(&kv_value).unwrap(); let combined_value_hash = combine_hash(&self_actual_value_hash, &referenced_value_hash).unwrap(); @@ -1223,45 +1203,23 @@ impl GroveDb { "expected merk to contain value at key".to_string(), ))?; - // Absolute path to the referenced merk and the key in that merk: - let (referenced_path, referenced_key) = { + let referenced_value_hash = { let mut full_path = path_from_reference_path_type( reference_path.clone(), &path.to_vec(), Some(&key), )?; - let key = full_path.pop().ok_or_else(|| { - Error::MissingReference( - "can't resolve reference path to absolute path".to_owned(), + let item = self + .follow_reference( + (full_path.as_slice()).into(), + true, + Some(transaction), + grove_version, ) - })?; - (full_path, key) + .unwrap()?; + value_hash(&item.serialize(grove_version)?).unwrap() }; - // Open another subtree, one that is referenced: - let referenced_merk = self - .open_transactional_merk_at_path( - (referenced_path.as_slice()).into(), - transaction, - None, - grove_version, - ) - .unwrap()?; - - // Get value hash of the referenced item - let (_, referenced_value_hash) = referenced_merk - .get_value_and_value_hash( - &referenced_key, - true, - None::<&fn(&[u8], &GroveVersion) -> Option>, - grove_version, - ) - .unwrap() - .map_err(MerkError)? - .ok_or(Error::CorruptedData( - "expected merk to contain value at key".to_string(), - ))?; - // Take the current item (reference) hash and combine it with referenced value's // hash let self_actual_value_hash = value_hash(&kv_value).unwrap(); From 2a6f1abea7bec5115e7c39940e449606fb03bdae Mon Sep 17 00:00:00 2001 From: Evgeny Fomin Date: Wed, 14 Aug 2024 16:13:23 +0200 Subject: [PATCH 3/4] reference insertion fixes --- grovedb/src/element/mod.rs | 11 ++++ grovedb/src/lib.rs | 4 +- grovedb/src/operations/insert/mod.rs | 81 +++++----------------------- grovedb/src/tests/mod.rs | 28 +++++----- 4 files changed, 39 insertions(+), 85 deletions(-) diff --git a/grovedb/src/element/mod.rs b/grovedb/src/element/mod.rs index ad2540e3..11089a9c 100644 --- a/grovedb/src/element/mod.rs +++ b/grovedb/src/element/mod.rs @@ -16,6 +16,9 @@ pub(crate) mod helpers; mod insert; #[cfg(any(feature = "full", feature = "verify"))] mod query; +use grovedb_costs::{cost_return_on_error_default, CostContext, CostsExt, OperationCost}; +use grovedb_merk::{tree::value_hash, CryptoHash}; +use grovedb_version::version::GroveVersion; #[cfg(any(feature = "full", feature = "verify"))] pub use query::QueryOptions; #[cfg(any(feature = "full", feature = "verify"))] @@ -153,6 +156,14 @@ impl Element { Element::SumTree(..) => "sum tree", } } + + pub(crate) fn value_hash( + &self, + grove_version: &GroveVersion, + ) -> CostContext> { + let bytes = cost_return_on_error_default!(self.serialize(grove_version)); + value_hash(&bytes).map(Result::Ok) + } } #[cfg(any(feature = "full", feature = "visualize"))] diff --git a/grovedb/src/lib.rs b/grovedb/src/lib.rs index 0c15d0c9..96c2d0de 100644 --- a/grovedb/src/lib.rs +++ b/grovedb/src/lib.rs @@ -1077,7 +1077,7 @@ impl GroveDb { grove_version, ) .unwrap()?; - value_hash(&item.serialize(grove_version)?).unwrap() + item.value_hash(grove_version).unwrap()? }; // Take the current item (reference) hash and combine it with referenced value's @@ -1217,7 +1217,7 @@ impl GroveDb { grove_version, ) .unwrap()?; - value_hash(&item.serialize(grove_version)?).unwrap() + item.value_hash(grove_version).unwrap()? }; // Take the current item (reference) hash and combine it with referenced value's diff --git a/grovedb/src/operations/insert/mod.rs b/grovedb/src/operations/insert/mod.rs index b7e00fc1..87f0f97b 100644 --- a/grovedb/src/operations/insert/mod.rs +++ b/grovedb/src/operations/insert/mod.rs @@ -7,6 +7,7 @@ use std::{collections::HashMap, option::Option::None}; use grovedb_costs::{ cost_return_on_error, cost_return_on_error_no_add, CostResult, CostsExt, OperationCost, }; +use grovedb_merk::tree::value_hash; #[cfg(feature = "full")] use grovedb_merk::{tree::NULL_HASH, Merk, MerkOptions}; use grovedb_path::SubtreePath; @@ -293,44 +294,18 @@ impl GroveDb { .wrap_with_cost(OperationCost::default()) ); - let (referenced_key, referenced_path) = reference_path.split_last().unwrap(); - let subtree_for_reference = cost_return_on_error!( + let referenced_item = cost_return_on_error!( &mut cost, - self.open_transactional_merk_at_path( - referenced_path.into(), - transaction, - Some(batch), - grove_version, - ) - ); - - let referenced_element_value_hash_opt = cost_return_on_error!( - &mut cost, - Element::get_value_hash( - &subtree_for_reference, - referenced_key, - true, + self.follow_reference( + reference_path.as_slice().into(), + false, + Some(transaction), grove_version ) ); - let referenced_element_value_hash = cost_return_on_error!( - &mut cost, - referenced_element_value_hash_opt - .ok_or({ - let reference_string = reference_path - .iter() - .map(hex::encode) - .collect::>() - .join("/"); - Error::MissingReference(format!( - "reference {}/{} can not be found", - reference_string, - hex::encode(key) - )) - }) - .wrap_with_cost(OperationCost::default()) - ); + let referenced_element_value_hash = + cost_return_on_error!(&mut cost, referenced_item.value_hash(grove_version)); cost_return_on_error!( &mut cost, @@ -452,46 +427,18 @@ impl GroveDb { path_from_reference_path_type(reference_path.clone(), path, Some(key)) .wrap_with_cost(OperationCost::default()) ); - - // TODO unwrap? - let (referenced_key, referenced_path) = reference_path.split_last().unwrap(); - let subtree_for_reference = cost_return_on_error!( + let referenced_item = cost_return_on_error!( &mut cost, - self.open_non_transactional_merk_at_path( - referenced_path.into(), - Some(batch), - grove_version - ) - ); - - // when there is no transaction, we don't want to use caching - let referenced_element_value_hash_opt = cost_return_on_error!( - &mut cost, - Element::get_value_hash( - &subtree_for_reference, - referenced_key, + self.follow_reference( + reference_path.as_slice().into(), false, + None, grove_version ) ); - let referenced_element_value_hash = cost_return_on_error!( - &mut cost, - referenced_element_value_hash_opt - .ok_or({ - let reference_string = reference_path - .iter() - .map(hex::encode) - .collect::>() - .join("/"); - Error::MissingReference(format!( - "reference {}/{} can not be found", - reference_string, - hex::encode(key) - )) - }) - .wrap_with_cost(OperationCost::default()) - ); + let referenced_element_value_hash = + cost_return_on_error!(&mut cost, referenced_item.value_hash(grove_version)); cost_return_on_error!( &mut cost, diff --git a/grovedb/src/tests/mod.rs b/grovedb/src/tests/mod.rs index fa48a900..f6b7b6d4 100644 --- a/grovedb/src/tests/mod.rs +++ b/grovedb/src/tests/mod.rs @@ -1190,7 +1190,12 @@ mod tests { ) .unwrap(); - assert!(matches!(result, Err(Error::MissingReference(_)))); + dbg!(&result); + + assert!(matches!( + result, + Err(Error::CorruptedReferencePathKeyNotFound(_)) + )); } #[test] @@ -1229,24 +1234,15 @@ mod tests { } // Add one more reference - db.insert( - [TEST_LEAF].as_ref(), - &keygen(MAX_REFERENCE_HOPS + 1), - Element::new_reference(ReferencePathType::AbsolutePathReference(vec![ - TEST_LEAF.to_vec(), - keygen(MAX_REFERENCE_HOPS), - ])), - None, - None, - grove_version, - ) - .unwrap() - .expect("expected insert"); - let result = db - .get( + .insert( [TEST_LEAF].as_ref(), &keygen(MAX_REFERENCE_HOPS + 1), + Element::new_reference(ReferencePathType::AbsolutePathReference(vec![ + TEST_LEAF.to_vec(), + keygen(MAX_REFERENCE_HOPS), + ])), + None, None, grove_version, ) From 847ceb7416d122efc64f05f07f9c7e9d2cbf180f Mon Sep 17 00:00:00 2001 From: Evgeny Fomin Date: Wed, 14 Aug 2024 16:33:09 +0200 Subject: [PATCH 4/4] fix ci --- grovedb/src/element/mod.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/grovedb/src/element/mod.rs b/grovedb/src/element/mod.rs index 11089a9c..b1667ffb 100644 --- a/grovedb/src/element/mod.rs +++ b/grovedb/src/element/mod.rs @@ -16,9 +16,6 @@ pub(crate) mod helpers; mod insert; #[cfg(any(feature = "full", feature = "verify"))] mod query; -use grovedb_costs::{cost_return_on_error_default, CostContext, CostsExt, OperationCost}; -use grovedb_merk::{tree::value_hash, CryptoHash}; -use grovedb_version::version::GroveVersion; #[cfg(any(feature = "full", feature = "verify"))] pub use query::QueryOptions; #[cfg(any(feature = "full", feature = "verify"))] @@ -37,6 +34,8 @@ use grovedb_visualize::visualize_to_vec; use crate::operations::proof::util::hex_to_ascii; #[cfg(any(feature = "full", feature = "verify"))] use crate::reference_path::ReferencePathType; +#[cfg(feature = "full")] +use crate::OperationCost; #[cfg(any(feature = "full", feature = "verify"))] /// Optional meta-data to be stored per element @@ -157,12 +156,13 @@ impl Element { } } + #[cfg(feature = "full")] pub(crate) fn value_hash( &self, - grove_version: &GroveVersion, - ) -> CostContext> { - let bytes = cost_return_on_error_default!(self.serialize(grove_version)); - value_hash(&bytes).map(Result::Ok) + grove_version: &grovedb_version::version::GroveVersion, + ) -> grovedb_costs::CostResult { + let bytes = grovedb_costs::cost_return_on_error_default!(self.serialize(grove_version)); + crate::value_hash(&bytes).map(Result::Ok) } }