Skip to content

Commit f432daa

Browse files
committed
fix(drive): serve unproved ranked reads through the paginated prover
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.
1 parent f05bf82 commit f432daa

12 files changed

Lines changed: 894 additions & 247 deletions

File tree

book/src/drive/ranked-index-examples.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -321,11 +321,11 @@ The response carries the skip back in `RankedEntries.skipped` (see [The Response
321321

322322
Three properties worth stating plainly:
323323

324-
- **The skip is attested, not walked.** grovedb proves the skipped region from the counted subtree commitments (`HashWithCount` / `HashWithCountAndSum`) rather than by traversing it. Both the prover's work and the proof's size stay `O(log n + k)` **at any offset**.
325-
- **There is therefore no offset ceiling.** An offset of 4 and an offset of four billion cost the same, so there is no denial-of-service lever a cap would close and a cap would only stop honest deep pagination.
324+
- **The skip is attested, not walked.** grovedb proves the skipped region from the counted subtree commitments (`HashWithCount` / `HashWithCountAndSum`) rather than by traversing it entry by entry. Both executors go through that prover, so the work and the proof's size stay `O(log n + k)` **at any offset**.
325+
- **There is therefore no offset ceiling.** An offset of 4 and an offset of four billion cost the same order of work — on either path — so there is no denial-of-service lever a cap would close, and a cap would only stop honest deep pagination.
326326
- **An offset past the end is a positive answer.** `entries` comes back empty and `skipped` is the ranking's *entire attested population*. "There are only 12 groups" is more information than a bare empty list.
327327

328-
On the **unproven** read there is nothing to attest and grovedb's read API does not report a short walk, so `skipped` simply echoes the requested offset. The proved and unproven paths therefore disagree in exactly one case — an offset past the end, where the unproven read reports the request and the proved one reports the truth. **Callers who need the population must prove.**
328+
On the **unproven** read the server derives the same number the same way — it extracts its page from that same counted envelope rather than walking the secondary — so the two paths no longer disagree anywhere, including past the end. What proving adds is that the value is *attested*: nothing in an unproven response is, so it is only as good as the node that sent it. **Callers who need to trust the population, rather than merely receive it, must still prove.**
329329

330330
## The Response
331331

packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h

Lines changed: 12 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/dapi-grpc/protos/platform/v0/platform.proto

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1407,12 +1407,18 @@ message GetDocumentsResponse {
14071407
// attested count, re-derived by the verifier from the counted
14081408
// subtree commitments in the proof bytes rather than trusted
14091409
// from this field; a proving client should use the verified
1410-
// value. On the unproven read there is nothing to attest and
1411-
// grovedb's read API does not report a short walk, so the server
1412-
// echoes the requested offset. The two therefore disagree in
1413-
// exactly one case — an offset past the end, where the unproven
1414-
// read reports the request and the proved one reports the truth.
1415-
// Callers who need the population must prove.
1410+
// value. An unproven response carries the same number — the
1411+
// server extracts its page from that same counted envelope — so
1412+
// the two no longer disagree anywhere, including past the end.
1413+
// What proving adds is that the value is *attested*: nothing in
1414+
// an unproven response is, so it is only as good as the node
1415+
// that sent it. Callers who need to trust the population, rather
1416+
// than merely receive it, must still prove.
1417+
//
1418+
// Do not assume this field equals the offset you requested. It
1419+
// equals the offset only when the skip succeeded; when the walk
1420+
// ran out of groups first it is smaller, and that is the answer
1421+
// rather than an inconsistency.
14161422
optional uint64 skipped = 2 [jstype = JS_STRING];
14171423
}
14181424

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

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1344,7 +1344,10 @@ impl<C> Platform<C> {
13441344
Some(rejection) => {
13451345
return Ok(QueryValidationResult::new_with_error(rejection));
13461346
}
1347-
None => return Err(e.into()),
1347+
None => {
1348+
annotate_ranked_chain_mismatch(&e);
1349+
return Err(e.into());
1350+
}
13481351
},
13491352
};
13501353

@@ -1445,6 +1448,11 @@ fn into_v1_ranked_entry(e: DriveRankedEntry) -> RankedEntry {
14451448
/// ancestor chain — not a single call site, and because the cost of
14461449
/// keeping it is one string comparison on an error path.
14471450
///
1451+
/// It is applied to both `prove` settings, and has to be: an unproven
1452+
/// ranked page is decoded out of an internally generated proof, so
1453+
/// anything that stops the envelope from being built stops both forms of
1454+
/// the request.
1455+
///
14481456
/// Historically: the non-paginated prover had no absence-proof shape
14491457
/// for "this axis secondary has no entries", so proving a ranking over
14501458
/// an index that held no documents failed with a merk-level "Cannot
@@ -1463,6 +1471,45 @@ fn into_v1_ranked_entry(e: DriveRankedEntry) -> RankedEntry {
14631471
/// narrow: any other `CorruptedData` still propagates as an internal
14641472
/// error, because for every other cause that classification is
14651473
/// correct.
1474+
/// Name the benign cause of a ranked chain-mismatch error in the log,
1475+
/// without reclassifying it.
1476+
///
1477+
/// A ranked page is committed to by an envelope built from several
1478+
/// independent storage reads, which are not isolated from a concurrent
1479+
/// block commit. A commit landing inside that window leaves the envelope's
1480+
/// ancestor chain unreconcilable, grovedb rejects it with a "chain
1481+
/// mismatch", and the request fails. The condition is benign and the query
1482+
/// layer re-runs a query whose committed height moved — but there is a
1483+
/// small window where it does not, and then this surfaces as an internal
1484+
/// error whose log line is indistinguishable from real storage corruption.
1485+
///
1486+
/// It stays an internal error on purpose: a chain mismatch is also what
1487+
/// genuine corruption looks like, and reclassifying it as retriable would
1488+
/// hide that. Only the operator-facing explanation is added, so whoever
1489+
/// reads the line at three in the morning knows which of the two to
1490+
/// suspect first, and that a *lone* occurrence under load is the race.
1491+
///
1492+
/// Applies to both `prove` settings. The proved path has always had this
1493+
/// exposure; unproved reads share it now that they are served from the
1494+
/// same envelope.
1495+
fn annotate_ranked_chain_mismatch(error: &drive::error::Error) {
1496+
let drive::error::Error::GroveDB(grove_error) = error else {
1497+
return;
1498+
};
1499+
let drive::query::GroveError::CorruptedData(message) = grove_error.as_ref() else {
1500+
return;
1501+
};
1502+
if !message.contains("chain mismatch") {
1503+
return;
1504+
}
1505+
tracing::warn!(
1506+
error = message.as_str(),
1507+
"ranked query failed to reconcile its proof's ancestor chain; an isolated \
1508+
occurrence under load is a query racing a block commit rather than corrupted \
1509+
state, but repeated occurrences — especially without load — are not"
1510+
);
1511+
}
1512+
14661513
fn empty_ranking_proof_rejection(error: &drive::error::Error) -> Option<QueryError> {
14671514
let drive::error::Error::GroveDB(grove_error) = error else {
14681515
return None;
@@ -1474,11 +1521,12 @@ fn empty_ranking_proof_rejection(error: &drive::error::Error) -> Option<QueryErr
14741521
return None;
14751522
}
14761523
Some(QueryError::InvalidArgument(
1477-
"this ranking has no groups yet, and an empty ranking cannot be proved: \
1478-
grovedb has no absence-proof shape for an empty axis secondary. Retry \
1479-
with `prove = false` — the unproven read answers the same request with \
1480-
an empty entry list. Once the index holds at least one document, the \
1481-
proved form works."
1524+
"this ranking could not be committed to: some merk along its path holds no \
1525+
entries, and merk cannot prove a key in an empty tree. An empty *ranking* is \
1526+
not itself the problem — a ranked index with no documents yet proves fine, as \
1527+
an empty page — so this is about the state of the indexed tree rather than the \
1528+
shape of the request. Dropping `prove` does not route around it: an unproven \
1529+
ranked page is extracted from the same envelope."
14821530
.to_string(),
14831531
))
14841532
}

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

Lines changed: 62 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2502,17 +2502,24 @@ mod ranked_tests {
25022502
assert_eq!(tail.skipped, Some(3));
25032503

25042504
// A window entirely past the end is an empty page, not an
2505-
// error. On this *unproven* path grovedb's read API doesn't
2506-
// report the short walk, so `skipped` echoes the request; the
2507-
// proved path is where it becomes the attested population.
2505+
// error, and `skipped` collapses to the population the walk
2506+
// actually found. That reaches the wire on this *unproven* path
2507+
// too — drive extracts the page from an internally generated
2508+
// paginated envelope, so the collapsed skip comes with it. It
2509+
// used to echo the requested 9 back instead.
25082510
let past_end = ranked_page(&platform, &state, paged(2, 9), version);
25092511
assert!(
25102512
past_end.entries.is_empty(),
25112513
"there is no rank 9 in a five-group ranking, and asking for one is not an \
25122514
error — got {:?}",
25132515
group_keys(&past_end.entries)
25142516
);
2515-
assert_eq!(past_end.skipped, Some(9));
2517+
assert_eq!(
2518+
past_end.skipped,
2519+
Some(5),
2520+
"the response reports the five groups the ranking holds, not the offset that \
2521+
was asked for"
2522+
);
25162523

25172524
// And the same page proves.
25182525
let result = platform
@@ -3031,4 +3038,55 @@ mod ranked_tests {
30313038
assert_ne!(label, "ranked", "no ORDER BY means no ranked routing");
30323039
}
30333040
}
3041+
3042+
/// The empty-tree mapper turns one grovedb failure into a
3043+
/// caller-facing argument error, and its value is entirely in how
3044+
/// *narrow* it is: every other `CorruptedData` must stay an internal
3045+
/// error, because for every other cause that classification is
3046+
/// correct. Nothing pinned either half before.
3047+
///
3048+
/// It also applies to both `prove` settings, which is not an
3049+
/// oversight — an unproven ranked page is decoded out of an
3050+
/// internally generated envelope, so whatever stops the envelope from
3051+
/// being built stops both forms of the request. That is why the
3052+
/// message must not tell a caller to retry without `prove`.
3053+
#[test]
3054+
fn the_empty_tree_mapper_matches_only_its_own_failure() {
3055+
use drive::error::Error as DriveError;
3056+
use drive::query::GroveError;
3057+
3058+
let mapped = empty_ranking_proof_rejection(&DriveError::GroveDB(Box::new(
3059+
GroveError::CorruptedData("Cannot create proof for empty tree".to_string()),
3060+
)))
3061+
.expect("the empty-tree failure is a caller-facing condition");
3062+
let QueryError::InvalidArgument(message) = mapped else {
3063+
panic!("an empty-tree failure must be an argument error, not a server fault");
3064+
};
3065+
assert!(
3066+
!message.contains("Retry with `prove = false`"),
3067+
"the message must not send callers down a path that no longer exists — an \
3068+
unproven read is served from the same envelope: {message}"
3069+
);
3070+
3071+
// Everything else stays an internal error.
3072+
for other in [
3073+
GroveError::CorruptedData("some unrelated corruption".to_string()),
3074+
GroveError::PathNotFound("no such subtree".to_string()),
3075+
] {
3076+
let label = format!("{other:?}");
3077+
assert!(
3078+
empty_ranking_proof_rejection(&DriveError::GroveDB(Box::new(other))).is_none(),
3079+
"only the empty-tree failure may be reclassified, not {label}"
3080+
);
3081+
}
3082+
assert!(
3083+
empty_ranking_proof_rejection(&DriveError::Drive(
3084+
drive::error::drive::DriveError::CorruptedDriveState(
3085+
"Cannot create proof for empty tree".to_string()
3086+
)
3087+
))
3088+
.is_none(),
3089+
"the marker string alone must not reclassify a non-grovedb error"
3090+
);
3091+
}
30343092
}

packages/rs-drive/src/drive/contract/insert/insert_contract/v0/tests/ranked_index_e2e_tests.rs

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1542,10 +1542,13 @@ fn an_offset_window_spanning_the_end_returns_the_short_tail() {
15421542
/// The page comes back empty and `skipped` collapses below the requested
15431543
/// offset — and *that shape* is the proof that the ranking holds exactly
15441544
/// `skipped` groups in total, because the counted commitments cover the whole
1545-
/// walk. It is the only way this surface reports a population, and the one
1546-
/// place the proved and unproven paths differ: the unproven read cannot see
1547-
/// the short walk (grovedb's read API returns an empty vector either way) and
1548-
/// reports the requested offset.
1545+
/// walk. It is the only way this surface reports a population.
1546+
///
1547+
/// Both paths report it. The unproven read gets its page out of an internally
1548+
/// generated envelope rather than by walking the secondary, so it carries the
1549+
/// same collapsed skip; it once echoed the requested offset back (grovedb's
1550+
/// direct read returns an empty vector either way and never says how far it
1551+
/// got), and this test is where that echo is pinned as gone.
15491552
#[test]
15501553
fn an_offset_past_the_end_returns_an_empty_page_whose_skip_attests_the_population() {
15511554
let (drive, contract) = setup_restaurants();
@@ -1557,8 +1560,9 @@ fn an_offset_past_the_end_returns_an_empty_page_whose_skip_attests_the_populatio
15571560
"there is no rank 12 in a five-group ranking"
15581561
);
15591562
assert_eq!(
1560-
page.skipped, 12,
1561-
"the unproven read echoes the requested offset — it has nothing to attest with"
1563+
page.skipped, 5,
1564+
"the unproven read reports the population it actually reached, not the requested \
1565+
offset of 12"
15621566
);
15631567

15641568
let verified = verified_ranked_avg_page(&drive, &contract, 3, 12);
@@ -1568,6 +1572,11 @@ fn an_offset_past_the_end_returns_an_empty_page_whose_skip_attests_the_populatio
15681572
"skipped < offset with an empty page is a proof that the ranking holds exactly \
15691573
five groups"
15701574
);
1575+
assert_eq!(
1576+
page.skipped, verified.skipped,
1577+
"the two paths agree on the population; proving buys the attestation, not a \
1578+
different number"
1579+
);
15711580

15721581
assert_grovedb_is_consistent(&drive);
15731582
}

0 commit comments

Comments
 (0)