Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: reference insertion without batch + verify #327

Merged
merged 4 commits into from
Aug 14, 2024
Merged

fix: reference insertion without batch + verify #327

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7481e40
add tests
fominok Aug 14, 2024
35a97e1
fix verify function
fominok Aug 14, 2024
2a6f1ab
reference insertion fixes
fominok Aug 14, 2024
847ceb7
fix ci
fominok Aug 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions grovedb/src/element/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,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
Expand Down Expand Up @@ -153,6 +155,15 @@ impl Element {
Element::SumTree(..) => "sum tree",
}
}

#[cfg(feature = "full")]
pub(crate) fn value_hash(
&self,
grove_version: &grovedb_version::version::GroveVersion,
) -> grovedb_costs::CostResult<grovedb_merk::CryptoHash, crate::Error> {
let bytes = grovedb_costs::cost_return_on_error_default!(self.serialize(grove_version));
crate::value_hash(&bytes).map(Result::Ok)
}
}

#[cfg(any(feature = "full", feature = "visualize"))]
Expand Down
80 changes: 19 additions & 61 deletions grovedb/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -928,7 +928,7 @@
transaction: TransactionArg,
verify_references: bool,
grove_version: &GroveVersion,
) -> Result<HashMap<Vec<Vec<u8>>, (CryptoHash, CryptoHash, CryptoHash)>, Error> {

Check warning on line 931 in grovedb/src/lib.rs

View workflow job for this annotation

GitHub Actions / clippy

very complex type used. Consider factoring parts into `type` definitions 

warning: very complex type used. Consider factoring parts into `type` definitions
   --> grovedb/src/lib.rs:931:10
    |
931 |     ) -> Result<HashMap<Vec<Vec<u8>>, (CryptoHash, CryptoHash, CryptoHash)>, Error> {
    |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity
    = note: `#[warn(clippy::type_complexity)]` on by default
if let Some(transaction) = transaction {
let root_merk = self
.open_transactional_merk_at_path(
Expand Down Expand Up @@ -969,7 +969,7 @@
batch: Option<&'db StorageBatch>,
verify_references: bool,
grove_version: &GroveVersion,
) -> Result<HashMap<Vec<Vec<u8>>, (CryptoHash, CryptoHash, CryptoHash)>, Error> {

Check warning on line 972 in grovedb/src/lib.rs

View workflow job for this annotation

GitHub Actions / clippy

very complex type used. Consider factoring parts into `type` definitions 

warning: very complex type used. Consider factoring parts into `type` definitions
   --> grovedb/src/lib.rs:972:10
    |
972 |     ) -> Result<HashMap<Vec<Vec<u8>>, (CryptoHash, CryptoHash, CryptoHash)>, Error> {
    |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity
let mut all_query = Query::new();
all_query.insert_all();

Expand Down Expand Up @@ -1063,46 +1063,26 @@
"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(

Check warning on line 1067 in grovedb/src/lib.rs

View workflow job for this annotation

GitHub Actions / clippy

variable does not need to be mutable 

warning: variable does not need to be mutable
    --> grovedb/src/lib.rs:1067:29
     |
1067 |                         let mut full_path = path_from_reference_path_type(
     |                             ----^^^^^^^^^
     |                             |
     |                             help: remove this `mut`
     |
     = note: `#[warn(unused_mut)]` on by default
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()?;
item.value_hash(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<ValueDefinedCostType>>,
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();
Expand All @@ -1127,7 +1107,7 @@
transaction: &Transaction,
verify_references: bool,
grove_version: &GroveVersion,
) -> Result<HashMap<Vec<Vec<u8>>, (CryptoHash, CryptoHash, CryptoHash)>, Error> {

Check warning on line 1110 in grovedb/src/lib.rs

View workflow job for this annotation

GitHub Actions / clippy

very complex type used. Consider factoring parts into `type` definitions 

warning: very complex type used. Consider factoring parts into `type` definitions
    --> grovedb/src/lib.rs:1110:10
     |
1110 |     ) -> Result<HashMap<Vec<Vec<u8>>, (CryptoHash, CryptoHash, CryptoHash)>, Error> {
     |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity
let mut all_query = Query::new();
all_query.insert_all();

Expand Down Expand Up @@ -1223,45 +1203,23 @@
"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(

Check warning on line 1207 in grovedb/src/lib.rs

View workflow job for this annotation

GitHub Actions / clippy

variable does not need to be mutable 

warning: variable does not need to be mutable
    --> grovedb/src/lib.rs:1207:29
     |
1207 |                         let mut full_path = path_from_reference_path_type(
     |                             ----^^^^^^^^^
     |                             |
     |                             help: remove this `mut`
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()?;
item.value_hash(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<ValueDefinedCostType>>,
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();
Expand Down
81 changes: 14 additions & 67 deletions grovedb/src/operations/insert/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use grovedb_costs::{
cost_return_on_error, cost_return_on_error_no_add, CostResult, CostsExt, OperationCost,
};
use grovedb_merk::tree::value_hash;

Check warning on line 10 in grovedb/src/operations/insert/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

unused import: `grovedb_merk::tree::value_hash` 

warning: unused import: `grovedb_merk::tree::value_hash`
  --> grovedb/src/operations/insert/mod.rs:10:5
   |
10 | use grovedb_merk::tree::value_hash;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(unused_imports)]` on by default
#[cfg(feature = "full")]
use grovedb_merk::{tree::NULL_HASH, Merk, MerkOptions};
use grovedb_path::SubtreePath;
Expand Down Expand Up @@ -114,16 +115,16 @@
})
}

fn insert_on_transaction<'db, 'b, B: AsRef<[u8]>>(
&self,
path: SubtreePath<'b, B>,
key: &[u8],
element: Element,
options: InsertOptions,
transaction: &'db Transaction,
batch: &StorageBatch,
grove_version: &GroveVersion,
) -> CostResult<(), Error> {

Check warning on line 127 in grovedb/src/operations/insert/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

this function has too many arguments (8/7) 

warning: this function has too many arguments (8/7)
   --> grovedb/src/operations/insert/mod.rs:118:5
    |
118 | /     fn insert_on_transaction<'db, 'b, B: AsRef<[u8]>>(
119 | |         &self,
120 | |         path: SubtreePath<'b, B>,
121 | |         key: &[u8],
...   |
126 | |         grove_version: &GroveVersion,
127 | |     ) -> CostResult<(), Error> {
    | |______________________________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
check_grovedb_v0_with_cost!(
"insert_on_transaction",
grove_version
Expand Down Expand Up @@ -214,16 +215,16 @@
/// first make sure other merk exist
/// if it exists, then create merk to be inserted, and get root hash
/// we only care about root hash of merk to be inserted
fn add_element_on_transaction<'db, B: AsRef<[u8]>>(
&'db self,
path: SubtreePath<B>,
key: &[u8],
element: Element,
options: InsertOptions,
transaction: &'db Transaction,
batch: &'db StorageBatch,
grove_version: &GroveVersion,
) -> CostResult<Merk<PrefixedRocksDbTransactionContext<'db>>, Error> {

Check warning on line 227 in grovedb/src/operations/insert/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

this function has too many arguments (8/7) 

warning: this function has too many arguments (8/7)
   --> grovedb/src/operations/insert/mod.rs:218:5
    |
218 | /     fn add_element_on_transaction<'db, B: AsRef<[u8]>>(
219 | |         &'db self,
220 | |         path: SubtreePath<B>,
221 | |         key: &[u8],
...   |
226 | |         grove_version: &GroveVersion,
227 | |     ) -> CostResult<Merk<PrefixedRocksDbTransactionContext<'db>>, Error> {
    | |________________________________________________________________________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
check_grovedb_v0_with_cost!(
"add_element_on_transaction",
grove_version
Expand Down Expand Up @@ -293,44 +294,18 @@
.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::<Vec<String>>()
.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,
Expand Down Expand Up @@ -452,46 +427,18 @@
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::<Vec<String>>()
.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,
Expand Down
Loading
Loading