Skip to content

Commit a8bb34f

Browse files
fix: nested cidx bubble-up mirrors count change to outer's secondary
Real audit finding: the cidx primary pre-state capture in execute_ops_on_path lists every mutating op variant EXCEPT the new GroveOp::ReplaceCountIndexedTreeRootKeys variant introduced for the cidx-aware bubble-up. When a NESTED cidx primary bubbles up to its OUTER cidx primary via the batch path: Level N (inner cidx primary): mutates fire, secondary mirrored, bubble emits ReplaceCountIndexedTreeRootKeys to level N-1. Level N-1 (outer cidx primary): receives the op at key=inner_key; handler `update_count_indexed_tree_item_preserve_flag_into_ batch_operations` correctly updates the inner_key element's bytes (new primary_root_key, secondary_root_key, count_value). But pre-state capture skipped this op type, so post-apply mirror walked an empty deltas list. Outer's secondary was not updated. The corruption was silent: H1-A integrity (verify_grovedb) still passed because the outer's stored value_hash is recomputed from the actual on-disk secondary root hash — the secondary just has stale content. Top-k / count-range queries on the outer returned stale counts. Fix: add the variant to the mutates match. With the fix, the outer's secondary entry for inner_key correctly moves from (old_count_be ‖ inner_key) to (new_count_be ‖ inner_key) when the inner's count changes. Test batch_insert_into_nested_cidx_primary_bubbles_count_up_outer_ secondary fails BEFORE the fix (asserts top[0] == (1, b"inner_cidx") but gets (0, b"inner_cidx")) and passes AFTER. Found via audit of the new code paths — there was no batch-path nested-cidx test before. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 0ea7b11 commit a8bb34f

2 files changed

Lines changed: 106 additions & 0 deletions

File tree

grovedb/src/batch/mod.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1914,6 +1914,17 @@ where
19141914
| GroveOp::InsertTreeWithRootHash { .. }
19151915
| GroveOp::ReplaceNonMerkTreeRoot { .. }
19161916
| GroveOp::InsertNonMerkTree { .. }
1917+
// Nested cidx: when a child cidx primary
1918+
// bubbles up to its OUTER cidx primary, the
1919+
// child's element bytes (count_value field)
1920+
// change as part of this op. The outer's
1921+
// secondary entry for the child must move
1922+
// from (old_count_be ‖ child_key) to
1923+
// (new_count_be ‖ child_key). Without this
1924+
// arm the outer's secondary silently reports
1925+
// stale counts even though H1-A integrity
1926+
// still passes.
1927+
| GroveOp::ReplaceCountIndexedTreeRootKeys { .. }
19171928
);
19181929
if mutates && !pre.contains_key(&key_bytes) {
19191930
let maybe_bytes = cost_return_on_error!(

grovedb/src/tests/count_indexed_tree_tests.rs

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2996,6 +2996,101 @@ mod tests {
29962996
assert!(matches!(result, Err(crate::Error::CorruptedData(_))));
29972997
}
29982998

2999+
#[test]
3000+
fn batch_insert_into_nested_cidx_primary_bubbles_count_up_outer_secondary() {
3001+
// Layout: TEST_LEAF / outer_cidx / inner_cidx
3002+
// (cidx) (cidx)
3003+
// Batch-inserts an item into inner_cidx's primary. The bubble-up
3004+
// should:
3005+
// - Mirror the count change inside inner's secondary (count
3006+
// for "item" goes None → 1).
3007+
// - Emit ReplaceCountIndexedTreeRootKeys to outer's primary
3008+
// level.
3009+
// - At outer's primary level, the pre/post element bytes for
3010+
// "inner_cidx" change (count_value 0 → 1), so outer's
3011+
// secondary entry for "inner_cidx" must move from
3012+
// (0_be ‖ inner_cidx) to (1_be ‖ inner_cidx).
3013+
//
3014+
// If outer's pre-state capture skips ReplaceCountIndexedTreeRootKeys
3015+
// ops, outer's secondary won't be mirrored — top-k on outer
3016+
// would silently return stale counts.
3017+
use crate::batch::QualifiedGroveDbOp;
3018+
3019+
let grove_version = GroveVersion::latest();
3020+
let db = make_test_grovedb(grove_version);
3021+
db.insert(
3022+
[TEST_LEAF].as_ref(),
3023+
b"outer_cidx",
3024+
Element::empty_count_indexed_tree(),
3025+
None,
3026+
None,
3027+
grove_version,
3028+
)
3029+
.unwrap()
3030+
.expect("create outer");
3031+
db.insert_into_count_indexed_tree(
3032+
[TEST_LEAF, b"outer_cidx"].as_ref(),
3033+
b"inner_cidx",
3034+
Element::empty_count_indexed_tree(),
3035+
None,
3036+
grove_version,
3037+
)
3038+
.unwrap()
3039+
.expect("create inner");
3040+
3041+
// Sanity: outer's top-k should currently show inner_cidx with
3042+
// count = 0 (newly created, empty inner).
3043+
let top = db
3044+
.count_indexed_top_k(
3045+
[TEST_LEAF, b"outer_cidx"].as_ref(),
3046+
10,
3047+
true,
3048+
None,
3049+
grove_version,
3050+
)
3051+
.unwrap()
3052+
.expect("outer top before");
3053+
assert_eq!(top.len(), 1);
3054+
assert_eq!(top[0], (0u64, b"inner_cidx".to_vec()));
3055+
3056+
// Now BATCH-insert an item into inner_cidx's primary.
3057+
let ops = vec![QualifiedGroveDbOp::insert_or_replace_op(
3058+
vec![
3059+
TEST_LEAF.to_vec(),
3060+
b"outer_cidx".to_vec(),
3061+
b"inner_cidx".to_vec(),
3062+
],
3063+
b"item".to_vec(),
3064+
Element::new_item(b"v".to_vec()),
3065+
)];
3066+
db.apply_batch(ops, None, None, grove_version)
3067+
.unwrap()
3068+
.expect("batch insert into nested cidx");
3069+
3070+
// Outer's top-k MUST now show inner_cidx with count = 1.
3071+
let top = db
3072+
.count_indexed_top_k(
3073+
[TEST_LEAF, b"outer_cidx"].as_ref(),
3074+
10,
3075+
true,
3076+
None,
3077+
grove_version,
3078+
)
3079+
.unwrap()
3080+
.expect("outer top after");
3081+
assert_eq!(top.len(), 1);
3082+
assert_eq!(
3083+
top[0],
3084+
(1u64, b"inner_cidx".to_vec()),
3085+
"outer's secondary must reflect inner's new count = 1"
3086+
);
3087+
3088+
let issues = db
3089+
.verify_grovedb(None, false, true, grove_version)
3090+
.expect("verify_grovedb");
3091+
assert!(issues.is_empty(), "verify_grovedb issues: {:?}", issues);
3092+
}
3093+
29993094
// =====================================================================
30003095
// Atomicity stress tests for batches mixing cidx + non-cidx ops.
30013096
//

0 commit comments

Comments
 (0)