fix(drive): keep cursor document in descending continuation-page proofs - #4541
Conversation
A documents query combining a startAfter/startAt cursor, a range clause
on the terminal index property, and descending order produced a proof
missing the cursor document's subtree layer, which no client can verify
("V1 proof is missing lower layer for non-empty tree at key 00").
The cursor fetch is merged into the proved query with limit + 1, one
slot reserved for the cursor document — a budget the prover spends in
root traversal order. Since grovedb's V4 merge propagates the inputs'
direction to the merged root, a descending page visits the index branch
before the cursor branch (key [0]), exhausts the budget mid-timeline,
and the prover omits the cursor subtree's lower layer.
Pin the merged root back to ascending after the merge so the cursor
branch spends its reserved slot first. Only the synthesized root flips —
each input query lands intact inside a subquery branch, preserving
in-branch (newest-first) result order — and the verifier never rebuilds
the merged query (it runs the cursor and main queries as separate subset
queries), so proof bytes change only for the previously unverifiable
shape and no client change is needed.
Fixes #4540
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 54 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🕓 Ready for review — 12 ahead in queue (commit 14cfca6) |
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (9.09%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## v4.2-dev #4541 +/- ##
============================================
- Coverage 87.80% 87.68% -0.13%
============================================
Files 2748 2748
Lines 355859 356325 +466
============================================
- Hits 312472 312427 -45
- Misses 43387 43898 +511
🚀 New features to boost your workflow:
|
Issue being fixed or feature implemented
Fixes #4540.
On protocol v14, a documents query combining a
startAfter/startAtcursor, a range clause on the terminal index property, and descending order returned a proof the client cannot verify:The rejection is correct — the cursor document is genuinely absent from the proof. The cursor fetch is merged into the proved query with
limit + 1(one slot reserved for the cursor document), and the prover spends that budget in root traversal order. grovedb's V4 merge propagates the inputs' direction onto the merged root, so a descending page visits the index branch before the cursor branch (key[0]); mid-timeline the index branch holds more thanlimit + 1matches, exhausts the budget, and the prover omits the cursor subtree's lower layer.verify_start_at_document_in_proofthen cannot extract the cursor document. This is the shape of every newest-first timeline continuation page, so any client paginating an[equality, $createdAt]index descending broke on page 2+.Not reproducible on protocol ≤ v13: the V3 merge left the synthesized root ascending, which silently protected the cursor-first invariant.
What was done?
Pin the merged root back to ascending after
PathQuery::mergeinconstruct_path_query_operations, restoring the cursor-first traversal the reservedlimit + 1slot depends on.Not addressed here (pre-existing, independent, noted in the issue): the cursor lowering excludes the cursor's entire terminal key, so documents tying the cursor's timestamp are skipped across page boundaries even when proofs verify.
How Has This Been Tested?
New round-trip regression test
test_proved_desc_range_continuation_page_includes_cursor_document: non-unique[category, $createdAt]index, 12 documents under onecategoryvalue, page 2 viastartAfter(and the inclusivestartAtvariant) with a$createdAt > 0range descending, proved withexecute_with_proof_only_get_elementsand checked against the unproved results. Without the fix the test fails with exactly the reported error; with it, both pages verify.Also ran the full
query_testssuite (56 tests),drivelibquery/verifyunit tests (711 + 270),query_tests_history,dashpay, andcargo clippy -p drive --all-targets— all green.Breaking Changes
None. Proof generation only; no state, fee, or consensus change. Proof bytes change only for the query shape that currently produces an unverifiable proof.
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code