feat(grovedb,merk): no-prove query_aggregate_count_and_sum accumulator#676
Merged
Conversation
Mirror Query::new_aggregate_count_and_sum_on_range on the no-proof path. One merk-internal walk returns the in-range (u64 count, i64 sum) pair from a ProvableCountProvableSumTree terminator, collapsing what consumers previously did as two parallel query_aggregate_count + query_aggregate_sum calls under a shared read transaction. Same cost class as the proof primitive (two O(log n) merk walks), but one round-trip instead of two. Adds: - merk/.../aggregate_count_and_sum/walk.rs: walk_count_and_sum classification walker accumulating (u128, i128) end-to-end so adversarial intermediate states cannot wrap mid-walk; narrowed at the public entry point. - merk/.../aggregate_count_and_sum/prove.rs: RefWalker entry point count_and_sum_aggregate_on_range narrowing to (u64, i64) with the same overflow check sum_aggregate_on_range uses. - merk/src/merk/get.rs: Merk::count_and_sum_aggregate_on_range with PCPS-only tree-type gate (same shape the proof-side primitive rejects single-axis hosts with) and (0, 0) on empty merk. - grovedb/src/operations/get/query.rs: GroveDb::query_aggregate_count_and_sum mirroring query_aggregate_sum body — upfront leaf-shape validation, opens leaf merk, delegates, wraps merk error with path context. - grovedb-version: new query_aggregate_count_and_sum_on_range slot (0 in v1/v2/v3) — V0-routed like the count/sum siblings. Tests: - 10 merk-level no-prove tests covering each Range* variant, the full-range happy path, empty merk, disjoint range, mixed-sign sums, and PCPS-only enforcement across every other tree type. Each test cross-checks the no-prove walker against the prove-side primitive on the same merk so the two paths cannot silently diverge. - 16 grovedb-level no-prove tests covering the same range-shape sweep end-to-end, plus carrier-shape rejection (the leaf-only validator must fire even when the dispatcher would accept the same query), empty-path rejection, NormalTree-at-merk rejection, single-axis ProvableCountSumTree rejection, and GROVE_V2 version-gate routing. Each fixture call asserts (count, sum) against what the prove-side verifier extracts for the same path query, pinning prove/no-prove equivalence the way the existing sum/count tests do. Carrier (per-key) variant is deferred — not needed for Dash Platform's AVG no-prove dispatcher and noted as out-of-scope in the entry-point docstring. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR adds a combined no-proof count-and-sum aggregate range query API ( ChangesCount-and-Sum Aggregate Range Query
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #676 +/- ##
===========================================
+ Coverage 91.41% 91.42% +0.01%
===========================================
Files 235 236 +1
Lines 66867 67053 +186
===========================================
+ Hits 61127 61305 +178
- Misses 5740 5748 +8
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@grovedb/src/tests/aggregate_count_and_sum_query_tests.rs`:
- Around line 496-520: The test function
no_proof_combined_v0_envelope_rejected_by_version_gate is misnamed because it
asserts the query succeeds under GROVE_V2; rename the test to reflect the
asserted success (for example
no_proof_combined_v0_envelope_allowed_by_version_gate or
no_proof_combined_v0_envelope_succeeds_under_v2) so the function name aligns
with the assertions in PathQuery::new_aggregate_count_and_sum_on_range and the
subsequent call to grove_db.query_aggregate_count_and_sum that expects success.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 696403ab-7a9a-483f-8987-af946da64661
📒 Files selected for processing (12)
grovedb-version/src/version/grovedb_versions.rsgrovedb-version/src/version/v1.rsgrovedb-version/src/version/v2.rsgrovedb-version/src/version/v3.rsgrovedb/src/operations/get/query.rsgrovedb/src/tests/aggregate_count_and_sum_query_tests.rsgrovedb/src/tests/mod.rsmerk/src/merk/get.rsmerk/src/proofs/query/aggregate_count_and_sum/mod.rsmerk/src/proofs/query/aggregate_count_and_sum/prove.rsmerk/src/proofs/query/aggregate_count_and_sum/tests.rsmerk/src/proofs/query/aggregate_count_and_sum/walk.rs
…nd_sum The `open_transactional_merk_at_path` error arm was untested — codecov flagged 6 uncovered lines in the new entry point. Add a test that queries a non-existent intermediate path to exercise the wrap-and-return branch before the merk walker is invoked. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CodeRabbit pointed out the test name said "rejected by version gate" but the body asserts success under GROVE_V2 (the slot is 0). Rename to no_proof_combined_v0_envelope_accepted_under_v2 so the name matches the assertion — avoids false signal during failure triage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
QuantumExplorer
added a commit
to dashpay/platform
that referenced
this pull request
May 20, 2026
…call Bumps the grovedb pin to 9db515ef (head of dashpay/grovedb#676), which adds the combined no-prove `query_aggregate_count_and_sum` accumulator — a merk-internal `(u128, i128)` walk narrowed to `(u64, i64)` at the entry point, mirroring the proof-side `AggregateCountAndSumOnRange` shape. Rewires `flat_aggregate_count_and_sum` in the joint count+sum executor to issue one combined accumulator call against `aggregate_count_and_sum_path_query` instead of two parallel `query_aggregate_count` + `query_aggregate_sum` calls. Same cost class (O(log n) at the merk layer either way) but halves the round-trip count and removes the need for the shared-tx-across-two-reads plumbing on the flat branch. Compound `In + range` per-In fan-out still issues one accumulator call per branch under a shared read transaction — now one call per branch instead of two. The integration lands ahead of the grovedb PR's review so the platform side can exercise the primitive end-to-end. If the grovedb PR's review surfaces changes, the platform pin re-bumps trivially. Updated docstrings on `drive_document_count_and_sum_query/mod.rs`, `drive_dispatcher.rs`, `executors/range_no_proof.rs`, and the `book/src/drive/average-index-examples.md` chapter section to reflect the single-call aggregate branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Local binding instead of a multi-line macro argument expression — easier for llvm-cov line attribution and otherwise equivalent. The macro behavior is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…range
Two consecutive `match try_from { Ok(v) => v, Err(_) => return ... }`
arms collapse to one `.map_err()...and_then(...)` chain. Same behavior
(both narrowing failures still surface as `CorruptedData` with a
descriptive message), fewer instrumented lines for llvm-cov to track on
unreachable defense-in-depth branches.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `(u128, i128) → (u64, i64)` narrow inside `count_and_sum_aggregate_on_range` was the last defense-in-depth path codecov was flagging — practically unreachable on an honest merk (PCPS maintains every aggregate as (u64, i64) at every level), but still worth pinning so the diagnostic doesn't bitrot. Extract the narrow into a free `narrow_count_and_sum(u128, i128) -> Result<(u64, i64), Error>` helper so it can be exercised without scaffolding a corrupted tree, and add 4 unit tests: - happy-path for boundary values (0, u64::MAX, i64::MAX, i64::MIN) - count overflow (count_u128 > u64::MAX) - sum positive overflow (sum_i128 > i64::MAX) - sum negative overflow (sum_i128 < i64::MIN) Bumps patch coverage past the 90% threshold. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
QuantumExplorer
added a commit
to dashpay/platform
that referenced
this pull request
May 20, 2026
dashpay/grovedb#676 (the no-prove `query_aggregate_count_and_sum` primitive) was squash-merged into develop as commit 60f29685. The content is identical to the PR-branch head 9db515ef that the previous commit pinned; just moves to the canonical develop ancestry. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the no-prove sibling of
Query::new_aggregate_count_and_sum_on_range:GroveDb::query_aggregate_count_and_sumreturns(u64 count, i64 sum)from a single merk-internal walk over aProvableCountProvableSumTree(PCPS) terminator. Same call shape asquery_aggregate_sum/query_aggregate_count, but yields both metrics at once.Why
Consumers that need both metrics on the no-proof path currently issue two parallel accumulator calls under a shared read transaction. Same cost class (two O(log n) merk walks), but two grovedb round-trips and two merk-internal traversals. Dash Platform's AVG no-prove dispatcher (dashpay/platform#3690) does this today — see
flat_aggregate_count_and_sumatpackages/rs-drive/src/query/drive_document_count_and_sum_query/executors/range_no_proof.rs. A combined no-prove primitive collapses those into one call, mirroring what the proof-side combined primitive already does for the prove path.What changed
merk/.../aggregate_count_and_sum/walk.rs(new) —walk_count_and_sumclassification walker accumulating(u128, i128)end-to-end so adversarial intermediate states cannot wrap mid-walk.merk/.../aggregate_count_and_sum/prove.rs—RefWalker::count_and_sum_aggregate_on_rangenarrowing to(u64, i64)with the same overflow checksum_aggregate_on_rangeuses.merk/src/merk/get.rs—Merk::count_and_sum_aggregate_on_rangewith PCPS-only tree-type gate (same shape the proof-side primitive rejects single-axis hosts with) and(0, 0)on empty merk.grovedb/src/operations/get/query.rs—GroveDb::query_aggregate_count_and_summirroringquery_aggregate_sum's body. Up-front leaf-shape validation via the pre-existingvalidate_leaf_aggregate_count_and_sum_on_range, opens leaf merk, delegates, wraps merk error with path context.grovedb-version— newquery_aggregate_count_and_sum_on_rangeslot (0in v1/v2/v3) — V0-routed like the count/sum siblings.Tests
Range*variant, full-range happy path, empty merk, disjoint range, mixed-sign sums, and PCPS-only enforcement across every other tree type. Each cross-checks the no-prove walker against the prove-side primitive on the same merk so the two paths cannot silently diverge.NormalTree-at-merk rejection, single-axisProvableCountSumTreerejection, andGROVE_V2version-gate routing. Each fixture call asserts(count, sum)against what the prove-side verifier extracts for the same path query, pinning prove/no-prove equivalence the way the existing sum/count tests do.Out of scope
query_aggregate_count_and_sum_per_key) variant — not needed for Dash Platform's AVG no-prove dispatcher (the carrier-AVG case is prove-only) and explicitly deferred to a separate PR. The entry-point docstring notes this.Query::new_aggregate_count_and_sum_on_range, the proof-side primitive, the verifier, or the carrier shape.Test plan
cargo test -p grovedb-merk --features minimal— 667 passed (10 new)cargo test -p grovedb --features minimal— 1849 passed (16 new)cargo clippy -p grovedb-merk --features minimal -- -D warnings— cleancargo clippy -p grovedb --features minimal -- -D warnings— clean🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests