build(platform)!: adopt GroveDB 6.0.0 and the published grovedb-bincode 2.1.0 - #4635
build(platform)!: adopt GroveDB 6.0.0 and the published grovedb-bincode 2.1.0#4635QuantumExplorer wants to merge 7 commits into
Conversation
…de 2.1.0 Advance GroveDB to develop 985ece65 (6.0.0) and rust-dashcore to dev 057e79cd, use rocksdb 0.25.0 from crates.io instead of the rust-rocksdb fork, and make the workspace bincode dependency an alias for the published grovedb-bincode 2.1.0 so GroveDB, rust-dashcore and Platform share one set of bincode traits without a Cargo patch. Adapt to the GroveDB API changes: batch options in Drive's grove operations, element verification aligned with the prover's right-to-left traversal, which GroveDB now checks strictly, and the new backward reference element variants rendered by the FFI and wasm SDK path element queries. BREAKING CHANGE: Rust consumers serializing Platform or Core types must use the published grovedb-bincode 2.1.0 crate; bincode trait identities now come from it. Wire encodings and the C ABI are unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
librocksdb-sys 0.19.0+11.8.1 (pulled in by rocksdb 0.25) targets the RocksDB 11 C API, which dropped the deprecated in_range callback from rocksdb_slicetransform_create. The Docker image and the librocksdb GitHub action still built RocksDB 10.4.2 and 9.9.3, so bindgen produced the six-argument signature and the rocksdb crate failed to compile against it. Build 11.8.1 in both places to match the vendored version. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
GroveDB now refuses a V1 layer proof that is read in the other op family (grovedb #863): a layer proof is emitted entirely in the family of the direction its query walks. When a document query carries a startAt or startAfter cursor, the prover merges the one-key cursor lookup into the page proof, and since grovedb's merge needs every input to walk the same way, the cursor layer is emitted in the page's orderBy direction. The verifier rebuilt that lookup as a fresh ascending single-key query, so every proved descending page with a cursor failed with InvalidProof. Read the cursor layer in the page's direction when it is verified as a subset of the page proof. The direction comes from the same query builder the prover uses and never depends on the cursor document; a single key decodes identically either way. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…der and cursor The proved contestant-votes helper verified every proof with a fixed ascending, unlimited query regardless of the request it had just sent. GroveDB now refuses a layer proof read in the other op family, so the descending request failed with InvalidProof. Mirror the request's order, count and cursor into the verifier query, as the SDK's proof verifier does. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
cargo fmt --check fails on v4.2-dev since #4218 landed this test unformatted. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (38)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe pull request updates workspace dependencies and RocksDB, fixes cursor pagination and proof-direction handling, disables backward-reference propagation in GroveDB operations, and adds rendering support for new backward-reference element variants. ChangesPlatform alignment and query behavior
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The dependency alignment, proof pagination, and element-rendering updates have no established merge-blocking issue in the reviewed changes. Sequence Diagram(s)sequenceDiagram
participant Client
participant QueryBuilder
participant GroveDB
participant ProofVerifier
Client->>QueryBuilder: Send cursor, limit, and ordering
QueryBuilder->>GroveDB: Build merged page query
GroveDB->>ProofVerifier: Provide page proof and root hash
ProofVerifier->>Client: Return verified page results
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
…ntrusted-bincode The dependency bump and GroveDB adaptations now live in #4635; this branch keeps only the untrusted-decoding adoption on top of it.
|
✅ Final review complete — no blockers (commit 7b596ff) · triage: critical · Phase 2 only (queue backlog) |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v4.2-dev #4635 +/- ##
============================================
- Coverage 85.34% 84.38% -0.97%
============================================
Files 2795 2796 +1
Lines 373566 375555 +1989
============================================
- Hits 318827 316909 -1918
- Misses 54739 58646 +3907
🚀 New features to boost your workflow:
|
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Phase 2 only (queue backlog)
Both supplied findings are confirmed against the exact head. The GroveDB upgrade exposes a direction mismatch that prevents verification of descending primary-key pages with a cursor, and the updated contestant-vote proof helper has no calls exercising its newly forwarded cursor or limit. Verification was source-based; reviewer-reported test results were not independently rerun.
Source: reviewer 1: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 2: gpt-6-astra (agent: phase2-reviewer, role: ffi-engineer); reviewer 3: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 4: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6-astra (agent: astra-verifier, role: final-verifier)
Review provenance
- Triage:
criticalbygpt-6-astra(effort low) — This upgrades consensus-critical storage and serialization dependencies and changes cryptographic proof verification and cursor traversal, so regressions could affect state consistency, data compatibility, or acceptance of proofs across Platform and its SDKs. - Phase 1 reviewers: not run (skipped for throughput: 20 PRs queued, above the 10 limit)
- Fresh verifier:
gpt-6-astra— final-verifier; agentastra-verifier - Phase 2 reviewers:
gpt-6-astra— general (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— ffi-engineer (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— security-auditor (completed, effort xhigh); agentphase2-reviewer
🔴 1 blocking | 🟡 1 suggestion(s)
🤖 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/src/verify/document/verify_start_at_document_in_proof/v0/mod.rs`:
- [BLOCKING] packages/rs-drive/src/verify/document/verify_start_at_document_in_proof/v0/mod.rs:59-63: Align primary-key cursor verification with the merged proof direction
For a non-history document query ordered by `$id desc` with a cursor, the cursor lookup and main query address the same primary-key tree. Their merged query therefore operates directly on that tree, but `construct_path_query_operations` unconditionally sets `merged.query.query.left_to_right = true` at `query/mod.rs:2061`. These new lines instead select descending verification. The newly pinned GroveDB explicitly rejects upright proof operations in a right-to-left V1 walk, so this valid page cannot be verified. The ascending-root override is justified for separate cursor and index branches, but not when both queries land on the same primary-key path. Preserve the requested document-layer direction in the prover for this case and add ascending/descending primary-key cursor round-trip tests. Reverting only the cursor verifier to ascending is insufficient: `verify_proof_keep_serialized_v0` subsequently verifies the main subset in the requested descending direction.
In `packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/masternode_vote/mod.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/masternode_vote/mod.rs:3079-3081: Exercise the forwarded cursor and limit in a proved vote request
All four calls to `get_proved_contestant_votes` in `test_proved_contestant_votes_query_request` pass `None` for both the count and cursor. Consequently, the new cursor conversion never executes, and reverting the verifier's `limit` and `start_at` fields to their previous hardcoded `None` values would leave this test passing. The neighboring non-proved pagination test does not exercise proof verification. Add proved requests with a nonempty cursor and a limit smaller than the available results, covering inclusive/exclusive cursors in ascending and descending order, and compare the returned identifiers with the corresponding slices of the full voter sequence.
…ges onto its own layer A startAt/startAfter page proof merges the one-key cursor lookup with the page query. For an index-ordered page the two live under different subtrees, so the merge synthesizes a root above both and that root is walked ascending so the cursor branch spends its reserved limit + 1 slot first; the verifier never sees that root. A $id-ordered page addresses the primary-key tree directly (a history-keeping type's cursor sits one level below it), so the merge point is the page query's own root layer. Flipping that layer ascending contradicts the descending direction the verifier reads it in, and the reserved slot makes an inclusive page return limit + 1 rows matching the page query. GroveDB 6 rejects both: "inverted op in a left-to-right walk" and "Proof returns more data than limit". When the merge lands on the page query's own layer, keep the requested direction, and reserve the extra slot only when the page's items do not already cover the cursor key. Round-trip tests page the family contract by $id in both directions with inclusive and exclusive cursors, for plain and history-keeping document types, verifying every page's proof. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The proved contestant-votes test only requested whole result sets, so the helper's forwarding of the request's count and cursor into the verifier query was never exercised. Add paged proved requests over the 50-voter contender: a limit smaller than the result set with inclusive and exclusive cursors in both directions, plus a short final page, each compared with the matching slice of the full ascending voter sequence. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ntrusted-bincode Brings the shared-layer cursor merge fix and the paged vote test from #4635.
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Phase 2 only (queue backlog)
Both prior findings are fixed at head 7b596ff, and no in-scope findings remain. Independent locked, offline, all-features validation passed all 57 Drive query tests, all 4 history-query tests, and the proved contestant-vote pagination test; the full-range diff check also passed. Full-workspace, browser, device, and live-network validation were not repeated.
Source: reviewer 1: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 2: gpt-6-astra (agent: phase2-reviewer, role: ffi-engineer); reviewer 3: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 4: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6-astra (agent: astra-verifier, role: final-verifier)
Review provenance
- Triage:
criticalbygpt-6-astra(effort low) — This upgrades consensus-critical GroveDB and RocksDB storage dependencies and changes cryptographic proof traversal, cursor merging, and pagination limits, where subtle incompatibilities could break state processing, proof verification, or persisted-data compatibility across Platform consumers. - Phase 1 reviewers: not run (skipped for throughput: 23 PRs queued, above the 10 limit)
- Fresh verifier:
gpt-6-astra— final-verifier; agentastra-verifier - Phase 2 reviewers:
gpt-6-astra— general (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— ffi-engineer (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— security-auditor (completed, effort xhigh); agentphase2-reviewer
Issue being fixed or feature implemented
Platform pinned GroveDB at
6fc7e1e8together with the QuantumExplorer rust-rocksdb fork, and rust-dashcore at a revision from before GroveDB's bincode moved to the publishedgrovedb-bincodecrate. This PR advances those dependencies and adapts Drive, the FFI and the wasm SDK to the GroveDB API changes, without changing how Platform decodes anything. The untrusted-decoding adoption in #4625 builds on top of it.Companion changes are merged: GroveDB #948 (develop
985ece6574f5fc8fe80db7b304a9a701c0f4afa6) and rust-dashcore #1005 (dev057e79cd3e1eeb29676288e9aab76b867d8e707f).What was done?
985ece65(6.0.0, about fifty commits since6fc7e1e8, including the V1 proof hardening series) and rust-dashcore to dev057e79cd. Userocksdb0.25.0 from crates.io instead of the rust-rocksdb fork. Make the workspacebincodedependency an alias for the publishedgrovedb-bincode2.1.0 so GroveDB, rust-dashcore and Platform share one set of bincode traits; no Cargo patch is needed.librocksdbGitHub action, matchinglibrocksdb-sys0.19.0+11.8.1. RocksDB 11's C API dropped thein_rangeslice-transform callback, so bindgen against the 10.x headers no longer compiles therocksdbcrate.orderBydirection, and GroveDB fix(scripts): update configure_test_network for hpmn #863 now rejects a layer proof read in the other op family, which failed every proved descending page with a cursor. When a$id-ordered page merges its cursor lookup onto the primary-key layer itself (also for history-keeping types, whose cursor sits one level below), the prover now keeps the requested direction and reserves the extra limit slot only when the page's items do not already cover the cursor, since GroveDB 6 also rejects a layer returning more rows than the verifier's limit; round-trip tests page by$idin both directions with inclusive and exclusive cursors. The drive-abci contestant-votes test helper now verifies with the requested order and cursor, as the SDK's proof verifier does, and the proved test pages with cursors and limits in both directions.How Has This Been Tested?
Local macOS:
cargo fmt --all --check,cargo clippy --workspace --all-targets --all-features --locked -- --no-deps -D warnings, and the CI Rust job's nextest package set (--all-features, same test filter): 14,855 tests passed, none failed. The Dockerdeps-rocksdbstage was built with RocksDB 11.8.1. No device or browser execution and no live network upgrade were exercised.Breaking Changes
Rust consumers that serialize Platform or Core types must use the published
grovedb-bincode2.1.0 crate, since bincode trait identities now come from it. Existing bincode encodings and the C ABI are unchanged.Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit