Skip to content

fix(batch): enforce reference hop limits for existing refs #733

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

Closed
QuantumExplorer wants to merge 1 commit into from
Closed

fix(batch): enforce reference hop limits for existing refs #733

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions grovedb-version/src/version/grovedb_versions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ pub struct GroveDBApplyBatchVersions {
pub apply_batch_with_element_flags_update: FeatureVersion,
pub apply_partial_batch_with_element_flags_update: FeatureVersion,
pub estimated_case_operations_for_batch: FeatureVersion,
pub validate_reference_hop_limits: FeatureVersion,
}

#[derive(Clone, Debug, Default)]
Expand Down
1 change: 1 addition & 0 deletions grovedb-version/src/version/v1.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ pub const GROVE_V1: GroveVersion = GroveVersion {
apply_batch_with_element_flags_update: 0,
apply_partial_batch_with_element_flags_update: 0,
estimated_case_operations_for_batch: 0,
validate_reference_hop_limits: 0,
},
element: GroveDBElementMethodVersions {
delete: 0,
Expand Down
1 change: 1 addition & 0 deletions grovedb-version/src/version/v2.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ pub const GROVE_V2: GroveVersion = GroveVersion {
apply_batch_with_element_flags_update: 0,
apply_partial_batch_with_element_flags_update: 0,
estimated_case_operations_for_batch: 0,
validate_reference_hop_limits: 0,
},
element: GroveDBElementMethodVersions {
delete: 0,
Expand Down
1 change: 1 addition & 0 deletions grovedb-version/src/version/v3.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ pub const GROVE_V3: GroveVersion = GroveVersion {
apply_batch_with_element_flags_update: 0,
apply_partial_batch_with_element_flags_update: 0,
estimated_case_operations_for_batch: 0,
validate_reference_hop_limits: 1,
},
element: GroveDBElementMethodVersions {
delete: 0,
Expand Down
34 changes: 11 additions & 23 deletions grovedb/src/batch/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1389,25 +1389,13 @@ where
.split_last()
.expect("path validated non-empty above");

// Fast path: `recursions_allowed == 1` means the user-declared
// `max_reference_hop` budget allows exactly one more hop. Under
// the well-formed-user contract, that one hop must land on an
// `Item` (or `SumItem` / `ItemWithSumItem`) terminal — pointing
// at another `Reference` would violate the user's own budget.
//
// For an `Item` terminal the merk-stored `value_hash` IS the
// terminal's simple hash `H(serialize(item))`, which is exactly
// what `insert_reference` bakes into the dependent ref via
// `Op::PutCombinedReference`. So we can skip a full element
// decode and read the value_hash directly.
//
// Ill-formed input (`max_hop = 1` pointing at a `Reference`)
// is out of scope: this fast path would return the target's
// merk-combined hash as if it were a simple hash, producing a
// hash mismatch that `verify_grovedb` later reports. The
// contract is the user's to uphold; we don't pay the price of
// an extra dispatch on every well-formed hop=1 ref.
if recursions_allowed == 1 {
if grove_version
.grovedb_versions
.apply_batch
.validate_reference_hop_limits
== 0
&& recursions_allowed == 1
{
let merk = match self.merks.entry(reference_path.to_vec()) {
HashMapEntry::Occupied(o) => o.into_mut(),
HashMapEntry::Vacant(v) => v.insert(cost_return_on_error!(
Expand Down Expand Up @@ -1448,10 +1436,10 @@ where
return Ok(referenced_element_value_hash).wrap_with_cost(cost);
}

// Slow path: `recursions_allowed > 1`. Dispatch on whether the
// target is being modified in this same batch. Neither branch
// needs the merk handle here — the helpers open (or reuse the
// cached) merk themselves via `self.merks.entry(..)`.
// Dispatch on whether the target is being modified in this same
// batch. The on-disk branch always decodes the target so a final
// hop that lands on another reference is rejected with
// ReferenceLimit instead of borrowing the intermediate value hash.
if let Some(referenced_path) = intermediate_reference_info {
// Target is in batch (refresh). Hop through the op's new
// path; budget decrements by one for this hop.
Expand Down
110 changes: 109 additions & 1 deletion grovedb/src/tests/batch_unit_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ mod tests {
};
use crate::reference_path::ReferencePathType;
use crate::tests::{common::EMPTY_PATH, make_empty_grovedb, make_test_grovedb, TEST_LEAF};
use crate::Element;
use crate::{Element, Error};

// ===================================================================
// Group 1: NonMerkTreeMeta::to_tree_type() and count()
Expand Down Expand Up @@ -1146,6 +1146,114 @@ mod tests {
assert_eq!(result, Element::new_item(b"hop1_val".to_vec()));
}

#[test]
fn test_batch_ref_hop_one_to_existing_reference_errors() {
let grove_version = GroveVersion::latest();
let db = make_test_grovedb(grove_version);

db.insert(
[TEST_LEAF].as_ref(),
b"target",
Element::new_item(b"hop1_val".to_vec()),
None,
None,
grove_version,
)
.unwrap()
.expect("insert target");
db.insert(
[TEST_LEAF].as_ref(),
b"existing_ref",
Element::new_reference_with_hops(
ReferencePathType::AbsolutePathReference(vec![
TEST_LEAF.to_vec(),
b"target".to_vec(),
]),
Some(1),
),
None,
None,
grove_version,
)
.unwrap()
.expect("insert existing reference");

let ops = vec![QualifiedGroveDbOp::insert_or_replace_op(
vec![TEST_LEAF.to_vec()],
b"ref_to_ref".to_vec(),
Element::new_reference_with_hops(
ReferencePathType::AbsolutePathReference(vec![
TEST_LEAF.to_vec(),
b"existing_ref".to_vec(),
]),
Some(1),
),
)];

assert!(matches!(
db.apply_batch(ops, None, None, grove_version).unwrap(),
Err(Error::ReferenceLimit)
));
let issues = db
.verify_grovedb(None, true, true, grove_version)
.expect("verify grovedb after rejected batch");
assert!(issues.is_empty(), "verification issues: {:?}", issues);
}

#[test]
fn test_batch_ref_hop_one_to_existing_reference_legacy_version_allows() {
let mut legacy_version = GroveVersion::latest().clone();
legacy_version
.grovedb_versions
.apply_batch
.validate_reference_hop_limits = 0;
let grove_version = &legacy_version;
let db = make_test_grovedb(grove_version);

db.insert(
[TEST_LEAF].as_ref(),
b"target",
Element::new_item(b"hop1_val".to_vec()),
None,
None,
grove_version,
)
.unwrap()
.expect("insert target");
db.insert(
[TEST_LEAF].as_ref(),
b"existing_ref",
Element::new_reference_with_hops(
ReferencePathType::AbsolutePathReference(vec![
TEST_LEAF.to_vec(),
b"target".to_vec(),
]),
Some(1),
),
None,
None,
grove_version,
)
.unwrap()
.expect("insert existing reference");

let ops = vec![QualifiedGroveDbOp::insert_or_replace_op(
vec![TEST_LEAF.to_vec()],
b"ref_to_ref".to_vec(),
Element::new_reference_with_hops(
ReferencePathType::AbsolutePathReference(vec![
TEST_LEAF.to_vec(),
b"existing_ref".to_vec(),
]),
Some(1),
),
)];

db.apply_batch(ops, None, None, grove_version)
.unwrap()
.expect("legacy hop validation should accept final-hop references");
}

/// RefreshReference with trust_refresh_reference=false reads the element
/// from disk before processing.
/// RefreshReference on an element that is NOT a reference on disk.
Expand Down
Loading