fix(drive): serve unproved ranked reads through the paginated prover by shumkov · Pull Request #4382 · dashpay/platform

shumkov · 2026-08-12T18:03:43Z

Issue being fixed or feature implemented

Ranked queries (SELECT <agg> GROUP BY <prop> ORDER BY <agg> LIMIT k OFFSET m) accept an unbounded
OFFSET, and the comment justifying that said:

grovedb's paginated prover attests the skipped region from the counted subtree commitments instead
of walking it, so proving OFFSET 4 and OFFSET 4_000_000_000 are the same O(log n + k) work and
the same proof size. There is no denial-of-service lever here to cap.

That was true of the proving path and false of the unproved one, which a client selects with
prove on the wire. prove = false called grovedb's direct read, whose offset skip is a per-entry
walk — one iterator step and one decode per skipped entry — so the skip alone cost
Θ(min(offset, population)).

Measured on a 1,000,000-group fixture (release build, median of 5–9 reps):

`prove`	`OFFSET 0`	`OFFSET 4e9`	ratio
`false`	15 µs	457 ms	30,000×
`true`	112 µs	38 µs (695-byte proof)	0.3×

Nothing contained it: ranked queries carry no fee, drive_dispatcher does no cost accounting, a
spawn_blocking query cannot be cancelled by client disconnect or stream reset (the closure outlives
a dropped JoinHandle), and the gateway's only rate limit is per source IP across the whole Platform
service — so a query competes with broadcastStateTransition rather than having its own budget. At
roughly 1,950 requests/minute from one unauthenticated IP, prove = false plus a large OFFSET was
~7× oversubscription of a 2-vCPU node whose cores are shared with consensus.

What was done?

execute_top_k_no_proof no longer calls indexed_{count,sum,avg}_top_k_paginated. It generates the
same envelope execute_top_k_with_proof returns and extracts the page from it with
GroveDb::verify_indexed_axis_top_k_paginated, so the skipped region is attested from counted subtree
commitments rather than walked, at any offset. Both executors now go through one shared
prove_page_with_cost over a path built once per request.

Measured for the full prove → verify → extract round trip: 78 / 90 / 121 / 129 µs at N = 1e3 /
1e4 / 1e5 / 1e6. Verification is flat in offset and in N and scales only with k. Worst case is
+113 µs absolute at offset 0 with k = 1; from N = 100k the round trip is cheaper than the direct
read outright, and the deep-offset lever goes from 440 ms to a flat 48 µs. The ratio between the
cheapest and the most expensive request on this surface collapses from ~30,000× to ~1×, which is the
property that matters on an unmetered, unauthenticated, uncancellable surface.

grovedb's verifier is called rather than drive's verify::document_ranked wrapper because drive-abci
builds drive with default-features = false, features = ["server", …], so the verify feature — and
therefore that wrapper — is not compiled there. GroveDb::verify_indexed_axis_top_k_paginated carries
no cfg gating and is available in the server build.

No platform-version gate, deliberately: queries do not touch state, the only production caller is
the v1 query handler (document_query/v1/mod.rs) with transaction: None, and no query cost feeds
block execution or fees. This is not consensus-visible. No Cargo edits, no feature flags, no grovedb
change, no proto field change.

Also in this PR:

Three comments that asserted things the code did not do, one of which was the load-bearing
justification for leaving OFFSET uncapped (mode_detection.rs), one claiming the prove and
no-proof paths shared a grovedb path (they did not; they now do), and one claiming the dispatcher
does fee accounting (it has no cost code at all). They land with the fix rather than before it:
rewriting "there is no denial-of-service lever here to cap" into an accurate description while
shipping no mitigation would have published an unmitigated unauthenticated remote DoS, with a
worked recipe, in a public repo.
A fourth, found during review: the drive-abci empty-ranking rejection told callers to "Retry
with prove = false" — a route this change removes — and attributed the failure to a prover that is
retired and not on this path. Reworded; it also gets its first test.
The reshape from grovedb's AxisEntries into drive's RankedEntrys, plus the entries.len() > k
bound, previously existed only in the verifier. Rather than duplicate ~35 lines into the executor it
moved to DriveDocumentRankedQuery::ranked_entries_from, shared by the server read and the client
verifier, so the two sides of the surface cannot drift on entry shape. verify_ranked_top_k_proof_v0
shrank by 40 lines.
An operator-facing warn naming the one new benign failure mode (below) so its log line is not
mistaken for storage corruption. The error itself is deliberately not reclassified — a chain
mismatch is also what genuine corruption looks like, and making it retriable would hide that.

How Has This Been Tested?

Two defects, two independent pins, both verified red-before-green by reverting the implementation in
the working tree.

1. Result equality — reading_through_the_prover_returns_what_the_direct_read_returned: 144 cases
(3 axes × 2 directions × k ∈ {1, 4, 100} — 100 being MAX_RANKED_LIMIT — × offsets
{0, 1, 5, 11, 12, 13, 100, 4e9}) against a 12-group population where three groups share every
aggregate value, the values straddle zero (the axis sort keys encode sign), and the averages are
non-integral, so tie ordering, the sign boundary and the fixed point's floor toward −∞ are all
exercised at every offset rather than only at the ends. The oracle for entries is a live call to the
old grovedb primitives; skipped is asserted from arithmetic, min(offset, population).

Reverting the implementation: 4 tests red in drive plus the wire assertion in drive-abci, all on
skipped.

2. Cost — a_deep_offset_does_not_cost_a_walk_of_the_skipped_region: asserts flatness through
drive's own executor, via a new pub(super) execute_top_k_no_proof_with_cost (public API
unchanged; the public method drops the cost exactly as before). Both seek_count and
storage_loaded_bytes are checked — the same OperationCost fields fees/op.rs already reads to
compute charged credits — against a bound derived from the secondary's AVL height (worst case
~1.44·log₂n, so 2·log₂n; a log₂n bound would be tighter than the tree's own invariant and would flake
on a legitimate rebalance), plus a vacuity guard that fails if the fixture ever stops being able to
catch a walk, plus a size tripwire for the "proof size is offset-independent" claim that nothing
previously pinned. Over 200 groups:

offset	executor seeks	executor bytes	direct-read seeks	direct-read bytes
0	36	4,458	5	635
100	35	4,515	105	30,835
200	23	2,414	204	60,671
4e9	23	2,414	204	60,671

This pin was verified against the case that defeats a naive one — a direct read that also reports
a truthful skipped by following its empty result with a count aggregate. That variant satisfies every
skipped assertion while restoring the 457 ms walk, and the cost test catches it:
the executor's seeks must not grow with the offset: 105 at offset 100 against 5 at offset 0. Without
it, the security defect itself had no regression evidence — only the behaviour change did.

Suites, on the rebased tree (rebased onto v4.2-dev @ f05bf82dc9, no conflicts):

cargo test -p drive --lib → 3327 passed, 0 failed
cargo test -p drive-abci --lib query:: → 607 passed, 0 failed, 1 pre-existing ignored
cargo test -p drive-proof-verifier → 262 passed
cargo fmt --check --all → clean
cargo clippy --workspace --all-features → clean
drive also compiles server-only, verify-only, and for wasm32-unknown-unknown via
wasm-drive-verify

Verification caveat, stated rather than buried: --all-features build scripts for grovedb
(grovedbg) and tenderdash-proto download release artifacts from GitHub, and those downloads failed
repeatedly on the machine used here. The clippy gate is green only after pre-populating both build
caches with the artifacts fetched out-of-band — the grovedbg zip verified against the SHA256 constant
the build script itself hardcodes, and the tenderdash archive fetched via api.github.com and
repackaged under the directory naming the archive/ endpoint produces. The compilation and lints
themselves are genuine; the artifact fetch was not performed by the build scripts. CI fetches these
normally and is the authority.

Review: six independent reviewers on the diff (correctness, adversarial proof-handling, performance,
test quality, comment/doc accuracy, plus a cross-model pass). Their must-fixes are folded, including
two that invalidated claims made in this PR's earlier drafts.

Breaking Changes

No API or wire format change, but one wire-visible behaviour change on unproved responses.

RankedPage::skipped, which reaches the wire as
GetDocumentsResponseV1.ResultData.Ranked.skipped, stops echoing the request and starts reporting
the truth. grovedb's direct read returns an empty vector when the walk exhausts during the skip and
never reports how far it got, so drive had no choice but to echo the requested offset. Through the
envelope it gets the attested count. On a ranking holding 5 groups:

request	`skipped` before	`skipped` after
`LIMIT 2 OFFSET 9`, `prove = false`	`9`	`5`
`LIMIT 2 OFFSET 9`, `prove = true`	`5`	`5` (unchanged)

Only the past-the-end case changes; when the skip succeeds, skipped still equals the requested
offset. The proved and unproved paths no longer diverge at all — what proving adds is that the number
is attested (re-derived client-side from bytes bound to a root hash checked against consensus),
not that it differs.

A client asserting skipped == requested_offset will see a different value. A client using it as the
rank base for entries[i] — its documented purpose — is unaffected, and gains a population count on
the unproved path it previously had to prove for. The contract is corrected in every place that stated
the old behaviour: platform.proto, the Objective-C generated client (the only generated client that
carries proto prose; all others are symbol-only), book/src/drive/ranked-index-examples.md, and the
Rust docs.

For release notes:

Mixed-network window. The ranked surface first appears in the v4.2.0-dev.1 tag, so a network
mixing v4.2.0-dev.1 nodes with newer ones returns the echoed offset from one and the population
from the other for the same unproved request. Devnet-only exposure, and the proto's new "do not
assume this field equals the offset you requested" advice is safe against both.
One unproved OFFSET = u32::MAX request now yields a ranking's exact population, where before it
took ~32 binary-search requests. Not a leak — document state is world-readable and the proved path
already attested this number — but the cost of obtaining it changed.

One new failure mode, documented at the executor. The prover makes several independent storage
reads and, at transaction = None, is not isolated from a concurrent commit; a commit landing
mid-proof can produce an envelope whose ancestor chain no longer reconciles, which fails verification
where the direct read had no cross-layer consistency requirement. The proved path has always carried
this exposure — what is new is the unproved path sharing it. query/service.rs re-runs a query whose
committed block height moved, which absorbs almost all of it; the residual is the window between a
commit becoming visible and that height being published.

Follow-ups (not in this PR)

A grovedb read-form of the counted skip (option A from the design discussion). Roughly 12 of the
~14 merk opens this path performs serve the envelope rather than the answer. A read-only entry point
on the same counted descent would be ~2 opens and strictly cheaper than both current paths at every
offset, erasing the shallow-corner regression. Needs profiling first.
Snapshot isolation in grovedb. start_transaction is a bare self.db.transaction() with no
set_snapshot, which is the root cause of the torn-chain failure above and the reason a read-time
root-hash self-check cannot be made race-free.
Unbounded recursion in merk's count-offset verifier on hostile bytes — surfaced by review,
pre-existing and not introduced here, but security-relevant and unreported elsewhere. The verifier
deliberately disables the AVL balance check (necessary for collapsed offset proofs), so reconstructed
tree depth is bounded only by op-stream length and verify_count_offset_shape walks it by unbounded
recursion. Unreachable when the prover is ours, but the same verifier runs on untrusted bytes in
rs-drive-proof-verifier and wasm-drive-verify, capped only by a 16 MiB bincode limit — enough to
nest hundreds of thousands of Parent ops and overflow a client's or wasm's stack. Reported as
suspected; no malicious op stream was constructed.
A transaction = Some(_) executor test. The production caller passes None, and grovedb's
TxRef::new borrows a supplied transaction unchanged so both mechanisms read through the caller's
handle identically — an API-contract gap rather than a behaviour risk.
yarn workspace @dashevo/dapi-grpc build is broken locally, dying on a PnP loader error partway
through after rewriting unrelated core clients with protoc-gen-js churn. The proto comment here was
therefore hand-synced into the Objective-C client; the next successful regeneration is the proper fix.

Checklist:

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have added or updated relevant unit/integration/functional/e2e tests
I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

I have assigned this pull request to a milestone

Summary by CodeRabbit

Improvements
- Ranked queries now use consistent pagination behavior for verified and unverified reads.
- Requests beyond the available results return an empty page with the actual number of available entries skipped.
- Large offsets maintain predictable performance without requiring linear scans.
- Unverified ranked results now use the same validated pagination data as verified results.
Bug Fixes
- Improved handling and messaging for empty rankings, invalid proofs, and ranking data inconsistencies.
- Preserved clear distinction between client input errors and internal query failures.
Documentation
- Clarified ranked pagination, proof verification, offsets, and past-end behavior.

The `prove = false` arm of the ranked query called grovedb's direct read, whose offset skip is a per-entry walk: one iterator step and one decode per skipped entry, so the skip alone cost Theta(min(offset, population)). `OFFSET` has no ceiling, ranked queries carry no fee, the dispatcher does no cost accounting, a `spawn_blocking` query cannot be cancelled by client disconnect or stream reset, and the gateway rate-limits per source IP across the whole Platform service rather than per method. Measured on a 1,000,000-group fixture: 15 us at OFFSET 0, 457 ms at OFFSET 4e9. At ~1,950 req/min from one unauthenticated IP that is roughly 7x oversubscription of a 2-vCPU node shared with consensus. `execute_top_k_no_proof` now generates the same envelope the prove path returns and extracts the page from it with grovedb's verifier, so the skipped region is attested from counted subtree commitments instead of walked. The full prove -> verify -> extract round trip measures 78/90/121/129 us at N = 1e3/1e4/1e5/1e6; verification is flat in offset and in N and scales only with k. The deep-offset lever becomes a flat 48 us. The cost is +113 us absolute in one corner -- offset 0 with a small k -- and from N = 100k the round trip is cheaper than the direct read outright. grovedb's verifier is called rather than drive's `verify::document_ranked` wrapper because drive-abci builds drive without the `verify` feature, so the wrapper is not compiled there. No Cargo edits, no feature flags, no grovedb change, no proto field change, and no platform-version gate: queries do not touch state, the only production caller is the v1 query handler with `transaction: None`, and no query cost feeds block execution or fees. BEHAVIOUR CHANGE, wire-visible on unproved responses `RankedPage::skipped` -- `GetDocumentsResponseV1.ResultData.Ranked.skipped` on the wire -- stops echoing the request and starts reporting the truth. grovedb's direct read returns an empty vector when the walk exhausts during the skip and never reports how far it got, so drive had no choice but to echo the requested offset. Through the envelope it gets the attested count. On a five-group ranking, `LIMIT 2 OFFSET 9` with `prove = false` now reports `skipped = 5` rather than `9`; the proved path is unchanged. Only the past-the-end case differs, and the two paths no longer diverge at all -- what proving adds is that the value is attested, not that it differs. A client asserting `skipped == requested_offset` will see a different value; a client using it as the rank base for `entries[i]`, its documented purpose, is unaffected and gains a population count it previously had to prove for. The contract is corrected in every place that stated the old behaviour: `platform.proto`, the Objective-C generated client that carries proto prose verbatim, the developer book, and the Rust docs. COMMENTS CORRECTED Three comments asserted things the code did not do, and two of them were policy: "there is no denial-of-service lever here to cap" justified the uncapped OFFSET. They land with the fix rather than before it, because rewriting them into an accurate description while shipping no mitigation would have published an unmitigated unauthenticated remote DoS, with a worked recipe, in a public repo. A fourth surfaced during review: the drive-abci empty-ranking rejection told callers to retry with `prove = false`, a route this change removes, and blamed a prover that is retired and not on the path. TESTS Two defects, two independent pins, both verified red before green. - Result equality: 144 cases (3 axes x 2 directions x k in {1, 4, 100} x offsets {0, 1, 5, 11, 12, 13, 100, 4e9}) against a population where three groups share every aggregate value, the values straddle zero, and the averages are non-integral -- so tie ordering, the sign boundary and the fixed point's rounding are exercised at every offset. The oracle is a live call to the old grovedb primitives; `skipped` is asserted from arithmetic, `min(offset, population)`. Reverting the implementation: 4 tests red in drive plus the wire assertion in drive-abci, all on `skipped`. - Cost: asserted through drive's own executor via a new `execute_top_k_no_proof_with_cost` (public API unchanged), on both `seek_count` and `storage_loaded_bytes` -- the same `OperationCost` fields `fees/op.rs` already reads to compute charged credits -- against a bound derived from the secondary's AVL height rather than a chosen constant, plus a guard that fails if the fixture stops being able to catch a walk, plus a size tripwire for the "proof size is offset-independent" claim that nothing previously pinned. Over 200 groups the direct read costs 5 -> 105 -> 204 -> 204 seeks and 635 -> 30,835 -> 60,671 -> 60,671 bytes at offsets 0/100/200/4e9; the executor stays at 36 -> 35 -> 23 -> 23 and 4,458 -> 4,515 -> 2,414 -> 2,414. This pin was verified against the case that defeats a naive one -- a direct read that also reports a truthful `skipped` by following its empty result with a count aggregate, satisfying every `skipped` assertion while restoring the walk. It goes red: "the executor's seeks must not grow with the offset: 105 at offset 100 against 5 at offset 0". Without it the security defect had no regression evidence; only the behaviour change did. - `empty_ranking_proof_rejection` gets its first test, including the negative case pinning how deliberately narrow its match is. drive --lib 3324 passed; drive-abci --lib query:: 607 passed; clippy and fmt clean. drive also compiles server-only, verify-only, and for wasm32-unknown-unknown via wasm-drive-verify. Verification here discards the reconstructed root hash rather than comparing it against a locally read one: query reads are not snapshot-isolated, `finalize_block` makes a commit visible before publishing the height the query layer's retry guard compares, and a locally read root is not an independent trust boundary in any case. The reasoning is recorded at the call site.

thepastaclaw · 2026-08-12T18:06:09Z

⛔ Blockers found — Opus deferred (commit 577fe3d)
Canonical validated blockers: 1

coderabbitai · 2026-08-12T18:06:32Z

Warning

Review limit reached

@shumkov, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 7 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 58b1566a-b16a-472f-bca4-a74746e5cd2e

📥 Commits

Reviewing files that changed from the base of the PR and between f432daa and 577fe3d.

📒 Files selected for processing (3)

packages/rs-drive-abci/src/query/document_query/v1/mod.rs
packages/rs-drive-abci/src/query/document_query/v1/tests.rs
packages/rs-drive/src/query/drive_document_ranked_query/tests.rs

📝 Walkthrough

Walkthrough

Unproved ranked reads now build and verify the same paginated proof envelope as proved reads. Shared decoding validates ranked entries and returns actual skipped populations. Tests and API documentation cover past-end offsets, empty rankings, proof sizes, and offset costs.

Changes

Ranked pagination

Layer / File(s)	Summary
Shared ranked execution and decoding `packages/rs-drive/src/query/drive_document_ranked_query/...`	Unproved execution now builds and verifies paginated proofs. Shared decoding validates entries and reports the actual skipped population.
Proof verification consolidation `packages/rs-drive/src/verify/document_ranked/verify_ranked_top_k_proof/v0/mod.rs`	Proof verification delegates ranked-entry conversion and `k` validation to the shared decoder.
Ranked query error handling `packages/rs-drive-abci/src/query/document_query/v1/mod.rs`, `packages/rs-drive-abci/src/query/document_query/v1/tests.rs`	Chain-mismatch errors receive warnings. Exact empty-ranking proof failures map to `InvalidArgument`; unrelated errors remain unchanged.
Pagination contract and regression coverage `packages/rs-drive/src/query/drive_document_ranked_query/tests.rs`, `packages/rs-drive/src/drive/contract/insert/insert_contract/v0/tests/ranked_index_e2e_tests.rs`, `packages/dapi-grpc/protos/platform/v0/platform.proto`, `packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h`, `book/src/drive/ranked-index-examples.md`	Tests and documentation define identical proved and unproved skip counts, including past-end requests, and verify cost and proof-size behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant RankedQuery
  participant ProofBuilder
  participant ProofVerifier
  Client->>RankedQuery: request ranked page
  RankedQuery->>ProofBuilder: construct paginated proof
  ProofBuilder-->>RankedQuery: proof envelope
  RankedQuery->>ProofVerifier: verify envelope and ranked entries
  ProofVerifier-->>RankedQuery: RankedPage with actual skipped count
  RankedQuery-->>Client: ranked response

Possibly related PRs

dashpay/platform#4266: Introduced the ranked aggregate and provable top-K query execution and verification stack extended by this change.

Suggested reviewers: quantumexplorer, lklimek

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely identifies the main change: routing unproved ranked reads through the paginated prover.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/ranked-unproved-read-through-prover

_{Comment @coderabbitai help to get the list of available commands.}

github-actions · 2026-08-12T18:08:19Z

📖 Book Preview built successfully.

Download the preview from the workflow artifacts.
To view locally: download the artifact, unzip, and open index.html.

Updated at 2026-08-12T19:24:52.188Z

coderabbitai

Actionable comments posted: 2

🧹 Nitpick comments (1)

packages/rs-drive/src/query/drive_document_ranked_query/tests.rs (1)
1682-1694: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider a separate tolerance constant for proof size.

byte_slack is derived as a storage-loaded-bytes allowance (256 bytes per tree level). Line 1690 reuses it as a proof-size tolerance. The two quantities are unrelated, so a later change to the storage allowance silently changes this tripwire. Define a distinct constant for the proof-size comparison.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/rs-drive/src/query/drive_document_ranked_query/tests.rs` around
lines 1682 - 1694, Define a dedicated proof-size tolerance constant near the
proof-size assertion, rather than reusing byte_slack. Update the proof_bytes_at
comparison to use this new constant, while leaving byte_slack exclusively for
storage-loaded-bytes allowances.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/rs-drive-abci/src/query/document_query/v1/tests.rs`:
- Around line 3058-3090: Update empty_ranking_proof_rejection and its tests so
only the exact supported GroveError::CorruptedData message “Cannot create proof
for empty tree” is reclassified as QueryError::InvalidArgument. Replace the
substring-based contains predicate with exact message matching, and add a test
case containing the marker within unrelated text that must remain unmapped.

In `@packages/rs-drive/src/query/drive_document_ranked_query/execute_top_k.rs`:
- Around line 171-180: Update the proof-read flow in the ranked query execution
around verify_indexed_axis_top_k_paginated to use snapshot isolation; if
unavailable, add a bounded retry at the dispatcher only when the error is the
specific chain-mismatch verification failure. Preserve immediate propagation for
all other proof or GroveDB errors.

---

Nitpick comments:
In `@packages/rs-drive/src/query/drive_document_ranked_query/tests.rs`:
- Around line 1682-1694: Define a dedicated proof-size tolerance constant near
the proof-size assertion, rather than reusing byte_slack. Update the
proof_bytes_at comparison to use this new constant, while leaving byte_slack
exclusively for storage-loaded-bytes allowances.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 09b317c6-215d-46bb-8681-50ff49f5eb3c

📥 Commits

Reviewing files that changed from the base of the PR and between f05bf82 and f432daa.

📒 Files selected for processing (12)

book/src/drive/ranked-index-examples.md
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h
packages/dapi-grpc/protos/platform/v0/platform.proto
packages/rs-drive-abci/src/query/document_query/v1/mod.rs
packages/rs-drive-abci/src/query/document_query/v1/tests.rs
packages/rs-drive/src/drive/contract/insert/insert_contract/v0/tests/ranked_index_e2e_tests.rs
packages/rs-drive/src/query/drive_document_ranked_query/execute_top_k.rs
packages/rs-drive/src/query/drive_document_ranked_query/executors/top_k_no_proof.rs
packages/rs-drive/src/query/drive_document_ranked_query/mod.rs
packages/rs-drive/src/query/drive_document_ranked_query/mode_detection.rs
packages/rs-drive/src/query/drive_document_ranked_query/tests.rs
packages/rs-drive/src/verify/document_ranked/verify_ranked_top_k_proof/v0/mod.rs

codecov · 2026-08-12T18:17:33Z

Codecov Report

❌ Patch coverage is 85.61151% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.51%. Comparing base (f05bf82) to head (577fe3d).

Files with missing lines	Patch %	Lines
...s/rs-drive-abci/src/query/document_query/v1/mod.rs	65.90%	15 Missing ⚠️
...query/drive_document_ranked_query/execute_top_k.rs	90.56%	5 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           v4.2-dev    #4382      +/-   ##
============================================
+ Coverage     87.49%   87.51%   +0.02%     
============================================
  Files          2672     2672              
  Lines        340400   340406       +6     
============================================
+ Hits         297819   297905      +86     
+ Misses        42581    42501      -80

Components	Coverage Δ
dpp	`88.87% <ø> (ø)`
drive	`86.19% <94.73%> (+<0.01%)`	⬆️
drive-abci	`89.29% <65.90%> (+0.07%)`	⬆️
sdk	`∅ <ø> (∅)`
dapi-client	`∅ <ø> (∅)`
platform-version	`∅ <ø> (∅)`
platform-value	`92.92% <ø> (ø)`
platform-wallet	`∅ <ø> (∅)`
drive-proof-verifier	`48.02% <ø> (ø)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

… commit A ranked page is committed to by an envelope built from several independent storage reads, and those are not isolated from a concurrent block commit. A commit landing inside that window leaves the ancestor chain unreconcilable, grovedb rejects it, and the request fails. Before this surface read through the prover, only `prove = true` was exposed; now the default read path is too. `query::service` already re-runs a query whose committed height moved, which absorbs almost all of it, but `finalize_block` makes a commit visible before publishing that height, and a request finishing inside that gap is not retried. Rebuild the envelope instead of returning: the condition is transient and a fresh envelope over settled state resolves it. Deliberately narrow. Only the chain mismatch is retried, bounded at two extra attempts (~400 us total), and genuine corruption produces the same rejection every time and so still surfaces — one envelope later, with a `warn` that says a burst under load is the race while a persistent or unloaded occurrence is not. Also covers three defensive paths that had no tests: the shared entry decoder's axis-shape and `k` guards, unreachable through grovedb today and therefore worth pinning precisely because nothing else can reach them, and the chain-mismatch detector's narrowness. Reviewed-by: CodeRabbit (retry suggestion adopted; its exact-match suggestion for the empty-tree marker was rejected — grovedb wraps merk's constant in its own prefix, so exact matching would stop the mapper from ever firing)

shumkov · 2026-08-12T19:18:14Z

Responding to the nitpick from the review body (it has no thread of its own): separate tolerance constant for proof size — agreed and done in 577fe3d.

There's now a proof_size_slack, derived from what an envelope actually carries per level of the counted descent, with a comment recording why it must stay distinct from byte_slack: the latter bounds storage reads, the two are unrelated quantities, and sharing one constant would let a change to either silently move the other's tripwire. That was a fair catch.

For the record, the two inline comments are answered in their own threads: the retry suggestion was adopted, and the exact-match suggestion for the empty-tree marker was rejected because grovedb wraps merk's constant in its own prefix, so exact matching would prevent the mapper from ever firing.

QuantumExplorer

You should not use the proved path for this, instead there are unproved ways that will make this fast, even faster.

thepastaclaw

Preliminary review — Codex only

The prover-backed ranked read removes the offset-proportional walk, but the new handler-local retry crosses the state-publication boundary without refreshing the captured PlatformState. A retry that succeeds after the GroveDB commit can therefore return new-state results or proof bytes with the previous block's metadata and signature, so this requires changes before merge.
Source: reviewer backend model gpt-5.6-sol; final verifier backend model gpt-5.6-sol. openclaw-agent/cliproxy/gpt-5.6-sol is orchestration-only and not reviewer evidence.

Validated blockers were found in the Codex precheck. Opus is deferred until a fresh Codex revalidation clears the blocker gate.

Review provenance

Codex reviewers: gpt-5.6-sol — general (completed)
Verifier: gpt-5.6-sol — verifier
Sonnet: not run (deferred by blocker gate)

🔴 1 blocking

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-drive-abci/src/query/document_query/v1/mod.rs`:
- [BLOCKING] packages/rs-drive-abci/src/query/document_query/v1/mod.rs:1367-1369: Retrying only Drive execution can pair new-state results with old block metadata
  The retry re-executes the Drive request while retaining the `platform_state` reference captured by `QueryService` before the query began. This is unsafe in the exact commit-visible/guard-not-yet-published window the retry is intended to cover: `update_state_cache_v0` publishes the new PlatformState before the database transaction commits, `finalize_block` commits GroveDB, and only afterward stores the new `committed_block_height_guard`. A query that captured the old state before publication can have its first envelope torn by the commit, then successfully rebuild against the newly committed GroveDB state here. Because the guard still has the old height, the service post-check sees it equal to the captured old state's height and accepts the response. Lines 1409-1417 then attach metadata—and, for proved responses, the old block signature and block ID—from that old PlatformState to new-state data or proof bytes. The retry must restart at a boundary that reloads PlatformState and reruns the service consistency checks; a successful local retry cannot safely be wrapped with the existing state object.

thepastaclaw · 2026-08-12T20:06:28Z

+                        if is_ranked_chain_mismatch(&e) && attempts_left > 0 {
+                            attempts_left -= 1;
+                            continue;


🔴 Blocking: Retrying only Drive execution can pair new-state results with old block metadata

The retry re-executes the Drive request while retaining the platform_state reference captured by QueryService before the query began. This is unsafe in the exact commit-visible/guard-not-yet-published window the retry is intended to cover: update_state_cache_v0 publishes the new PlatformState before the database transaction commits, finalize_block commits GroveDB, and only afterward stores the new committed_block_height_guard. A query that captured the old state before publication can have its first envelope torn by the commit, then successfully rebuild against the newly committed GroveDB state here. Because the guard still has the old height, the service post-check sees it equal to the captured old state's height and accepts the response. Lines 1409-1417 then attach metadata—and, for proved responses, the old block signature and block ID—from that old PlatformState to new-state data or proof bytes. The retry must restart at a boundary that reloads PlatformState and reruns the service consistency checks; a successful local retry cannot safely be wrapped with the existing state object.

_{source: ['codex']}

shumkov requested a review from QuantumExplorer as a code owner August 12, 2026 18:03

github-actions Bot added this to the v4.2.0 milestone Aug 12, 2026

coderabbitai Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread packages/rs-drive-abci/src/query/document_query/v1/tests.rs

Comment thread packages/rs-drive/src/query/drive_document_ranked_query/execute_top_k.rs

QuantumExplorer requested changes Aug 12, 2026

View reviewed changes

thepastaclaw suggested changes Aug 12, 2026

View reviewed changes

Navigation Menu

Uh oh!

fix(drive): serve unproved ranked reads through the paginated prover - #4382

fix(drive): serve unproved ranked reads through the paginated prover#4382
shumkov wants to merge 2 commits into
v4.2-devfrom
fix/ranked-unproved-read-through-prover

shumkov commented Aug 12, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

thepastaclaw commented Aug 12, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Aug 12, 2026 •

edited

Loading

Review limit reached

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

github-actions Bot commented Aug 12, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Aug 12, 2026 •

edited

Loading

Uh oh!

shumkov commented Aug 12, 2026

Uh oh!

QuantumExplorer left a comment

Uh oh!

thepastaclaw left a comment

Uh oh!

thepastaclaw Aug 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

shumkov commented Aug 12, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue being fixed or feature implemented

What was done?

How Has This Been Tested?

Breaking Changes

Follow-ups (not in this PR)

Checklist:

Summary by CodeRabbit

Uh oh!

thepastaclaw commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

github-actions Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

shumkov commented Aug 12, 2026

Uh oh!

QuantumExplorer left a comment

Choose a reason for hiding this comment

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

Preliminary review — Codex only

Review provenance

Uh oh!

thepastaclaw Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shumkov commented Aug 12, 2026 •

edited by coderabbitai Bot

Loading

thepastaclaw commented Aug 12, 2026 •

edited

Loading

coderabbitai Bot commented Aug 12, 2026 •

edited

Loading

github-actions Bot commented Aug 12, 2026 •

edited

Loading

codecov Bot commented Aug 12, 2026 •

edited

Loading