diff --git a/docs/COUNTED_SKIP_DESIGN.md b/docs/COUNTED_SKIP_DESIGN.md new file mode 100644 index 000000000..be0d2edb9 --- /dev/null +++ b/docs/COUNTED_SKIP_DESIGN.md @@ -0,0 +1,246 @@ +# Counted skip for unproved ranked reads — agreed design (v2) + +**Status:** agreed, implemented on this branch. +**Branch:** `fix/counted-skip-unproved-read`, based on `a2791bb` (`develop`). +**Supersedes:** the v1 emitter-extraction proposal, preserved in git history at `356af1e8` +(`git show 356af1e8:docs/COUNTED_SKIP_DESIGN.md`). Nothing in v1 was found unsound; v2 is +strictly smaller and was reached independently by two research passes. + +## 1. History, and why the unproved route is the fix rather than an optimization + +`indexed_axis_top_k_paginated_generic` (`grovedb/src/operations/indexed_tree.rs`) skipped the +offset by stepping a storage iterator once per skipped entry — `Θ(min(offset, N))`, a +consensus-reachable linear walk (~440 ms at deep offsets on the measured Platform fixture). All +three axes (Count u64 / Sum i64 / Avg i128 fixed-point) funnel through this one function. + +The first shipped mitigation (Platform PR #4382) routed the unproved read through +prove-then-verify internally: flat ~130 µs at any offset. It then drew a **blocking review** for a +structural reason: its handler-local retry re-executed the Drive request while still holding the +`platform_state` captured before the query began, so in the window where new state is published +but the block-height guard has not yet updated, a retry could attach the *old* block's signature +and metadata to *new*-state proof bytes. A counted unproved walk has no proof envelope and needs +no retry, so that failure mode ceases to exist — this design does not work around that review, it +removes the thing the review objected to. + +v1 of this document proposed extracting the emitter's decision logic (`decide.rs`) and adding a +merk-level `read.rs` + `Merk::read_count_offset_on_range`. Two later research passes (run +independently, then compared) both rejected that shape as unnecessarily risky and converged on the +design below; the owner confirmed the direction. + +## 2. Facts about `a2791bb` the design rests on (verified by both passes) + +1. **No unproved counted-skip / rank-select primitive exists.** v1's negative claim holds: + `merk/src/proofs/query/count_offset/` is `emit / mod / prove / tests / verify`, every entry + point produces proof ops, and nothing else in the workspace descends by count. +2. **But no-proof aggregate-aware walks are established precedent.** + `Merk::count_aggregate_on_range` (`merk/src/merk/get.rs:377`), `sum_aggregate_on_range` + (`:431`) and `count_and_sum_aggregate_on_range` (`:491`) walk the tree using link aggregates + with no proof machinery, and every `aggregate_*` proof module has a no-proof `walk.rs` sibling. + `count_offset/` is the only one missing its own — and grovedb's own proved rank-of-key + generator already trusts this family (`proof/indexed_axis/generate.rs:571` computes the rank + with the *unproved* count walk). +3. **Everything a counted descent needs is public.** `Merk::walk` (`merk/src/merk/mod.rs:523`) + yields a `RefWalker`; `RefWalker::{tree, walk}`, `TreeNode::{link, key, aggregate_data}` and + `Link::aggregate_data` are all public. The load-bearing fact: **`Link::aggregate_data()` + reads off `Link::Reference`** (`merk/src/tree/link.rs:209`), so a child subtree's population + is known *before* paying a fetch for it. (It panics on `Link::Modified`, which a freshly + opened read-only merk never holds — same exposure as merk's own walkers.) The implementation + deliberately does **not** use the public `AggregateData::as_count_u64`, which silently + returns 0 for non-count variants; it uses a strict matcher that treats any non-provable-count + aggregate as corruption. v1's claim that a new entry point had to live inside merk (because + `use_tree` is `pub(crate)`) was wrong. +4. **Counted rank equals iterator position on any valid secondary.** The secondary's merk key is + the complete sort key — order-preserving axis encoding ‖ original item key + (`make_axis_secondary_key`), so in-order traversal = lexicographic order = exactly what + `raw_iter` produces, ties broken by item key inside the key bytes, total order, both + directions. Every valid row contributes structural count exactly 1: the secondaries are + `ProvableCountTree` (count axis) / `ProvableCountProvableSumTree` (sum, avg), the only writer + (`mirror_indexed_axis_to_secondary`) writes exactly one count-1 payload per primary entry, and + `verify_indexed_axis_content` enforces it. + +## 3. The agreed design + +A grovedb-local counted traversal in `operations/indexed_tree.rs`, reached through the public +`Merk::walk`. No merk change of any kind. Specialized to `RangeFull` — the paginated caller only +ever scans the whole secondary, so `classify_subtree`, inherited bounds, and the proof flow's +shape-rejection rules are all unnecessary here. + +- **`offset == 0` keeps the raw-iterator path unchanged** — it is the overwhelmingly common + shape, and one iterator seek plus `k` sequential steps is cheaper than loading a root-to-leaf + merk path. The counted traversal serves `offset > 0` only. +- **Whole-population shortcut:** the root node is already loaded by the open, and its aggregate + is the population `N`. `offset ≥ N` returns empty with **zero** additional fetches — the + past-the-end case, which is the original DoS lever, costs the open and nothing else. +- **Counted descent**, per loaded node, children visited in direction order (ascending: left + first; descending: right first): + 1. Read the first child's count from its **link**, without fetching the child. + 2. If that count ≤ remaining offset: subtract and skip the child entirely (no fetch). This is + the counted skip. + 3. Otherwise fetch (`RefWalker::walk`) and recurse. + 4. Consume the node itself: its own structural count (aggregate − left link − right link, via + `checked_sub`) must be exactly 1 — anything else is corruption and fails loud + (`Error::CorruptedData`) rather than silently diverging from positional order. If offset + remains, burn it; else emit the node's key into the page. + 5. Recurse into the second child only while the page is not full; return the moment it is — + a reader has no obligation to keep walking, unlike the prover, which must hash-bind the + rest of the tree. +- The page holds secondary **keys** only; the caller decodes them exactly as before (the old + loop also discarded values). Cost is `O(depth + k)` node fetches for `offset > 0`. +- The three paginated APIs return `IndexedTopKPage { entries, skipped }`, where `skipped` is the + **true** skipped count `min(offset, population)` — read from the root aggregate at zero extra + cost. The old linear read structurally could not report this (an offset past the end just + exhausted the iterator); the proved path attests the same quantity through its count + commitments, so unproved and proved reads now agree on it. Like the entries, the unproved + value is the local tree's claim, not independently verifiable. + +### Where the two research passes differed (recorded per the consolidation request) + +Both passes agreed on: the negative primitive claim; the walk-family precedent; link-count skip +without fetching; the root shortcut; own-count==1 fail-loud; the `offset == 0` fast path; the +`RangeFull` specialization; O(depth + k) cost; and proof-byte neutrality by construction. They +differed on two points: + +1. **Placement.** One pass preferred a merk-level `count_offset/walk.rs` twin with an entry in + `get.rs` (maximal conformance to the walk-family precedent, merk-level testability); the other + preferred the grovedb-local specialization (no new public merk API, no cross-crate release + coordination, no general-range machinery on the hot path). The owner chose grovedb-local. If a + general-range or cross-crate primitive is ever wanted, the merk-level twin is the shape — and + it must live under `merk::proofs::query`, because `classify_subtree` / + `SubtreeClassification` are `pub(super)` there. +2. **Open-cost constant** (≈2 vs ≈3–5 seeks before the descent). Settled empirically by the cost + regression test rather than argued. + +## 4. Behavior in corrupt or concurrent states (unchanged posture from v1, re-confirmed) + +- **Ghost rows** (storage rows absent from the tree, or vice versa): the old read iterates + storage, the new read walks the tree, so results in a *drifted* secondary differ. That state + is corruption by definition (`verify_grovedb` flags it; the drift tests build it deliberately), + and the proved path already walks the tree — the change makes unproved reads agree with proved + reads instead of with the raw-storage accident. +- **Corrupt keys inside the skipped region** are no longer decoded (not visiting them is the + point); returned keys are still validated, and a visited node with own-count ≠ 1 fails loud. +- **Snapshot consistency: restored after a blocking Platform review finding.** The first + implementation fetched nodes through independent `RefWalker` point-gets on a snapshotless + transaction; a commit landing mid-descent could hand back a child from a newer state than its + resident parent, and because merk's child loads never verify the fetched child against the + parent's recorded link hash — and the count cross-checks cannot see a same-population update — + the result was a *silently mixed page*. (The proved path survives the same torn reads because + verification's ancestor-chain reconciliation rejects them; the unproved read has no such + check, which is exactly why it needed the snapshot.) A second review round tightened the + boundary further: pinning only the traversal still left **root-key discovery** outside the + view, and a commit rotating the secondary root between discovery and traversal could leave the + old root key resolving to a *demoted child* in the newer view — an internally consistent + subtree that every count check accepts, silently truncating the page. The final shape + therefore pins the *entire read*: one raw iterator (implicit RocksDB snapshot at creation plus + the transaction's uncommitted-write overlay) is created under the parent merk's prefix, + re-reads the indexed element to obtain the authoritative secondary root key, is retargeted to + the secondary's prefix (`PrefixedRocksDbRawIterator::retarget`, same underlying iterator, same + snapshot), and then serves the root fetch, the descent, and the collect. **Nothing the page is + built from is read outside that one view**; the ordinary validated open still runs first, but + purely for validation and the offset-0 fast path — none of its loads are trusted as page data. + This restores, and slightly exceeds, the guarantee the replaced single-`KVIterator` linear + scan had. The transaction-overlay half is pinned by an always-on test; the + commit-interleaving half is not deterministically testable (no way to pause a synchronous + read between fetches) and rests on RocksDB's iterator-snapshot contract, as the old code's + guarantee did. +- **One function, two views.** `offset == 0` serves the storage-iterator view; `offset > 0` + serves the merk-tree view. Identical on any clean secondary; in a drifted one, a caller paging + `offset = 0, k, 2k, …` could see a discontinuity between the first page and the rest. Accepted: + the drifted state is corruption, and the pages a drifted state produces are not consistent + under the old code either (they include rows `verify_grovedb` rejects). + +## 5. Proof-byte neutrality — how it is proven rather than asserted + +The consensus-frozen envelope must not move. Three lines of proof, strongest first: + +1. **By construction:** the diff touches only `grovedb/src/operations/indexed_tree.rs` and test + files. No file under `merk/` and no grovedb proof module is modified — `git diff --stat` + against the base commit is checkable in review. The new code calls only read-only public + traversal APIs (`Merk::walk` / `RefWalker::walk`) and shares zero code with proof emission, + so no code path exists through which emitted bytes could change. +2. **Empirically:** the entire existing **proof** suites — `merk` `count_offset` tests, grovedb + `indexed_axis_proof_tests`, `indexed_axis_offset_proof_tests`, `count_offset_paginated_tests` + — pass with **zero edits**. Any proof-test edit would be a red flag, not a rebase. (One + non-proof test *was* deliberately edited: a drift-suite assertion that pinned the linear + skip's decode-during-skip behavior — see §7.4.) +3. **Root-hash pinning:** existing tests assert verified proofs against the live root hash, so + even an indirect state-shape change would surface as hash mismatches. + +This is why v1's golden-digest machinery is not needed: it existed to police an edit to +`emit.rs`, and v2 makes no such edit. + +## 6. Measured costs + +Measured with the in-repo harness (`indexed_axis_paginated_cost_tests::measure_paginated_costs`, +release build, N-row Count-axis secondaries, `k ∈ {1, 100}`, wall-clock = min of 3 runs on a +loaded shared machine — the seek/byte counters are the machine-independent signal). "linear" is +the pre-change implementation, kept verbatim as a test-only baseline; the harness also asserts +both paths return identical rows at every point. Selected rows (`k = 1`; full grid in the +harness output): + +| N | offset | counted (seeks / bytes / µs) | linear (seeks / bytes / µs) | +|---:|---:|---:|---:| +| 1e6 | 0 | 5 / 629 / 7 | 5 / 629 / 6 | +| 1e6 | N−1 | 24 / 5,985 / 32 | 1,000,004 / 316 MB / 306,699 | +| 1e6 | ≥ N | 5 / 817 / 9 | 1,000,004 / 316 MB / 312,300 | + +(Numbers are from the final fully-pinned implementation. Relative to the original point-get +walk, the pinned view costs two extra seeks — the in-view re-read of the indexed element that +carries the authoritative secondary root key, and the root node fetch — and charges full +prefixed key bytes per fetch. Wall-clock is unchanged; the past-the-end shape stays flat at +every N.) + +What the numbers establish: + +- **Offset 0 has zero regression** — identical seeks, bytes, and wall-clock in both directions + and at both `k` values, at every N (the fast path *is* the old code, and the always-on + `paginated_offset_zero_costs_exactly_plain_top_k` test pins the equality). +- **The deep-offset seek count is the tree depth plus the two pinned-view discovery reads**: + 13 → 17 → 20 → 24 across N = 1e3 → 1e6, logarithmic exactly as designed; wall-clock 14–37 µs + against the linear walk's 268 µs–312 ms. +- **Past-the-end — the DoS lever — is flat 5 seeks / ~817 B / ≤ 11 µs at every N**: the + in-view element and root reads alone answer it with zero descent. +- Against the previously measured prove-then-verify route (78–129 µs flat, plus its proof + construction/serialization/verification CPU), the counted read measures 6–37 µs at `k = 1` — + the "unproved is even faster" claim is now measured, not modeled. +- **The one corner where the old path was faster, measured and accepted:** tiny positive offsets. + At `offset = 1, k = 100` the counted path costs 107–112 seeks / ~14 KB / 141–155 µs against the + linear read's 105 seeks / ~32 KB / ~30 µs — near-identical counters, ~5× wall-clock, because k + tree-node point-gets are slower than k sequential iterator steps. The crossover to + counted-wins sits around offset ≈ a few hundred rows. Absolute worst measured cost is ~155 µs + (the same order as prove-then-verify's flat floor), the shape decays as offset grows, and the + alternative — a threshold hybrid falling back to the linear skip for small offsets — would make + skipped-region error semantics depend on the offset value. Accepted as-is; §8 keeps the hybrid + on record if max-cost-per-request ever matters more than semantic uniformity. + +## 7. Test plan + +1. **Equality grid** (pins behavior; green before and after): the unchanged `indexed_*_top_k` + iterator path is the oracle — `paginated(k, offset) == top_k(offset + k)[offset..]` across all + three axes, both directions, offsets at and around subtree boundaries / population − 1 / + population / past-end, `k ∈ {0, 1, small, larger-than-remainder}`, and tie-heavy populations + exercising the item-key tiebreak. +2. **Offset-0 non-regression pin:** `paginated(k, 0)` must cost exactly what `top_k(k)` costs in + `seek_count` — the fast path is the old code, and this assertion keeps it that way. +3. **The counted-skip assertion** — the caller-usable proof that the skip is counted rather than + linear. Platform's executors discard `CostContext`, so this must live grovedb-side, and it + can: every indexed API returns `CostResult`. On an N-row fixture, deep-offset `seek_count` + must stay within `offset-0 seek_count + small multiple of the AVL depth bound + (1.44·log2(N+2))`, and past-the-end must not exceed the offset-0 cost. **Red before this + change** (the linear walk pays ~N seeks), green after — the red run is recorded in the commit + message. The bound encodes *why* ("the offset costs at most one descent"), so it fails again + if anyone reintroduces per-entry skip work. +4. **Existing suites** — every proof suite passes with zero edits (§5). Exactly one existing + test changed, deliberately: the drift-suite case that asserted the linear skip decodes every + skipped row (`indexed_tree_secondary_drift_tests`, the `offset = 1` assertion). Under the + counted skip, a malformed-but-tree-resident row still occupies its position (it is counted) + but its key is never decoded, so the read serves the positionally identical page instead of + erroring; the assertion now pins that contract. This is §4's second bullet made concrete — + detection of skipped-region corruption belongs to `verify_grovedb` and to the shapes that + actually decode the row. + +## 8. Explicitly out of scope + +- Snapshot isolation for unproved reads (pre-existing, storage-layer). +- A general-range merk-level walker and the large-`k` iterator-hybrid collect — neither is needed + by the only caller; both are recorded above should the need appear. diff --git a/grovedb/src/lib.rs b/grovedb/src/lib.rs index a214771b9..6c8082b60 100644 --- a/grovedb/src/lib.rs +++ b/grovedb/src/lib.rs @@ -246,6 +246,13 @@ use grovedb_visualize::DebugByteVectors; /// callable from outside the crate but its return type unnameable. #[cfg(feature = "minimal")] pub use operations::get::{AxisAggregateValue, PathQueryRun}; +// Gated on `minimal` alone, matching `operations::indexed_tree` itself: the +// only APIs that produce an `IndexedTopKPage` are the paginated indexed-axis +// reads, which need storage. A verify-only build consumes proofs and can +// never name this type, so widening the gate here to `any(minimal, verify)` +// only breaks that cut on an unresolved import. +#[cfg(feature = "minimal")] +pub use operations::indexed_tree::IndexedTopKPage; #[cfg(any(feature = "minimal", feature = "verify"))] pub use query::{ aggregate_sum_path_query::AggregateSumPathQuery, AggregateKind, GroveBranchQueryResult, diff --git a/grovedb/src/operations/get/run_path_query.rs b/grovedb/src/operations/get/run_path_query.rs index ae680c7b1..ae9b56a12 100644 --- a/grovedb/src/operations/get/run_path_query.rs +++ b/grovedb/src/operations/get/run_path_query.rs @@ -509,6 +509,7 @@ impl GroveDb { transaction, grove_version ) + .map_ok(|page| page.entries) )), IndexAxis::Sum => AxisEntries::Sum(cost_return_on_error!( &mut cost, @@ -520,6 +521,7 @@ impl GroveDb { transaction, grove_version ) + .map_ok(|page| page.entries) )), IndexAxis::Avg => AxisEntries::Avg(cost_return_on_error!( &mut cost, @@ -531,6 +533,7 @@ impl GroveDb { transaction, grove_version ) + .map_ok(|page| page.entries) )), }; Ok(entries).wrap_with_cost(cost) diff --git a/grovedb/src/operations/indexed_tree.rs b/grovedb/src/operations/indexed_tree.rs index 7859a6e37..81ac1f3ce 100644 --- a/grovedb/src/operations/indexed_tree.rs +++ b/grovedb/src/operations/indexed_tree.rs @@ -41,12 +41,13 @@ use grovedb_merk::{ }, merk::KVIterator, proofs::Query, + tree::{kv::ValueDefinedCostType, AggregateData, TreeNode}, Merk, TreeType, }; use grovedb_path::SubtreePath; use grovedb_storage::{ rocksdb_storage::{PrefixedRocksDbTransactionContext, RocksDbStorage}, - Storage, StorageBatch, StorageContext, + RawIterator, Storage, StorageBatch, StorageContext, }; use grovedb_version::version::GroveVersion; @@ -1297,28 +1298,7 @@ impl GroveDb { self.open_validated_axis_secondary(path, axis, tx_ref, grove_version) ); - let mut all_query = Query::new(); - all_query.left_to_right = !descending; - all_query.insert_all(); - - let mut iter = KVIterator::new(secondary_merk.storage.raw_iter(), &all_query) - .unwrap_add_cost(&mut cost); - - let mut results = Vec::with_capacity(k as usize); - while results.len() < k as usize { - match iter.next_kv().unwrap_add_cost(&mut cost) { - Some((secondary_key, _)) => match decode(&secondary_key) { - Some(decoded) => results.push(decoded), - None => { - return Err(corrupted_secondary_key_error(axis, &secondary_key)) - .wrap_with_cost(cost); - } - }, - None => break, - } - } - - Ok(results).wrap_with_cost(cost) + collect_top_k_via_iterator(&secondary_merk, axis, k, descending, &decode).add_cost(cost) } /// One implementation of the `indexed__top_k_paginated` shape. @@ -1332,7 +1312,7 @@ impl GroveDb { transaction: TransactionArg, grove_version: &GroveVersion, decode: impl Fn(&[u8]) -> Option<(T, Vec)>, - ) -> CostResult)>, Error> + ) -> CostResult, Error> where B: AsRef<[u8]> + 'b, { @@ -1346,46 +1326,111 @@ impl GroveDb { let secondary_merk = cost_return_on_error!( &mut cost, - self.open_validated_axis_secondary(path, axis, tx_ref, grove_version) - ); - - let mut all_query = Query::new(); - all_query.left_to_right = !descending; - all_query.insert_all(); - - let mut iter = KVIterator::new(secondary_merk.storage.raw_iter(), &all_query) + self.open_validated_axis_secondary(path.clone(), axis, tx_ref, grove_version) + ); + + // `offset == 0` is the overwhelmingly common shape, and the raw + // iterator is the cheapest way to serve it: one directional seek + // plus `k` sequential steps, no tree-path loads. It shares the + // `top_k` core's implementation, so "offset 0 costs exactly what + // plain top-k costs" is structural, not coincidental. Zero offset + // skips zero entries, so `skipped = min(0, population) = 0` needs + // no tree read. + if offset == 0 { + return collect_top_k_via_iterator(&secondary_merk, axis, k, descending, &decode) + .map_ok(|entries| IndexedTopKPage { + entries, + skipped: 0, + }) + .add_cost(cost); + } + // The open above serves validation (path shape, element variant, + // axis compatibility) and the offset-0 fast path only. For the + // counted read, nothing it loaded is trusted as page data. + drop(secondary_merk); + + // `offset > 0`: counted skip. NOTHING THE PAGE IS BUILT FROM IS + // READ OUTSIDE ONE PINNED VIEW: a single raw iterator (implicit + // RocksDB snapshot at creation, plus the transaction's own + // uncommitted writes) serves the indexed element's re-read — the + // authoritative secondary root key — and then, retargeted to the + // secondary's prefix, the root node and every descent and collect + // fetch. Discovering the root key outside the view would let a + // commit that rotates the secondary root between discovery and + // traversal leave the old root key resolving to a *demoted child* + // in the newer view: an internally consistent subtree that every + // count check accepts, silently truncating the page. Re-reading + // the element inside the view closes that hole. + let Some((parent_path, indexed_key)) = path.derive_parent() else { + // Unreachable: the validated open above already rejected the + // root path. + return Err(Error::InvalidPath( + "cannot query an indexed tree at the root path".to_string(), + )) + .wrap_with_cost(cost); + }; + let parent_prefix = + RocksDbStorage::build_prefix(parent_path.clone()).unwrap_add_cost(&mut cost); + let primary_prefix = RocksDbStorage::build_prefix(path).unwrap_add_cost(&mut cost); + let secondary_prefix = RocksDbStorage::secondary_prefix_for(&primary_prefix, axis.tag()) + .unwrap_add_cost(&mut cost); + let parent_ctx = self + .db + .get_transactional_storage_context_by_subtree_prefix(parent_prefix, None, tx_ref) .unwrap_add_cost(&mut cost); - // Skip `offset` entries; surface corruption defensively. - let mut skipped: u64 = 0; - while skipped < offset { - match iter.next_kv().unwrap_add_cost(&mut cost) { - Some((secondary_key, _)) => { - if decode(&secondary_key).is_none() { - return Err(corrupted_secondary_key_error(axis, &secondary_key)) - .wrap_with_cost(cost); - } - skipped += 1; - } - None => return Ok(Vec::new()).wrap_with_cost(cost), + // The pinned view. Created under the parent merk's prefix to read + // the indexed element, then retargeted to the secondary's prefix + // for the traversal — same underlying iterator, same snapshot. + let mut view = parent_ctx.raw_iter(); + let parent_node = match cost_return_on_error!( + &mut cost, + snapshot_fetch_node(&mut view, indexed_key, grove_version) + ) { + Some(node) => node, + None => { + return Err(Error::CorruptedData( + "indexed-tree element is absent from the read snapshot — the tree was \ + removed between validation and read" + .to_string(), + )) + .wrap_with_cost(cost); } - } + }; + let element = cost_return_on_error_no_add!( + cost, + Element::deserialize(parent_node.value_as_slice(), grove_version).map_err(|e| { + Error::CorruptedData(format!("indexed-tree element failed to deserialize: {e}")) + }) + ); + let secondary_root_key = cost_return_on_error_no_add!( + cost, + axis_secondary_root_key_from_element(axis, &element) + ); + let view = view.retarget(secondary_prefix); - let mut results = Vec::with_capacity(k as usize); - while results.len() < k as usize { - match iter.next_kv().unwrap_add_cost(&mut cost) { - Some((secondary_key, _)) => match decode(&secondary_key) { - Some(decoded) => results.push(decoded), - None => { - return Err(corrupted_secondary_key_error(axis, &secondary_key)) - .wrap_with_cost(cost); - } - }, - None => break, + let (secondary_keys, skipped) = cost_return_on_error!( + &mut cost, + counted_skip_page( + view, + secondary_root_key, + offset, + u64::from(k), + !descending, + grove_version + ) + ); + let mut entries = Vec::with_capacity(secondary_keys.len()); + for secondary_key in secondary_keys { + match decode(&secondary_key) { + Some(decoded) => entries.push(decoded), + None => { + return Err(corrupted_secondary_key_error(axis, &secondary_key)) + .wrap_with_cost(cost); + } } } - - Ok(results).wrap_with_cost(cost) + Ok(IndexedTopKPage { entries, skipped }).wrap_with_cost(cost) } /// One implementation of the `indexed__range` shape. The @@ -1496,13 +1541,19 @@ impl GroveDb { /// entries in the directional scan before collecting up to `k` /// results. /// - /// `offset = 0` is equivalent to plain `indexed_count_top_k`. The - /// skip is performed at the secondary's storage iterator level — - /// this is not a verifiable / proof-bounded skip; for the provable - /// variant use - /// [`Self::prove_indexed_count_top_k_paginated`] which relies on the - /// merk-level count-offset proof to commit the skipped count via - /// `HashWithCount`. + /// `offset = 0` is equivalent to plain `indexed_count_top_k` and is + /// served by the same storage-iterator scan. A positive `offset` is + /// skipped by counted descent over the secondary merk — subtrees are + /// consumed from their aggregate counts without loading their + /// entries, so the skip costs `O(log n)` node loads rather than + /// `O(offset)` iterator steps. The returned + /// [`IndexedTopKPage::skipped`] is the true skipped count, + /// `min(offset, population)` — an offset past the end reports the + /// secondary's population rather than echoing the request. Neither + /// shape is a verifiable / proof-bounded read; for the provable + /// variant use [`Self::prove_indexed_count_top_k_paginated`] which + /// relies on the merk-level count-offset proof to commit the skipped + /// count via `HashWithCount`. pub fn indexed_count_top_k_paginated<'b, B, P>( &self, path: P, @@ -1511,7 +1562,7 @@ impl GroveDb { descending: bool, transaction: TransactionArg, grove_version: &GroveVersion, - ) -> CostResult)>, Error> + ) -> CostResult, Error> where B: AsRef<[u8]> + 'b, P: Into>, @@ -1706,9 +1757,11 @@ impl GroveDb { /// Paginated form of [`Self::indexed_sum_top_k`]. Skips `offset` /// entries in the directional scan before collecting up to `k` - /// results. `offset = 0` is equivalent to plain - /// `indexed_sum_top_k`; the skip is iterator-level only (not - /// proof-bounded). + /// results. `offset = 0` is equivalent to plain `indexed_sum_top_k`; + /// a positive `offset` is skipped by counted descent over the + /// secondary merk in `O(log n)` node loads, and + /// [`IndexedTopKPage::skipped`] reports the true + /// `min(offset, population)`. Not a verifiable / proof-bounded read. pub fn indexed_sum_top_k_paginated<'b, B, P>( &self, path: P, @@ -1717,7 +1770,7 @@ impl GroveDb { descending: bool, transaction: TransactionArg, grove_version: &GroveVersion, - ) -> CostResult)>, Error> + ) -> CostResult, Error> where B: AsRef<[u8]> + 'b, P: Into>, @@ -2030,7 +2083,11 @@ impl GroveDb { /// Paginated form of [`Self::indexed_avg_top_k`]. Skips `offset` /// entries in the directional scan before collecting up to `k` - /// results. + /// results. `offset = 0` is equivalent to plain `indexed_avg_top_k`; + /// a positive `offset` is skipped by counted descent over the + /// secondary merk in `O(log n)` node loads, and + /// [`IndexedTopKPage::skipped`] reports the true + /// `min(offset, population)`. Not a verifiable / proof-bounded read. pub fn indexed_avg_top_k_paginated<'b, B, P>( &self, path: P, @@ -2039,7 +2096,7 @@ impl GroveDb { descending: bool, transaction: TransactionArg, grove_version: &GroveVersion, - ) -> CostResult)>, Error> + ) -> CostResult, Error> where B: AsRef<[u8]> + 'b, P: Into>, @@ -2212,34 +2269,7 @@ impl GroveDb { &mut cost, Element::get(&parent_merk, indexed_key, true, grove_version).map_err(Error::MerkError) ); - match (axis, element.underlying()) { - // PCIT carries a single secondary; only Count axis is valid. - (IndexAxis::Count, Element::ProvableCountIndexedTree(_, secondary, ..)) => { - Ok(secondary.clone()).wrap_with_cost(cost) - } - // PSIT carries a single secondary; only Sum axis is valid. - (IndexAxis::Sum, Element::ProvableSumIndexedTree(_, secondary, ..)) => { - Ok(secondary.clone()).wrap_with_cost(cost) - } - // PCPSIT carries a TLV of 1..=3 axis-tagged secondaries; the - // requested axis must appear in the TLV. - (_, Element::ProvableCountProvableSumIndexedTree(_, _, _, axes, _)) => { - let want_tag = axis.tag(); - match axes.iter().find(|(t, _)| *t == want_tag) { - Some((_, sec)) => Ok(sec.clone()).wrap_with_cost(cost), - None => Err(Error::InvalidPath(format!( - "{:?} axis not indexed at this path", - axis - ))) - .wrap_with_cost(cost), - } - } - _ => Err(Error::InvalidPath(format!( - "{:?} axis not indexed at this path", - axis - ))) - .wrap_with_cost(cost), - } + axis_secondary_root_key_from_element(axis, &element).wrap_with_cost(cost) } /// Delete an item from a `CountIndexedTree` element. Removes the @@ -2614,6 +2644,523 @@ fn corrupted_secondary_key_error(axis: IndexAxis, secondary_key: &[u8]) -> Error )) } +/// Directional top-`k` collect over the secondary's storage iterator. +/// Shared by the `top_k` core and the paginated core's `offset == 0` fast +/// path, so "offset 0 costs exactly what plain top-k costs" is a +/// structural fact rather than two implementations kept in sync. +fn collect_top_k_via_iterator<'db, S: StorageContext<'db>, T>( + secondary_merk: &Merk, + axis: IndexAxis, + k: u16, + descending: bool, + decode: &impl Fn(&[u8]) -> Option<(T, Vec)>, +) -> CostResult)>, Error> { + let mut cost = OperationCost::default(); + + let mut all_query = Query::new(); + all_query.left_to_right = !descending; + all_query.insert_all(); + + let mut iter = + KVIterator::new(secondary_merk.storage.raw_iter(), &all_query).unwrap_add_cost(&mut cost); + + let mut results = Vec::with_capacity(k as usize); + while results.len() < k as usize { + match iter.next_kv().unwrap_add_cost(&mut cost) { + Some((secondary_key, _)) => match decode(&secondary_key) { + Some(decoded) => results.push(decoded), + None => { + return Err(corrupted_secondary_key_error(axis, &secondary_key)) + .wrap_with_cost(cost); + } + }, + None => break, + } + } + + Ok(results).wrap_with_cost(cost) +} + +/// Strict provable-count read of an aggregate. The counted skip only ever +/// runs against axis secondaries, whose tree types (`ProvableCountTree` / +/// `ProvableCountProvableSumTree`) bind a provable count into every node; +/// any other aggregate shape here is corruption, never a fallback. +/// Deliberately not `AggregateData::as_count_u64`, which returns 0 for +/// non-count variants — silently, which is exactly what this matcher +/// exists to prevent. Mirrors merk's `pub(super)` +/// `provable_count_from_aggregate` (unreachable from here). +#[inline] +fn provable_count_from_aggregate(aggregate: AggregateData) -> Result { + match aggregate { + AggregateData::ProvableCount(c) + | AggregateData::ProvableCountAndSum(c, _) + | AggregateData::ProvableCountAndProvableSum(c, _) => Ok(c), + other => Err(Error::CorruptedData(format!( + "indexed secondary node carries a non-provable-count aggregate: {:?}", + other + ))), + } +} + +/// One page of an `indexed__top_k_paginated` read. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct IndexedTopKPage { + /// Page entries, `(axis_value, original_key)`, in directional order. + pub entries: Vec<(T, Vec)>, + /// How many entries the offset actually skipped: + /// `min(offset, population)`. When the offset runs past the end this + /// reports the secondary's true population instead of echoing the + /// request — the same quantity the proved path attests through its + /// count commitments, though here it is the local tree's unverified + /// claim, like the entries themselves. + pub skipped: u64, +} + +/// Mutable state threaded through the counted descent. +struct CountedPageState { + /// In-range entries still to skip before returning starts. + offset_remaining: u64, + /// Page slots still to fill. The recursion is only ever entered while + /// this is non-zero and unwinds the moment it reaches zero. + limit_remaining: u64, + /// `true` = ascending (left child first), `false` = descending. + left_to_right: bool, +} + +/// Pure extraction of the per-axis `secondary_root_key` from an +/// indexed-tree element. Shared by the merk-backed reader (which drives +/// every per-axis query API's validation) and by the counted paginated +/// path's pinned-view re-read, so the two agree on axis-compatibility by +/// construction. +fn axis_secondary_root_key_from_element( + axis: IndexAxis, + element: &Element, +) -> Result>, Error> { + match (axis, element.underlying()) { + // PCIT carries a single secondary; only Count axis is valid. + (IndexAxis::Count, Element::ProvableCountIndexedTree(_, secondary, ..)) => { + Ok(secondary.clone()) + } + // PSIT carries a single secondary; only Sum axis is valid. + (IndexAxis::Sum, Element::ProvableSumIndexedTree(_, secondary, ..)) => { + Ok(secondary.clone()) + } + // PCPSIT carries a TLV of 1..=3 axis-tagged secondaries; the + // requested axis must appear in the TLV. + (_, Element::ProvableCountProvableSumIndexedTree(_, _, _, axes, _)) => { + let want_tag = axis.tag(); + match axes.iter().find(|(t, _)| *t == want_tag) { + Some((_, sec)) => Ok(sec.clone()), + None => Err(Error::InvalidPath(format!( + "{:?} axis not indexed at this path", + axis + ))), + } + } + _ => Err(Error::InvalidPath(format!( + "{:?} axis not indexed at this path", + axis + ))), + } +} + +/// Recursion ceiling for the counted descent. An AVL tree cannot exceed +/// 1.44·64 ≈ 93 levels even at the u64 population limit, so anything +/// deeper means a corrupt link structure (e.g. a cyclic link) — fail with +/// an error instead of overflowing the stack. +const COUNTED_SKIP_MAX_DEPTH: u32 = 128; + +/// Read a child link's provable count. A present link points at a +/// non-empty subtree, whose count is therefore at least 1 — a zero is +/// corruption, not an empty side (that is `None`). +fn provable_count_from_link(link: Option<&grovedb_merk::tree::Link>) -> Result { + match link { + None => Ok(0), + Some(link) => match provable_count_from_aggregate(link.aggregate_data())? { + 0 => Err(Error::CorruptedData( + "secondary link is present but carries aggregate count 0".to_string(), + )), + count => Ok(count), + }, + } +} + +/// Serve one page of secondary keys at `offset` in directional order by +/// counted descent over the secondary merk: subtrees whose whole +/// population fits inside the remaining offset are consumed from their +/// parent's link aggregate without being fetched, so the skip costs one +/// root-to-position path instead of one step per skipped entry. Returns +/// the raw secondary keys (`sort_key ‖ item_key`) plus the true skipped +/// count, `min(offset, population)`; values are never needed — the +/// caller decodes keys exactly as the iterator path does. +/// +/// **Every node in the page comes from one pinned view.** The caller +/// hands in the raw iterator already carrying the view that the +/// secondary root key was discovered in (retargeted to the secondary's +/// prefix), and the root fetch, descent, and collect all go through it. +/// A RocksDB transaction iterator pins an implicit snapshot of the +/// committed state at creation plus the transaction's own uncommitted +/// writes — the same consistency guarantee the replaced linear scan had +/// from its single `KVIterator`. Independent point-gets through the +/// (snapshotless) transaction would not have it: a commit landing +/// mid-descent could hand back a child from a newer state than its +/// resident parent, and merk's child loads do not verify the child +/// against the parent's recorded link hash, so the result would be a +/// silently mixed page rather than an error. +fn counted_skip_page( + mut iter: I, + root_key: Option>, + offset: u64, + limit: u64, + left_to_right: bool, + grove_version: &GroveVersion, +) -> CostResult<(Vec>, u64), Error> { + let mut cost = OperationCost::default(); + + let Some(root_key) = root_key else { + // Empty secondary: nothing to skip, nothing to return. + return Ok((Vec::new(), 0)).wrap_with_cost(cost); + }; + + // The root key was read from the indexed element inside this same + // view, so a miss here is corruption, not a race. + let root = match cost_return_on_error!( + &mut cost, + snapshot_fetch_node(&mut iter, &root_key, grove_version) + ) { + Some(root) => root, + None => { + return Err(Error::CorruptedData( + "secondary root node named by the indexed element is absent from the same \ + read snapshot" + .to_string(), + )) + .wrap_with_cost(cost); + } + }; + + let population = cost_return_on_error_no_add!( + cost, + root.aggregate_data() + .map_err(|e| Error::CorruptedData(format!("secondary aggregate_data: {e}"))) + .and_then(provable_count_from_aggregate) + ); + let skipped = offset.min(population); + if limit == 0 || offset >= population { + return Ok((Vec::new(), skipped)).wrap_with_cost(cost); + } + let mut state = CountedPageState { + offset_remaining: offset, + limit_remaining: limit, + left_to_right, + }; + // Pre-allocation is a hint, not a promise: `limit` is caller-supplied + // and `population` comes from an on-disk aggregate, so an unclamped + // capacity would let a huge limit (or a forged aggregate) reserve + // memory the page can never fill. The vector grows past the clamp + // only by actually being filled, one visited node at a time. + const PAGE_CAPACITY_CLAMP: usize = 1024; + let page_len = (population - offset).min(limit) as usize; + let mut out = Vec::with_capacity(page_len.min(PAGE_CAPACITY_CLAMP)); + cost_return_on_error!( + &mut cost, + counted_skip_collect(&mut iter, &root, &mut state, &mut out, 0, grove_version) + ); + Ok((out, skipped)).wrap_with_cost(cost) +} + +/// Fetch and decode one merk node from the pinned iterator view. Returns +/// `Ok(None)` when the key is absent from that view. +fn snapshot_fetch_node( + iter: &mut I, + key: &[u8], + grove_version: &GroveVersion, +) -> CostResult, Error> { + let mut cost = OperationCost::default(); + iter.seek(key).unwrap_add_cost(&mut cost); + match iter.key().unwrap_add_cost(&mut cost) { + Some(found) if found == key => {} + _ => return Ok(None).wrap_with_cost(cost), + } + let Some(bytes) = iter.value().unwrap_add_cost(&mut cost) else { + return Ok(None).wrap_with_cost(cost); + }; + TreeNode::decode( + key.to_vec(), + bytes, + None:: Option>, + grove_version, + ) + .map(Some) + .map_err(|e| Error::CorruptedData(format!("secondary node failed to decode: {e}"))) + .wrap_with_cost(cost) +} + +/// Recursive counted descent over nodes served by the pinned iterator. +/// Entered only on subtrees whose population exceeds the remaining +/// offset (the caller and both descend sites guarantee it) and only +/// while the page has room. +fn counted_skip_collect( + iter: &mut I, + node: &TreeNode, + state: &mut CountedPageState, + out: &mut Vec>, + depth: u32, + grove_version: &GroveVersion, +) -> CostResult<(), Error> { + let mut cost = OperationCost::default(); + + if depth > COUNTED_SKIP_MAX_DEPTH { + return Err(Error::CorruptedData(format!( + "secondary tree exceeds the maximum plausible depth {COUNTED_SKIP_MAX_DEPTH} — link \ + structure is corrupt" + ))) + .wrap_with_cost(cost); + } + + let node_count = cost_return_on_error_no_add!( + cost, + node.aggregate_data() + .map_err(|e| Error::CorruptedData(format!("secondary aggregate_data: {e}"))) + .and_then(provable_count_from_aggregate) + ); + let left_count = cost_return_on_error_no_add!(cost, provable_count_from_link(node.link(true))); + let right_count = + cost_return_on_error_no_add!(cost, provable_count_from_link(node.link(false))); + + // Every valid secondary row contributes structural count exactly 1 + // (`mirror_indexed_axis_to_secondary` writes nothing else, and + // `verify_indexed_axis_content` enforces it). This is a payload check + // on the node's own count value — a link whose cached count disagrees + // with its child's real subtree is caught separately, by the + // cross-check in `counted_skip_descend`. + let own_count = node_count + .checked_sub(left_count) + .and_then(|n| n.checked_sub(right_count)); + if own_count != Some(1) { + return Err(Error::CorruptedData(format!( + "indexed secondary node must have own structural count 1: aggregate {} with child \ + counts {} + {}", + node_count, left_count, right_count + ))) + .wrap_with_cost(cost); + } + + let (first_is_left, first_count, second_count) = if state.left_to_right { + (true, left_count, right_count) + } else { + (false, right_count, left_count) + }; + + // First child in directional order: consumed wholesale from the link + // aggregate (no fetch) when its entire population fits inside the + // remaining offset; descended into otherwise. + if first_count > 0 { + if first_count <= state.offset_remaining { + state.offset_remaining -= first_count; + } else { + cost_return_on_error!( + &mut cost, + counted_skip_descend( + iter, + node, + first_is_left, + first_count, + state, + out, + depth, + grove_version + ) + ); + if state.limit_remaining == 0 { + return Ok(()).wrap_with_cost(cost); + } + } + } + + // The node itself: burn one unit of offset, or emit its key. The + // entry invariant (population > offset, page not full) makes the + // emit branch safe without re-checking `limit_remaining`. + if state.offset_remaining > 0 { + state.offset_remaining -= 1; + } else { + out.push(node.key().to_vec()); + state.limit_remaining -= 1; + if state.limit_remaining == 0 { + return Ok(()).wrap_with_cost(cost); + } + } + + // Second child. With consistent aggregates the offset can never + // swallow it whole (this frame was entered because its subtree + // outlasts the offset), but the count arithmetic keeps the skip + // branch as the safe symmetric action. + if second_count > 0 { + if second_count <= state.offset_remaining { + state.offset_remaining -= second_count; + } else { + cost_return_on_error!( + &mut cost, + counted_skip_descend( + iter, + node, + !first_is_left, + second_count, + state, + out, + depth, + grove_version + ) + ); + } + } + + Ok(()).wrap_with_cost(cost) +} + +/// Fetch one child from the pinned view and recurse into it. +/// +/// `link_count` is the aggregate count read off the parent's link — the +/// number that authorized this descent (and that whole-subtree skips +/// trust without fetching). It is cross-checked against the loaded +/// child's own aggregate, so a link whose cached count disagrees with +/// its subtree fails loud instead of shifting every position after it. +#[allow(clippy::too_many_arguments)] +fn counted_skip_descend( + iter: &mut I, + parent: &TreeNode, + left: bool, + link_count: u64, + state: &mut CountedPageState, + out: &mut Vec>, + depth: u32, + grove_version: &GroveVersion, +) -> CostResult<(), Error> { + let mut cost = OperationCost::default(); + let child_key = match parent.link(left) { + Some(link) => link.key().to_vec(), + None => { + // The caller only descends where the link (and its non-zero + // count) was just read, so this is unreachable short of a + // logic error — fail loud regardless. + return Err(Error::CorruptedData( + "secondary descend without a link".to_string(), + )) + .wrap_with_cost(cost); + } + }; + let child = match cost_return_on_error!( + &mut cost, + snapshot_fetch_node(iter, &child_key, grove_version) + ) { + Some(child) => child, + None => { + // The parent's link names a key the pinned view does not + // contain: corruption (or a view predating the parent, which + // a single snapshot rules out). + return Err(Error::CorruptedData( + "secondary link is present but its child is absent from the read snapshot" + .to_string(), + )) + .wrap_with_cost(cost); + } + }; + let child_count = cost_return_on_error_no_add!( + cost, + child + .aggregate_data() + .map_err(|e| Error::CorruptedData(format!("secondary aggregate_data: {e}"))) + .and_then(provable_count_from_aggregate) + ); + if child_count != link_count { + return Err(Error::CorruptedData(format!( + "secondary link claims aggregate count {link_count} but its subtree carries \ + {child_count}" + ))) + .wrap_with_cost(cost); + } + counted_skip_collect(iter, &child, state, out, depth + 1, grove_version).add_cost(cost) +} + +#[cfg(test)] +impl GroveDb { + /// The pre-counted-skip linear implementation, specialized to the + /// Count axis (the generic `decode`/`axis` parameters are the only + /// change from the replaced code). Kept as the measurement baseline + /// for `indexed_axis_paginated_cost_tests::measure_paginated_costs`; + /// compiled only for tests, never reachable in production builds. + pub(crate) fn legacy_linear_indexed_count_top_k_paginated<'b, B, P>( + &self, + path: P, + k: u16, + offset: u64, + descending: bool, + transaction: TransactionArg, + grove_version: &GroveVersion, + ) -> CostResult)>, Error> + where + B: AsRef<[u8]> + 'b, + P: Into>, + { + let mut cost = OperationCost::default(); + let tx = TxRef::new(&self.db, transaction); + let tx_ref = tx.as_ref(); + + let secondary_merk = cost_return_on_error!( + &mut cost, + self.open_validated_axis_secondary( + path.into(), + IndexAxis::Count, + tx_ref, + grove_version + ) + ); + + let mut all_query = Query::new(); + all_query.left_to_right = !descending; + all_query.insert_all(); + + let mut iter = KVIterator::new(secondary_merk.storage.raw_iter(), &all_query) + .unwrap_add_cost(&mut cost); + + let mut skipped: u64 = 0; + while skipped < offset { + match iter.next_kv().unwrap_add_cost(&mut cost) { + Some((secondary_key, _)) => { + if decode_secondary_key(&secondary_key).is_none() { + return Err(corrupted_secondary_key_error( + IndexAxis::Count, + &secondary_key, + )) + .wrap_with_cost(cost); + } + skipped += 1; + } + None => return Ok(Vec::new()).wrap_with_cost(cost), + } + } + + let mut results = Vec::with_capacity(k as usize); + while results.len() < k as usize { + match iter.next_kv().unwrap_add_cost(&mut cost) { + Some((secondary_key, _)) => match decode_secondary_key(&secondary_key) { + Some(decoded) => results.push(decoded), + None => { + return Err(corrupted_secondary_key_error( + IndexAxis::Count, + &secondary_key, + )) + .wrap_with_cost(cost); + } + }, + None => break, + } + } + + Ok(results).wrap_with_cost(cost) + } +} + #[cfg(test)] mod count_value_as_sum_tests { //! The count-axis secondary stores count_value as an i64 sum item; diff --git a/grovedb/src/tests/axis_descent_proof_tests.rs b/grovedb/src/tests/axis_descent_proof_tests.rs index d4e976015..06722e0ca 100644 --- a/grovedb/src/tests/axis_descent_proof_tests.rs +++ b/grovedb/src/tests/axis_descent_proof_tests.rs @@ -182,7 +182,7 @@ mod tests { ) .unwrap() .expect("direct read"); - assert_eq!(entries_as_sum(&entries), direct.as_slice()); + assert_eq!(entries_as_sum(&entries), direct.entries.as_slice()); // ...and to the standalone envelope over the same state. let standalone_bytes = db @@ -234,7 +234,7 @@ mod tests { ) .unwrap() .expect("direct"); - assert_eq!(entries_as_sum(&entries), direct.as_slice()); + assert_eq!(entries_as_sum(&entries), direct.entries.as_slice()); } other => panic!("expected AxisEntries, got {other:?}"), } @@ -358,7 +358,7 @@ mod tests { ) .unwrap() .expect("direct"); - assert_eq!(entries_as_sum(&entries), direct.as_slice()); + assert_eq!(entries_as_sum(&entries), direct.entries.as_slice()); } other => panic!("expected AxisEntries, got {other:?}"), } @@ -411,7 +411,7 @@ mod tests { .expect("direct"); assert_eq!( entries_as_sum(entries.as_ref().expect("present")), - direct.as_slice() + direct.entries.as_slice() ); } } @@ -906,7 +906,7 @@ mod tests { .indexed_count_top_k_paginated(path.as_ref(), 2, 0, true, None, grove_version) .unwrap() .expect("direct count top-k"); - assert_eq!(entries, AxisEntries::Count(direct)); + assert_eq!(entries, AxisEntries::Count(direct.entries)); } other => panic!("expected AxisEntries, got {other:?}"), } diff --git a/grovedb/src/tests/indexed_axis_paginated_cost_tests.rs b/grovedb/src/tests/indexed_axis_paginated_cost_tests.rs new file mode 100644 index 000000000..793953362 --- /dev/null +++ b/grovedb/src/tests/indexed_axis_paginated_cost_tests.rs @@ -0,0 +1,869 @@ +//! Cost and equality pins for the unproved paginated ranked reads. +//! +//! The offset skip in `indexed__top_k_paginated` must be *counted* — +//! one descent through the secondary merk, skipping whole subtrees via link +//! aggregate counts — rather than *linear* (one storage-iterator step per +//! skipped entry). Platform-side executors discard `CostContext`, so the +//! assertion that the skip is counted has to live here, where +//! `OperationCost` is visible: seek counts at a deep offset are bounded by +//! the tree depth, not by the offset. +//! +//! The equality tests pin that the counted path returns byte-identical +//! entries in identical order to the storage-iterator order, using the +//! unchanged `indexed__top_k` iterator path as the oracle. + +#[cfg(test)] +mod tests { + use grovedb_costs::CostContext; + use grovedb_version::version::GroveVersion; + + use crate::{ + batch::QualifiedGroveDbOp, + operations::proof::indexed_axis::AxisEntries, + tests::{make_test_grovedb, TempGroveDb, TEST_LEAF}, + Element, GroveDb, + }; + + /// Rows in the large tie-heavy fixture. Big enough that a linear skip + /// (≈ one seek per skipped row) is orders of magnitude over the + /// depth-bounded budget the cost test allows. + const N: u64 = 600; + const K: u16 = 5; + + /// AVL depth bound for a tree of `n` keys: 1.44·log2(n + 2). + fn avl_depth_bound(n: u64) -> u32 { + (1.44 * ((n + 2) as f64).log2()).ceil() as u32 + } + + /// PCIT with `n` Item children inserted via chunked batches. Every + /// Item child derives count = 1, so the count axis is maximally + /// tie-heavy and the secondary order is exactly the item-key tiebreak. + fn make_pcit_with_rows(n: u64, grove_version: &GroveVersion) -> TempGroveDb { + let db = make_test_grovedb(grove_version); + db.apply_batch( + vec![QualifiedGroveDbOp::insert_or_replace_op( + vec![TEST_LEAF.to_vec()], + b"cidx".to_vec(), + Element::empty_provable_count_indexed_tree(), + )], + None, + None, + grove_version, + ) + .unwrap() + .expect("create PCIT"); + const CHUNK: u64 = 50_000; + let mut next = 0u64; + while next < n { + let end = (next + CHUNK).min(n); + let ops = (next..end) + .map(|i| { + QualifiedGroveDbOp::insert_or_replace_op( + vec![TEST_LEAF.to_vec(), b"cidx".to_vec()], + format!("k{i:07}").into_bytes(), + Element::new_item(vec![]), + ) + }) + .collect(); + db.apply_batch(ops, None, None, grove_version) + .unwrap() + .expect("populate PCIT batch chunk"); + next = end; + } + db + } + + fn make_pcit_with_n_tied_rows(grove_version: &GroveVersion) -> TempGroveDb { + make_pcit_with_rows(N, grove_version) + } + + /// PCIT with count-tree children whose derived counts contain both + /// distinct values and ties (3×3 and 2×9), so the axis-value ordering + /// and the item-key tiebreak are both exercised. + fn make_pcit_with_mixed_counts(grove_version: &GroveVersion) -> (TempGroveDb, usize) { + let dataset: &[(&[u8], u64)] = &[ + (b"a", 5), + (b"b", 12), + (b"c", 1), + (b"d", 7), + (b"e", 20), + (b"f", 3), + (b"g", 3), + (b"h", 3), + (b"i", 9), + (b"j", 9), + ]; + let db = make_test_grovedb(grove_version); + db.insert( + [TEST_LEAF].as_ref(), + b"cidx", + Element::empty_provable_count_indexed_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert PCIT"); + for (key, count) in dataset { + db.insert_into_count_indexed_tree( + [TEST_LEAF, b"cidx"].as_ref(), + key, + Element::empty_count_tree(), + None, + grove_version, + ) + .unwrap() + .expect("insert count-tree child"); + for i in 0..*count { + db.insert( + [TEST_LEAF, b"cidx", key].as_ref(), + &i.to_be_bytes(), + Element::new_item(vec![]), + None, + None, + grove_version, + ) + .unwrap() + .expect("populate count-tree child"); + } + } + (db, dataset.len()) + } + + /// PCPSIT (all three axes) with count-sum-tree children whose derived + /// sums include a tie group (10 appears three times), so the sum and + /// avg axes exercise the item-key tiebreak too. + fn make_pcpsit_with_mixed_sums(grove_version: &GroveVersion) -> (TempGroveDb, usize) { + let dataset: &[(&[u8], u64, i64)] = &[ + (b"a", 2, 10), + (b"b", 4, 100), + (b"c", 5, -25), + (b"d", 1, 0), + (b"e", 3, 9), + (b"f", 1, 10), + (b"g", 2, 10), + ]; + let db = make_test_grovedb(grove_version); + db.insert( + [TEST_LEAF].as_ref(), + b"pcpsit", + Element::empty_provable_count_provable_sum_indexed_tree(vec![ + (0u8, None), + (1u8, None), + (2u8, None), + ]) + .expect("canonical axes"), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert PCPSIT"); + for (key, count, sum) in dataset { + db.insert_into_provable_count_provable_sum_indexed_tree( + [TEST_LEAF, b"pcpsit"].as_ref(), + key, + Element::empty_count_sum_tree(), + None, + grove_version, + ) + .unwrap() + .expect("insert count-sum-tree child"); + for i in 0..*count { + db.insert( + [TEST_LEAF, b"pcpsit", key].as_ref(), + &i.to_be_bytes(), + Element::new_sum_item(if i == 0 { *sum } else { 0 }), + None, + None, + grove_version, + ) + .unwrap() + .expect("populate count-sum-tree child"); + } + } + (db, dataset.len()) + } + + // ----------------------------------------------------------------- + // The counted-skip assertion: offset cost is bounded by tree depth, + // never by the offset itself. + // ----------------------------------------------------------------- + + #[test] + fn paginated_offset_skip_is_counted_not_linear() { + let grove_version = GroveVersion::latest(); + let db = make_pcit_with_n_tied_rows(grove_version); + let path = [TEST_LEAF, b"cidx"]; + + for descending in [false, true] { + let CostContext { + value: page0, + cost: cost_0, + } = db.indexed_count_top_k_paginated( + path.as_ref(), + K, + 0, + descending, + None, + grove_version, + ); + let page0 = page0.expect("offset 0"); + assert_eq!(page0.entries.len(), K as usize); + assert_eq!(page0.skipped, 0, "offset 0 skips nothing"); + + let far_offset = N - K as u64; + let CostContext { + value: page_far, + cost: cost_far, + } = db.indexed_count_top_k_paginated( + path.as_ref(), + K, + far_offset, + descending, + None, + grove_version, + ); + let page_far = page_far.expect("deep offset"); + assert_eq!( + page_far.entries.len(), + K as usize, + "last full page must exist" + ); + assert_eq!( + page_far.skipped, far_offset, + "true skipped at a deep offset" + ); + + let depth_bound = avl_depth_bound(N); + + // A counted skip pays at most one root-to-position descent (plus + // the k-collect walk) for the offset — never per-skipped-entry + // work. A linear skip pays ≈ one seek per skipped row (~595 + // here), which blows this budget by an order of magnitude. + assert!( + cost_far.seek_count <= cost_0.seek_count + 3 * depth_bound, + "offset skip is walking entries (descending={descending}): seek_count {} at \ + offset {} vs {} at offset 0 (depth bound {})", + cost_far.seek_count, + far_offset, + cost_0.seek_count, + depth_bound, + ); + // ~300 B per fetched node; 512 B/level keeps 2x headroom while + // staying an order of magnitude under the skipped region's bytes. + assert!( + cost_far.storage_loaded_bytes + <= cost_0.storage_loaded_bytes + u64::from(depth_bound) * 512, + "offset skip is loading the skipped region (descending={descending}): {} bytes \ + at offset {} vs {} at offset 0", + cost_far.storage_loaded_bytes, + far_offset, + cost_0.storage_loaded_bytes, + ); + + // Past the end: the root aggregate alone proves the offset + // exceeds the population — no descent at all. The counted path + // pays the open plus exactly two pinned-view discovery reads + // (the indexed element carrying the authoritative secondary + // root key, then the root node itself, both through the one + // snapshot the whole page is served from). Offset 0 costs open + // + iterator seek + K collect steps, so "no descent" pins as: + // past-end + the collect work fits inside the offset-0 budget + // plus those two discovery reads. + let CostContext { + value: past, + cost: cost_past, + } = db.indexed_count_top_k_paginated( + path.as_ref(), + K, + N + 100, + descending, + None, + grove_version, + ); + let past = past.expect("past-end offset"); + assert!( + past.entries.is_empty(), + "past-end offset must return an empty page" + ); + assert_eq!( + past.skipped, N, + "past-end must report the true population, not echo the request" + ); + assert!( + cost_past.seek_count + u32::from(K) <= cost_0.seek_count + 2, + "past-end offset must not descend (descending={descending}): seek_count {} vs \ + {} at offset 0 (k = {K}, 2 pinned-view discovery reads allowed)", + cost_past.seek_count, + cost_0.seek_count, + ); + } + } + + /// An empty secondary has no root to descend into: any offset must + /// return an empty page without erroring, on both the iterator path + /// (offset 0) and the counted path (offset > 0). + #[test] + fn paginated_on_empty_secondary_returns_empty_at_any_offset() { + let grove_version = GroveVersion::latest(); + let db = make_pcit_with_rows(0, grove_version); + let path = [TEST_LEAF, b"cidx"]; + + for descending in [false, true] { + for offset in [0u64, 1, 1_000_000] { + let page = db + .indexed_count_top_k_paginated( + path.as_ref(), + 3, + offset, + descending, + None, + grove_version, + ) + .unwrap() + .expect("empty secondary must serve an empty page"); + assert!( + page.entries.is_empty(), + "offset={offset} descending={descending} must be empty" + ); + assert_eq!(page.skipped, 0, "an empty secondary has nothing to skip"); + } + } + } + + // ----------------------------------------------------------------- + // Offset-0 non-regression: the common shape stays on the iterator + // path, at exactly the plain top-k cost. + // ----------------------------------------------------------------- + + #[test] + fn paginated_offset_zero_costs_exactly_plain_top_k() { + let grove_version = GroveVersion::latest(); + let db = make_pcit_with_n_tied_rows(grove_version); + let path = [TEST_LEAF, b"cidx"]; + + for descending in [false, true] { + let CostContext { + value: plain, + cost: cost_plain, + } = db.indexed_count_top_k(path.as_ref(), K, descending, None, grove_version); + let CostContext { + value: paginated, + cost: cost_paginated, + } = db.indexed_count_top_k_paginated( + path.as_ref(), + K, + 0, + descending, + None, + grove_version, + ); + let paginated = paginated.expect("paginated offset 0"); + assert_eq!( + plain.expect("plain top-k"), + paginated.entries, + "offset 0 must return exactly the top-k page (descending={descending})" + ); + assert_eq!(paginated.skipped, 0); + assert_eq!( + cost_plain.seek_count, cost_paginated.seek_count, + "offset 0 must stay on the iterator path: seek_count diverged \ + (descending={descending})" + ); + assert_eq!( + cost_plain.storage_loaded_bytes, cost_paginated.storage_loaded_bytes, + "offset 0 must stay on the iterator path: loaded bytes diverged \ + (descending={descending})" + ); + } + } + + // ----------------------------------------------------------------- + // Equality grids: the counted path must return byte-identical pages + // in identical order to the iterator order. Oracle: the unchanged + // `indexed__top_k` iterator path, sliced. + // ----------------------------------------------------------------- + + #[test] + fn counted_paginated_matches_iterator_oracle_count_axis() { + let grove_version = GroveVersion::latest(); + let (db, population) = make_pcit_with_mixed_counts(grove_version); + let path = [TEST_LEAF, b"cidx"]; + + for descending in [false, true] { + let full = db + .indexed_count_top_k( + path.as_ref(), + population as u16, + descending, + None, + grove_version, + ) + .unwrap() + .expect("full ordered scan (oracle)"); + assert_eq!(full.len(), population); + + for offset in [0u64, 1, 2, 3, 4, 5, 8, 9, 10, 11, 15] { + for k in [0u16, 1, 3, population as u16] { + let page = db + .indexed_count_top_k_paginated( + path.as_ref(), + k, + offset, + descending, + None, + grove_version, + ) + .unwrap() + .expect("paginated page"); + let start = (offset as usize).min(population); + let end = (start + k as usize).min(population); + assert_eq!( + page.entries, + full[start..end], + "count axis page mismatch at offset={offset} k={k} \ + descending={descending}" + ); + assert_eq!( + page.skipped, + offset.min(population as u64), + "true skipped at offset={offset} k={k}" + ); + } + } + } + } + + #[test] + fn counted_paginated_matches_iterator_oracle_sum_and_avg_axes() { + let grove_version = GroveVersion::latest(); + let (db, population) = make_pcpsit_with_mixed_sums(grove_version); + let path = [TEST_LEAF, b"pcpsit"]; + + for descending in [false, true] { + let full_sum = db + .indexed_sum_top_k( + path.as_ref(), + population as u16, + descending, + None, + grove_version, + ) + .unwrap() + .expect("full sum scan (oracle)"); + let full_avg = db + .indexed_avg_top_k( + path.as_ref(), + population as u16, + descending, + None, + grove_version, + ) + .unwrap() + .expect("full avg scan (oracle)"); + assert_eq!(full_sum.len(), population); + assert_eq!(full_avg.len(), population); + + for offset in [0u64, 1, 2, 3, 5, 6, 7, 9] { + for k in [0u16, 1, 2, population as u16] { + let start = (offset as usize).min(population); + let end = (start + k as usize).min(population); + + let sum_page = db + .indexed_sum_top_k_paginated( + path.as_ref(), + k, + offset, + descending, + None, + grove_version, + ) + .unwrap() + .expect("sum page"); + assert_eq!( + sum_page.entries, + full_sum[start..end], + "sum axis page mismatch at offset={offset} k={k} \ + descending={descending}" + ); + assert_eq!(sum_page.skipped, (offset).min(population as u64)); + + let avg_page = db + .indexed_avg_top_k_paginated( + path.as_ref(), + k, + offset, + descending, + None, + grove_version, + ) + .unwrap() + .expect("avg page"); + assert_eq!( + avg_page.entries, + full_avg[start..end], + "avg axis page mismatch at offset={offset} k={k} \ + descending={descending}" + ); + assert_eq!(avg_page.skipped, (offset).min(population as u64)); + } + } + } + } + + #[test] + fn counted_paginated_matches_iterator_oracle_across_subtree_boundaries() { + let grove_version = GroveVersion::latest(); + let db = make_pcit_with_n_tied_rows(grove_version); + let path = [TEST_LEAF, b"cidx"]; + + // Offsets chosen to land inside, at, and across internal subtree + // boundaries of a ~600-key AVL tree, plus the exact-population and + // past-end edges. + for descending in [false, true] { + let full = db + .indexed_count_top_k(path.as_ref(), N as u16, descending, None, grove_version) + .unwrap() + .expect("full ordered scan (oracle)"); + assert_eq!(full.len(), N as usize); + + for offset in [1u64, 7, 63, 64, 65, 250, 511, N - 6, N - 1, N, N + 17] { + let page = db + .indexed_count_top_k_paginated( + path.as_ref(), + K, + offset, + descending, + None, + grove_version, + ) + .unwrap() + .expect("paginated page"); + let start = (offset as usize).min(N as usize); + let end = (start + K as usize).min(N as usize); + assert_eq!( + page.entries, + full[start..end], + "page mismatch at offset={offset} descending={descending}" + ); + assert_eq!( + page.skipped, + offset.min(N), + "true skipped at offset={offset}" + ); + } + } + } + + // ----------------------------------------------------------------- + // Cross-path parity: the unproved `skipped` is the same quantity the + // proved path attests. + // + // A client reading `skipped` off the wire cannot tell which path + // served it, so the two must report the same number for the same + // request — including the past-the-end shape, where both must report + // the population rather than echoing the request. The proved side + // re-derives its value from the counted subtree commitments in the + // proof bytes; the unproved side reads the secondary's root + // aggregate. Nothing structural keeps those in step, so it is pinned + // here rather than assumed. + // ----------------------------------------------------------------- + + #[test] + fn unproved_skipped_equals_the_proved_paths_attested_skipped() { + let grove_version = GroveVersion::latest(); + let (db, population) = make_pcit_with_mixed_counts(grove_version); + let path: &[&[u8]] = &[TEST_LEAF, b"cidx"]; + let pop = population as u64; + + for descending in [false, true] { + // Offsets spanning: nothing skipped, mid-page, the last row, + // exactly the population, just past it, and absurdly past it + // (the original denial-of-service lever). + for offset in [0u64, 1, 5, pop - 1, pop, pop + 1, 4_000_000_000] { + for k in [0u16, 1, 3] { + let unproved = db + .indexed_count_top_k_paginated( + path, + k, + offset, + descending, + None, + grove_version, + ) + .unwrap() + .expect("unproved page"); + + let proof = db + .prove_indexed_count_top_k_paginated( + path, + k, + offset, + descending, + None, + grove_version, + ) + .unwrap() + .expect("prove page"); + let proved = GroveDb::verify_indexed_count_top_k_paginated( + &proof, + path, + k, + offset, + descending, + grove_version, + ) + .expect("verify page"); + + assert_eq!( + unproved.skipped, proved.skipped, + "skipped disagrees between paths at offset={offset} k={k} \ + descending={descending}" + ); + // Both paths must also agree that `min(offset, + // population)` is what that number means — equality + // alone would be satisfied by two identically wrong + // values. + assert_eq!( + unproved.skipped, + offset.min(pop), + "skipped is not min(offset, population) at offset={offset} k={k} \ + descending={descending}" + ); + + let proved_entries = match &proved.entries { + AxisEntries::Count(v) => v.clone(), + other => panic!("count axis proof returned {other:?}"), + }; + assert_eq!( + unproved.entries, proved_entries, + "entries disagree between paths at offset={offset} k={k} \ + descending={descending}" + ); + } + } + } + } + + /// Always-on differential against the pre-change implementation + /// (`legacy_linear_indexed_count_top_k_paginated`, kept verbatim as a + /// test-only baseline): identical entries in identical order at every + /// probed offset/k/direction, plus the `skipped` semantics the legacy + /// shape could not report. The release-mode measurement harness runs + /// the same comparison at 1e3–1e6 rows; this pins it in CI at a size + /// CI can afford. + #[test] + fn counted_path_agrees_with_the_pre_change_linear_implementation() { + let grove_version = GroveVersion::latest(); + const ROWS: u64 = 120; + let db = make_pcit_with_rows(ROWS, grove_version); + let path = [TEST_LEAF, b"cidx"]; + + for descending in [false, true] { + for offset in [0u64, 1, 3, 60, ROWS - 1, ROWS, ROWS + 80] { + for k in [1u16, 7] { + let counted = db + .indexed_count_top_k_paginated( + path.as_ref(), + k, + offset, + descending, + None, + grove_version, + ) + .unwrap() + .expect("counted read"); + let legacy = db + .legacy_linear_indexed_count_top_k_paginated( + path.as_ref(), + k, + offset, + descending, + None, + grove_version, + ) + .unwrap() + .expect("legacy linear read"); + assert_eq!( + counted.entries, legacy, + "counted and legacy diverge at offset={offset} k={k} \ + descending={descending}" + ); + assert_eq!( + counted.skipped, + offset.min(ROWS), + "true skipped at offset={offset} k={k} descending={descending}" + ); + } + } + } + } + + /// The counted read's pinned view must be the *transaction's* view — + /// snapshot of committed state plus the transaction's own uncommitted + /// writes — not a bare DB snapshot. Rows inserted inside an open + /// transaction must be visible (and counted) by a paginated read + /// through that same transaction before commit, exactly as they were + /// through the old single-iterator scan. + /// + /// The other half of the consistency property — that a commit landing + /// mid-descent cannot produce a page mixing two states — is not + /// deterministically testable from here: the read is one synchronous + /// call with no way to pause between node fetches. It rests on + /// RocksDB's iterator-snapshot contract, the same contract the + /// replaced implementation relied on for the same guarantee. + #[test] + fn counted_read_sees_the_transactions_uncommitted_writes() { + let grove_version = GroveVersion::latest(); + let db = make_pcit_with_rows(20, grove_version); + let path = [TEST_LEAF, b"cidx"]; + + let tx = db.start_transaction(); + for i in 20..30u64 { + db.insert_into_count_indexed_tree( + [TEST_LEAF, b"cidx"].as_ref(), + format!("k{i:07}").as_bytes(), + Element::new_item(vec![]), + Some(&tx), + grove_version, + ) + .unwrap() + .expect("insert within tx"); + } + + // Through the transaction, all 30 rows exist: a counted read at + // offset 25 must see the uncommitted tail. + let in_tx = db + .indexed_count_top_k_paginated(path.as_ref(), 3, 25, false, Some(&tx), grove_version) + .unwrap() + .expect("counted read inside the transaction"); + assert_eq!(in_tx.skipped, 25, "population through the tx is 30"); + assert_eq!( + in_tx + .entries + .iter() + .map(|(_, key)| key.clone()) + .collect::>(), + vec![ + b"k0000025".to_vec(), + b"k0000026".to_vec(), + b"k0000027".to_vec() + ], + ); + + // Without the transaction, the committed view still has 20 rows: + // the same offset is past the end. + let outside = db + .indexed_count_top_k_paginated(path.as_ref(), 3, 25, false, None, grove_version) + .unwrap() + .expect("counted read outside the transaction"); + assert!(outside.entries.is_empty()); + assert_eq!(outside.skipped, 20, "committed population is 20"); + } + + // ----------------------------------------------------------------- + // Manual measurement harness — not run in CI. Run with: + // cargo test -p grovedb measure_paginated_costs -- --ignored --nocapture + // ----------------------------------------------------------------- + + /// Measures the counted paginated read against the pre-change linear + /// loop (kept verbatim as + /// `GroveDb::legacy_linear_indexed_count_top_k_paginated`) on the + /// N-group Count-axis secondary shape, and prints `OperationCost` + /// counters plus wall-clock. The counters are the trustworthy, + /// machine-independent signal; wall-clock is reported because it is + /// what a node operator feels, but it is noisy on a loaded machine — + /// check the load average before quoting it. + #[test] + #[ignore] + fn measure_paginated_costs() { + use std::time::Instant; + + let grove_version = GroveVersion::latest(); + println!(); + println!( + "wall-clock is min of 3 runs on a shared machine — treat seeks/bytes as the signal" + ); + println!("| n | k | offset | path | seeks | loaded bytes | rows | wall µs |"); + println!("|---|---|---|---|---|---|---|---|"); + + for n in [1_000u64, 10_000, 100_000, 1_000_000] { + let db = make_pcit_with_rows(n, grove_version); + let path = [TEST_LEAF, b"cidx"]; + for k in [1u16, 100] { + // `offset = 1` is the worst case for the counted collect + // phase: the whole page is gathered through tree-node + // point-gets instead of sequential iterator steps. + for offset in [0u64, 1, n - 1, n, 4_000_000_000] { + let mut counted_wall = u128::MAX; + let mut counted = None; + for _ in 0..3 { + let started = Instant::now(); + let run = db.indexed_count_top_k_paginated( + path.as_ref(), + k, + offset, + false, + None, + grove_version, + ); + counted_wall = counted_wall.min(started.elapsed().as_micros()); + counted = Some(run); + } + let CostContext { + value: counted_rows, + cost: counted_cost, + } = counted.expect("three runs happened"); + let counted_page = counted_rows.expect("counted read"); + + let mut linear_wall = u128::MAX; + let mut linear = None; + for _ in 0..3 { + let started = Instant::now(); + let run = db.legacy_linear_indexed_count_top_k_paginated( + path.as_ref(), + k, + offset, + false, + None, + grove_version, + ); + linear_wall = linear_wall.min(started.elapsed().as_micros()); + linear = Some(run); + } + let CostContext { + value: linear_rows, + cost: linear_cost, + } = linear.expect("three runs happened"); + let linear_rows = linear_rows.expect("legacy linear read"); + + assert_eq!( + counted_page.entries, linear_rows, + "counted and linear paths diverged at n={n} k={k} offset={offset}" + ); + assert_eq!( + counted_page.skipped, + offset.min(n), + "true skipped at n={n} k={k} offset={offset}" + ); + + println!( + "| {n} | {k} | {offset} | counted | {} | {} | {} | {} |", + counted_cost.seek_count, + counted_cost.storage_loaded_bytes, + counted_page.entries.len(), + counted_wall, + ); + println!( + "| {n} | {k} | {offset} | linear | {} | {} | {} | {} |", + linear_cost.seek_count, + linear_cost.storage_loaded_bytes, + linear_rows.len(), + linear_wall, + ); + } + } + } + } +} diff --git a/grovedb/src/tests/indexed_tree_secondary_drift_tests.rs b/grovedb/src/tests/indexed_tree_secondary_drift_tests.rs index 5d27a9003..b0e360543 100644 --- a/grovedb/src/tests/indexed_tree_secondary_drift_tests.rs +++ b/grovedb/src/tests/indexed_tree_secondary_drift_tests.rs @@ -795,16 +795,19 @@ mod tests { } /// A secondary row whose key is shorter than the axis's 8-byte sort prefix - /// carries no recoverable `(count, original_key)` pair. Every direct count - /// query must raise `CorruptedData` naming the axis and the expected width - /// — dropping the row instead would let a short-key row silently shrink a - /// top-k page or a range listing while the caller sees a successful result. + /// carries no recoverable `(count, original_key)` pair. Every place that + /// decodes a row must raise `CorruptedData` naming the axis and the + /// expected width — dropping the row instead would let a short-key row + /// silently shrink a top-k page or a range listing while the caller sees a + /// successful result. /// - /// Each of the three query shapes decodes in a different place — the plain - /// walk, the pagination *skip* loop, the pagination *collect* loop, and the - /// range walk — so all four are driven here. + /// The decode sites driven here: the plain top-k walk, the paginated + /// collect at `offset == 0`, and the range walk. A paginated read with + /// `offset > 0` skips by counted descent and decodes only *returned* rows, + /// so its skip phase deliberately does not error — that contract is + /// asserted here too. #[test] - fn a_short_secondary_key_is_reported_by_every_count_query_shape() { + fn a_short_secondary_key_is_reported_by_every_shape_that_decodes_it() { let gv = GroveVersion::latest(); let db = make_test_grovedb(gv); make_pcit_with(&db, b"pcit", &[b"a", b"b"], gv); @@ -833,11 +836,23 @@ mod tests { .expect_err("paginated collect must surface the malformed row"), "paginated (offset 0, collect loop)", ); - assert_short_key_corruption( + // A paginated read with `offset > 0` skips by counted descent over + // the secondary merk: skipped rows are consumed from aggregate + // counts and their keys are never decoded. The malformed row is a + // real tree row here, so it still occupies its position (descending + // order is [0xff…, b, a] and offset 1 lands on b — the same + // position the storage iterator would assign), but only *returned* + // rows are decoded, so the skip no longer surfaces it as an error. + // Detecting malformed rows in the skipped region is + // `verify_grovedb`'s job (asserted above) and the collect loops' + // (asserted below for every shape that reads the row itself). + assert_eq!( db.indexed_count_top_k_paginated([TEST_LEAF, b"pcit"].as_ref(), 1, 1, true, None, gv) .unwrap() - .expect_err("paginated skip must surface the malformed row"), - "paginated (offset 1, skip loop)", + .expect("counted skip passes the malformed row without decoding it") + .entries, + vec![(1u64, b"b".to_vec())], + "descending order is [0xff…, b, a]; the malformed row is counted at offset 0", ); assert_short_key_corruption( db.indexed_count_range( @@ -854,6 +869,17 @@ mod tests { "range", ); + // When the malformed row is the RETURNED position of a counted read + // (ascending order is [a, b, 0xff…]; offset 2 selects it), the + // counted path must decode-and-refuse it exactly as the iterator + // paths do — skipped rows go undecoded, returned rows never do. + assert_short_key_corruption( + db.indexed_count_top_k_paginated([TEST_LEAF, b"pcit"].as_ref(), 1, 2, false, None, gv) + .unwrap() + .expect_err("a returned malformed row must surface through the counted path"), + "paginated (offset 2, counted path, returned row)", + ); + // Ascending top-k stops before reaching the malformed row, so the // well-formed prefix of the index still reads cleanly — the error above // is the decoder refusing a specific row, not the query failing wholesale. @@ -1178,6 +1204,135 @@ mod tests { tx.commit().expect("tx commit"); } + /// Rebind a PCIT's parent element so its `secondary_root_key` names a + /// key that does not exist in the count secondary, leaving every row + /// and hash otherwise intact. Produces the state a counted read must + /// treat as corruption: the element is readable and valid, but the + /// root it names resolves to nothing. + fn rebind_count_secondary_root_key_to( + db: &TempGroveDb, + pcit_path: &[&[u8]], + bogus_key: Vec, + gv: &GroveVersion, + ) { + use grovedb_merk::element::reconstruct::ElementReconstructExtensions; + + let tx = db.start_transaction(); + let batch = StorageBatch::new(); + let owned: Vec<&[u8]> = pcit_path.to_vec(); + let path: SubtreePath<&[u8]> = owned.as_slice().into(); + let (parent_path, pcit_key) = path.derive_parent().expect("non-root PCIT"); + + let (primary_root_hash, primary_root_key, primary_aggregate) = { + let primary = db + .open_transactional_merk_at_path(path.clone(), &tx, Some(&batch), gv) + .unwrap() + .expect("open PCIT primary"); + primary + .root_hash_key_and_aggregate_data() + .unwrap() + .expect("primary root state") + }; + + let mut parent_merk = db + .open_transactional_merk_at_path(parent_path.clone(), &tx, Some(&batch), gv) + .unwrap() + .expect("open parent merk"); + let pcit_element = Element::get(&parent_merk, pcit_key, true, gv) + .unwrap() + .expect("PCIT element"); + let secondary_root_key = match pcit_element.underlying() { + Element::ProvableCountIndexedTree(_, s, ..) => s.clone(), + other => panic!("not a PCIT element: {other:?}"), + }; + let secondary_root_hash = { + let secondary = db + .open_indexed_secondary_at_path( + path.clone(), + IndexAxis::Count, + secondary_root_key, + &tx, + Some(&batch), + gv, + ) + .unwrap() + .expect("open count secondary"); + let (hash, _, _) = secondary + .root_hash_key_and_aggregate_data() + .unwrap() + .expect("secondary root state"); + hash + }; + + let rebound = pcit_element + .reconstruct_with_two_root_keys(primary_root_key, Some(bogus_key), primary_aggregate) + .expect("reconstruct PCIT element"); + rebound + .insert_count_indexed_subtree( + &mut parent_merk, + pcit_key, + primary_root_hash, + secondary_root_hash, + None, + gv, + ) + .unwrap() + .expect("rebind PCIT element"); + + let mut merk_cache = std::collections::HashMap::new(); + merk_cache.insert(parent_path.clone(), parent_merk); + db.propagate_changes_with_transaction(merk_cache, parent_path, &tx, &batch, gv) + .unwrap() + .expect("propagate rebind"); + db.db + .commit_multi_context_batch(batch, Some(&tx)) + .unwrap() + .expect("commit rebind"); + tx.commit().expect("tx commit"); + } + + /// A parent element whose `secondary_root_key` names a node that does + /// not exist must fail loud through the counted path. The pinned view + /// read the element itself, so a dangling root key is corruption, not + /// a race to retry through — and it must never be silently served as + /// an empty or truncated page. The offset-0 iterator path, which + /// never consults the root key, still reads the physical rows: the + /// contrast is the two-views posture the design doc records. + #[test] + fn a_dangling_secondary_root_key_fails_loud_through_the_counted_path() { + let gv = GroveVersion::latest(); + let db = make_test_grovedb(gv); + make_pcit_with(&db, b"pcit", &[b"a", b"b"], gv); + rebind_count_secondary_root_key_to( + &db, + &[TEST_LEAF, b"pcit"], + b"no-such-node".to_vec(), + gv, + ); + + let err = db + .indexed_count_top_k_paginated([TEST_LEAF, b"pcit"].as_ref(), 1, 1, false, None, gv) + .unwrap() + .expect_err("a dangling secondary root key must error, not serve a page"); + match err { + Error::CorruptedData(message) => assert!( + message.contains("absent from the same read snapshot"), + "unexpected corruption message: {message}" + ), + other => panic!("expected CorruptedData, got {other:?}"), + } + + // The iterator path reads the physical keyspace and is untouched + // by the dangling root key. + assert_eq!( + db.indexed_count_top_k_paginated([TEST_LEAF, b"pcit"].as_ref(), 2, 0, false, None, gv) + .unwrap() + .expect("offset-0 path reads physical rows") + .entries, + vec![(1u64, b"a".to_vec()), (1u64, b"b".to_vec())], + ); + } + /// PSIT: an orphan row in the sum secondary is removed and the root /// returns to the pristine twin's — the repair covers the sum axis, not /// just count. diff --git a/grovedb/src/tests/mod.rs b/grovedb/src/tests/mod.rs index 50aa09f0d..d125b1e92 100644 --- a/grovedb/src/tests/mod.rs +++ b/grovedb/src/tests/mod.rs @@ -57,6 +57,7 @@ mod get_cost_estimator_tests; mod grove_query_result_tests; mod indexed_axis_nested_and_bounds_tests; mod indexed_axis_offset_proof_tests; +mod indexed_axis_paginated_cost_tests; mod indexed_axis_proof_tests; mod indexed_tree_secondary_drift_tests; mod indexed_tree_security_regression_tests; diff --git a/grovedb/src/tests/provable_count_indexed_tree_tests.rs b/grovedb/src/tests/provable_count_indexed_tree_tests.rs index 8622dd777..ff97d1c81 100644 --- a/grovedb/src/tests/provable_count_indexed_tree_tests.rs +++ b/grovedb/src/tests/provable_count_indexed_tree_tests.rs @@ -1040,7 +1040,7 @@ mod tests { .unwrap() .expect("page 1"); assert_eq!( - page1, + page1.entries, vec![(20u64, b"eve".to_vec()), (12u64, b"bob".to_vec())] ); @@ -1057,7 +1057,7 @@ mod tests { .unwrap() .expect("page 2"); assert_eq!( - page2, + page2.entries, vec![(7u64, b"dave".to_vec()), (5u64, b"alice".to_vec())] ); @@ -1073,7 +1073,7 @@ mod tests { ) .unwrap() .expect("page 3"); - assert_eq!(page3, vec![(1u64, b"carol".to_vec())]); + assert_eq!(page3.entries, vec![(1u64, b"carol".to_vec())]); // Offset beyond total → empty. let beyond = db @@ -1087,7 +1087,7 @@ mod tests { ) .unwrap() .expect("offset beyond"); - assert!(beyond.is_empty()); + assert!(beyond.entries.is_empty()); // offset=0 ≡ plain top_k. let plain = db @@ -1105,7 +1105,7 @@ mod tests { ) .unwrap() .expect("paginated offset 0"); - assert_eq!(plain, paginated); + assert_eq!(plain, paginated.entries); } #[test] diff --git a/grovedb/src/tests/provable_count_provable_sum_indexed_tree_tests.rs b/grovedb/src/tests/provable_count_provable_sum_indexed_tree_tests.rs index 5d9eb5bbc..864134821 100644 --- a/grovedb/src/tests/provable_count_provable_sum_indexed_tree_tests.rs +++ b/grovedb/src/tests/provable_count_provable_sum_indexed_tree_tests.rs @@ -1395,7 +1395,7 @@ mod tests { .unwrap() .expect("page 1"); assert_eq!( - page1, + page1.entries, vec![ (25 * AVG_SCALE, b"bob".to_vec()), (5 * AVG_SCALE, b"alice".to_vec()), @@ -1415,7 +1415,7 @@ mod tests { .unwrap() .expect("page 2"); assert_eq!( - page2, + page2.entries, vec![(3 * AVG_SCALE, b"eve".to_vec()), (0, b"dave".to_vec())] ); @@ -1431,7 +1431,7 @@ mod tests { ) .unwrap() .expect("beyond"); - assert!(beyond.is_empty()); + assert!(beyond.entries.is_empty()); } #[test] diff --git a/grovedb/src/tests/provable_sum_indexed_tree_tests.rs b/grovedb/src/tests/provable_sum_indexed_tree_tests.rs index 5d212fade..324382ab6 100644 --- a/grovedb/src/tests/provable_sum_indexed_tree_tests.rs +++ b/grovedb/src/tests/provable_sum_indexed_tree_tests.rs @@ -918,7 +918,7 @@ mod tests { .unwrap() .expect("page 1"); assert_eq!( - page1, + page1.entries, vec![(100i64, b"frank".to_vec()), (12, b"bob".to_vec())] ); @@ -934,7 +934,7 @@ mod tests { .unwrap() .expect("page 2"); assert_eq!( - page2, + page2.entries, vec![(5i64, b"alice".to_vec()), (0, b"dave".to_vec())] ); @@ -950,7 +950,7 @@ mod tests { ) .unwrap() .expect("beyond"); - assert!(beyond.is_empty()); + assert!(beyond.entries.is_empty()); // offset=0 ≡ plain top_k. let plain = db @@ -968,7 +968,7 @@ mod tests { ) .unwrap() .expect("pag offset 0"); - assert_eq!(plain, pag); + assert_eq!(plain, pag.entries); } #[test] diff --git a/grovedb/src/tests/run_path_query_tests.rs b/grovedb/src/tests/run_path_query_tests.rs index d29b8c0e4..efaae33ec 100644 --- a/grovedb/src/tests/run_path_query_tests.rs +++ b/grovedb/src/tests/run_path_query_tests.rs @@ -207,7 +207,7 @@ mod tests { .expect("unified top-k"); assert_eq!( run_entries(run), - AxisEntries::Sum(direct), + AxisEntries::Sum(direct.entries), "top-k k={k} offset={offset} descending={descending}" ); } @@ -475,7 +475,7 @@ mod tests { .expect("direct branch read"); assert_eq!( entries.as_ref().expect("present branch"), - &AxisEntries::Sum(direct), + &AxisEntries::Sum(direct.entries), "branch {}", String::from_utf8_lossy(present) ); @@ -735,7 +735,7 @@ mod tests { ) .unwrap() .expect("unified count top-k"); - assert_eq!(run_entries(run), AxisEntries::Count(direct)); + assert_eq!(run_entries(run), AxisEntries::Count(direct.entries)); // Bounded on the count axis, with bounds deliberately below and // above the u64 domain so the count clamp is exercised (the sum @@ -842,7 +842,7 @@ mod tests { ) .unwrap() .expect("unified avg top-k"); - assert_eq!(run_entries(run), AxisEntries::Avg(direct)); + assert_eq!(run_entries(run), AxisEntries::Avg(direct.entries)); // Bounded on the avg axis takes the i128 bounds unclamped — the // avg domain is the whole i128 range. diff --git a/storage/src/rocksdb_storage/storage_context/raw_iterator.rs b/storage/src/rocksdb_storage/storage_context/raw_iterator.rs index c6593746f..bb3559329 100644 --- a/storage/src/rocksdb_storage/storage_context/raw_iterator.rs +++ b/storage/src/rocksdb_storage/storage_context/raw_iterator.rs @@ -46,6 +46,22 @@ pub struct PrefixedRocksDbRawIterator { pub(super) raw_iterator: I, } +impl PrefixedRocksDbRawIterator { + /// Re-scope this iterator to a different subtree prefix while keeping + /// the underlying RocksDB iterator — and therefore its pinned snapshot + /// view — alive. A multi-subtree read that must be internally + /// consistent (e.g. discovering a subtree's root key in one subtree + /// and then traversing that subtree) can hop scopes without giving up + /// the view; consuming `self` makes the hand-off explicit, so one + /// pinned view serves one subtree at a time, in sequence. + pub fn retarget(self, prefix: SubtreePrefix) -> Self { + Self { + prefix, + raw_iterator: self.raw_iterator, + } + } +} + impl RawIterator for PrefixedRocksDbRawIterator> { fn seek_to_first(&mut self) -> CostContext<()> { self.raw_iterator.seek(self.prefix);