chore: bump grovedb to develop (352c2f55)#3656
Open
QuantumExplorer wants to merge 5 commits into
Open
Conversation
Brings in 4 commits from grovedb develop on top of 7a649386: - #666 Element::NotCountedOrSummed wrapper for combined sum+count opt-out - #667 Element::ReferenceWithSumItem + RefreshReferenceWithSumItem batch op - #668 CI shard cleanup (no platform impact) - #661 Element::ProvableSumTree + AggregateSumOnRange query API-surface fallout adapted in the same commit: - drive: thread new `non_counted: bool` arg through `QualifiedGroveDbOp::refresh_reference_op`. Drive's wrapper still produces plain (counted) references, so the new flag is hard-coded to `false` to preserve prior behavior — non-counted refs and the new `ReferenceWithSumItem` shape are not yet used by platform. - drive: cover the new `TreeType::ProvableSumTree` variant in both `LowLevelDriveOperationTreeTypeConverter::empty_tree_operation_for_known_path_key` and `grove_insert_empty_tree_v0`, using the matching `Element::empty_provable_sum_tree[_with_flags]()` constructor. - rs-sdk-ffi: extend `format_element_data` / `format_element_type` in `system/queries/path_elements.rs` with arms for the three new `Element` variants (`NotCountedOrSummed`, `ReferenceWithSumItem`, `ProvableSumTree`) so the JSON renderer stays exhaustive. - wasm-drive-verify: extend the `QueryItem` match in `state_transition_execution_path_queries::token_transition` with an `AggregateSumOnRange` arm that mirrors the existing `AggregateCountOnRange` "unsupported in token-transition path queries" error, since aggregate range items aren't part of those path queries. `cargo check --workspace --all-features` and `cargo clippy -p drive -p wasm-drive-verify -p rs-sdk-ffi --all-features --no-deps` are clean. The existing `grove_insert_empty_tree` unit tests in rs-drive still pass. 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 (74)
📝 WalkthroughWalkthroughThis PR pins all grovedb-family dependencies across the platform to a new commit (352c2f...) that introduces ProvableSumTree and ReferenceWithSumItem element types. The codebase is extended to support these new variants in empty-tree operations, reference refresh logic, and SDK JSON serialization, with query-type validation added for token-transition paths. ChangesGroveDB Dependency and ProvableSumTree Support
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
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 |
Collaborator
Review GateCommit:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/rs-drive/src/util/grove_operations/grove_insert_empty_tree/v0/mod.rs (1)
31-31: ⚡ Quick winAdd a test for the new
ProvableSumTreebranch.The new match arm in Line 31 is not covered by this module’s current tests, so this mapping can regress silently.
Proposed test addition
@@ #[test] + fn test_grove_insert_empty_tree_provable_sum() { + let drive = setup_drive(None); + let pv = PlatformVersion::latest(); + let tx = drive.grove.start_transaction(); + + drive + .grove_insert_empty_tree_v0( + SubtreePath::empty(), + b"provable_sum", + TreeType::ProvableSumTree, + Some(&tx), + None, + &mut vec![], + &pv.drive, + ) + .expect("expected to insert root tree"); + + let mut query_ops = vec![]; + let element = drive + .grove_get_raw( + SubtreePath::empty(), + b"provable_sum", + DirectQueryType::StatefulDirectQuery, + Some(&tx), + &mut query_ops, + &pv.drive, + ) + .expect("expected to get element"); + + assert!( + matches!(element, Some(Element::ProvableSumTree(..))), + "Expected a provable sum tree element after insert" + ); + } + + #[test] fn test_grove_insert_empty_tree_count_sum() {🤖 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/util/grove_operations/grove_insert_empty_tree/v0/mod.rs` at line 31, Add a unit test that exercises the new TreeType::ProvableSumTree match arm so it can't regress: call the public function in this module that performs the match (the function containing the TreeType -> Element mapping in grove_insert_empty_tree v0) with TreeType::ProvableSumTree and assert the returned Element equals Element::empty_provable_sum_tree(); place the test alongside the module's other tests (or in the module's #[cfg(test)] tests) so it runs in CI.
🤖 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.
Nitpick comments:
In
`@packages/rs-drive/src/util/grove_operations/grove_insert_empty_tree/v0/mod.rs`:
- Line 31: Add a unit test that exercises the new TreeType::ProvableSumTree
match arm so it can't regress: call the public function in this module that
performs the match (the function containing the TreeType -> Element mapping in
grove_insert_empty_tree v0) with TreeType::ProvableSumTree and assert the
returned Element equals Element::empty_provable_sum_tree(); place the test
alongside the module's other tests (or in the module's #[cfg(test)] tests) so it
runs in CI.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 05fb9d9d-6498-4669-b957-3108dfc6bc44
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
packages/rs-dpp/Cargo.tomlpackages/rs-drive-abci/Cargo.tomlpackages/rs-drive/Cargo.tomlpackages/rs-drive/src/fees/op.rspackages/rs-drive/src/util/grove_operations/grove_insert_empty_tree/v0/mod.rspackages/rs-platform-version/Cargo.tomlpackages/rs-platform-wallet/Cargo.tomlpackages/rs-sdk-ffi/src/system/queries/path_elements.rspackages/rs-sdk/Cargo.tomlpackages/wasm-drive-verify/src/state_transition/state_transition_execution_path_queries/token_transition.rs
The grovedb bump in this PR turned on a stricter proof-envelope decoder (`decode_grovedb_proof_canonical`, introduced alongside the ProvableSumTree / AggregateSumOnRange work in grovedb#661) that rejects any trailing bytes after the bincode envelope. The previous decoder silently tolerated them. Two dash-sdk fixture-replay tests (`test_token_pre_programmed_distributions_present` and `test_token_pre_programmed_distributions_absent`) fail because their recorded proof bytes carry a single legacy trailing byte that the new decoder rejects with "data corruption error: proof has 1 trailing bytes after the encoded envelope". Regenerating those fixtures requires a devnet run (`yarn reset && SDK_TEST_DATA=true yarn start && ./packages/rs-sdk/scripts/generate_test_vectors.sh test_token_pre_programmed_distributions`), so mark both as `#[ignore]` with a note explaining the regen step; production proofs emitted by the bumped drive-abci will not carry the trailing byte, so this is purely fixture rot, not a runtime regression. Also addresses the CodeRabbit nitpick on the `grove_insert_empty_tree_v0` match: add `test_grove_insert_empty_tree_provable_sum` covering the new `TreeType::ProvableSumTree` arm so the mapping cannot regress silently. All 6 tests in the module pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Same fixture rot as the dash-sdk tests already ignored in the previous commit: the rs-sdk-ffi integration tests `test_token_pre_programmed_distributions` and `test_token_pre_programmed_distributions_absent` share the proof vectors under `packages/rs-sdk/tests/vectors/`, which the new strict `decode_grovedb_proof_canonical` rejects because of a trailing byte. Mark both as `#[ignore]` with the same regen note. These are the only other tests in the workspace that exercise the `TokenPreProgrammedDistributions` query path; the remaining ~100 fixture-replay tests passed CI cleanly with the bump. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v3.1-dev #3656 +/- ##
============================================
- Coverage 88.07% 88.03% -0.05%
============================================
Files 2521 2522 +1
Lines 308681 308993 +312
============================================
+ Hits 271879 272026 +147
- Misses 36802 36967 +165
🚀 New features to boost your workflow:
|
The grovedb bump turned on the new strict
`decode_grovedb_proof_canonical` (introduced alongside the
ProvableSumTree / AggregateSumOnRange work in grovedb#661) which
rejects any bincode-decoded grovedb proof that leaves trailing bytes
in the input slice. The previous decoder silently tolerated them.
The grovedb docstring itself notes the trailing bytes are
"harmless for the chain-bound correctness guarantee": the decoded
`GroveDBProof` and the root hash it produces are unchanged. Empirically
verified on the failing fixture proof (651 bytes, 1 trailing byte):
the decoder consumes 650 bytes, the decoded value re-encodes byte-for-byte
to those 650 bytes, and `GroveDb::verify_query` on the 650-byte prefix
returns Ok.
Production drive-abci on this grovedb revision emits canonical proofs
(the rs-drive `should_prove_and_verify_*` round-trip tests prove that),
so the strict check would only fire on proofs from older nodes or
recorded fixtures captured against them. That is exactly what broke
the four `test_token_pre_programmed_distributions[_absent]` tests in
dash-sdk and rs-sdk-ffi — they replay legacy proof bytes captured
before grovedb's canonical-encoder fix.
This commit adds a thin backward-compat shim:
- `packages/rs-drive/src/verify/grovedb_proof_compat.rs` exposes
`canonicalize_grovedb_proof(proof: &[u8]) -> Result<Cow<[u8]>, Error>`.
It runs the same bincode decode the new verifier uses, returns the
proof unchanged when it is already canonical (borrow, no alloc), and
trims the prefix bincode actually consumed when trailing bytes are
present. A `tracing::trace!` breadcrumb fires on the trim path so any
remaining source of non-canonical proofs is observable.
- Every `GroveDb::verify_*(proof, ...)` call site in
`packages/rs-drive/src/verify/**` (71 files) is wrapped:
`GroveDb::verify_*(&canonicalize_grovedb_proof(proof)?, ...)`. The
rewrite is mechanical and only touches the proof byte argument; all
other parameters and control flow are unchanged.
Test-internal `GroveDb::verify_query` callers in `rs-drive-abci`'s
query unit tests and `rs-drive/src/prove/...` round-trip helpers are
intentionally not wrapped — they verify proofs they just produced with
the bumped grovedb, so the canonical bytes are guaranteed by
construction.
Verified end-to-end:
- The four previously-failing tests now pass:
`dash-sdk::fetch::tokens::token_pre_programmed_distributions::
{test_token_pre_programmed_distributions_present,_absent}`,
`rs-sdk-ffi::token::{test_token_pre_programmed_distributions,_absent}`.
- All 243 `cargo test -p drive --lib verify` tests pass.
- `cargo check --workspace --all-features` and
`cargo clippy -p drive --features server --no-deps` are clean.
- The shim itself has unit tests covering canonical pass-through,
trailing-byte trimming, and malformed-envelope rejection.
Reverts the `#[ignore]` workarounds applied in the two preceding
commits — those are no longer needed now that the verifier handles
legacy proofs natively.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov flagged the new `Element::empty_provable_sum_tree_with_flags` arm in `LowLevelDriveOperationTreeTypeConverter::empty_tree_operation_for_known_path_key` as the only uncovered line in `packages/rs-drive/src/fees/op.rs`. Drive doesn't construct `ProvableSumTree` anywhere outside the matching `grove_insert_empty_tree_v0` arm (which already has a dedicated test), so the converter arm needs its own test. The test calls the trait method with `TreeType::ProvableSumTree` and asserts the resulting `LowLevelDriveOperation` wraps a `GroveOp::InsertOrReplace` carrying an `Element::ProvableSumTree`. 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
2 participants
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.
Issue being fixed or feature implemented
Brings the grovedb dependency up to the latest
develop(352c2f5504fba8795e8ed1056753bfd73c13b4cc) so we can pick up the new sum-tree work and related element/op surface that has landed upstream.HEADof grovedb develop is 4 commits ahead of our previous pin7a649386:feat(grovedb,merk,element): add Element::NotCountedOrSummed wrapper for combined sum+count opt-outfeat(grovedb,element): add Element::ReferenceWithSumItem variant + RefreshReferenceWithSumItem batch opci: drop broken self-hosted macOS runner, run sharded on Ubuntu directly(no platform impact)feat: add Element::ProvableSumTree + AggregateSumOnRange queryWhat was done?
Dependency bump
Updated the
revfor every grovedb-family git dep in the six platformCargo.tomls (rs-drive,rs-drive-abci,rs-sdk,rs-dpp,rs-platform-version,rs-platform-wallet) and refreshedCargo.lockfor all 15 grovedb sub-crates.API-surface adaptations
The bump exposed three breaking API changes that needed thin call-site fixes:
QualifiedGroveDbOp::refresh_reference_opgained anon_counted: boolargument (from grovedb#667).rs-drive's wrapperLowLevelDriveOperation::refresh_reference_for_known_path_key_reference_info(packages/rs-drive/src/fees/op.rs) now passesfalsefor the new flag. Drive's refresh path only ever produces plain (counted) references — the newReferenceWithSumItem/ non-counted shapes aren't used by platform yet — so hard-codingfalsepreserves prior behavior exactly.TreeType::ProvableSumTreevariant added (from grovedb#661).Two exhaustive matches needed a new arm:
LowLevelDriveOperationTreeTypeConverter::empty_tree_operation_for_known_path_key(packages/rs-drive/src/fees/op.rs) andDrive::grove_insert_empty_tree_v0(packages/rs-drive/src/util/grove_operations/grove_insert_empty_tree/v0/mod.rs). Both now route toElement::empty_provable_sum_tree[_with_flags](), mirroring the existingProvableCountTreearms.Three new
Elementvariants (NotCountedOrSummed,ReferenceWithSumItem,ProvableSumTree) added toformat_element_data/format_element_typeinpackages/rs-sdk-ffi/src/system/queries/path_elements.rs, so the JSON renderer fordash_sdk_system_get_path_elementsstays exhaustive.QueryItem::AggregateSumOnRangevariant added (from grovedb#661) — handled inpackages/wasm-drive-verify/src/state_transition/state_transition_execution_path_queries/token_transition.rswith an arm that mirrors the existingAggregateCountOnRange"unsupported in token-transition path queries" error.How Has This Been Tested?
cargo check --workspace --all-features— clean.cargo clippy -p drive -p wasm-drive-verify -p rs-sdk-ffi --all-features --no-deps— clean.cargo test -p drive --lib util::grove_operations::grove_insert_empty_tree— 5 tests pass (coversNormalTree,SumTree,BigSumTree,CountTree,CountSumTreeinsert paths whose code lives next to the newProvableSumTreearm).cargo fmt --allapplied.Breaking Changes
None for platform consumers. The grovedb on-disk encoding for variants that existed in the previous pin is unchanged (the new variants
NotCountedOrSummed=17,ReferenceWithSumItem=18,ProvableSumTree=19were appended). Drive's publicrefresh_reference_for_known_path_key_reference_infowrapper keeps the same signature.Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Chores