diff --git a/grovedb-version/src/version/grovedb_versions.rs b/grovedb-version/src/version/grovedb_versions.rs index b63af3694..eaf2eb31a 100644 --- a/grovedb-version/src/version/grovedb_versions.rs +++ b/grovedb-version/src/version/grovedb_versions.rs @@ -55,34 +55,41 @@ pub struct GroveDBApplyBatchVersions { /// /// - `0` (V1..V3): the caller-declared `TreeType` carried by the op is /// taken at face value. - /// - `1` (V4+): the stored element is read and its actual type used - /// instead, and a declared/stored mismatch involving an indexed tree is - /// rejected. Closes an indexed type-confusion — a declared type hiding a - /// stored indexed primary skips the per-axis secondary sweep and leaves + /// - `1` (V4+): the ACTUAL stored type is used instead, and a + /// declared/stored mismatch involving an indexed tree is rejected. + /// Closes an indexed type-confusion — a declared type hiding a stored + /// indexed primary skips the per-axis secondary sweep and leaves /// authenticated stale rows — and a `CommitmentTree` case where the /// declared type sends the op down the wrong emptiness path, orphaning - /// its non-Merk data. Costs one extra stored-element read per op, which - /// is why it cannot apply to the released versions. + /// its non-Merk data. + /// + /// The stored element comes from data the apply already loads (the + /// emptiness pre-scan's own read, or the old value the merk delete + /// surfaces through the old-value observer), so V4 charges exactly the + /// V1..V3 cost per op. The slot still gates the check because it flips + /// an accepted/rejected outcome — a mismatched delete that V1..V3 + /// accept is refused on V4+ when an indexed tree is involved. pub delete_tree_cleanup_type_source: FeatureVersion, - /// Whether a batch overwrite (`InsertOrReplace` / `Replace` / `Patch` of a - /// non-reference element, with tree-override protection off) reads the - /// stored element to detect an indexed tree being overwritten. + /// Whether a batch overwrite (`InsertOrReplace` / `Replace` / `Patch`, + /// with tree-override protection off) classifies the element it + /// displaces to detect an indexed tree being overwritten. /// - /// - `0` (V1..V3): no read. Overwrites keep their released cost shape. - /// - `1` (V4+): the stored element is read and, when it is an indexed - /// tree, the overwrite is classified — the safe subset (empty indexed or - /// non-indexed replacement) schedules the per-axis secondary storage for - /// cleanup, and an ambiguous non-empty indexed replacement is refused. - /// Without the read, overwriting an indexed primary would orphan its - /// secondary namespaces at their derived prefixes. + /// - `0` (V1..V3): no classification. Overwrites keep their released + /// accepted/rejected outcomes. + /// - `1` (V4+): the displaced element is classified — the safe subset + /// (empty indexed or non-indexed replacement, references included) + /// schedules the per-axis secondary storage for cleanup, and an + /// ambiguous non-empty indexed replacement is refused. Without this, + /// overwriting an indexed primary would orphan its secondary + /// namespaces at their derived prefixes. /// - /// Costs one extra stored-element read per overwrite-capable op, which - /// measurably changes tracked costs (+1 seek, +129 loaded bytes on the - /// repo's own cost tests) — cost feeds fees, so like - /// [`Self::delete_tree_cleanup_type_source`] it cannot apply to the - /// released versions. The hole it closes needs an indexed tree to be the - /// element being overwritten, which cannot occur before the version that - /// introduces indexed trees. + /// The old element bytes come from the node the merk walk fetched + /// anyway to rewrite the key, surfaced through the old-value observer — + /// no dedicated stored-element read, so V4 charges exactly the V1..V3 + /// cost per overwrite-capable op. Like + /// [`Self::delete_tree_cleanup_type_source`] the slot gates behaviour, + /// not cost: a non-empty indexed replacement that would be accepted + /// blind on V1..V3 is refused on V4+. pub overwrite_indexed_cleanup_inspection: FeatureVersion, } diff --git a/grovedb-version/src/version/v4.rs b/grovedb-version/src/version/v4.rs index 7b53a2033..9be0893f6 100644 --- a/grovedb-version/src/version/v4.rs +++ b/grovedb-version/src/version/v4.rs @@ -4,18 +4,22 @@ //! and no longer is — gates land here as they are written. Currently flipped: //! //! - `apply_batch.delete_tree_cleanup_type_source: 1` — a batch `DeleteTree` -//! reads the stored element and uses its ACTUAL type to select cleanup -//! namespaces, rejecting a declared/stored mismatch that involves an indexed -//! tree. V1..V3 keep taking the declared type at face value. Costs one extra -//! stored-element read per op, which is why it cannot apply to the released -//! versions. +//! uses the stored element's ACTUAL type to select cleanup namespaces, +//! rejecting a declared/stored mismatch that involves an indexed tree. +//! V1..V3 keep taking the declared type at face value. The stored element +//! comes from data the apply already loads (the emptiness pre-scan's own +//! read, or the old value the merk delete surfaces through the old-value +//! observer), so V4 charges exactly the V1..V3 cost — the gate exists +//! because it flips an accepted/rejected outcome, not because of cost. //! -//! - `apply_batch.overwrite_indexed_cleanup_inspection: 1` — a batch overwrite -//! of a non-reference element (with tree-override protection off) reads the -//! stored element to detect an indexed tree being overwritten, scheduling -//! its per-axis secondary storage for cleanup or refusing the ambiguous -//! case. Same shape as the gate above: one extra stored-element read per -//! overwrite-capable op, so V1..V3 keep their released cost shape. +//! - `apply_batch.overwrite_indexed_cleanup_inspection: 1` — a batch +//! overwrite (with tree-override protection off, references included) +//! classifies the element it displaces to detect an indexed tree being +//! overwritten, scheduling its per-axis secondary storage for cleanup or +//! refusing the ambiguous case. The old bytes come from the node the merk +//! walk fetched anyway to rewrite the key, so — like the gate above — +//! V1..V3 cost is charged exactly and only the accepted/rejected outcome +//! is gated. //! //! - `proof.terminal_non_merk_tree_child_hash: 1` — a V1 proof that reports a //! `CommitmentTree` / `MmrTree` / `BulkAppendTree` / diff --git a/grovedb/src/batch/estimated_costs/average_case_costs.rs b/grovedb/src/batch/estimated_costs/average_case_costs.rs index 548832952..b79da061a 100644 --- a/grovedb/src/batch/estimated_costs/average_case_costs.rs +++ b/grovedb/src/batch/estimated_costs/average_case_costs.rs @@ -12,23 +12,18 @@ use grovedb_costs::{ }; #[cfg(feature = "minimal")] use grovedb_merk::estimated_costs::average_case_costs::{ - add_average_case_get_merk_node, add_average_case_merk_has_value, average_case_merk_propagate, - EstimatedLayerInformation, + add_average_case_merk_has_value, average_case_merk_propagate, EstimatedLayerInformation, }; use grovedb_merk::{ element::tree_type::ElementTreeTypeExtensions, tree::AggregateData, tree_type::TreeType, RootHashKeyAndAggregateData, }; #[cfg(feature = "minimal")] -use grovedb_merk::{tree::kv::KV, tree_type::CostSize}; -#[cfg(feature = "minimal")] use grovedb_storage::rocksdb_storage::RocksDbStorage; #[cfg(feature = "minimal")] -use grovedb_storage::{worst_case_costs::WorstKeyLength, Storage}; +use grovedb_storage::worst_case_costs::WorstKeyLength; use grovedb_version::version::GroveVersion; #[cfg(feature = "minimal")] -use integer_encoding::VarInt; -#[cfg(feature = "minimal")] use itertools::Itertools; use crate::Element; @@ -432,78 +427,6 @@ fn indexed_axes_for_tree_type(tree_type: TreeType) -> Vec= 1`: an overwrite-capable op -/// (`InsertOrReplace` / `Replace` / `Patch`, excluding bare references) -/// with tree-override validation OFF reads the existing element from the -/// op's own Merk to detect an indexed tree being overwritten. Modeled as -/// one average-case Merk node read of this layer's estimated element, -/// skipped when the layer is estimated to be empty — a get on an empty -/// Merk is free, which is what keeps fresh-insert estimates identical to -/// applied costs. -/// - `delete_tree_cleanup_type_source >= 1`: a `DeleteTree` op builds the -/// parent storage context and reads the stored element so cleanup -/// namespaces follow its ACTUAL type. Modeled as the context prefix hash -/// plus one seek loading the layered element cost — the exact -/// `storage_loaded_bytes` shape `Element::get_from_storage` reports for -/// tree elements. -fn add_average_case_v4_read_gate_costs( - cost: &mut OperationCost, - path: &KeyInfoPath, - key: &KeyInfo, - op: &GroveOp, - layer_element_estimates: &EstimatedLayerInformation, - batch_apply_options: &BatchApplyOptions, - grove_version: &GroveVersion, -) -> Result<(), Error> { - let apply_batch = &grove_version.grovedb_versions.apply_batch; - match op { - GroveOp::InsertOrReplace { element } - | GroveOp::Replace { element } - | GroveOp::Patch { element, .. } - if apply_batch.overwrite_indexed_cleanup_inspection >= 1 - && !batch_apply_options.validate_insertion_does_not_override_tree - && !matches!(element, Element::Reference(..)) - && !layer_element_estimates - .estimated_layer_count - .estimated_to_be_empty() => - { - let estimated_element_size = layer_element_estimates - .estimated_layer_sizes - .value_with_feature_and_flags_size(grove_version) - .map_err(Error::MerkError)?; - add_average_case_get_merk_node( - cost, - key.max_length() as u32, - estimated_element_size, - layer_element_estimates.tree_type.inner_node_type(), - ) - .map_err(Error::MerkError)?; - } - GroveOp::DeleteTree(tree_type, _) if apply_batch.delete_tree_cleanup_type_source >= 1 => { - *cost += RocksDbStorage::get_storage_context_cost(path.as_vec()); - let flags_size = layer_element_estimates - .estimated_layer_sizes - .layered_flags_size() - .map_err(Error::MerkError)? - .map(|f| f + f.required_space() as u32) - .unwrap_or_default(); - cost.seek_count += 1; - cost.storage_loaded_bytes += KV::layered_value_byte_cost_size_for_key_and_value_lengths( - key.max_length() as u32, - tree_type.cost_size() + flags_size, - layer_element_estimates.tree_type.inner_node_type(), - ) as u64; - } - _ => {} - } - Ok(()) -} - #[cfg(feature = "minimal")] /// Cache for subtree paths for average case scenario costs. #[derive(Default)] @@ -594,7 +517,7 @@ impl TreeCache for AverageCaseTreeCacheKnownPaths { path: &KeyInfoPath, ops_at_path_by_key: BTreeMap, _ops_by_qualified_paths: &BTreeMap>, GroveOp>, - batch_apply_options: &BatchApplyOptions, + _batch_apply_options: &BatchApplyOptions, _flags_update: &mut G, _split_removal_bytes: &mut SR, grove_version: &GroveVersion, @@ -661,18 +584,6 @@ impl TreeCache for AverageCaseTreeCacheKnownPaths { .count(); for (key, op) in ops_at_path_by_key.into_iter() { - cost_return_on_error_no_add!( - cost, - add_average_case_v4_read_gate_costs( - &mut cost, - path, - &key, - &op, - layer_element_estimates, - batch_apply_options, - grove_version, - ) - ); cost_return_on_error!( &mut cost, op.average_case_cost(&key, layer_element_estimates, false, grove_version) @@ -1048,13 +959,7 @@ mod tests { // case cost if it doesn't already exist assert_eq!(cost.storage_cost, average_case_cost.storage_cost); assert_eq!(cost.hash_node_calls, average_case_cost.hash_node_calls); - // With `overwrite_indexed_cleanup_inspection >= 1` (V4+, which is - // what `latest()` resolves to) the estimator charges one node read - // for indexed-overwrite detection on this non-empty layer. The - // applied path performs the same read but gets it for free here: - // the only existing node is the Merk's root, already in memory - // from opening the Merk. - assert_eq!(cost.seek_count + 1, average_case_cost.seek_count); + assert_eq!(cost.seek_count, average_case_cost.seek_count); // Seek Count explanation (this isn't 100% sure - needs to be verified) // 1 to get root merk @@ -1062,8 +967,6 @@ mod tests { // 1 to get previous element // 1 to insert // 1 to insert node above - // 1 for the gated indexed-overwrite inspection read (estimator only - // pays it here; see comment above) // Replaced parent Value -> 76 // 1 for the flag option (but no flags) @@ -1076,19 +979,17 @@ mod tests { // Loaded // For root key 1 byte // For root tree item 69 bytes - // For the inspection read 150 bytes (average-case encoded node with - // both links present, keyed and valued from the layer estimates) assert_eq!( average_case_cost, OperationCost { - seek_count: 6, + seek_count: 5, // todo: why is this 5 storage_cost: StorageCost { added_bytes: 115, replaced_bytes: 75, removed_bytes: NoStorageRemoval, }, - storage_loaded_bytes: 259, + storage_loaded_bytes: 109, hash_node_calls: 8, sinsemilla_hash_calls: 0, } @@ -1246,22 +1147,16 @@ mod tests { // case cost if it doesn't already exist assert_eq!(average_case_cost.storage_cost.added_bytes, 0); - // +1 seek and +238 storage_loaded_bytes vs. V1..V3: with - // `overwrite_indexed_cleanup_inspection >= 1` (V4+, which is what - // `latest()` resolves to) and tree-override validation off, the - // estimator models the applied path's read of the existing element - // for indexed-overwrite detection — one average-case node read of - // this layer's estimated element. assert_eq!( average_case_cost, OperationCost { - seek_count: 42, + seek_count: 41, storage_cost: StorageCost { added_bytes: 0, replaced_bytes: 5594, removed_bytes: NoStorageRemoval, }, - storage_loaded_bytes: 7907, + storage_loaded_bytes: 7669, hash_node_calls: 79, sinsemilla_hash_calls: 0, } @@ -2499,252 +2394,4 @@ mod tests { actual.storage_loaded_bytes ); } - - /// The V4 `overwrite_indexed_cleanup_inspection` gate charges one extra - /// stored-element read per overwrite-capable op in the APPLIED path, so - /// the estimator must move by exactly one modeled node read between V3 - /// and V4 — and must NOT move when the gate's own applied-side - /// conditions (tree-override validation on, bare-reference element) - /// suppress the read. - #[test] - fn test_average_case_overwrite_inspection_read_gated_v4_vs_v3() { - use grovedb_merk::tree::TreeNode; - use grovedb_version::version::v3::GROVE_V3; - - use crate::batch::BatchApplyOptions; - - let v3 = &GROVE_V3; - let v4 = GroveVersion::latest(); - assert!( - v4.grovedb_versions - .apply_batch - .overwrite_indexed_cleanup_inspection - >= 1 - ); - - let layer = EstimatedLayerInformation { - tree_type: TreeType::NormalTree, - estimated_layer_count: EstimatedLevel(1, false), - estimated_layer_sizes: AllItems(4, 8, None), - }; - let ops = vec![QualifiedGroveDbOp::replace_op( - vec![], - b"key1".to_vec(), - Element::new_item(b"12345678".to_vec()), - )]; - let estimate = |grove_version, options| { - let mut paths = HashMap::new(); - paths.insert(KeyInfoPath(vec![]), layer); - GroveDb::estimated_case_operations_for_batch( - AverageCaseCostsType(paths), - ops.clone(), - options, - |_cost, _old_flags, _new_flags| Ok(false), - |_flags, _removed_key_bytes, _removed_value_bytes| { - Ok((NoStorageRemoval, NoStorageRemoval)) - }, - grove_version, - ) - .cost_as_result() - .expect("expected to estimate costs") - }; - - let est_v3 = estimate(v3, None); - let est_v4 = estimate(v4, None); - - // Exactly one average-case node read of this layer's estimated - // element — the same shape `add_average_case_get_merk_node` charges. - let estimated_element_size = layer - .estimated_layer_sizes - .value_with_feature_and_flags_size(v4) - .expect("expected element size"); - let expected_loaded = TreeNode::average_case_encoded_tree_size( - 4, - estimated_element_size, - layer.tree_type.inner_node_type(), - ) as u64; - assert_eq!(est_v4.seek_count, est_v3.seek_count + 1); - assert_eq!( - est_v4.storage_loaded_bytes, - est_v3.storage_loaded_bytes + expected_loaded - ); - assert_eq!(est_v4.storage_cost, est_v3.storage_cost); - assert_eq!(est_v4.hash_node_calls, est_v3.hash_node_calls); - - // Tree-override validation ON suppresses the applied read (the - // ungated validation branch runs instead), so the estimator must - // charge nothing for the gate. - let validated = BatchApplyOptions { - validate_insertion_does_not_override_tree: true, - ..Default::default() - }; - assert_eq!(estimate(v4, Some(validated)), est_v3); - - // Bare references are excluded from the applied inspection, so a - // reference overwrite estimates identically on V3 and V4. - let ops = vec![QualifiedGroveDbOp::replace_op( - vec![], - b"key1".to_vec(), - Element::new_reference(ReferencePathType::SiblingReference(b"key2".to_vec())), - )]; - let est_ref_v3 = { - let mut paths = HashMap::new(); - paths.insert(KeyInfoPath(vec![]), layer); - GroveDb::estimated_case_operations_for_batch( - AverageCaseCostsType(paths), - ops.clone(), - None, - |_cost, _old_flags, _new_flags| Ok(false), - |_flags, _removed_key_bytes, _removed_value_bytes| { - Ok((NoStorageRemoval, NoStorageRemoval)) - }, - v3, - ) - .cost_as_result() - .expect("expected to estimate costs") - }; - let est_ref_v4 = { - let mut paths = HashMap::new(); - paths.insert(KeyInfoPath(vec![]), layer); - GroveDb::estimated_case_operations_for_batch( - AverageCaseCostsType(paths), - ops, - None, - |_cost, _old_flags, _new_flags| Ok(false), - |_flags, _removed_key_bytes, _removed_value_bytes| { - Ok((NoStorageRemoval, NoStorageRemoval)) - }, - v4, - ) - .cost_as_result() - .expect("expected to estimate costs") - }; - assert_eq!(est_ref_v4, est_ref_v3); - } - - /// The V4 `delete_tree_cleanup_type_source` gate reads the stored - /// element (through a freshly built parent storage context) on every - /// applied `DeleteTree` op. That read is deterministic — a raw - /// storage get, no Merk caching involved — so the estimator's modeled - /// read must match the applied one EXACTLY: same seek, same loaded - /// bytes, same context-prefix hash calls, per gate, field for field. - #[test] - fn test_average_case_delete_tree_type_source_read_estimated_vs_applied_parity() { - use grovedb_merk::{merk::NodeType, tree::kv::KV, tree_type::CostSize}; - use grovedb_version::version::v3::GROVE_V3; - - let v3 = &GROVE_V3; - let v4 = GroveVersion::latest(); - assert!( - v4.grovedb_versions - .apply_batch - .delete_tree_cleanup_type_source - >= 1 - ); - - let ops = vec![QualifiedGroveDbOp::delete_tree_op( - vec![b"tree1".to_vec()], - b"key1".to_vec(), - TreeType::NormalTree, - SubelementsDeletionBehavior::Error, - )]; - - let apply = |grove_version| { - let db = make_empty_grovedb(); - let tx = db.start_transaction(); - db.insert( - EMPTY_PATH, - b"tree1", - Element::empty_tree(), - None, - Some(&tx), - grove_version, - ) - .unwrap() - .expect("insert parent tree"); - db.insert( - [b"tree1".as_slice()].as_ref(), - b"key1", - Element::empty_tree(), - None, - Some(&tx), - grove_version, - ) - .unwrap() - .expect("insert child tree"); - db.apply_batch(ops.clone(), None, Some(&tx), grove_version) - .cost_as_result() - .expect("apply delete tree batch") - }; - - let estimate = |grove_version| { - let mut paths = HashMap::new(); - paths.insert( - KeyInfoPath(vec![]), - EstimatedLayerInformation { - tree_type: TreeType::NormalTree, - estimated_layer_count: EstimatedLevel(1, false), - estimated_layer_sizes: AllSubtrees(5, NoSumTrees, None), - }, - ); - paths.insert( - KeyInfoPath(vec![KeyInfo::KnownKey(b"tree1".to_vec())]), - EstimatedLayerInformation { - tree_type: TreeType::NormalTree, - estimated_layer_count: EstimatedLevel(1, false), - estimated_layer_sizes: AllSubtrees(4, NoSumTrees, None), - }, - ); - GroveDb::estimated_case_operations_for_batch( - AverageCaseCostsType(paths), - ops.clone(), - None, - |_cost, _old_flags, _new_flags| Ok(false), - |_flags, _removed_key_bytes, _removed_value_bytes| { - Ok((NoStorageRemoval, NoStorageRemoval)) - }, - grove_version, - ) - .cost_as_result() - .expect("expected to estimate costs") - }; - - let applied_v3 = apply(v3); - let applied_v4 = apply(v4); - let est_v3 = estimate(v3); - let est_v4 = estimate(v4); - - // The stored element is an empty tree with no flags under a 4-byte - // key in a normal-node parent — the exact layered cost - // `Element::get_from_storage` reports for the read. - let expected_loaded = KV::layered_value_byte_cost_size_for_key_and_value_lengths( - 4, - TreeType::NormalTree.cost_size(), - NodeType::NormalNode, - ) as u64; - - for (v4_cost, v3_cost, side) in [ - (&applied_v4, &applied_v3, "applied"), - (&est_v4, &est_v3, "estimated"), - ] { - assert_eq!(v4_cost.seek_count, v3_cost.seek_count + 1, "{side} seeks"); - assert_eq!( - v4_cost.storage_loaded_bytes, - v3_cost.storage_loaded_bytes + expected_loaded, - "{side} loaded bytes" - ); - assert_eq!( - v4_cost.storage_cost, v3_cost.storage_cost, - "{side} storage cost" - ); - } - // Context-prefix hashing must move identically on both sides — - // that is the estimated-vs-applied parity for the gate's - // `get_transactional_storage_context` build. - assert_eq!( - applied_v4.hash_node_calls - applied_v3.hash_node_calls, - est_v4.hash_node_calls - est_v3.hash_node_calls, - "context-prefix hash calls" - ); - } } diff --git a/grovedb/src/batch/estimated_costs/worst_case_costs.rs b/grovedb/src/batch/estimated_costs/worst_case_costs.rs index 48f454730..0689de98e 100644 --- a/grovedb/src/batch/estimated_costs/worst_case_costs.rs +++ b/grovedb/src/batch/estimated_costs/worst_case_costs.rs @@ -12,19 +12,17 @@ use grovedb_costs::{ }; #[cfg(feature = "minimal")] use grovedb_merk::estimated_costs::worst_case_costs::{ - add_worst_case_get_merk_node, add_worst_case_merk_has_value, worst_case_merk_propagate, - WorstCaseLayerInformation, MERK_BIGGEST_VALUE_SIZE, + add_worst_case_merk_has_value, worst_case_merk_propagate, WorstCaseLayerInformation, + MERK_BIGGEST_VALUE_SIZE, }; use grovedb_merk::{ element::tree_type::ElementTreeTypeExtensions, tree::AggregateData, tree_type::TreeType, RootHashKeyAndAggregateData, }; #[cfg(feature = "minimal")] -use grovedb_merk::{tree::kv::KV, tree_type::CostSize}; -#[cfg(feature = "minimal")] use grovedb_storage::rocksdb_storage::RocksDbStorage; #[cfg(feature = "minimal")] -use grovedb_storage::{worst_case_costs::WorstKeyLength, Storage}; +use grovedb_storage::worst_case_costs::WorstKeyLength; use grovedb_version::version::GroveVersion; #[cfg(feature = "minimal")] use itertools::Itertools; @@ -36,7 +34,6 @@ use crate::{ key_info::KeyInfo, mode::BatchRunMode, BatchApplyOptions, GroveOp, KeyInfoPath, RefreshReferenceMode, TreeCache, }, - estimated_costs::worst_case_costs::WORST_CASE_FLAGS_LEN, Error, GroveDb, }; @@ -412,63 +409,6 @@ impl GroveOp { } } -#[cfg(feature = "minimal")] -/// Model the stored-element reads the GROVE_V4 apply gates add, keeping the -/// worst-case estimate an upper bound of the applied cost per gate. On -/// V1..V3 both gates are 0 and this charges nothing, preserving released -/// estimates. See the average-case counterpart for the shape of each gated -/// read; the worst case sizes the loaded node with the Merk-wide maxima -/// (`MERK_BIGGEST_VALUE_SIZE` / `WORST_CASE_FLAGS_LEN`) instead of layer -/// estimates. -fn add_worst_case_v4_read_gate_costs( - cost: &mut OperationCost, - path: &KeyInfoPath, - key: &KeyInfo, - op: &GroveOp, - worst_case_layer_information: &WorstCaseLayerInformation, - batch_apply_options: &BatchApplyOptions, - grove_version: &GroveVersion, -) -> Result<(), Error> { - let apply_batch = &grove_version.grovedb_versions.apply_batch; - // A get on an empty Merk is free, so a layer known to hold nothing - // charges no inspection read — mirroring the average-case skip and - // keeping fresh-insert estimates unchanged. - let layer_known_empty = matches!( - worst_case_layer_information, - WorstCaseLayerInformation::MaxElementsNumber(0) - | WorstCaseLayerInformation::NumberOfLevels(0) - ); - match op { - GroveOp::InsertOrReplace { element } - | GroveOp::Replace { element } - | GroveOp::Patch { element, .. } - if apply_batch.overwrite_indexed_cleanup_inspection >= 1 - && !batch_apply_options.validate_insertion_does_not_override_tree - && !matches!(element, Element::Reference(..)) - && !layer_known_empty => - { - add_worst_case_get_merk_node( - cost, - key.max_length() as u32, - MERK_BIGGEST_VALUE_SIZE, - TreeType::NormalTree.inner_node_type(), - ) - .map_err(Error::MerkError)?; - } - GroveOp::DeleteTree(tree_type, _) if apply_batch.delete_tree_cleanup_type_source >= 1 => { - *cost += RocksDbStorage::get_storage_context_cost(path.as_vec()); - cost.seek_count += 1; - cost.storage_loaded_bytes += KV::layered_value_byte_cost_size_for_key_and_value_lengths( - key.max_length() as u32, - tree_type.cost_size() + WORST_CASE_FLAGS_LEN, - TreeType::NormalTree.inner_node_type(), - ) as u64; - } - _ => {} - } - Ok(()) -} - #[cfg(feature = "minimal")] /// Cache for subtree paths for worst case scenario costs. #[derive(Default)] @@ -525,7 +465,7 @@ impl TreeCache for WorstCaseTreeCacheKnownPaths { path: &KeyInfoPath, ops_at_path_by_key: BTreeMap, _ops_by_qualified_paths: &BTreeMap>, GroveOp>, - batch_apply_options: &BatchApplyOptions, + _batch_apply_options: &BatchApplyOptions, _flags_update: &mut G, _split_removal_bytes: &mut SR, grove_version: &GroveVersion, @@ -557,18 +497,6 @@ impl TreeCache for WorstCaseTreeCacheKnownPaths { } for (key, op) in ops_at_path_by_key.into_iter() { - cost_return_on_error_no_add!( - cost, - add_worst_case_v4_read_gate_costs( - &mut cost, - path, - &key, - &op, - worst_case_layer_element_estimates, - batch_apply_options, - grove_version, - ) - ); cost_return_on_error!( &mut cost, op.worst_case_cost( @@ -679,22 +607,16 @@ mod tests { worst_case_cost.storage_cost.added_bytes ); - // +1 seek and +65682 storage_loaded_bytes vs. V1..V3: with - // `overwrite_indexed_cleanup_inspection >= 1` (V4+, which is what - // `latest()` resolves to) and tree-override validation off, the - // estimator models the applied path's read of the existing element - // for indexed-overwrite detection — one worst-case node read sized - // at `MERK_BIGGEST_VALUE_SIZE`. assert_eq!( worst_case_cost, OperationCost { - seek_count: 6, + seek_count: 5, storage_cost: StorageCost { added_bytes: 115, replaced_bytes: 65535, // todo: verify removed_bytes: NoStorageRemoval, }, - storage_loaded_bytes: 131473, + storage_loaded_bytes: 65791, hash_node_calls: 8, // todo: verify why sinsemilla_hash_calls: 0, } @@ -864,19 +786,16 @@ mod tests { worst_case_cost.storage_cost.added_bytes ); - // +1 seek and +65682 storage_loaded_bytes vs. V1..V3: the gated - // worst-case indexed-overwrite inspection read, as in - // `test_batch_root_one_tree_insert_op_worst_case_costs`. assert_eq!( worst_case_cost, OperationCost { - seek_count: 39, + seek_count: 38, storage_cost: StorageCost { added_bytes: 115, replaced_bytes: 2228190, // todo: verify removed_bytes: NoStorageRemoval, }, - storage_loaded_bytes: 2302576, + storage_loaded_bytes: 2236894, hash_node_calls: 74, sinsemilla_hash_calls: 0, } @@ -1643,138 +1562,4 @@ mod tests { cost_count, ); } - - /// Worst-case counterpart of the average-case V4-gate parity test: the - /// `overwrite_indexed_cleanup_inspection` gate moves the estimate by - /// exactly one worst-case node read between V3 and V4, and only when - /// the gate's applied-side conditions hold. - #[test] - fn test_worst_case_overwrite_inspection_read_gated_v4_vs_v3() { - use grovedb_merk::{ - estimated_costs::worst_case_costs::MERK_BIGGEST_VALUE_SIZE, tree::TreeNode, - }; - use grovedb_version::version::v3::GROVE_V3; - - use crate::batch::BatchApplyOptions; - - let v3 = &GROVE_V3; - let v4 = GroveVersion::latest(); - assert!( - v4.grovedb_versions - .apply_batch - .overwrite_indexed_cleanup_inspection - >= 1 - ); - - let ops = vec![QualifiedGroveDbOp::replace_op( - vec![], - b"key1".to_vec(), - Element::new_item(b"12345678".to_vec()), - )]; - let estimate = |grove_version, options| { - let mut paths = HashMap::new(); - paths.insert(KeyInfoPath(vec![]), MaxElementsNumber(1)); - GroveDb::estimated_case_operations_for_batch( - WorstCaseCostsType(paths), - ops.clone(), - options, - |_cost, _old_flags, _new_flags| Ok(false), - |_flags, _removed_key_bytes, _removed_value_bytes| { - Ok((NoStorageRemoval, NoStorageRemoval)) - }, - grove_version, - ) - .cost_as_result() - .expect("expected to estimate costs") - }; - - let est_v3 = estimate(v3, None); - let est_v4 = estimate(v4, None); - - let expected_loaded = TreeNode::worst_case_encoded_tree_size( - 4, - MERK_BIGGEST_VALUE_SIZE, - TreeType::NormalTree.inner_node_type(), - ) as u64; - assert_eq!(est_v4.seek_count, est_v3.seek_count + 1); - assert_eq!( - est_v4.storage_loaded_bytes, - est_v3.storage_loaded_bytes + expected_loaded - ); - assert_eq!(est_v4.storage_cost, est_v3.storage_cost); - assert_eq!(est_v4.hash_node_calls, est_v3.hash_node_calls); - - // Tree-override validation ON suppresses the applied read, so the - // estimator must charge nothing for the gate. - let validated = BatchApplyOptions { - validate_insertion_does_not_override_tree: true, - ..Default::default() - }; - assert_eq!(estimate(v4, Some(validated)), est_v3); - } - - /// Worst-case counterpart of the average-case `DeleteTree` gate test: - /// the `delete_tree_cleanup_type_source` gate moves the estimate by - /// exactly the context-prefix build plus one worst-case layered - /// element load between V3 and V4. - #[test] - fn test_worst_case_delete_tree_type_source_read_gated_v4_vs_v3() { - use grovedb_merk::{merk::NodeType, tree::kv::KV, tree_type::CostSize}; - use grovedb_version::version::v3::GROVE_V3; - - use crate::estimated_costs::worst_case_costs::WORST_CASE_FLAGS_LEN; - - let v3 = &GROVE_V3; - let v4 = GroveVersion::latest(); - assert!( - v4.grovedb_versions - .apply_batch - .delete_tree_cleanup_type_source - >= 1 - ); - - let ops = vec![QualifiedGroveDbOp::delete_tree_op( - vec![b"tree1".to_vec()], - b"key1".to_vec(), - TreeType::NormalTree, - SubelementsDeletionBehavior::Error, - )]; - let estimate = |grove_version| { - let mut paths = HashMap::new(); - paths.insert(KeyInfoPath(vec![]), MaxElementsNumber(1)); - paths.insert( - KeyInfoPath(vec![KeyInfo::KnownKey(b"tree1".to_vec())]), - MaxElementsNumber(1), - ); - GroveDb::estimated_case_operations_for_batch( - WorstCaseCostsType(paths), - ops.clone(), - None, - |_cost, _old_flags, _new_flags| Ok(false), - |_flags, _removed_key_bytes, _removed_value_bytes| { - Ok((NoStorageRemoval, NoStorageRemoval)) - }, - grove_version, - ) - .cost_as_result() - .expect("expected to estimate costs") - }; - - let est_v3 = estimate(v3); - let est_v4 = estimate(v4); - - let expected_loaded = KV::layered_value_byte_cost_size_for_key_and_value_lengths( - 4, - TreeType::NormalTree.cost_size() + WORST_CASE_FLAGS_LEN, - NodeType::NormalNode, - ) as u64; - assert_eq!(est_v4.seek_count, est_v3.seek_count + 1); - assert_eq!( - est_v4.storage_loaded_bytes, - est_v3.storage_loaded_bytes + expected_loaded - ); - assert_eq!(est_v4.storage_cost, est_v3.storage_cost); - // One blake3 block hashed to build the parent path's context prefix. - assert_eq!(est_v4.hash_node_calls, est_v3.hash_node_calls + 1); - } } diff --git a/grovedb/src/batch/indexed_tree/delete_tree.rs b/grovedb/src/batch/indexed_tree/delete_tree.rs index 6cb2f09dd..de99b263f 100644 --- a/grovedb/src/batch/indexed_tree/delete_tree.rs +++ b/grovedb/src/batch/indexed_tree/delete_tree.rs @@ -2,41 +2,30 @@ //! V4 gate. //! //! A batch `DeleteTree` op carries a caller-declared `TreeType`, which on -//! V1..V3 is taken at face value (reading the stored element costs an extra -//! seek + load per op, and cost feeds fees on the released versions). On V4+ -//! the stored element is read and its ACTUAL type selects the cleanup -//! namespaces — a declared type that hid a stored indexed primary would skip -//! the per-axis secondary sweep and leave authenticated stale rows. +//! V1..V3 is taken at face value. On V4+ the stored element's ACTUAL type +//! selects the cleanup namespaces — a declared type that hid a stored indexed +//! primary would skip the per-axis secondary sweep and leave authenticated +//! stale rows. The stored element comes from data already loaded on every +//! path that needs it (the emptiness pre-scan's own read, or the old value +//! the merk delete surfaces through the old-value observer), so V4 charges +//! exactly what V1..V3 charge. -use grovedb_costs::{cost_return_on_error, CostResult, CostsExt, OperationCost}; -use grovedb_merk::{ - element::{get::ElementFetchFromStorageExtensions, tree_type::ElementTreeTypeExtensions}, - TreeType, -}; -use grovedb_storage::StorageContext; -use grovedb_version::version::GroveVersion; +use grovedb_merk::{element::tree_type::ElementTreeTypeExtensions, TreeType}; use crate::{Element, Error}; /// Treat the tree type carried by `DeleteTree` as a checked claim, not as /// storage-ownership authority. Cleanup namespaces must be selected from the -/// element that is actually stored at the target key. -pub(crate) fn validate_delete_tree_type<'db, S: StorageContext<'db>>( - parent_storage: &S, - key: &[u8], +/// element that is actually stored at the target key — which the caller has +/// already loaded (this function performs no reads and charges no cost). +pub(crate) fn validate_delete_tree_type( + stored_element: &Element, declared_tree_type: TreeType, - grove_version: &GroveVersion, -) -> CostResult { - let mut cost = OperationCost::default(); - let stored_element = cost_return_on_error!( - &mut cost, - Element::get_from_storage(parent_storage, key, grove_version).map_err(Error::MerkError) - ); +) -> Result { let Some(actual_tree_type) = stored_element.tree_type() else { return Err(Error::InvalidBatchOperation( "DeleteTree target exists but is not a tree element", - )) - .wrap_with_cost(cost); + )); }; // Reject a mismatch only when an indexed tree is involved on either // side. The security property this guards is cleanup-namespace @@ -58,8 +47,7 @@ pub(crate) fn validate_delete_tree_type<'db, S: StorageContext<'db>>( { return Err(Error::InvalidBatchOperation( "DeleteTree declared tree type does not match the stored element", - )) - .wrap_with_cost(cost); + )); } - Ok(actual_tree_type).wrap_with_cost(cost) + Ok(actual_tree_type) } diff --git a/grovedb/src/batch/indexed_tree/mod.rs b/grovedb/src/batch/indexed_tree/mod.rs index 4d0965462..fdccd0929 100644 --- a/grovedb/src/batch/indexed_tree/mod.rs +++ b/grovedb/src/batch/indexed_tree/mod.rs @@ -13,17 +13,21 @@ //! in one batch is supported — the level executor opens the fresh primary //! and secondaries from the in-batch element, and the bubble-up emits //! `InsertAggregateIndexedTreeRootKeys`. -//! - [`overwrite`] runs inside the per-op loop when tree-override protection -//! is OFF and the op could overwrite an existing element (V4+ only — the -//! stored-element read [`inspect_cidx_overwrite`] starts with is gated by -//! `overwrite_indexed_cleanup_inspection` so released versions keep their -//! cost shape). Indexed → non-indexed and indexed → empty indexed are -//! allowed and schedule the old tree's storage for cleanup; indexed → -//! non-empty indexed is rejected as ambiguous. -//! - [`delete_tree`] holds the other V4-gated stored-element read: +//! - [`overwrite`] classifies overwrite-capable ops that displaced an +//! existing element (V4+ only, gated by +//! `overwrite_indexed_cleanup_inspection`). [`classify_cidx_overwrite`] +//! runs against the OLD element bytes the merk apply already fetched — +//! surfaced through the merk old-value observer — so it performs no +//! storage read of its own and V4 costs match V1..V3 exactly. Indexed → +//! non-indexed and indexed → empty indexed are allowed and schedule the +//! old tree's storage for cleanup; indexed → non-empty indexed is +//! rejected as ambiguous. +//! - [`delete_tree`] holds the other V4 gate's check: //! [`validate_delete_tree_type`] treats the tree type a `DeleteTree` op //! carries as a checked claim rather than storage-ownership authority, so -//! cleanup namespaces are selected from what is actually stored. +//! cleanup namespaces are selected from what is actually stored. It takes +//! the already-loaded stored element (from the emptiness pre-scan's own +//! read or the merk old-value observer) and performs no read itself. //! - [`pre_state`] runs against an indexed primary's level just before the //! merk apply: [`capture_indexed_pre_state`] validates the level's ops //! against the indexed-primary rules and reads each mutated key's *old* @@ -55,7 +59,7 @@ use grovedb_merk::{element::costs::ElementCostExtensions, Merk}; use grovedb_storage::StorageContext; use grovedb_version::version::GroveVersion; pub(crate) use mirror::{apply_indexed_secondary_mirror_post_apply, read_post_apply_transitions}; -pub(crate) use overwrite::inspect_cidx_overwrite; +pub(crate) use overwrite::classify_cidx_overwrite; pub(crate) use pre_state::capture_indexed_pre_state; pub(crate) use preflight::reject_indexed_overwrite_with_descendants; diff --git a/grovedb/src/batch/indexed_tree/overwrite.rs b/grovedb/src/batch/indexed_tree/overwrite.rs index 71737aab8..8a0a901f7 100644 --- a/grovedb/src/batch/indexed_tree/overwrite.rs +++ b/grovedb/src/batch/indexed_tree/overwrite.rs @@ -1,23 +1,18 @@ //! Classification of batch overwrites that target an existing indexed tree. //! -//! Runs inside the per-op loop when tree-override protection is OFF and the -//! op could overwrite an existing element — on V4+ only: the stored-element -//! read this starts with is gated by `overwrite_indexed_cleanup_inspection`, -//! so released versions keep their cost shape. +//! Runs on V4+ only (`overwrite_indexed_cleanup_inspection`), against the OLD +//! element bytes the merk apply already fetched: when a batch put lands on an +//! existing key, the tree walk loads that node to rewrite it, and the batch +//! layer receives its stored value through the merk old-value observer. The +//! classification therefore adds no storage read and no tracked cost — which +//! is also why V1..V3 (which skip the classification entirely) and V4 charge +//! identical costs for overwrite-capable ops. use std::collections::BTreeMap; -use grovedb_costs::{ - cost_return_on_error, cost_return_on_error_no_add, CostResult, CostsExt, OperationCost, -}; -use grovedb_merk::{element::costs::ElementCostExtensions, Merk}; -use grovedb_storage::StorageContext; use grovedb_version::version::GroveVersion; -use crate::{ - batch::{GroveOp, KeyInfo}, - Element, Error, -}; +use crate::{batch::GroveOp, Element, Error}; /// What "empty" means for each indexed variant offered as a replacement: /// `Some(true)` for an empty indexed element, `Some(false)` for a non-empty @@ -45,18 +40,26 @@ fn replacement_indexed_emptiness(new_element: &Element) -> Option { } } -/// Classify an `op_could_overwrite` insert at `path / key_info` against -/// the existing primary-merk entry, when tree-override protection is -/// **OFF** for this batch. Allows indexed-tree safe-subset overwrites and -/// rejects the ambiguous ones: +/// Classify an `op_could_overwrite` insert at `path / key` against the +/// element it displaced (`old_value` — the stored bytes surfaced by the merk +/// old-value observer), when the op reached the merk apply without +/// tree-override protection rejecting it. Allows indexed-tree safe-subset +/// overwrites and rejects the ambiguous ones: /// /// | existing | new | outcome | /// |------------------------|----------------------------|-------------------------------| -/// | none | * | `Ok(None)` | /// | non-indexed | * | `Ok(None)` | -/// | indexed | non-indexed | `Ok(Some(indexed_path))` | -/// | indexed | empty indexed | `Ok(Some(indexed_path))` | -/// | indexed | non-empty indexed | `Err(NotSupported)` | +/// | indexed | non-indexed | `Ok(Some(indexed_path))` | +/// | indexed | empty indexed | `Ok(Some(indexed_path))` | +/// | indexed | non-empty indexed | `Err(NotSupported)` | +/// +/// (The "no existing element" row of the old table cannot occur here: the +/// observer only fires for keys that exist, and a fresh insert never +/// classifies — exactly the case that used to pay a wasted read. The +/// `Err(NotSupported)` row is normally preempted too: the ungated +/// empty-at-batch-insertion guard in the op loop refuses any NON-EMPTY +/// indexed element before the merk apply runs, so the arm here is defense +/// in depth should that guard ever be relaxed.) /// /// When `Ok(Some(cidx_path))` is returned, the caller should push /// `cidx_path` onto its `cidx_overwrite_cleanup_paths` list so the @@ -77,43 +80,19 @@ fn replacement_indexed_emptiness(new_element: &Element) -> Option { /// (`verify_consistency_of_operations`) only blocks writes under /// `Delete` / `DeleteTree` paths; it does not know about safe-subset /// cidx-overwrite cleanup, so the descendant-check lives here. -pub(crate) fn inspect_cidx_overwrite<'db, S: StorageContext<'db>>( - primary_merk: &Merk, +pub(crate) fn classify_cidx_overwrite( + old_value: &[u8], path: &[Vec], - key_info: &KeyInfo, + key: &[u8], new_element: &Element, ops_by_qualified_paths: &BTreeMap>, GroveOp>, grove_version: &GroveVersion, -) -> CostResult>>, Error> { - let mut cost = OperationCost::default(); - - let maybe_existing = cost_return_on_error!( - &mut cost, - primary_merk - .get( - key_info.get_key_clone().as_slice(), - true, - Some(&Element::value_defined_cost_for_serialized_value), - grove_version, - ) - .map_err(|e| Error::CorruptedData(format!( - "unable to check for existing element: {e}" - ))) - ); - - let Some(existing_bytes) = maybe_existing else { - return Ok(None).wrap_with_cost(cost); - }; - - let existing_element = cost_return_on_error_no_add!( - cost, - Element::deserialize(existing_bytes.as_slice(), grove_version).map_err(|_| { - Error::CorruptedData("unable to deserialize existing element".to_string()) - }) - ); +) -> Result>>, Error> { + let existing_element = Element::deserialize(old_value, grove_version) + .map_err(|_| Error::CorruptedData("unable to deserialize existing element".to_string()))?; if !existing_element.is_indexed_tree() { - return Ok(None).wrap_with_cost(cost); + return Ok(None); } if matches!(replacement_indexed_emptiness(new_element), Some(false)) { @@ -124,15 +103,14 @@ pub(crate) fn inspect_cidx_overwrite<'db, S: StorageContext<'db>>( post-apply cleanup also clears it). DeleteTree the old indexed tree and re-create \ the new state in a follow-up batch" .to_string(), - )) - .wrap_with_cost(cost); + )); } // Safe subset: indexed → non-indexed OR indexed → empty indexed. // Schedule the OLD indexed tree's storage namespaces for cleanup. Its path is - // `path + key_info`. + // `path + key`. let mut cidx_path = path.to_vec(); - cidx_path.push(key_info.get_key_clone()); + cidx_path.push(key.to_vec()); // CONSISTENCY CHECK: writes UNDER the cidx's path in the same batch // would be silently lost when the post-apply cleanup clears the prefix. @@ -140,13 +118,10 @@ pub(crate) fn inspect_cidx_overwrite<'db, S: StorageContext<'db>>( // This loop is currently UNREACHABLE, and deliberately kept anyway. // The reason it cannot fire is self-cancelling: a descendant write makes // the deeper level bubble a `ReplaceTreeRootKey` into this key's slot - // before the shallower level runs, so by the time this function reads the - // existing element it no longer sees an indexed tree and returns above, - // never reaching here. In other words the very condition being checked - // for is what prevents the check from running. Verified by instrumenting - // both this loop and the function entry: for a PCIT overwritten with a - // plain tree alongside a write underneath it, the function is entered and - // returns early, and the loop is never reached. + // before the shallower level runs, so by the time this classification + // sees the displaced element it no longer is an indexed tree and the + // function returns above, never reaching here. In other words the very + // condition being checked for is what prevents the check from running. // // It stays because the unreachability is a property of the ORDER the // levels are processed in, not of this function — reorder the bubble-up, @@ -161,10 +136,9 @@ pub(crate) fn inspect_cidx_overwrite<'db, S: StorageContext<'db>>( safe-subset-overwritten in the same batch; the post-apply cleanup \ would silently clear the descendant write. Split into two batches: \ delete + recreate first, then populate.", - )) - .wrap_with_cost(cost); + )); } } - Ok(Some(cidx_path)).wrap_with_cost(cost) + Ok(Some(cidx_path)) } diff --git a/grovedb/src/batch/mod.rs b/grovedb/src/batch/mod.rs index 1467d1383..12a75936d 100644 --- a/grovedb/src/batch/mod.rs +++ b/grovedb/src/batch/mod.rs @@ -68,7 +68,8 @@ use grovedb_merk::{ value_hash, AggregateData, NULL_HASH, }, tree_type::{CostSize, TreeType, SUM_ITEM_COST_SIZE}, - CryptoHash, Error as MerkError, Merk, MerkType, Op, RootHashKeyAndAggregateData, + CryptoHash, Error as MerkError, Merk, MerkType, OldValueDisposition, Op, + RootHashKeyAndAggregateData, }; use grovedb_path::SubtreePath; use grovedb_storage::{ @@ -1452,6 +1453,12 @@ struct TreeCacheMerkByPath { /// ambiguous. See the cidx-overwrite handling in /// `execute_ops_on_path`. cidx_overwrite_cleanup_paths: Vec>>, + /// Qualified path → ACTUAL stored tree type of every `DeleteTree` target + /// this apply deleted, captured (V4+ only) from the old element bytes the + /// merk delete surfaces through the old-value observer. Consumed by + /// `apply_batch`'s post-apply phase to select cleanup namespaces from + /// what was really stored rather than what the op declared. + deleted_tree_actual_types: Vec<(Vec>, TreeType)>, } impl fmt::Debug for TreeCacheMerkByPath { @@ -1460,6 +1467,96 @@ impl fmt::Debug for TreeCacheMerkByPath { } } +/// V4 cleanup data collected while the batch body applied, handed from +/// `apply_batch_structure` back to the outer `apply_batch*` functions, which +/// run the corresponding post-apply storage cleanup passes. Both vecs are +/// empty on V1..V3. +#[derive(Default)] +struct BatchApplyCaptures { + /// Cidx primary paths displaced by a safe-subset overwrite; their old + /// primary subtree storage + per-axis secondary namespaces get cleared. + cidx_overwrite_cleanup_paths: Vec>>, + /// `(qualified_path, ACTUAL stored tree type)` of every `DeleteTree` + /// target that was really deleted; cleanup namespaces are selected from + /// the actual type, not the declared one. + deleted_tree_actual_types: Vec<(Vec>, TreeType)>, +} + +/// Result of the pre-apply `DeleteTree` scan shared by +/// `apply_batch_with_element_flags_update` and +/// `apply_partial_batch_with_element_flags_update`. +#[derive(Default)] +struct DeleteTreePreScan { + /// Deleted non-merk-tree paths whose data namespace gets cleared + /// post-apply. Filled pre-apply from DECLARED types on V1..V3 only; on + /// V4+ it starts empty and `classify_captured_delete_trees` fills it + /// from the captured ACTUAL types. + non_merk_delete_paths: Vec>>, + /// Deleted merk-tree paths for the recursive `find_subtrees` clear. + /// Same V1..V3 / V4+ split as above. + merk_delete_paths: Vec>>, + /// Deleted indexed-primary paths for the per-axis secondary sweep. + /// Same V1..V3 / V4+ split as above. + cidx_primary_delete_paths: Vec>>, + /// Paths whose `Skip`-behavior `DeleteTree` found a non-empty tree; + /// their ops are filtered out of the batch before `apply_body`. + skipped_delete_paths: HashSet>>, + /// V4+ only: qualified path → deletion behavior of every `DeleteTree` + /// op, so `classify_captured_delete_trees` can honor the behavior when + /// folding captured actual types into the cleanup lists. + delete_tree_behaviors: HashMap>, SubelementsDeletionBehavior>, +} + +/// V4+ post-apply classification: fold the `(qualified_path, ACTUAL stored +/// tree type)` pairs captured by the merk old-value observer into the +/// cleanup lists, honoring each op's deletion behavior. On V1..V3 the +/// captures are empty and this is a no-op — the lists were already built +/// pre-apply from the declared types, exactly as released. +fn classify_captured_delete_trees( + captures: Vec<(Vec>, TreeType)>, + behaviors: &HashMap>, SubelementsDeletionBehavior>, + non_merk_delete_paths: &mut Vec>>, + merk_delete_paths: &mut Vec>>, + cidx_primary_delete_paths: &mut Vec>>, +) { + for (qualified_path, actual_tree_type) in captures { + // Ops the pre-scan did not register (e.g. add-on DeleteTree ops + // returned by a partial batch's callback) keep their released + // no-cleanup behaviour. + let Some(behavior) = behaviors.get(&qualified_path) else { + continue; + }; + match behavior { + SubelementsDeletionBehavior::DontCheckWithNoCleanup => { + // No primary storage cleanup — but an indexed primary still + // needs its secondary namespaces cleared, because they live + // outside the primary's prefix and are invisible to + // find_subtrees. is_indexed_primary() (not + // is_count_indexed_primary): PSIT and PCPSIT must also queue + // for the all-axis sweep, which clears all three axis tags + // unconditionally and so is correct for every variant. + if actual_tree_type.is_indexed_primary() { + cidx_primary_delete_paths.push(qualified_path); + } + } + SubelementsDeletionBehavior::DeleteChildren + | SubelementsDeletionBehavior::Error + | SubelementsDeletionBehavior::Skip => { + if actual_tree_type.uses_non_merk_data_storage() { + non_merk_delete_paths.push(qualified_path); + } else { + // is_indexed_primary(): PSIT/PCPSIT primaries also need + // their path queued for the all-axis secondary sweep. + if actual_tree_type.is_indexed_primary() { + cidx_primary_delete_paths.push(qualified_path.clone()); + } + merk_delete_paths.push(qualified_path); + } + } + } + } +} + #[allow(dead_code)] // get_batch_run_mode is defined for future use trait TreeCache { fn insert( @@ -1508,6 +1605,15 @@ trait TreeCache { fn take_cidx_overwrite_cleanup_paths(&mut self) -> Vec>> { Vec::new() } + + /// After all level processing completes, `apply_batch` calls this to + /// retrieve the `(qualified_path, actual_tree_type)` pairs captured + /// (V4+ only) for the `DeleteTree` targets that were really deleted, so + /// the post-apply cleanup can classify namespaces by the ACTUAL stored + /// type. Default impl returns an empty Vec. + fn take_deleted_tree_actual_types(&mut self) -> Vec<(Vec>, TreeType)> { + Vec::new() + } } impl<'db, S, F, F2> TreeCacheMerkByPath @@ -2216,6 +2322,10 @@ where std::mem::take(&mut self.cidx_overwrite_cleanup_paths) } + fn take_deleted_tree_actual_types(&mut self) -> Vec<(Vec>, TreeType)> { + std::mem::take(&mut self.deleted_tree_actual_types) + } + fn update_base_merk_root_key( &mut self, root_key: Option>, @@ -2288,6 +2398,14 @@ where None }; + // V4 gates: keys whose ops need the OLD element they displace. The + // merk apply surfaces those bytes for free through the old-value + // observer below — the walker fetched the node anyway to rewrite or + // delete it — so no dedicated stored-element read (and no extra + // tracked cost) is issued. On V1..V3 both maps stay empty. + let mut pending_overwrite_inspections: BTreeMap, Element> = BTreeMap::new(); + let mut pending_delete_tree_checks: BTreeMap, TreeType> = BTreeMap::new(); + let mut batch_operations: Vec<(Vec, Op)> = vec![]; for (key_info, op) in ops_at_path_by_key.into_iter() { match op { @@ -2367,59 +2485,29 @@ where .wrap_with_cost(cost); } } - // NOTE: bare `Reference` overwrites are deliberately still - // excluded here. Including them (so a reference overwriting an - // indexed tree schedules the per-axis secondary cleanup) costs - // one extra stored-element read on EVERY reference overwrite, - // which measurably changes tracked cost — +1 seek and +79 - // storage_loaded_bytes on the repo's own refresh-reference cost - // tests. Cost feeds fees, and references over plain trees are - // shipped functionality on GROVE_V1/V2/V3, so paying that read - // unconditionally is a live-path behaviour change. - // - // The hole it would close requires an indexed tree to be the - // element being overwritten, which cannot occur on any released - // version (indexed trees are introduced by this PR). Closing it - // therefore belongs with the protocol version that activates - // indexed trees, gated so live versions keep today's cost. } else if op_could_overwrite - && !matches!(&element, Element::Reference(..)) && grove_version .grovedb_versions .apply_batch .overwrite_indexed_cleanup_inspection >= 1 { - // Tree-override protection is OFF; let the - // cidx helper classify the overwrite (safe - // subset → schedule cleanup, ambiguous → err, - // non-cidx → no-op). + // Register the key so the merk old-value observer can + // classify what this op displaces (safe subset → + // schedule cleanup, ambiguous → err, non-indexed → + // no-op). The observer only fires when the key + // actually exists, and the bytes it sees are the node + // the merk walk fetched anyway — so unlike the + // pre-V4 shape of this gate, no dedicated + // stored-element read is issued and V4 charges + // exactly the V1..V3 cost for every + // overwrite-capable op. // - // Gated exactly like `delete_tree_cleanup_type_source`: - // the classification starts with a stored-element read, - // which costs +1 seek and +129 loaded bytes per - // overwrite-capable op. Cost feeds fees, so V1..V3 must - // keep their released cost shape — and the hole this - // closes needs an indexed tree to be the element being - // overwritten, which cannot occur before the version - // that introduces indexed trees. The bare-Reference - // exclusion above is the same principle applied to the - // reference overwrite path. - let merk = self.merks.get(path).expect("the Merk is cached"); - let maybe_cleanup_path = cost_return_on_error!( - &mut cost, - indexed_tree::inspect_cidx_overwrite( - merk, - path, - &key_info, - &element, - ops_by_qualified_paths, - grove_version, - ) - ); - if let Some(cidx_path) = maybe_cleanup_path { - self.cidx_overwrite_cleanup_paths.push(cidx_path); - } + // Bare `Reference` overwrites are included: with the + // read gone there is no cost argument for leaving a + // reference that overwrites an indexed tree unswept. + pending_overwrite_inspections + .insert(key_info.get_key_clone(), element.clone()); } // Mirror the per-merk insert guard: wrapper children are @@ -3136,7 +3224,7 @@ where ) ); } - GroveOp::DeleteTree(_tree_type, _) => { + GroveOp::DeleteTree(tree_type, _) => { // CountIndexedTree owns two child Merks (primary + // secondary). The standard DeleteTree path runs // find_subtrees on the primary's prefix and clears @@ -3148,6 +3236,21 @@ where // tree_type and clears the secondary prefix there; // here we just emit the merk-level delete the same // way as for any other tree. + // + // On V4+ the declared type is a checked claim, not + // authority: register the key so the old-value observer + // can validate the declaration against the stored + // element the merk delete surfaces, and capture the + // ACTUAL type for the post-apply cleanup-namespace + // classification. + if grove_version + .grovedb_versions + .apply_batch + .delete_tree_cleanup_type_source + >= 1 + { + pending_delete_tree_checks.insert(key_info.get_key_clone(), tree_type); + } cost_return_on_error_into!( &mut cost, Element::delete_into_batch_operations( @@ -3442,9 +3545,66 @@ where let merk = self.merks.get_mut(path).expect("the Merk is cached"); + // V4 gate results collected by the old-value observer while the merk + // apply runs. The observer is infallible, so rejections are stashed + // here and returned right after the apply — nothing has been + // committed at that point (the level's writes only live in the + // pending storage batch, which the caller discards on error). + let mut old_value_gate_error: Option = None; + let mut cidx_overwrite_cleanups: Vec>> = vec![]; + let mut deleted_tree_captures: Vec<(Vec, TreeType)> = vec![]; + let mut old_value_observer = + |key: &[u8], old_value: &[u8], disposition: OldValueDisposition| { + if old_value_gate_error.is_some() { + return; + } + match disposition { + OldValueDisposition::Replaced => { + let Some(new_element) = pending_overwrite_inspections.get(key) else { + return; + }; + match indexed_tree::classify_cidx_overwrite( + old_value, + path, + key, + new_element, + ops_by_qualified_paths, + grove_version, + ) { + Ok(Some(cidx_path)) => cidx_overwrite_cleanups.push(cidx_path), + Ok(None) => {} + Err(e) => old_value_gate_error = Some(e), + } + } + OldValueDisposition::Deleted => { + let Some(declared_tree_type) = pending_delete_tree_checks.get(key) else { + return; + }; + let outcome = Element::deserialize(old_value, grove_version) + .map_err(|_| { + Error::CorruptedData( + "unable to deserialize deleted element".to_string(), + ) + }) + .and_then(|stored_element| { + indexed_tree::validate_delete_tree_type( + &stored_element, + *declared_tree_type, + ) + }); + match outcome { + Ok(actual_tree_type) => { + deleted_tree_captures.push((key.to_vec(), actual_tree_type)) + } + Err(e) => old_value_gate_error = Some(e), + } + } + } + }; + cost_return_on_error!( &mut cost, - merk.apply_unchecked::<_, Vec, _, _, _, _, _>( + merk.apply_unchecked_with_old_value_observer::<_, Vec, _, _, _, _, _, _>( &batch_operations, &[], Some(batch_apply_options.as_merk_options()), @@ -3585,11 +3745,28 @@ where } } }, + &mut old_value_observer, grove_version, ) .map_err(|e| Error::CorruptedData(e.to_string())) ); + // Surface any V4 gate rejection the observer recorded. The apply's + // cost has been charged (the batch fails as a whole, and none of its + // storage writes commit), which mirrors how every other mid-apply + // rejection behaves. + if let Some(gate_error) = old_value_gate_error { + return Err(gate_error).wrap_with_cost(cost); + } + self.cidx_overwrite_cleanup_paths + .extend(cidx_overwrite_cleanups); + for (key, actual_tree_type) in deleted_tree_captures { + let mut qualified_path = path.to_vec(); + qualified_path.push(key); + self.deleted_tree_actual_types + .push((qualified_path, actual_tree_type)); + } + // Post-apply: if this level was a cidx primary, mirror each // mutation to the secondary and capture the secondary's // post-mirror state into `cidx_secondary_after_apply` so the @@ -3663,19 +3840,23 @@ impl GroveDb { /// are returned /// Runs the level-by-level batch propagation. /// - /// Returns `(leftover_ops, cidx_overwrite_cleanup_paths)`: + /// Returns `(leftover_ops, captures)`: /// - `leftover_ops` is `Some(...)` only if a `batch_pause_height` /// was set and pruning paused before reaching the root. - /// - `cidx_overwrite_cleanup_paths` is the list of cidx primary - /// paths whose old storage (primary subtree + secondary - /// namespace) must be cleaned up post-apply because of a safe- - /// subset cidx-overwrite (see `execute_ops_on_path`). Empty when - /// no such overwrites occurred. + /// - `captures` is the [`BatchApplyCaptures`] collected while the + /// body applied, for the caller's post-apply cleanup passes: + /// `cidx_overwrite_cleanup_paths` lists the cidx primary paths + /// whose old storage (primary subtree + secondary namespaces) + /// must be cleared because of a safe-subset cidx-overwrite (see + /// `execute_ops_on_path`), and `deleted_tree_actual_types` maps + /// each really-deleted `DeleteTree` target to its ACTUAL stored + /// tree type so cleanup namespaces follow the truth rather than + /// the op's declaration. Both are empty on V1..V3. fn apply_batch_structure, F, SR>( batch_structure: BatchStructure, batch_apply_options: Option, grove_version: &GroveVersion, - ) -> CostResult<(Option, Vec>>), Error> + ) -> CostResult<(Option, BatchApplyCaptures), Error> where F: FnMut(&StorageCost, Option, &mut ElementFlags) -> Result, SR: FnMut( @@ -4216,15 +4397,20 @@ impl GroveDb { } if current_level == stop_level { // we need to pause the batch execution - let cidx_overwrite_cleanup_paths = - merk_tree_cache.take_cidx_overwrite_cleanup_paths(); - return Ok((Some(ops_by_level_paths), cidx_overwrite_cleanup_paths)) - .wrap_with_cost(cost); + let captures = BatchApplyCaptures { + cidx_overwrite_cleanup_paths: merk_tree_cache + .take_cidx_overwrite_cleanup_paths(), + deleted_tree_actual_types: merk_tree_cache.take_deleted_tree_actual_types(), + }; + return Ok((Some(ops_by_level_paths), captures)).wrap_with_cost(cost); } current_level = current_level.saturating_sub(1); } - let cidx_overwrite_cleanup_paths = merk_tree_cache.take_cidx_overwrite_cleanup_paths(); - Ok((None, cidx_overwrite_cleanup_paths)).wrap_with_cost(cost) + let captures = BatchApplyCaptures { + cidx_overwrite_cleanup_paths: merk_tree_cache.take_cidx_overwrite_cleanup_paths(), + deleted_tree_actual_types: merk_tree_cache.take_deleted_tree_actual_types(), + }; + Ok((None, captures)).wrap_with_cost(cost) } /// Method to propagate updated subtree root hashes up to GroveDB root @@ -4256,7 +4442,7 @@ impl GroveDb { Error, >, grove_version: &GroveVersion, - ) -> CostResult<(Option, Vec>>), Error> { + ) -> CostResult<(Option, BatchApplyCaptures), Error> { check_grovedb_v0_with_cost!( "apply_body", grove_version.grovedb_versions.apply_batch.apply_body @@ -4274,6 +4460,7 @@ impl GroveDb { get_secondary_merks_fn, indexed_secondary_after_apply: Default::default(), cidx_overwrite_cleanup_paths: Default::default(), + deleted_tree_actual_types: Default::default(), } ) ); @@ -4311,7 +4498,7 @@ impl GroveDb { Error, >, grove_version: &GroveVersion, - ) -> CostResult<(Option, Vec>>), Error> { + ) -> CostResult<(Option, BatchApplyCaptures), Error> { check_grovedb_v0_with_cost!( "continue_partial_apply_body", grove_version @@ -4333,6 +4520,7 @@ impl GroveDb { get_secondary_merks_fn, indexed_secondary_after_apply: Default::default(), cidx_overwrite_cleanup_paths: Default::default(), + deleted_tree_actual_types: Default::default(), } ) ); @@ -4795,127 +4983,85 @@ impl GroveDb { } } - /// Applies batch of operations on GroveDB - pub fn apply_batch_with_element_flags_update( - &self, - ops: Vec, - batch_apply_options: Option, - update_element_flags_function: impl FnMut( - &StorageCost, - Option, - &mut ElementFlags, - ) -> Result, - split_removal_bytes_function: impl FnMut( - &mut ElementFlags, - u32, // key removed bytes - u32, // value removed bytes - ) -> Result< - (StorageRemovedBytes, StorageRemovedBytes), - Error, - >, - transaction: TransactionArg, + /// Like [`open_batch_transactional_merk_at_path`] + /// (Self::open_batch_transactional_merk_at_path) with `new_merk: false`, + /// for a caller that has ALREADY read (and paid for) the parent element + /// at `path`: the open's own parent fetch is skipped so the read is + /// charged exactly once. Only valid for non-root paths. + fn open_batch_transactional_merk_with_parent_element<'db, B: AsRef<[u8]>>( + &'db self, + storage_batch: &'db StorageBatch, + path: SubtreePath, + tx: &'db Transaction, + parent_element: Element, grove_version: &GroveVersion, - ) -> CostResult<(), Error> { - check_grovedb_v0_with_cost!( - "apply_batch_with_element_flags_update", - grove_version - .grovedb_versions - .apply_batch - .apply_batch_with_element_flags_update - ); + ) -> CostResult>, Error> { let mut cost = OperationCost::default(); - let tx = TxRef::new(&self.db, transaction); - - if ops.is_empty() { - return Ok(()).wrap_with_cost(cost); - } - - // Check batch operation consistency BEFORE preprocessing so that - // conflicting ops (e.g., CommitmentTreeInsert + Delete on the same - // path/key) are caught before any work is done. - let check_batch_operation_consistency = batch_apply_options - .as_ref() - .map(|batch_options| !batch_options.disable_operation_consistency_check) - .unwrap_or(true); - - if check_batch_operation_consistency { - let consistency_result = QualifiedGroveDbOp::verify_consistency_of_operations(&ops); - if !consistency_result.is_empty() { - return Err(Error::InvalidBatchOperation( - "batch operations fail consistency checks", - )) - .wrap_with_cost(cost); - } + if path.derive_parent().is_none() { + return Err(Error::CorruptedCodeExecution( + "open_batch_transactional_merk_with_parent_element requires a non-root path", + )) + .wrap_with_cost(cost); } - - cost_return_on_error!( - &mut cost, - indexed_tree::reject_indexed_overwrite_with_descendants( - self, - &ops, - tx.as_ref(), + let storage = self + .db + .get_transactional_storage_context(path, Some(storage_batch), tx) + .unwrap_add_cost(&mut cost); + if let Some((root_key, tree_type)) = parent_element.root_key_and_tree_type_owned() { + Merk::open_layered_with_root_key( + storage, + root_key, + tree_type, + Some(&Element::value_defined_cost_for_serialized_value), grove_version, ) - ); - - // `StorageBatch` collects all operations (preprocessing + apply_body) - // for a single atomic commit at the end. - let storage_batch = StorageBatch::new(); - - // Preprocess CommitmentTreeInsert ops: execute Sinsemilla operations - // then convert to ReplaceTreeRootKey ops - let ops = cost_return_on_error!( - &mut cost, - self.preprocess_commitment_tree_ops(ops, tx.as_ref(), &storage_batch, grove_version) - ); - - // Preprocess MmrTreeAppend ops: execute MMR operations - // then convert to ReplaceTreeRootKey ops - let ops = cost_return_on_error!( - &mut cost, - self.preprocess_mmr_tree_ops(ops, tx.as_ref(), &storage_batch, grove_version) - ); - - // Preprocess BulkAppend ops: execute bulk append operations - // then convert to ReplaceTreeRootKey ops - let ops = cost_return_on_error!( - &mut cost, - self.preprocess_bulk_append_ops(ops, tx.as_ref(), &storage_batch, grove_version) - ); - - // Preprocess DenseTreeInsert ops: execute dense tree operations - // then convert to ReplaceTreeRootKey ops - let ops = cost_return_on_error!( - &mut cost, - self.preprocess_dense_tree_ops(ops, tx.as_ref(), &storage_batch, grove_version) - ); - - // Collect paths of subtrees being deleted, separated by type. - // - // Non-Merk trees (MmrTree, BulkAppendTree, DenseTree, CommitmentTree) - // store their data in the data storage namespace of their subtree path - // (e.g. MMR nodes, buffer entries, dense tree values). When apply_body - // processes a DeleteTree op it removes the Element from the parent Merk - // and clears the subtree's Merk metadata, but does NOT clear the raw - // data these tree types wrote. Without explicit cleanup, deleting a - // non-Merk tree would leave orphaned data that could corrupt a new tree - // inserted at the same path. - // - // Standard Merk trees also need cleanup: when a Merk subtree has child - // subtrees, deleting the parent key in the parent Merk does NOT clear - // the child subtree's storage. We must recursively find and clear all - // nested subtrees. - let mut non_merk_delete_paths: Vec>> = Vec::new(); - let mut merk_delete_paths: Vec>> = Vec::new(); - // CountIndexedTree primary deletes: alongside the primary's - // recursive cleanup (via merk_delete_paths), the secondary's - // storage namespace at Blake3(primary_prefix ‖ 0x01) must be - // cleared explicitly — find_subtrees only walks primary keys. - let mut cidx_primary_delete_paths: Vec>> = Vec::new(); - // Track paths skipped due to SubelementsDeletionBehavior::Skip so we can - // filter the corresponding ops out of the batch before apply_body. - let mut skipped_delete_paths: HashSet>> = HashSet::new(); + .map_err(|e| { + Error::CorruptedData(format!("cannot open a subtree with given root key: {e}")) + }) + .add_cost(cost) + } else { + Err(Error::CorruptedPath( + "cannot open a subtree as parent exists but is not a tree".to_string(), + )) + .wrap_with_cost(cost) + } + } + /// Pre-apply scan over a batch's `DeleteTree` ops, shared by + /// `apply_batch_with_element_flags_update` and + /// `apply_partial_batch_with_element_flags_update`. + /// + /// On V1..V3 this is the released behaviour verbatim: the DECLARED tree + /// type is taken at face value, driving both the emptiness checks and + /// the pre-apply classification of cleanup paths. + /// + /// On V4+ (`delete_tree_cleanup_type_source >= 1`) cleanup namespaces + /// must follow the ACTUAL stored type instead — but reading the stored + /// element here would add a charged read per op, which is exactly what + /// this gate used to cost. So classification moves to after + /// `apply_body`, driven by the old element bytes the merk delete + /// surfaces for free through the old-value observer (which also rejects + /// declared/stored mismatches involving an indexed tree). The only work + /// left here is what must happen before the apply: the Error/Skip + /// emptiness checks. Those already read the stored element on V1..V3 — + /// directly for declared non-merk types, or inside the child-merk open + /// for merk types — so V4 does that single read up front, derives the + /// ACTUAL type from it, and hands the element to the open. Same read + /// count, same bytes: V4 charges exactly the released cost. + fn scan_delete_tree_ops<'db>( + &'db self, + ops: &[QualifiedGroveDbOp], + storage_batch: &'db StorageBatch, + tx: &'db Transaction, + grove_version: &GroveVersion, + ) -> CostResult { + let mut cost = OperationCost::default(); + let mut scan = DeleteTreePreScan::default(); + let capture_actual_types = grove_version + .grovedb_versions + .apply_batch + .delete_tree_cleanup_type_source + >= 1; for op in ops.iter() { if let GroveOp::DeleteTree(tree_type, subelements_deletion_behavior) = &op.op && let Some(key) = op.key.as_ref() @@ -4923,41 +5069,138 @@ impl GroveDb { let mut child_path = op.path.to_path(); child_path.push(key.as_slice().to_vec()); - let parent_path_vec = op.path.to_path(); - let parent_path: SubtreePath> = parent_path_vec.as_slice().into(); - // Gated: reading the stored element to pick cleanup namespaces - // costs an extra seek + load per DeleteTree op, so it cannot - // apply to the released versions. V1..V3 keep taking the - // caller-declared type at face value; V4+ reads the truth. - let resolved_tree_type = if grove_version - .grovedb_versions - .apply_batch - .delete_tree_cleanup_type_source - >= 1 - { - let parent_storage = self - .db - .get_transactional_storage_context( - parent_path, - Some(&storage_batch), - tx.as_ref(), - ) - .unwrap_add_cost(&mut cost); - cost_return_on_error!( - &mut cost, - indexed_tree::validate_delete_tree_type( - &parent_storage, - key.as_slice(), - *tree_type, - grove_version, - ) - ) - } else { - *tree_type - }; - let tree_type = &resolved_tree_type; + if capture_actual_types { + scan.delete_tree_behaviors + .insert(child_path.clone(), *subelements_deletion_behavior); + match subelements_deletion_behavior { + SubelementsDeletionBehavior::DontCheckWithNoCleanup + | SubelementsDeletionBehavior::DeleteChildren => { + // Nothing to check pre-apply; cleanup paths come + // from the captured actual types after apply. + } + SubelementsDeletionBehavior::Error | SubelementsDeletionBehavior::Skip => { + let parent_path_vec = op.path.to_path(); + let parent_path: SubtreePath> = + parent_path_vec.as_slice().into(); + let parent_storage = self + .db + .get_transactional_storage_context( + parent_path, + Some(storage_batch), + tx, + ) + .unwrap_add_cost(&mut cost); + let stored_element = cost_return_on_error!( + &mut cost, + Element::get_from_storage( + &parent_storage, + key.as_slice(), + grove_version, + ) + .map_err(|e| { + Error::CorruptedData(format!( + "unable to get element for delete tree emptiness \ + check: {e}" + )) + }) + ); + let actual_tree_type = cost_return_on_error_no_add!( + cost, + indexed_tree::validate_delete_tree_type( + &stored_element, + *tree_type + ) + ); + let is_empty = if actual_tree_type.uses_non_merk_data_storage() { + // Non-Merk trees: element-level entry count, + // read off the element loaded above. + stored_element.non_merk_entry_count().unwrap_or(0) == 0 + } else { + // Standard Merk trees: use is_empty_tree_except + // to account for other delete ops in the same + // batch. + // + // Exclude DeleteTree ops with Skip policy — + // those might not execute if their target is + // non-empty, so we cannot assume they will + // delete their key. + let batch_deleted_keys = ops + .iter() + .filter_map(|other_op| match &other_op.op { + GroveOp::Delete => { + if other_op.path.to_path() == child_path { + Some(other_op.key.as_ref()?.as_slice().to_vec()) + } else { + None + } + } + GroveOp::DeleteTree( + _, + SubelementsDeletionBehavior::Skip, + ) => None, + GroveOp::DeleteTree(..) => { + if other_op.path.to_path() == child_path { + Some(other_op.key.as_ref()?.as_slice().to_vec()) + } else { + None + } + } + _ => None, + }) + .collect::>>(); + let batch_deleted_keys_refs: std::collections::BTreeSet<&[u8]> = + batch_deleted_keys.iter().map(|k| k.as_slice()).collect(); + + let child_merk = cost_return_on_error!( + &mut cost, + self.open_batch_transactional_merk_with_parent_element( + storage_batch, + child_path.as_slice().into(), + tx, + stored_element, + grove_version, + ) + ); + + child_merk + .is_empty_tree_except(batch_deleted_keys_refs) + .unwrap_add_cost(&mut cost) + }; + + if !is_empty { + match subelements_deletion_behavior { + SubelementsDeletionBehavior::Error => { + return Err(Error::DeletingNonEmptyTree( + "trying to do a batch delete operation for a non \ + empty tree, but options not allowing this", + )) + .wrap_with_cost(cost); + } + SubelementsDeletionBehavior::Skip => { + scan.skipped_delete_paths.insert(child_path); + } + SubelementsDeletionBehavior::DontCheckWithNoCleanup + | SubelementsDeletionBehavior::DeleteChildren => { + return Err(Error::CorruptedCodeExecution( + "batch delete: DontCheckWithNoCleanup / \ + DeleteChildren behaviors are handled before the \ + non-empty-tree check and must not reach this \ + match arm", + )) + .wrap_with_cost(cost); + } + } + } + } + } + continue; + } - // Per-op emptiness check based on the SubelementsDeletionBehavior policy. + // V1..V3: released behaviour, byte for byte — the declared + // tree type is taken at face value. + // + // Per-op emptiness check based on the + // SubelementsDeletionBehavior policy. match subelements_deletion_behavior { SubelementsDeletionBehavior::DontCheckWithNoCleanup => { // No emptiness check and no post-apply storage cleanup. @@ -4973,7 +5216,7 @@ impl GroveDb { // the DeleteTree. The sweep clears all three axis tags // unconditionally, so this is correct for every variant. if tree_type.is_indexed_primary() { - cidx_primary_delete_paths.push(child_path); + scan.cidx_primary_delete_paths.push(child_path); } continue; } @@ -4991,8 +5234,8 @@ impl GroveDb { .db .get_transactional_storage_context( parent_path, - Some(&storage_batch), - tx.as_ref(), + Some(storage_batch), + tx, ) .unwrap_add_cost(&mut cost); let element = cost_return_on_error!( @@ -5046,9 +5289,9 @@ impl GroveDb { let child_merk = cost_return_on_error!( &mut cost, self.open_batch_transactional_merk_at_path( - &storage_batch, + storage_batch, child_path.as_slice().into(), - tx.as_ref(), + tx, false, grove_version, ) @@ -5069,7 +5312,7 @@ impl GroveDb { .wrap_with_cost(cost); } SubelementsDeletionBehavior::Skip => { - skipped_delete_paths.insert(child_path); + scan.skipped_delete_paths.insert(child_path); continue; } // DontCheckWithNoCleanup / DeleteChildren never @@ -5093,17 +5336,131 @@ impl GroveDb { } if tree_type.uses_non_merk_data_storage() { - non_merk_delete_paths.push(child_path); + scan.non_merk_delete_paths.push(child_path); } else { // is_indexed_primary(): PSIT/PCPSIT primaries also need // their path queued for the all-axis secondary sweep. if tree_type.is_indexed_primary() { - cidx_primary_delete_paths.push(child_path.clone()); + scan.cidx_primary_delete_paths.push(child_path.clone()); } - merk_delete_paths.push(child_path); + scan.merk_delete_paths.push(child_path); } } } + Ok(scan).wrap_with_cost(cost) + } + + /// Applies batch of operations on GroveDB + pub fn apply_batch_with_element_flags_update( + &self, + ops: Vec, + batch_apply_options: Option, + update_element_flags_function: impl FnMut( + &StorageCost, + Option, + &mut ElementFlags, + ) -> Result, + split_removal_bytes_function: impl FnMut( + &mut ElementFlags, + u32, // key removed bytes + u32, // value removed bytes + ) -> Result< + (StorageRemovedBytes, StorageRemovedBytes), + Error, + >, + transaction: TransactionArg, + grove_version: &GroveVersion, + ) -> CostResult<(), Error> { + check_grovedb_v0_with_cost!( + "apply_batch_with_element_flags_update", + grove_version + .grovedb_versions + .apply_batch + .apply_batch_with_element_flags_update + ); + let mut cost = OperationCost::default(); + let tx = TxRef::new(&self.db, transaction); + + if ops.is_empty() { + return Ok(()).wrap_with_cost(cost); + } + + // Check batch operation consistency BEFORE preprocessing so that + // conflicting ops (e.g., CommitmentTreeInsert + Delete on the same + // path/key) are caught before any work is done. + let check_batch_operation_consistency = batch_apply_options + .as_ref() + .map(|batch_options| !batch_options.disable_operation_consistency_check) + .unwrap_or(true); + + if check_batch_operation_consistency { + let consistency_result = QualifiedGroveDbOp::verify_consistency_of_operations(&ops); + if !consistency_result.is_empty() { + return Err(Error::InvalidBatchOperation( + "batch operations fail consistency checks", + )) + .wrap_with_cost(cost); + } + } + + cost_return_on_error!( + &mut cost, + indexed_tree::reject_indexed_overwrite_with_descendants( + self, + &ops, + tx.as_ref(), + grove_version, + ) + ); + + // `StorageBatch` collects all operations (preprocessing + apply_body) + // for a single atomic commit at the end. + let storage_batch = StorageBatch::new(); + + // Preprocess CommitmentTreeInsert ops: execute Sinsemilla operations + // then convert to ReplaceTreeRootKey ops + let ops = cost_return_on_error!( + &mut cost, + self.preprocess_commitment_tree_ops(ops, tx.as_ref(), &storage_batch, grove_version) + ); + + // Preprocess MmrTreeAppend ops: execute MMR operations + // then convert to ReplaceTreeRootKey ops + let ops = cost_return_on_error!( + &mut cost, + self.preprocess_mmr_tree_ops(ops, tx.as_ref(), &storage_batch, grove_version) + ); + + // Preprocess BulkAppend ops: execute bulk append operations + // then convert to ReplaceTreeRootKey ops + let ops = cost_return_on_error!( + &mut cost, + self.preprocess_bulk_append_ops(ops, tx.as_ref(), &storage_batch, grove_version) + ); + + // Preprocess DenseTreeInsert ops: execute dense tree operations + // then convert to ReplaceTreeRootKey ops + let ops = cost_return_on_error!( + &mut cost, + self.preprocess_dense_tree_ops(ops, tx.as_ref(), &storage_batch, grove_version) + ); + + // Collect paths of subtrees being deleted (so their storage can be + // cleaned up after apply_body) and run the pre-apply emptiness + // checks / Skip filtering. On V1..V3 the cleanup lists are filled + // here from the DECLARED tree types; on V4+ they stay empty and are + // filled after apply_body from the ACTUAL stored types captured by + // the merk old-value observer. See `scan_delete_tree_ops`. + let DeleteTreePreScan { + mut non_merk_delete_paths, + mut merk_delete_paths, + mut cidx_primary_delete_paths, + skipped_delete_paths, + delete_tree_behaviors, + } = cost_return_on_error!( + &mut cost, + self.scan_delete_tree_ops(&ops, &storage_batch, tx.as_ref(), grove_version) + ); // Filter out DeleteTree ops that were skipped due to // SubelementsDeletionBehavior::Skip on non-empty trees. @@ -5134,7 +5491,7 @@ impl GroveDb { // 5. Remove operation from the tree, repeat until there are operations to do; // 6. Add root leaves save operation to the batch // 7. Apply storage_cost batch - let (_leftover, cidx_overwrite_cleanup_paths) = cost_return_on_error!( + let (_leftover, batch_apply_captures) = cost_return_on_error!( &mut cost, self.apply_body( ops, @@ -5166,6 +5523,23 @@ impl GroveDb { ) ); + let BatchApplyCaptures { + cidx_overwrite_cleanup_paths, + deleted_tree_actual_types, + } = batch_apply_captures; + + // V4+: fold the `(path, ACTUAL stored type)` pairs captured during + // the apply into the cleanup lists (no-op on V1..V3, where the + // captures are empty and the lists were already built pre-apply from + // the declared types). + classify_captured_delete_trees( + deleted_tree_actual_types, + &delete_tree_behaviors, + &mut non_merk_delete_paths, + &mut merk_delete_paths, + &mut cidx_primary_delete_paths, + ); + // Clean up data storage for deleted non-Merk trees. for child_path in &non_merk_delete_paths { let child_subtree_path: SubtreePath> = child_path.as_slice().into(); @@ -5465,194 +5839,24 @@ impl GroveDb { self.preprocess_dense_tree_ops(ops, tx.as_ref(), &storage_batch, grove_version) ); - // See comment in apply_batch_with_element_flags_update for why - // deleted tree subtrees need explicit storage cleanup, and why - // emptiness checks are needed (H2). - let mut non_merk_delete_paths: Vec>> = Vec::new(); - let mut merk_delete_paths: Vec>> = Vec::new(); - let mut cidx_primary_delete_paths: Vec>> = Vec::new(); - let mut batch_apply_options = batch_apply_options.unwrap_or_default(); - let mut skipped_delete_paths: HashSet>> = HashSet::new(); - - for op in ops.iter() { - if let GroveOp::DeleteTree(tree_type, subelements_deletion_behavior) = &op.op - && let Some(key) = op.key.as_ref() - { - let mut child_path = op.path.to_path(); - child_path.push(key.as_slice().to_vec()); - - let parent_path_vec = op.path.to_path(); - let parent_path: SubtreePath> = parent_path_vec.as_slice().into(); - // Gated: reading the stored element to pick cleanup namespaces - // costs an extra seek + load per DeleteTree op, so it cannot - // apply to the released versions. V1..V3 keep taking the - // caller-declared type at face value; V4+ reads the truth. - let resolved_tree_type = if grove_version - .grovedb_versions - .apply_batch - .delete_tree_cleanup_type_source - >= 1 - { - let parent_storage = self - .db - .get_transactional_storage_context( - parent_path, - Some(&storage_batch), - tx.as_ref(), - ) - .unwrap_add_cost(&mut cost); - cost_return_on_error!( - &mut cost, - indexed_tree::validate_delete_tree_type( - &parent_storage, - key.as_slice(), - *tree_type, - grove_version, - ) - ) - } else { - *tree_type - }; - let tree_type = &resolved_tree_type; - match subelements_deletion_behavior { - SubelementsDeletionBehavior::DontCheckWithNoCleanup => { - // No emptiness check and no post-apply storage cleanup. - // The caller guarantees the subtree is already empty. - // is_indexed_primary() (not is_count_indexed_primary): - // PSIT and PCPSIT DeleteTree ops must also queue their - // primary path for the all-axis secondary sweep below, - // otherwise their sum/avg secondary namespaces survive - // the DeleteTree. The sweep clears all three axis tags - // unconditionally, so this is correct for every variant. - if tree_type.is_indexed_primary() { - cidx_primary_delete_paths.push(child_path); - } - continue; - } - SubelementsDeletionBehavior::DeleteChildren => { - // No emptiness check, but still perform post-apply - // storage cleanup to remove child subtree storage. - } - SubelementsDeletionBehavior::Error | SubelementsDeletionBehavior::Skip => { - let is_empty = if tree_type.uses_non_merk_data_storage() { - let parent_path_vec = op.path.to_path(); - let parent_path: SubtreePath> = - parent_path_vec.as_slice().into(); - let parent_storage = self - .db - .get_transactional_storage_context( - parent_path, - Some(&storage_batch), - tx.as_ref(), - ) - .unwrap_add_cost(&mut cost); - let element = cost_return_on_error!( - &mut cost, - Element::get_from_storage( - &parent_storage, - key.as_slice(), - grove_version, - ) - .map_err(|e| { - Error::CorruptedData(format!( - "unable to get element for delete tree emptiness \ - check: {e}" - )) - }) - ); - element.non_merk_entry_count().unwrap_or(0) == 0 - } else { - // Exclude DeleteTree ops with Skip policy — those - // might not execute if their target is non-empty. - let batch_deleted_keys = ops - .iter() - .filter_map(|other_op| match &other_op.op { - GroveOp::Delete => { - if other_op.path.to_path() == child_path { - Some(other_op.key.as_ref()?.as_slice().to_vec()) - } else { - None - } - } - GroveOp::DeleteTree(_, SubelementsDeletionBehavior::Skip) => { - None - } - GroveOp::DeleteTree(..) => { - if other_op.path.to_path() == child_path { - Some(other_op.key.as_ref()?.as_slice().to_vec()) - } else { - None - } - } - _ => None, - }) - .collect::>>(); - let batch_deleted_keys_refs: std::collections::BTreeSet<&[u8]> = - batch_deleted_keys.iter().map(|k| k.as_slice()).collect(); - - let child_merk = cost_return_on_error!( - &mut cost, - self.open_batch_transactional_merk_at_path( - &storage_batch, - child_path.as_slice().into(), - tx.as_ref(), - false, - grove_version, - ) - ); - - child_merk - .is_empty_tree_except(batch_deleted_keys_refs) - .unwrap_add_cost(&mut cost) - }; - - if !is_empty { - match subelements_deletion_behavior { - SubelementsDeletionBehavior::Error => { - return Err(Error::DeletingNonEmptyTree( - "trying to do a batch delete operation for a non \ - empty tree, but options not allowing this", - )) - .wrap_with_cost(cost); - } - SubelementsDeletionBehavior::Skip => { - skipped_delete_paths.insert(child_path); - continue; - } - // DontCheckWithNoCleanup / DeleteChildren never - // reach the emptiness-check block above (they - // either skip the check or delete children - // unconditionally). Return a graceful error - // rather than panicking if that invariant is - // ever broken. - SubelementsDeletionBehavior::DontCheckWithNoCleanup - | SubelementsDeletionBehavior::DeleteChildren => { - return Err(Error::CorruptedCodeExecution( - "batch delete: DontCheckWithNoCleanup / DeleteChildren \ - behaviors are handled before the non-empty-tree check \ - and must not reach this match arm", - )) - .wrap_with_cost(cost); - } - } - } - } - } - - if tree_type.uses_non_merk_data_storage() { - non_merk_delete_paths.push(child_path); - } else { - // is_indexed_primary(): PSIT/PCPSIT primaries also need - // their path queued for the all-axis secondary sweep. - if tree_type.is_indexed_primary() { - cidx_primary_delete_paths.push(child_path.clone()); - } - merk_delete_paths.push(child_path); - } - } - } + // Collect paths of subtrees being deleted (so their storage can be + // cleaned up after apply_body) and run the pre-apply emptiness + // checks / Skip filtering. On V1..V3 the cleanup lists are filled + // here from the DECLARED tree types; on V4+ they stay empty and are + // filled after apply_body from the ACTUAL stored types captured by + // the merk old-value observer. See `scan_delete_tree_ops`. + let DeleteTreePreScan { + mut non_merk_delete_paths, + mut merk_delete_paths, + mut cidx_primary_delete_paths, + skipped_delete_paths, + delete_tree_behaviors, + } = cost_return_on_error!( + &mut cost, + self.scan_delete_tree_ops(&ops, &storage_batch, tx.as_ref(), grove_version) + ); // Filter out DeleteTree ops that were skipped due to // SubelementsDeletionBehavior::Skip on non-empty trees. @@ -5687,7 +5891,7 @@ impl GroveDb { // 5. Remove operation from the tree, repeat until there are operations to do; // 6. Add root leaves save operation to the batch // 7. Apply storage_cost batch - let (left_over_operations, partial_cidx_overwrite_cleanup_paths) = cost_return_on_error!( + let (left_over_operations, partial_captures) = cost_return_on_error!( &mut cost, self.apply_body( ops, @@ -5772,7 +5976,7 @@ impl GroveDb { let continue_storage_batch = StorageBatch::new(); - let (_leftover_unused, continue_cidx_overwrite_cleanup_paths) = cost_return_on_error!( + let (_leftover_unused, continue_captures) = cost_return_on_error!( &mut cost, self.continue_partial_apply_body( left_over_operations, @@ -5805,6 +6009,28 @@ impl GroveDb { ) ); + let BatchApplyCaptures { + cidx_overwrite_cleanup_paths: partial_cidx_overwrite_cleanup_paths, + deleted_tree_actual_types: partial_deleted_tree_actual_types, + } = partial_captures; + let BatchApplyCaptures { + cidx_overwrite_cleanup_paths: continue_cidx_overwrite_cleanup_paths, + deleted_tree_actual_types: continue_deleted_tree_actual_types, + } = continue_captures; + + // V4+: fold captures from BOTH applies into the cleanup lists + // (no-op on V1..V3). The overwrite-cleanup paths are unioned below. + classify_captured_delete_trees( + partial_deleted_tree_actual_types + .into_iter() + .chain(continue_deleted_tree_actual_types) + .collect(), + &delete_tree_behaviors, + &mut non_merk_delete_paths, + &mut merk_delete_paths, + &mut cidx_primary_delete_paths, + ); + // Clean up data storage for deleted non-Merk trees. for child_path in &non_merk_delete_paths { let child_subtree_path: SubtreePath> = child_path.as_slice().into(); diff --git a/grovedb/src/batch/single_insert_cost_tests.rs b/grovedb/src/batch/single_insert_cost_tests.rs index 73cd4b835..29bd5496f 100644 --- a/grovedb/src/batch/single_insert_cost_tests.rs +++ b/grovedb/src/batch/single_insert_cost_tests.rs @@ -19,8 +19,10 @@ mod tests { use integer_encoding::VarInt; use intmap::IntMap; + use grovedb_merk::TreeType; + use crate::{ - batch::QualifiedGroveDbOp, + batch::{QualifiedGroveDbOp, SubelementsDeletionBehavior}, reference_path::ReferencePathType::SiblingReference, tests::{common::EMPTY_PATH, make_empty_grovedb}, Element, @@ -1584,21 +1586,19 @@ mod tests { assert_eq!( cost, OperationCost { - // +1 seek and +129 storage_loaded_bytes vs. V1..V3: with + // Identical to the `_v3_keeps_live_costs` companion below: // `overwrite_indexed_cleanup_inspection >= 1` (V4+, which is - // what `latest()` resolves to) and tree-override validation - // off, the batch path reads each existing element for - // indexed-overwrite detection, preventing silent - // secondary-storage orphaning. The `_v3_keeps_live_costs` - // companion below pins that released versions do NOT pay - // this read. - seek_count: 8, + // what `latest()` resolves to) classifies each overwritten + // element for indexed-overwrite detection, but it does so + // from the old value the merk walk already fetched, so the + // gate adds no seeks and no loaded bytes over V1..V3. + seek_count: 7, storage_cost: StorageCost { added_bytes: 4, replaced_bytes: 285, removed_bytes: NoStorageRemoval }, - storage_loaded_bytes: 509, + storage_loaded_bytes: 380, hash_node_calls: 12, sinsemilla_hash_calls: 0, } @@ -1619,6 +1619,75 @@ mod tests { ); } + #[test] + fn test_batch_plain_overwrites_and_tree_delete_cost_parity_v3_v4() { + // A batch touching NO indexed trees must cost byte-for-byte the same + // under GROVE_V3 and GROVE_V4. Both V4 gates + // (`overwrite_indexed_cleanup_inspection` and + // `delete_tree_cleanup_type_source`) derive the old element from + // data the apply already loads — the merk walk's own fetch of the + // node being rewritten or deleted, and the emptiness pre-scan's own + // read — instead of issuing a dedicated stored-element read, so + // their classification work is invisible to tracked cost. + let run = |grove_version: &GroveVersion| { + let db = make_empty_grovedb(); + let tx = db.start_transaction(); + let ops = vec![ + QualifiedGroveDbOp::insert_or_replace_op( + vec![], + b"key1".to_vec(), + Element::new_item([0u8; 30].to_vec()), + ), + QualifiedGroveDbOp::insert_or_replace_op( + vec![], + b"keyref".to_vec(), + Element::new_reference(SiblingReference(b"key1".to_vec())), + ), + QualifiedGroveDbOp::insert_or_replace_op( + vec![], + b"tree".to_vec(), + Element::empty_tree(), + ), + ]; + db.apply_batch(ops, None, Some(&tx), grove_version) + .value + .expect("expected to execute setup batch"); + + // Every op shape the V4 gates watch: an item overwrite via + // InsertOrReplace, a reference overwrite via Replace, and a + // DeleteTree of a plain tree with the emptiness check on. + let ops = vec![ + QualifiedGroveDbOp::insert_or_replace_op( + vec![], + b"key1".to_vec(), + Element::new_item([1u8; 30].to_vec()), + ), + QualifiedGroveDbOp::replace_op( + vec![], + b"keyref".to_vec(), + Element::new_reference(SiblingReference(b"key1".to_vec())), + ), + QualifiedGroveDbOp::delete_tree_op( + vec![], + b"tree".to_vec(), + TreeType::NormalTree, + SubelementsDeletionBehavior::Error, + ), + ]; + db.apply_batch(ops, None, Some(&tx), grove_version) + }; + + let v3 = run(&grovedb_version::version::v3::GROVE_V3); + let v4 = run(&grovedb_version::version::v4::GROVE_V4); + v3.value.as_ref().expect("v3 batch should apply"); + v4.value.as_ref().expect("v4 batch should apply"); + assert_eq!( + v3.cost, v4.cost, + "a batch of plain overwrites plus a plain DeleteTree must \ + produce an identical CostResult under V3 and V4" + ); + } + #[test] fn test_batch_root_one_update_cost_right_above_value_required_cost_of_2_with_refresh_reference_v3_keeps_live_costs( ) { @@ -1701,11 +1770,11 @@ mod tests { OperationCost { // The exact constants this batch shape cost on the released // versions. `overwrite_indexed_cleanup_inspection` is 0 on - // V1..V3, so the stored-element read the V4 test above pays - // (+1 seek, +129 loaded bytes) must NOT happen here — cost - // feeds fees, and released versions must keep their released - // cost shape. If this test starts failing, a live fee just - // changed. + // V1..V3; on V4+ the gate classifies overwrites from the + // already-fetched old value, so the SAME constants hold in + // the V4 test above. Cost feeds fees, and released versions + // must keep their released cost shape. If this test starts + // failing, a live fee just changed. seek_count: 7, storage_cost: StorageCost { added_bytes: 4, diff --git a/grovedb/src/estimated_costs/mod.rs b/grovedb/src/estimated_costs/mod.rs index 2f7f3f0bd..937e93411 100644 --- a/grovedb/src/estimated_costs/mod.rs +++ b/grovedb/src/estimated_costs/mod.rs @@ -29,4 +29,4 @@ //! Estimated costs mod average_case_costs; -pub(crate) mod worst_case_costs; +mod worst_case_costs; diff --git a/grovedb/src/tests/batch_delete_tree_tests.rs b/grovedb/src/tests/batch_delete_tree_tests.rs index b01e580c2..1ca687772 100644 --- a/grovedb/src/tests/batch_delete_tree_tests.rs +++ b/grovedb/src/tests/batch_delete_tree_tests.rs @@ -2430,4 +2430,283 @@ mod tests { Element::new_item(b"hello".to_vec()), ); } + + // =================================================================== + // Pre-scan branch coverage for the zero-cost V4 gates + // =================================================================== + + #[test] + fn test_batch_delete_tree_missing_key_errors_on_emptiness_check() { + // The V4 Error/Skip pre-scan reads the stored element up front (the + // same single read V1..V3 pay inside the child-merk open); a + // DeleteTree whose target does not exist fails that read. + let grove_version = GroveVersion::latest(); + let db = make_empty_grovedb(); + let result = db + .apply_batch( + vec![QualifiedGroveDbOp::delete_tree_op( + vec![], + b"missing".to_vec(), + TreeType::NormalTree, + SubelementsDeletionBehavior::Error, + )], + None, + None, + grove_version, + ) + .unwrap(); + match result { + Err(Error::CorruptedData(message)) => assert!( + message.contains("unable to get element for delete tree emptiness check"), + "unexpected message: {message}" + ), + other => panic!("expected CorruptedData, got {other:?}"), + } + } + + #[test] + fn test_batch_delete_tree_counts_same_batch_child_deletes_as_empty() { + // A parent tree containing one child tree may be DeleteTree'd with + // Error behavior in the same batch that also deletes the child: the + // emptiness check excludes keys other delete ops in the batch will + // remove. While scanning it must ignore plain Delete ops that target + // other paths and Skip-policy DeleteTree ops (which might not + // execute). + let grove_version = GroveVersion::latest(); + let db = make_empty_grovedb(); + db.insert( + EMPTY_PATH, + b"parent", + Element::empty_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert parent"); + db.insert( + [b"parent".as_slice()].as_ref(), + b"child", + Element::empty_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert child"); + db.insert( + EMPTY_PATH, + b"item", + Element::new_item(b"v".to_vec()), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert item"); + // A non-empty sibling tree deleted with Skip policy: skipped, and + // its op must not count toward any other tree's emptiness carve-out. + db.insert( + EMPTY_PATH, + b"busy", + Element::empty_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert busy"); + db.insert( + [b"busy".as_slice()].as_ref(), + b"row", + Element::new_item(b"x".to_vec()), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert row"); + + db.apply_batch( + vec![ + QualifiedGroveDbOp::delete_tree_op( + vec![], + b"parent".to_vec(), + TreeType::NormalTree, + SubelementsDeletionBehavior::Error, + ), + QualifiedGroveDbOp::delete_tree_op( + vec![b"parent".to_vec()], + b"child".to_vec(), + TreeType::NormalTree, + SubelementsDeletionBehavior::Error, + ), + QualifiedGroveDbOp::delete_op(vec![], b"item".to_vec()), + QualifiedGroveDbOp::delete_tree_op( + vec![], + b"busy".to_vec(), + TreeType::NormalTree, + SubelementsDeletionBehavior::Skip, + ), + ], + None, + None, + grove_version, + ) + .unwrap() + .expect("batch should apply: the child delete satisfies the parent's emptiness check"); + + assert!(db + .get(EMPTY_PATH, b"parent", None, grove_version) + .unwrap() + .is_err()); + assert!(db + .get(EMPTY_PATH, b"item", None, grove_version) + .unwrap() + .is_err()); + // busy was skipped (non-empty) and must survive with its row + assert!(db + .get(EMPTY_PATH, b"busy", None, grove_version) + .unwrap() + .is_ok()); + assert!(db + .get([b"busy".as_slice()].as_ref(), b"row", None, grove_version) + .unwrap() + .is_ok()); + } + + #[test] + fn test_batch_delete_tree_v3_takes_declared_types_at_face_value() { + // The released (V1..V3) pre-scan classifies cleanup paths from the + // DECLARED type without reading storage. Exercise each declared-type + // shape on GROVE_V3 against plain stored trees: an indexed declare + // with DontCheckWithNoCleanup, a non-merk declare with Error, an + // indexed declare with DeleteChildren, and Skip on a non-empty tree. + let grove_version = &grovedb_version::version::v3::GROVE_V3; + let db = make_empty_grovedb(); + for key in [b"a".as_slice(), b"b", b"c", b"d"] { + db.insert( + EMPTY_PATH, + key, + Element::empty_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert tree"); + } + db.insert( + [b"d".as_slice()].as_ref(), + b"row", + Element::new_item(b"x".to_vec()), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert row"); + + db.apply_batch( + vec![ + QualifiedGroveDbOp::delete_tree_op( + vec![], + b"a".to_vec(), + TreeType::ProvableCountIndexedTree, + SubelementsDeletionBehavior::DontCheckWithNoCleanup, + ), + QualifiedGroveDbOp::delete_tree_op( + vec![], + b"b".to_vec(), + TreeType::MmrTree, + SubelementsDeletionBehavior::Error, + ), + QualifiedGroveDbOp::delete_tree_op( + vec![], + b"c".to_vec(), + TreeType::ProvableSumIndexedTree, + SubelementsDeletionBehavior::DeleteChildren, + ), + QualifiedGroveDbOp::delete_tree_op( + vec![], + b"d".to_vec(), + TreeType::NormalTree, + SubelementsDeletionBehavior::Skip, + ), + ], + None, + None, + grove_version, + ) + .unwrap() + .expect("v3 batch should apply, taking every declared type at face value"); + + for key in [b"a".as_slice(), b"b", b"c"] { + assert!(db + .get(EMPTY_PATH, key, None, grove_version) + .unwrap() + .is_err()); + } + // d was skipped (non-empty) and must survive + assert!(db + .get(EMPTY_PATH, b"d", None, grove_version) + .unwrap() + .is_ok()); + } + + #[test] + fn test_batch_delete_tree_second_mismatch_after_first_gate_error() { + // Two mismatched DeleteTree ops at the same level: the old-value + // observer records the first rejection and short-circuits on the + // second; the whole batch aborts and nothing commits. + let grove_version = GroveVersion::latest(); + let db = make_empty_grovedb(); + for key in [b"x".as_slice(), b"y"] { + db.insert( + EMPTY_PATH, + key, + Element::empty_provable_count_indexed_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert pcit"); + } + let result = db + .apply_batch( + vec![ + QualifiedGroveDbOp::delete_tree_op( + vec![], + b"x".to_vec(), + TreeType::NormalTree, + SubelementsDeletionBehavior::DeleteChildren, + ), + QualifiedGroveDbOp::delete_tree_op( + vec![], + b"y".to_vec(), + TreeType::NormalTree, + SubelementsDeletionBehavior::DeleteChildren, + ), + ], + None, + None, + grove_version, + ) + .unwrap(); + match result { + Err(Error::InvalidBatchOperation(message)) => assert!( + message.contains("declared tree type does not match"), + "unexpected message: {message}" + ), + other => panic!("expected InvalidBatchOperation, got {other:?}"), + } + // Nothing committed — both indexed trees survive. + for key in [b"x".as_slice(), b"y"] { + assert!(db + .get(EMPTY_PATH, key, None, grove_version) + .unwrap() + .is_ok()); + } + } } diff --git a/grovedb/src/tests/batch_indexed_overwrite_tests.rs b/grovedb/src/tests/batch_indexed_overwrite_tests.rs index 7dd372939..f0d355ad1 100644 --- a/grovedb/src/tests/batch_indexed_overwrite_tests.rs +++ b/grovedb/src/tests/batch_indexed_overwrite_tests.rs @@ -10,7 +10,7 @@ //! back out through top-k. Each op variant that can carry a caller-supplied //! element therefore needs its own regression test; `Replace` is one of them. //! -//! `inspect_cidx_overwrite` runs when tree-override protection is off and +//! `classify_cidx_overwrite` runs when tree-override protection is off and //! decides whether replacing an existing indexed tree is safe. Indexed → empty //! indexed and indexed → non-indexed are allowed and schedule the old tree's //! storage for cleanup; indexed → *non-empty* indexed is refused because the @@ -48,7 +48,7 @@ mod tests { tags.into_iter().map(|t| (t, None)).collect() } - /// Tree-override protection off — the mode in which `inspect_cidx_overwrite` + /// Tree-override protection off — the mode in which `classify_cidx_overwrite` /// is consulted at all. fn overwrite_allowed() -> BatchApplyOptions { BatchApplyOptions { @@ -325,15 +325,15 @@ mod tests { ) .unwrap() .expect_err("indexed -> non-empty indexed is ambiguous and must be refused"); + // The refusal comes from the ungated empty-at-batch-insertion guard, + // which runs before the overwrite classification ever sees the op — + // a non-empty indexed element cannot enter a batch at all. match err { - Error::NotSupported(message) => assert!( - message.starts_with( - "overwriting an existing indexed tree with a NON-EMPTY indexed tree via the \ - batch path is not supported" - ), + Error::InvalidBatchOperation(message) => assert!( + message.contains("must be empty at the moment of batch insertion"), "unexpected message: {message}" ), - other => panic!("expected NotSupported, got {other:?}"), + other => panic!("expected InvalidBatchOperation, got {other:?}"), } assert_eq!( db.root_hash(None, gv).unwrap().expect("root hash"), diff --git a/grovedb/src/tests/batch_indexed_tree_tests.rs b/grovedb/src/tests/batch_indexed_tree_tests.rs index cf904f449..db98ef6ee 100644 --- a/grovedb/src/tests/batch_indexed_tree_tests.rs +++ b/grovedb/src/tests/batch_indexed_tree_tests.rs @@ -4,7 +4,7 @@ //! - Batch insert of empty indexed-tree primaries. //! - Mixed batches (cidx + non-cidx ops apply atomically). //! - DeleteTree on each variant cleans up secondary namespaces. -//! - inspect_cidx_overwrite: cidx → empty cidx safe; +//! - classify_cidx_overwrite: cidx → empty cidx safe; //! cidx → non-empty cidx rejected; cidx → non-cidx safe. //! - Batch capture-pre-state / apply-post-mirror integration. //! - Validation rejection: empty-cidx + descendant writes in same @@ -484,7 +484,7 @@ mod tests { } // ----------------------------------------------------------------- - // inspect_cidx_overwrite paths + // classify_cidx_overwrite paths // ----------------------------------------------------------------- #[test] @@ -566,12 +566,16 @@ mod tests { grove_version, ) .unwrap(); - assert!( - matches!(result, Err(Error::NotSupported(_))) - || matches!(result, Err(Error::InvalidInput(_))), - "expected NotSupported / InvalidInput, got {:?}", - result - ); + // Refused by the ungated empty-at-batch-insertion guard (a non-empty + // indexed element cannot enter a batch at all); the overwrite + // classifier's NotSupported stays as defense in depth behind it. + match result { + Err(Error::InvalidBatchOperation(message)) => assert!( + message.contains("must be empty at the moment of batch insertion"), + "expected the empty-at-insertion refusal, got: {message}" + ), + other => panic!("expected InvalidBatchOperation, got {:?}", other), + } } #[test] @@ -597,7 +601,7 @@ mod tests { ) .unwrap() .expect("populate"); - // Overwrite with a plain Item — inspect_cidx_overwrite's + // Overwrite with a plain Item — classify_cidx_overwrite's // "non-cidx" safe branch. db.apply_batch( vec![QualifiedGroveDbOp::insert_or_replace_op( @@ -975,13 +979,13 @@ mod tests { // ----------------------------------------------------------------- // Batch overwrite with Patch / Replace / InsertOrReplace into a // non-cidx primary that contains a cidx — exercises - // inspect_cidx_overwrite call site (L2322-2342) + // classify_cidx_overwrite call site (L2322-2342) // ----------------------------------------------------------------- #[test] fn batch_overwrite_cidx_with_safe_subset_schedules_cleanup() { // Empty PCIT exists; replace with a non-cidx Item. - // inspect_cidx_overwrite classifies as safe-subset overwrite + // classify_cidx_overwrite classifies as safe-subset overwrite // and queues the cidx-cleanup path. The new element bytes are // written, and the secondary namespace gets cleaned up. let grove_version = GroveVersion::latest(); @@ -997,7 +1001,7 @@ mod tests { .unwrap() .expect("create"); // Use a batch with override-tree off so the safe-subset - // classifier in inspect_cidx_overwrite runs. + // classifier in classify_cidx_overwrite runs. let ops = vec![QualifiedGroveDbOp::insert_or_replace_op( vec![TEST_LEAF.to_vec()], b"cidx".to_vec(), @@ -1008,13 +1012,12 @@ mod tests { validate_insertion_does_not_override_tree: false, ..Default::default() }; - // Either succeeds (safe overwrite path) or returns the - // ambiguous-cidx error from inspect_cidx_overwrite. Both - // exercise the call site at L2330. - let _ = db - .apply_batch(ops, Some(options), None, grove_version) - .unwrap(); - // Regardless of outcome the DB is consistent. + // Indexed → Item is the safe overwrite subset: the classifier + // schedules the old cidx's storage for cleanup and the batch + // must succeed. + db.apply_batch(ops, Some(options), None, grove_version) + .unwrap() + .expect("safe-subset cidx overwrite with an Item must succeed"); assert_verify_passes(&db, grove_version); } diff --git a/grovedb/src/tests/indexed_tree_security_regression_tests.rs b/grovedb/src/tests/indexed_tree_security_regression_tests.rs index 721dacfaf..97ac00276 100644 --- a/grovedb/src/tests/indexed_tree_security_regression_tests.rs +++ b/grovedb/src/tests/indexed_tree_security_regression_tests.rs @@ -280,27 +280,17 @@ fn batch_overwrite_cleans_psit_and_pcpsit_namespaces() { assert_verify_passes(&pcpsit, grove_version); } -/// DEFERRED — documents a known-open issue, not current behaviour. +/// Overwriting an indexed tree with a bare `Reference` schedules the per-axis +/// secondary cleanup like every other overwrite-capable op (closes issue +/// https://github.com/dashpay/grovedb/issues/776). /// -/// Overwriting an indexed tree with a bare `Reference` does not schedule the -/// per-axis secondary cleanup, so the secondary namespace is orphaned. The fix -/// is to route reference overwrites through `inspect_cidx_overwrite` like every -/// other overwrite-capable op — but that costs one extra stored-element read on -/// EVERY reference overwrite (+1 seek, +79 storage_loaded_bytes, measured by -/// `single_insert_cost_tests::test_batch_root_one_update_item_*_with_refresh_reference`). -/// Cost feeds fees, and references over plain trees are shipped functionality on -/// GROVE_V1/V2/V3, so paying that read unconditionally changes live behaviour. -/// -/// The hole requires an indexed tree to be the overwritten element, which cannot -/// happen on any released version — indexed trees are introduced by this PR. It -/// should therefore be closed together with the protocol version that activates -/// indexed trees, gated so live versions keep today's cost. Un-ignore this test -/// as part of that change. +/// This used to be deferred because routing reference overwrites through the +/// classifier started with a dedicated stored-element read (+1 seek, +79 +/// storage_loaded_bytes on every reference overwrite — a live cost change). +/// The V4 gate now classifies from the old value the merk walk already +/// fetched, so including references costs nothing and the hole is closed on +/// V4+ while V1..V3 keep their released behaviour. #[test] -#[ignore = "deferred, tracked in \ - https://github.com/dashpay/grovedb/issues/776 — closing this changes \ - reference-overwrite cost on live versions; gate with the protocol \ - version that activates indexed trees"] fn bare_reference_overwrite_cleans_indexed_storage() { let grove_version = GroveVersion::latest(); let db = make_test_grovedb(grove_version); @@ -659,11 +649,12 @@ fn batch_patch_cannot_forge_a_rootless_aggregate() { /// The DeleteTree cleanup-type fix is gated on V4: active there, absent on the /// released versions. /// -/// Reading the stored element to select cleanup namespaces costs an extra seek -/// and load per op, so applying it to V1..V3 would change tracked costs — and -/// therefore fees — on a shipped path. This pins both halves of the gate, so a -/// future change cannot quietly extend it to a released version (which would -/// be a consensus divergence) or drop it from V4 (which would reopen the +/// The check derives the stored type from data the apply already loads, so it +/// no longer costs anything — but it still flips an accepted/rejected +/// outcome: a mismatched declare that V1..V3 accept is refused on V4+ when an +/// indexed tree is involved. This pins both halves of the gate, so a future +/// change cannot quietly extend it to a released version (which would be a +/// consensus divergence) or drop it from V4 (which would reopen the /// type-confusion). #[test] fn delete_tree_cleanup_type_gate_is_v4_only() { @@ -684,7 +675,7 @@ fn delete_tree_cleanup_type_gate_is_v4_only() { .apply_batch .delete_tree_cleanup_type_source, 1, - "V4 must read the stored element to select cleanup namespaces" + "V4 must select cleanup namespaces from the stored element's type" ); // Behaviour: a mismatched declared type on a populated PCIT. diff --git a/grovedb/src/tests/provable_count_indexed_tree_tests.rs b/grovedb/src/tests/provable_count_indexed_tree_tests.rs index 3201ff2bf..35e7cab92 100644 --- a/grovedb/src/tests/provable_count_indexed_tree_tests.rs +++ b/grovedb/src/tests/provable_count_indexed_tree_tests.rs @@ -430,7 +430,7 @@ mod tests { #[test] fn pcit_batch_overwrite_existing_pcit_with_empty_pcit_clears_secondary() { - // Per `inspect_cidx_overwrite`: cidx → empty cidx is the safe + // Per `classify_cidx_overwrite`: cidx → empty cidx is the safe // subset and must succeed via batch (post-apply cleanup // clears the old secondary namespace at // Blake3(primary_prefix ‖ 0x01)). @@ -473,7 +473,7 @@ mod tests { #[test] fn pcit_batch_overwrite_existing_pcit_with_non_empty_pcit_is_rejected() { // indexed → non-empty indexed must be rejected by - // `inspect_cidx_overwrite` (storage-pointer ambiguity: the new + // `classify_cidx_overwrite` (storage-pointer ambiguity: the new // root keys would refer to on-disk data that post-apply // cleanup of the OLD cidx also clears). let grove_version = GroveVersion::latest(); @@ -497,12 +497,15 @@ mod tests { grove_version, ) .unwrap(); + // Refused by the ungated empty-at-batch-insertion guard (a non-empty + // indexed element cannot enter a batch at all); the overwrite + // classifier's NotSupported stays as defense in depth behind it. match result { - Err(Error::NotSupported(msg)) => assert!( - msg.contains("NON-EMPTY indexed tree"), - "expected NotSupported, got: {msg}" + Err(Error::InvalidBatchOperation(msg)) => assert!( + msg.contains("must be empty at the moment of batch insertion"), + "expected the empty-at-insertion refusal, got: {msg}" ), - other => panic!("expected NotSupported, got {:?}", other), + other => panic!("expected InvalidBatchOperation, got {:?}", other), } } diff --git a/merk/src/lib.rs b/merk/src/lib.rs index 06125a8de..71f39034d 100644 --- a/merk/src/lib.rs +++ b/merk/src/lib.rs @@ -51,8 +51,8 @@ pub use ed; pub use error::Error; #[cfg(feature = "minimal")] pub use tree::{ - BatchEntry, Link, MerkBatch, Op, PanicSource, HASH_BLOCK_SIZE, HASH_BLOCK_SIZE_U32, - HASH_LENGTH, HASH_LENGTH_U32, HASH_LENGTH_U32_X2, + BatchEntry, Link, MerkBatch, OldValueDisposition, Op, PanicSource, HASH_BLOCK_SIZE, + HASH_BLOCK_SIZE_U32, HASH_LENGTH, HASH_LENGTH_U32, HASH_LENGTH_U32_X2, }; pub use tree::{CryptoHash, TreeFeatureType}; pub use tree_type::{MaybeTree, TreeType}; diff --git a/merk/src/merk/apply.rs b/merk/src/merk/apply.rs index 4ae8b3c9a..4609e2932 100644 --- a/merk/src/merk/apply.rs +++ b/merk/src/merk/apply.rs @@ -14,7 +14,7 @@ use crate::{ merk::NodeType, tree::{ kv::{ValueDefinedCostType, KV}, - AuxMerkBatch, Walker, + AuxMerkBatch, OldValueDisposition, Walker, }, Error, Merk, MerkBatch, MerkOptions, }; @@ -345,6 +345,64 @@ where &mut Vec, ) -> Result<(bool, Option), Error>, R: FnMut(&Vec, u32, u32) -> Result<(StorageRemovedBytes, StorageRemovedBytes), Error>, + { + self.apply_unchecked_with_old_value_observer( + batch, + aux, + options, + old_specialized_cost, + value_defined_cost_fn, + get_temp_new_value_with_old_flags, + update_tree_value_based_on_costs, + section_removal_bytes, + &mut |_, _, _| {}, + grove_version, + ) + } + + /// Same as [`apply_unchecked`](Self::apply_unchecked), with an additional + /// `old_value_observer` called as `(key, old_value, disposition)` for + /// every batch op that overwrites or deletes an EXISTING node. + /// + /// The observed bytes are the node's stored value, which the tree walk + /// had to fetch anyway to rewrite or remove the node — the observer + /// therefore costs no extra storage read and adds nothing to the tracked + /// [`OperationCost`](grovedb_costs::OperationCost). Callers that need the + /// pre-op element (e.g. to select cleanup namespaces from the ACTUAL + /// stored type) should use this instead of issuing a separate charged + /// read before the apply. + /// + /// Ops on keys that do not exist in the tree never invoke the observer. + /// + /// # Safety + /// Same contract as [`apply_unchecked`](Self::apply_unchecked): keys in + /// `batch` must be sorted and unique. + pub fn apply_unchecked_with_old_value_observer( + &mut self, + batch: &MerkBatch, + aux: &AuxMerkBatch, + options: Option, + old_specialized_cost: &C, + value_defined_cost_fn: Option<&V>, + get_temp_new_value_with_old_flags: &T, + update_tree_value_based_on_costs: &mut U, + section_removal_bytes: &mut R, + old_value_observer: &mut O, + grove_version: &GroveVersion, + ) -> CostResult<(), Error> + where + KB: AsRef<[u8]>, + KA: AsRef<[u8]>, + C: Fn(&Vec, &Vec) -> Result, + V: Fn(&[u8], &GroveVersion) -> Option, + T: Fn(&Vec, &Vec) -> Result>, Error>, + U: FnMut( + &StorageCost, + &Vec, + &mut Vec, + ) -> Result<(bool, Option), Error>, + R: FnMut(&Vec, u32, u32) -> Result<(StorageRemovedBytes, StorageRemovedBytes), Error>, + O: FnMut(&[u8], &[u8], OldValueDisposition), { for (key, ..) in batch.iter() { if key.as_ref().len() > MAX_KEY_LENGTH { @@ -369,6 +427,7 @@ where get_temp_new_value_with_old_flags, update_tree_value_based_on_costs, section_removal_bytes, + old_value_observer, grove_version, ) .flat_map_ok(|(maybe_tree, key_updates)| { diff --git a/merk/src/test_utils/mod.rs b/merk/src/test_utils/mod.rs index eb1eeeb26..07e99821c 100644 --- a/merk/src/test_utils/mod.rs +++ b/merk/src/test_utils/mod.rs @@ -103,6 +103,7 @@ pub fn apply_memonly_unchecked( BasicStorageRemoval(value_bytes_to_remove), )) }, + &mut |_, _, _| {}, grove_version, ) .unwrap() @@ -163,6 +164,7 @@ pub fn apply_to_memonly( BasicStorageRemoval(value_bytes_to_remove), )) }, + &mut |_, _, _| {}, grove_version, ) .unwrap() diff --git a/merk/src/tree/mod.rs b/merk/src/tree/mod.rs index 97a7f8f5f..02b12e0c0 100644 --- a/merk/src/tree/mod.rs +++ b/merk/src/tree/mod.rs @@ -59,7 +59,7 @@ use kv::KV; #[cfg(feature = "minimal")] pub use link::Link; #[cfg(feature = "minimal")] -pub use ops::{AuxMerkBatch, BatchEntry, MerkBatch, Op, PanicSource}; +pub use ops::{AuxMerkBatch, BatchEntry, MerkBatch, OldValueDisposition, Op, PanicSource}; #[cfg(feature = "minimal")] pub use tree_feature_type::AggregateData; #[cfg(any(feature = "minimal", feature = "verify"))] diff --git a/merk/src/tree/ops.rs b/merk/src/tree/ops.rs index 9ec433d1e..5b7db36b4 100644 --- a/merk/src/tree/ops.rs +++ b/merk/src/tree/ops.rs @@ -129,6 +129,21 @@ impl fmt::Debug for Op { } } +/// How a batch operation displaced the stored value of an existing node. +/// +/// Passed to the old-value observer (see +/// [`Merk::apply_unchecked_with_old_value_observer`](crate::Merk)) together +/// with the node's key and its pre-op value bytes. The walker had to fetch +/// the node to rewrite or remove it, so surfacing the old value here is free +/// — no additional storage read and no additional tracked cost. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum OldValueDisposition { + /// A put-style op overwrote the node's value. + Replaced, + /// A delete-style op removed the node. + Deleted, +} + /// A single `(key, operation)` pair. pub type BatchEntry = (K, Op); @@ -172,7 +187,7 @@ where /// not require a non-empty tree. /// /// Keys in batch must be sorted and unique. - pub fn apply_to, C, V, T, U, R>( + pub fn apply_to, C, V, T, U, R, O>( maybe_tree: Option, batch: &MerkBatch, source: S, @@ -181,6 +196,7 @@ where get_temp_new_value_with_old_flags: &T, update_tree_value_based_on_costs: &mut U, section_removal_bytes: &mut R, + old_value_observer: &mut O, grove_version: &GroveVersion, ) -> CostContext, KeyUpdates), Error>> where @@ -193,6 +209,7 @@ where &mut Vec, ) -> Result<(bool, Option), Error>, R: FnMut(&Vec, u32, u32) -> Result<(StorageRemovedBytes, StorageRemovedBytes), Error>, + O: FnMut(&[u8], &[u8], OldValueDisposition), { let mut cost = OperationCost::default(); @@ -217,6 +234,7 @@ where get_temp_new_value_with_old_flags, update_tree_value_based_on_costs, section_removal_bytes, + old_value_observer, grove_version, ) .map_ok(|tree| { @@ -245,6 +263,7 @@ where get_temp_new_value_with_old_flags, update_tree_value_based_on_costs, section_removal_bytes, + old_value_observer, grove_version ) ) @@ -259,7 +278,7 @@ where /// Builds a `Tree` from a batch of operations. /// /// Keys in batch must be sorted and unique. - fn build, C, V, T, U, R>( + fn build, C, V, T, U, R, O>( batch: &MerkBatch, source: S, old_tree_cost: &C, @@ -267,6 +286,7 @@ where get_temp_new_value_with_old_flags: &T, update_tree_value_based_on_costs: &mut U, section_removal_bytes: &mut R, + old_value_observer: &mut O, grove_version: &GroveVersion, ) -> CostResult, Error> where @@ -279,6 +299,7 @@ where &mut Vec, ) -> Result<(bool, Option), Error>, R: FnMut(&Vec, u32, u32) -> Result<(StorageRemovedBytes, StorageRemovedBytes), Error>, + O: FnMut(&[u8], &[u8], OldValueDisposition), { let mut cost = OperationCost::default(); @@ -303,6 +324,7 @@ where get_temp_new_value_with_old_flags, update_tree_value_based_on_costs, section_removal_bytes, + old_value_observer, grove_version ) ) @@ -318,6 +340,7 @@ where get_temp_new_value_with_old_flags, update_tree_value_based_on_costs, section_removal_bytes, + old_value_observer, grove_version ) ) @@ -333,6 +356,7 @@ where get_temp_new_value_with_old_flags, update_tree_value_based_on_costs, section_removal_bytes, + old_value_observer, grove_version ) ) @@ -432,6 +456,7 @@ where get_temp_new_value_with_old_flags, update_tree_value_based_on_costs, section_removal_bytes, + old_value_observer, grove_version, ) ) @@ -458,6 +483,7 @@ where BasicStorageRemoval(value_bytes_to_remove), )) }, + &mut |_, _, _| {}, grove_version, ) } @@ -466,7 +492,7 @@ where /// `Walker::apply`_to, but requires a populated tree. /// /// Keys in batch must be sorted and unique. - fn apply_sorted, C, V, T, U, R>( + fn apply_sorted, C, V, T, U, R, O>( self, batch: &MerkBatch, old_specialized_cost: &C, @@ -474,6 +500,7 @@ where get_temp_new_value_with_old_flags: &T, update_tree_value_based_on_costs: &mut U, section_removal_bytes: &mut R, + old_value_observer: &mut O, grove_version: &GroveVersion, ) -> CostResult<(Option, KeyUpdates), Error> where @@ -486,6 +513,7 @@ where &mut Vec, ) -> Result<(bool, Option), Error>, R: FnMut(&Vec, u32, u32) -> Result<(StorageRemovedBytes, StorageRemovedBytes), Error>, + O: FnMut(&[u8], &[u8], OldValueDisposition), { let mut cost = OperationCost::default(); @@ -497,6 +525,23 @@ where let tree = if let Ok(index) = search { let (_, op) = &batch[index]; + // This node is being rewritten or removed, and the walker already + // holds its stored value — surface it to the observer before the + // op consumes it. This is the free source of "old element" bytes + // callers would otherwise have to re-read (and re-pay) from + // storage. + let disposition = match op { + Delete | DeleteLayered | DeleteLayeredMaybeSpecialized | DeleteMaybeSpecialized => { + OldValueDisposition::Deleted + } + _ => OldValueDisposition::Replaced, + }; + old_value_observer( + key_vec.as_slice(), + self.tree().value_ref().as_slice(), + disposition, + ); + // a key matches this node's key, apply op to this node match op { // TODO: take vec from batch so we don't need to clone @@ -667,6 +712,7 @@ where get_temp_new_value_with_old_flags, update_tree_value_based_on_costs, section_removal_bytes, + old_value_observer, grove_version, ) ); @@ -694,6 +740,7 @@ where get_temp_new_value_with_old_flags, update_tree_value_based_on_costs, section_removal_bytes, + old_value_observer, grove_version ) ) @@ -727,6 +774,7 @@ where get_temp_new_value_with_old_flags, update_tree_value_based_on_costs, section_removal_bytes, + old_value_observer, grove_version, ) ); @@ -754,6 +802,7 @@ where get_temp_new_value_with_old_flags, update_tree_value_based_on_costs, section_removal_bytes, + old_value_observer, grove_version ) ) @@ -803,6 +852,7 @@ where get_temp_new_value_with_old_flags, update_tree_value_based_on_costs, section_removal_bytes, + old_value_observer, grove_version, ) .add_cost(cost) @@ -813,7 +863,7 @@ where /// /// This recursion executes serially in the same thread, but in the future /// will be dispatched to workers in other threads. - fn recurse, C, V, T, U, R>( + fn recurse, C, V, T, U, R, O>( self, batch: &MerkBatch, mid: usize, @@ -824,6 +874,7 @@ where get_temp_new_value_with_old_flags: &T, update_tree_value_based_on_costs: &mut U, section_removal_bytes: &mut R, + old_value_observer: &mut O, grove_version: &GroveVersion, ) -> CostResult<(Option, KeyUpdates), Error> where @@ -836,6 +887,7 @@ where ) -> Result<(bool, Option), Error>, V: Fn(&[u8], &GroveVersion) -> Option, R: FnMut(&Vec, u32, u32) -> Result<(StorageRemovedBytes, StorageRemovedBytes), Error>, + O: FnMut(&[u8], &[u8], OldValueDisposition), { let mut cost = OperationCost::default(); @@ -864,6 +916,7 @@ where get_temp_new_value_with_old_flags, update_tree_value_based_on_costs, section_removal_bytes, + old_value_observer, grove_version, ) .map_ok(|(maybe_left, mut key_updates_left)| { @@ -901,6 +954,7 @@ where get_temp_new_value_with_old_flags, update_tree_value_based_on_costs, section_removal_bytes, + old_value_observer, grove_version, ) .map_ok(|(maybe_right, mut key_updates_right)| { @@ -1141,6 +1195,34 @@ mod test { tree::{tree_feature_type::TreeFeatureType::BasicMerkNode, *}, }; + /// `Walker::apply_to` with inert cost callbacks and a no-op old-value + /// observer — the shape every test in this module wants. Consolidated so + /// each case exercises the same plumbing (including the observer, which + /// fires whenever a batch op lands on an existing key). + fn apply_to_plain( + maybe_walker: Option>, + batch: &MerkBatch>, + grove_version: &GroveVersion, + ) -> CostContext, KeyUpdates), Error>> { + Walker::::apply_to( + maybe_walker, + batch, + PanicSource {}, + &|_, _| Ok(0), + None::<&fn(&[u8], &GroveVersion) -> Option>, + &|_, _| Ok(None), + &mut |_, _, _| Ok((false, None)), + &mut |_flags, key_bytes_to_remove, value_bytes_to_remove| { + Ok(( + BasicStorageRemoval(key_bytes_to_remove), + BasicStorageRemoval(value_bytes_to_remove), + )) + }, + &mut |_, _, _| {}, + grove_version, + ) + } + #[test] fn simple_insert() { let grove_version = GroveVersion::latest(); @@ -1301,51 +1383,49 @@ mod test { #[test] fn apply_empty_none() { let grove_version = GroveVersion::latest(); - let (maybe_tree, key_updates) = Walker::::apply_to::, _, _, _, _, _>( - None, - &[], - PanicSource {}, - &|_, _| Ok(0), - None::<&fn(&[u8], &GroveVersion) -> Option>, - &|_, _| Ok(None), - &mut |_, _, _| Ok((false, None)), - &mut |_flags, key_bytes_to_remove, value_bytes_to_remove| { - Ok(( - BasicStorageRemoval(key_bytes_to_remove), - BasicStorageRemoval(value_bytes_to_remove), - )) - }, - grove_version, - ) - .unwrap() - .expect("apply_to failed"); + let (maybe_tree, key_updates) = apply_to_plain(None, &[], grove_version) + .unwrap() + .expect("apply_to failed"); assert!(maybe_tree.is_none()); assert!(key_updates.updated_keys.is_empty()); assert!(key_updates.deleted_keys.is_empty()); } + #[test] + fn build_with_mid_delete_ignores_missing_key() { + // Building a fresh tree from a batch whose middle op is a Delete + // exercises `build`'s delete arm: the left half is built, then the + // right half is applied to it via `apply_sorted`, and the delete of + // a key that never existed is simply skipped. + let grove_version = GroveVersion::latest(); + let batch = vec![ + (vec![0], Put(vec![1], BasicMerkNode)), + (vec![1], Delete), + (vec![2], Put(vec![3], BasicMerkNode)), + ]; + let (maybe_tree, _key_updates) = apply_to_plain(None, &batch, grove_version) + .unwrap() + .expect("apply_to failed"); + let tree = maybe_tree.expect("expected tree"); + // Both puts landed; the deleted key never existed. + let mut keys = vec![tree.key().to_vec()]; + if let Some(child) = tree.child(true) { + keys.push(child.key().to_vec()); + } + if let Some(child) = tree.child(false) { + keys.push(child.key().to_vec()); + } + keys.sort(); + assert_eq!(keys, vec![vec![0], vec![2]]); + } + #[test] fn insert_empty_single() { let grove_version = GroveVersion::latest(); let batch = vec![(vec![0], Put(vec![1], BasicMerkNode))]; - let (maybe_tree, key_updates) = Walker::::apply_to( - None, - &batch, - PanicSource {}, - &|_, _| Ok(0), - None::<&fn(&[u8], &GroveVersion) -> Option>, - &|_, _| Ok(None), - &mut |_, _, _| Ok((false, None)), - &mut |_flags, key_bytes_to_remove, value_bytes_to_remove| { - Ok(( - BasicStorageRemoval(key_bytes_to_remove), - BasicStorageRemoval(value_bytes_to_remove), - )) - }, - grove_version, - ) - .unwrap() - .expect("apply_to failed"); + let (maybe_tree, key_updates) = apply_to_plain(None, &batch, grove_version) + .unwrap() + .expect("apply_to failed"); let tree = maybe_tree.expect("expected tree"); assert_eq!(tree.key(), &[0]); assert_eq!(tree.value_as_slice(), &[1]); @@ -1358,24 +1438,9 @@ mod test { fn insert_updated_single() { let grove_version = GroveVersion::latest(); let batch = vec![(vec![0], Put(vec![1], BasicMerkNode))]; - let (maybe_tree, key_updates) = Walker::::apply_to( - None, - &batch, - PanicSource {}, - &|_, _| Ok(0), - None::<&fn(&[u8], &GroveVersion) -> Option>, - &|_, _| Ok(None), - &mut |_, _, _| Ok((false, None)), - &mut |_flags, key_bytes_to_remove, value_bytes_to_remove| { - Ok(( - BasicStorageRemoval(key_bytes_to_remove), - BasicStorageRemoval(value_bytes_to_remove), - )) - }, - grove_version, - ) - .unwrap() - .expect("apply_to failed"); + let (maybe_tree, key_updates) = apply_to_plain(None, &batch, grove_version) + .unwrap() + .expect("apply_to failed"); assert!(key_updates.updated_keys.is_empty()); assert!(key_updates.deleted_keys.is_empty()); @@ -1384,24 +1449,9 @@ mod test { (vec![0], Put(vec![2], BasicMerkNode)), (vec![1], Put(vec![2], BasicMerkNode)), ]; - let (maybe_tree, key_updates) = Walker::::apply_to( - maybe_walker, - &batch, - PanicSource {}, - &|_, _| Ok(0), - None::<&fn(&[u8], &GroveVersion) -> Option>, - &|_, _| Ok(None), - &mut |_, _, _| Ok((false, None)), - &mut |_flags, key_bytes_to_remove, value_bytes_to_remove| { - Ok(( - BasicStorageRemoval(key_bytes_to_remove), - BasicStorageRemoval(value_bytes_to_remove), - )) - }, - grove_version, - ) - .unwrap() - .expect("apply_to failed"); + let (maybe_tree, key_updates) = apply_to_plain(maybe_walker, &batch, grove_version) + .unwrap() + .expect("apply_to failed"); let tree = maybe_tree.expect("expected tree"); assert_eq!(tree.key(), &[0]); assert_eq!(tree.value_as_slice(), &[2]); @@ -1417,24 +1467,9 @@ mod test { (vec![1], Put(vec![2], BasicMerkNode)), (vec![2], Put(vec![3], BasicMerkNode)), ]; - let (maybe_tree, key_updates) = Walker::::apply_to( - None, - &batch, - PanicSource {}, - &|_, _| Ok(0), - None::<&fn(&[u8], &GroveVersion) -> Option>, - &|_, _| Ok(None), - &mut |_, _, _| Ok((false, None)), - &mut |_flags, key_bytes_to_remove, value_bytes_to_remove| { - Ok(( - BasicStorageRemoval(key_bytes_to_remove), - BasicStorageRemoval(value_bytes_to_remove), - )) - }, - grove_version, - ) - .unwrap() - .expect("apply_to failed"); + let (maybe_tree, key_updates) = apply_to_plain(None, &batch, grove_version) + .unwrap() + .expect("apply_to failed"); assert!(key_updates.updated_keys.is_empty()); assert!(key_updates.deleted_keys.is_empty()); @@ -1444,24 +1479,9 @@ mod test { (vec![1], Put(vec![8], BasicMerkNode)), (vec![2], Delete), ]; - let (maybe_tree, key_updates) = Walker::::apply_to( - maybe_walker, - &batch, - PanicSource {}, - &|_, _| Ok(0), - None::<&fn(&[u8], &GroveVersion) -> Option>, - &|_, _| Ok(None), - &mut |_, _, _| Ok((false, None)), - &mut |_flags, key_bytes_to_remove, value_bytes_to_remove| { - Ok(( - BasicStorageRemoval(key_bytes_to_remove), - BasicStorageRemoval(value_bytes_to_remove), - )) - }, - grove_version, - ) - .unwrap() - .expect("apply_to failed"); + let (maybe_tree, key_updates) = apply_to_plain(maybe_walker, &batch, grove_version) + .unwrap() + .expect("apply_to failed"); let tree = maybe_tree.expect("expected tree"); assert_eq!(tree.key(), &[1]); assert_eq!(tree.value_as_slice(), &[8]);