fix: add version gating to proof generation methods - #513
Merged
QuantumExplorer merged 2 commits intoMar 6, 2026
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded five new proof-version fields to GroveDBOperationsProofVersions and initialized them in v1/v2; introduced GROVE_V3 and its descriptor; added a version-with-cost macro; implemented version-dispatched non-serialized proof paths with cost guards; updated many tests and public proof API call sites. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant GroveDB as GroveDB
participant VersionCheck as VersionChecker
participant ProofGen as ProofGenerator
participant CostSys as CostSystem
Client->>GroveDB: prove_query(path_query, options, grove_version)
GroveDB->>VersionCheck: check_grovedb_v0_or_v1_with_cost(grove_version)
VersionCheck-->>GroveDB: ok / Err(UnknownVersionMismatch)
alt version == 0
GroveDB->>ProofGen: prove_query_non_serialized_v0(path_query, options)
ProofGen->>CostSys: charge cost (v0 path)
ProofGen-->>GroveDB: GroveDBProof (Merkle-only)
else version == 1
GroveDB->>ProofGen: prove_query_non_serialized_v1(path_query, options)
ProofGen->>CostSys: charge cost (v1 path, mmr/bulk support)
ProofGen-->>GroveDB: GroveDBProof (layered)
end
GroveDB->>GroveDB: serialize GroveDBProof -> Vec<u8>
GroveDB-->>Client: Result<Vec<u8>, Error>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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
❌ Your patch status has failed because the patch coverage (31.91%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #513 +/- ##
===========================================
- Coverage 90.69% 90.69% -0.01%
===========================================
Files 182 182
Lines 49278 49311 +33
===========================================
+ Hits 44694 44723 +29
- Misses 4584 4588 +4
🚀 New features to boost your workflow:
|
QuantumExplorer
commented
Mar 5, 2026
| pub struct GroveDBOperationsProofVersions { | ||
| pub prove_query: FeatureVersion, | ||
| pub prove_query_many: FeatureVersion, | ||
| pub prove_query_v1: FeatureVersion, |
Member
Author
There was a problem hiding this comment.
there should eb no prove_query_v1, just prove_query
Contributor
There was a problem hiding this comment.
Addressed in 7b3a2a6 — prove_query_v1 field removed; the method now correctly references the existing prove_query version field.
| pub prove_trunk_chunk_non_serialized: FeatureVersion, | ||
| pub prove_branch_chunk: FeatureVersion, | ||
| pub prove_branch_chunk_non_serialized: FeatureVersion, | ||
| pub prove_query_v1_non_serialized: FeatureVersion, |
Member
Author
There was a problem hiding this comment.
there should be no v1, as it's the v1 of prove_query_non_serialized
Contributor
There was a problem hiding this comment.
Also addressed in 7b3a2a6 — prove_query_v1_non_serialized field removed. The v1 methods correctly share version fields with their base methods now.
QuantumExplorer
commented
Mar 5, 2026
Comment on lines
+107
to
+110
| check_grovedb_v0_with_cost!( | ||
| "prove_query_v1", | ||
| grove_version.grovedb_versions.operations.proof.prove_query | ||
| ); |
Member
Author
There was a problem hiding this comment.
This should not not be check_grovedb_v0_with_cost, because it's v1
Contributor
There was a problem hiding this comment.
Fixed in 441a4cd — now uses check_grovedb_v0_or_v1_with_cost! which correctly accepts both v0 and v1 for the v1 proof methods. Also added the new check_grovedb_v0_or_v1_with_cost macro (cost-returning variant) to support this.
thepastaclaw
approved these changes
Mar 5, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Contributor
There was a problem hiding this comment.
All three self-review comments have been addressed in the follow-up commits:
- 7b3a2a6: Removed
prove_query_v1andprove_query_v1_non_serializedversion fields — v1 methods now correctly reuseprove_queryandprove_query_non_serializedfields. - 441a4cd: Changed
prove_query_v1andprove_query_v1_non_serializedchecks fromcheck_grovedb_v0_with_cost!tocheck_grovedb_v0_or_v1_with_cost!— correctly accepts both v0 and v1.
Quick review of the final state:
- ✅
GroveDBOperationsProofVersionsstruct has 5 new fields (no v1 dupes) - ✅
GROVE_V1andGROVE_V2both initialize the new fields to0 - ✅ New
check_grovedb_v0_or_v1_with_cost!macro mirrors the existingcheck_grovedb_v0_or_v1but returns a cost-wrapped error - ✅ v0-only methods use
check_grovedb_v0_with_cost!, v1 methods usecheck_grovedb_v0_or_v1_with_cost!
LGTM — all version gates look correct.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
grovedb/src/operations/proof/generate.rs (2)
119-124:⚠️ Potential issue | 🟠 MajorAvoid shrinking the versioned proof API in this fix.
Making
prove_query_v1andprove_query_non_serialized_v1pub(crate)turns this bugfix into a breaking library change for downstream crates that call the version-pinned methods directly. If the intent is to funnel callers throughprove_query*, keep deprecated public wrappers for a release or document this as a semver-major change.Also applies to: 842-847
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@grovedb/src/operations/proof/generate.rs` around lines 119 - 124, The change made the version-pinned APIs prove_query_v1 and prove_query_non_serialized_v1 non-public (pub(crate)), which is a breaking change for downstream crates; restore a public surface by either reverting their visibility to pub or add back thin public wrappers named prove_query_v1 and prove_query_non_serialized_v1 that forward to the internal implementations (which can be renamed or kept pub(crate)), and mark those wrappers #[deprecated] with a message pointing to the new prove_query* entrypoints so callers are not broken immediately but guided to migrate.
99-102:⚠️ Potential issue | 🟠 MajorCall the V0 non-serialized helper directly.
prove_query_v0currently re-dispatches throughprove_query_non_serialized, so it depends onoperations.proof.prove_query_non_serializedinstead of theprove_queryversion that selected this branch. If those fields ever diverge, this can encode a V1 proof from the V0 path or reject an otherwise valid V0 call.Suggested fix
let proof = cost_return_on_error!( &mut cost, - self.prove_query_non_serialized(path_query, prove_options, grove_version) + self.prove_query_non_serialized_v0(path_query, prove_options, grove_version) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@grovedb/src/operations/proof/generate.rs` around lines 99 - 102, In prove_query_v0, avoid re-dispatching through prove_query_non_serialized (which ties V0 to the generic prove_query path); instead call the V0-specific non-serialized helper directly (the V0 helper function used for non-serialized proofs) so V0 uses its own implementation rather than operations.proof.prove_query_non_serialized; update the call site in prove_query_v0 to invoke that V0 non-serialized helper (keep prove_query and prove_query_non_serialized unchanged) to ensure V0 proofs remain self-contained.
🧹 Nitpick comments (1)
grovedb/src/tests/proof_coverage_tests.rs (1)
1919-1953: This now skips the public version-gating path.Calling
prove_query_non_serialized_v1directly only covers the internal helper rename. It no longer exercises the newprove_query_non_serializeddispatch/rejection logic that this PR adds. A companion test that drives the public method with an unsupportedprove_query_non_serializedversion would cover the actual regression surface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@grovedb/src/tests/proof_coverage_tests.rs` around lines 1919 - 1953, The test currently calls the internal helper prove_query_non_serialized_v1 directly and therefore skips the public dispatch/rejection path; update the test (or add a companion test) to call the public method prove_query_non_serialized with an unsupported version value (e.g., a fake/latest+1 or an explicit unsupported enum variant) and assert that it returns the expected rejection/error, while keeping the existing setup (inserts, PathQuery, grove_version) so the dispatch logic is exercised; reference prove_v1_non_serialized, prove_query_non_serialized, and prove_query_non_serialized_v1 when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@grovedb/src/operations/proof/generate.rs`:
- Around line 119-124: The change made the version-pinned APIs prove_query_v1
and prove_query_non_serialized_v1 non-public (pub(crate)), which is a breaking
change for downstream crates; restore a public surface by either reverting their
visibility to pub or add back thin public wrappers named prove_query_v1 and
prove_query_non_serialized_v1 that forward to the internal implementations
(which can be renamed or kept pub(crate)), and mark those wrappers #[deprecated]
with a message pointing to the new prove_query* entrypoints so callers are not
broken immediately but guided to migrate.
- Around line 99-102: In prove_query_v0, avoid re-dispatching through
prove_query_non_serialized (which ties V0 to the generic prove_query path);
instead call the V0-specific non-serialized helper directly (the V0 helper
function used for non-serialized proofs) so V0 uses its own implementation
rather than operations.proof.prove_query_non_serialized; update the call site in
prove_query_v0 to invoke that V0 non-serialized helper (keep prove_query and
prove_query_non_serialized unchanged) to ensure V0 proofs remain self-contained.
---
Nitpick comments:
In `@grovedb/src/tests/proof_coverage_tests.rs`:
- Around line 1919-1953: The test currently calls the internal helper
prove_query_non_serialized_v1 directly and therefore skips the public
dispatch/rejection path; update the test (or add a companion test) to call the
public method prove_query_non_serialized with an unsupported version value
(e.g., a fake/latest+1 or an explicit unsupported enum variant) and assert that
it returns the expected rejection/error, while keeping the existing setup
(inserts, PathQuery, grove_version) so the dispatch logic is exercised;
reference prove_v1_non_serialized, prove_query_non_serialized, and
prove_query_non_serialized_v1 when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a1ab11f9-d288-45e7-a309-e261e629632d
📒 Files selected for processing (2)
grovedb/src/operations/proof/generate.rsgrovedb/src/tests/proof_coverage_tests.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
grovedb/src/tests/provable_count_tree_comprehensive_test.rs (1)
122-123:⚠️ Potential issue | 🟠 MajorAdd
insert_all()to query to match full-tree semantics across this file.Both lines 122-123 and 752-753 build an empty Query without calling
insert_all(), despite the test intent and comment ("all items"). All other full-tree queries in this file (lines 235-236, 354-355, 481-482, 607-608) explicitly callinsert_all()afterQuery::new(). Without it, these tests query an empty set instead of validating proof generation for counted-tree content.Suggested change
- let query = Query::new(); // Empty query gets all items + let mut query = Query::new(); + query.insert_all(); let path_query = PathQuery::new_unsized(vec![b"counts".to_vec()], query);- let query = Query::new(); + let mut query = Query::new(); + query.insert_all(); let path_query = PathQuery::new_unsized(vec![b"counts".to_vec()], query);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@grovedb/src/tests/provable_count_tree_comprehensive_test.rs` around lines 122 - 123, The test builds empty queries with Query::new() then wraps them with PathQuery::new_unsized(...) but never calls insert_all(), so the queries return nothing; update both occurrences that construct the query used with PathQuery::new_unsized to call insert_all() on the Query (i.e., replace Query::new() with Query::new().insert_all() or call .insert_all() before creating the PathQuery) so the queries match full-tree semantics for counted-tree proofs.
♻️ Duplicate comments (1)
grovedb/src/operations/proof/generate.rs (1)
77-80:⚠️ Potential issue | 🔴 CriticalUse the v0/v1 guard here, or V1 proofs never reach the dispatcher.
prove_query_non_serialized()now handles both proof versions, but this v0-only gate rejectsoperations.proof.prove_query == 1first. That makes the publicprove_query()API unusable for the new V1/GROVE_V3 path and breaks the renamed test call sites that now go through it.Suggested fix
-use grovedb_version::{check_grovedb_v0_with_cost, version::GroveVersion}; +use grovedb_version::{ + check_grovedb_v0_or_v1_with_cost, check_grovedb_v0_with_cost, version::GroveVersion, +}; ... - check_grovedb_v0_with_cost!( + check_grovedb_v0_or_v1_with_cost!( "prove_query", grove_version.grovedb_versions.operations.proof.prove_query );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@grovedb/src/operations/proof/generate.rs` around lines 77 - 80, The v0-only check_grovedb_v0_with_cost! gate around prove_query blocks the V1 path so public prove_query() never reaches the dispatcher; change the guard to allow both v0 and v1 (or use the v0/v1-aware check macro) so prove_query_non_serialized() can handle both versions. Locate the call using check_grovedb_v0_with_cost! around "prove_query" and replace it with the v0/v1 guard variant (or remove the strict v0 macro) so operations.proof.prove_query == 1 is accepted and dispatches into prove_query_non_serialized().
🧹 Nitpick comments (1)
grovedb/src/tests/mod.rs (1)
1875-2059: Extract the shared proof fixture/assertion flow into helpers.These four tests rebuild nearly the same trees and repeat the same prove/verify/assert logic. A small helper parameterized by
&GroveVersionplus an optional expected hex string would make future proof-version updates much easier to keep in sync.Also applies to: 2062-2234, 2236-2434, 2436-2624
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@grovedb/src/tests/mod.rs` around lines 1875 - 2059, The test test_path_query_proofs_without_subquery_with_reference_for_version_2 (and the other tests listed) duplicate tree setup and prove/verify/assert flow; extract a small helper (e.g. build_test_tree_and_run_proof or assert_proof_for_version) that accepts &GroveVersion and an optional expected hex string and performs: create DB via make_test_grovedb, insert the same nodes (use the same keys: TEST_LEAF, ANOTHER_TEST_LEAF, innertree/innertree2/innertree3, key1..key5 and reference inserts), run temp_db.prove_query(&path_query, ...), call GroveDb::verify_query_raw, compare root hash to temp_db.root_hash, and call compare_result_tuples with expected serialized Elements; then replace the body of test_path_query_proofs_without_subquery_with_reference_for_version_2 (and the other repeated tests) with calls to that helper passing the grove version and expected hex string where needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@grovedb-version/src/version/mod.rs`:
- Line 35: Tests in grovedb-version need updating because GROVE_V3 was added to
the GROVE_VERSIONS array, which invalidates assertions in
grovedb-version/src/tests.rs; update the test expectations to reflect the new
version list and behavior: change any length assertion (currently asserting
GROVE_VERSIONS.len() == 2) to the new count including GROVE_V3, and update
expectations for GroveVersion::latest() to return GROVE_V3 (or otherwise compute
expected latest from GROVE_VERSIONS) so tests reference the GROVE_VERSIONS
constant and GroveVersion::latest() symbol instead of hard-coded old values.
In `@grovedb/src/tests/mod.rs`:
- Line 52: The tests currently reference GroveVersion::latest() for the proof-v1
dispatch paths; change them to use the concrete constant for the intended fixed
version (import and use GROVE_V3) so the tests are deterministic. Update the
imports at the top to include grovedb_version::version::v3::GROVE_V3 (in
addition to the existing GROVE_V2 import) and replace occurrences of
GroveVersion::latest() in the proof-v1 dispatch tests (the test blocks around
the "proof-v1" dispatch checks) with GROVE_V3.
In `@grovedb/src/tests/proof_coverage_tests.rs`:
- Line 17: Tests that assert V1 dispatch currently call GroveVersion::latest(),
which will break when latest() advances; change those V1-specific tests to use
the explicit V1 version constant instead. Import and use the concrete V1
constant (e.g. grovedb_version::version::v1::GROVE_V1) in place of
GroveVersion::latest() in the tests that validate operations.proof.prove_query
mapping to 1 (references: the test file's use of GroveVersion::latest() and the
proof coverage assertions that expect prove_query => 1).
- Around line 1950-1953: The test is calling prove_query_non_serialized_v1(...)
directly which bypasses the dispatcher; change it to call
prove_query_non_serialized(&path_query, None, grove_version) on db (the
dispatcher) and then assert the result is a GroveDBProof::V1 variant (e.g. match
or if let to confirm GroveDBProof::V1(_)) before unwrapping/using the inner V1
proof so the test verifies correct routing through the dispatcher.
In `@grovedb/src/tests/provable_count_sum_tree_tests.rs`:
- Around line 2656-2673: The current check_v1_layer_proof returns early when
layer.merk_proof is not ProofBytes::Merk, which prevents descending into
layer.lower_layers and misses counted child Merk layers under non-Merk parents;
update the function so that non-Merk cases do not return but instead skip
decoding and still iterate/recursively call check_v1_layer_proof on
layer.lower_layers (e.g., replace the '_ => return' behavior with a no-op or use
if let ProofBytes::Merk(...) { ... } and always recurse into lower_layers),
preserving the found_kvdigest_count update logic for Merk bytes.
---
Outside diff comments:
In `@grovedb/src/tests/provable_count_tree_comprehensive_test.rs`:
- Around line 122-123: The test builds empty queries with Query::new() then
wraps them with PathQuery::new_unsized(...) but never calls insert_all(), so the
queries return nothing; update both occurrences that construct the query used
with PathQuery::new_unsized to call insert_all() on the Query (i.e., replace
Query::new() with Query::new().insert_all() or call .insert_all() before
creating the PathQuery) so the queries match full-tree semantics for
counted-tree proofs.
---
Duplicate comments:
In `@grovedb/src/operations/proof/generate.rs`:
- Around line 77-80: The v0-only check_grovedb_v0_with_cost! gate around
prove_query blocks the V1 path so public prove_query() never reaches the
dispatcher; change the guard to allow both v0 and v1 (or use the v0/v1-aware
check macro) so prove_query_non_serialized() can handle both versions. Locate
the call using check_grovedb_v0_with_cost! around "prove_query" and replace it
with the v0/v1 guard variant (or remove the strict v0 macro) so
operations.proof.prove_query == 1 is accepted and dispatches into
prove_query_non_serialized().
---
Nitpick comments:
In `@grovedb/src/tests/mod.rs`:
- Around line 1875-2059: The test
test_path_query_proofs_without_subquery_with_reference_for_version_2 (and the
other tests listed) duplicate tree setup and prove/verify/assert flow; extract a
small helper (e.g. build_test_tree_and_run_proof or assert_proof_for_version)
that accepts &GroveVersion and an optional expected hex string and performs:
create DB via make_test_grovedb, insert the same nodes (use the same keys:
TEST_LEAF, ANOTHER_TEST_LEAF, innertree/innertree2/innertree3, key1..key5 and
reference inserts), run temp_db.prove_query(&path_query, ...), call
GroveDb::verify_query_raw, compare root hash to temp_db.root_hash, and call
compare_result_tuples with expected serialized Elements; then replace the body
of test_path_query_proofs_without_subquery_with_reference_for_version_2 (and the
other repeated tests) with calls to that helper passing the grove version and
expected hex string where needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b1f9ca99-b8ed-469a-8756-71c0fd2e8327
📒 Files selected for processing (11)
grovedb-version/src/version/mod.rsgrovedb-version/src/version/v3.rsgrovedb/src/operations/proof/generate.rsgrovedb/src/tests/commitment_tree_tests.rsgrovedb/src/tests/dense_tree_tests.rsgrovedb/src/tests/mmr_tree_tests.rsgrovedb/src/tests/mod.rsgrovedb/src/tests/proof_coverage_tests.rsgrovedb/src/tests/provable_count_sum_tree_tests.rsgrovedb/src/tests/provable_count_tree_comprehensive_test.rsgrovedb/src/tests/v1_proof_tests.rs
| }; | ||
|
|
||
| use grovedb_version::version::GroveVersion; | ||
| use grovedb_version::version::{v2::GROVE_V2, GroveVersion}; |
Contributor
There was a problem hiding this comment.
Pin the proof-v1 tests to a concrete version instead of latest().
Line 2063 and Line 2438 make these new dispatch tests follow whatever GroveVersion::latest() becomes next, so they stop being a deterministic check of the proof-v1 path introduced in this PR. Please import and use the intended fixed version constant here (likely GROVE_V3), the same way the legacy path is pinned to GROVE_V2.
🩹 Suggested change
-use grovedb_version::version::{v2::GROVE_V2, GroveVersion};
+use grovedb_version::version::{v2::GROVE_V2, v3::GROVE_V3, GroveVersion};
...
fn test_path_query_proofs_without_subquery_with_reference() {
- let grove_version = GroveVersion::latest();
+ let grove_version = &GROVE_V3;
...
fn test_path_query_proofs_without_subquery() {
- let grove_version = GroveVersion::latest();
+ let grove_version = &GROVE_V3;Also applies to: 2062-2234, 2436-2624
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@grovedb/src/tests/mod.rs` at line 52, The tests currently reference
GroveVersion::latest() for the proof-v1 dispatch paths; change them to use the
concrete constant for the intended fixed version (import and use GROVE_V3) so
the tests are deterministic. Update the imports at the top to include
grovedb_version::version::v3::GROVE_V3 (in addition to the existing GROVE_V2
import) and replace occurrences of GroveVersion::latest() in the proof-v1
dispatch tests (the test blocks around the "proof-v1" dispatch checks) with
GROVE_V3.
Contributor
There was a problem hiding this comment.
@QuantumExplorer, understood — I'll leave the tests using GroveVersion::latest() as-is.
🧠 Learnings used
Learnt from: QuantumExplorer
Repo: dashpay/grovedb PR: 397
File: grovedb/src/tests/provable_count_sum_tree_tests.rs:82-90
Timestamp: 2025-12-17T18:39:16.131Z
Learning: In Rust test code for grovedb, when a function returns a cost-wrapped result (CostResult), using .unwrap().map_err(|e| e) is acceptable to discard cost tracking by unwrapping the inner Result and converting the error type for test helpers. This pattern should be applied across test files that interact with cost-wrapped results, not just a single test, to ensure consistency in how test helpers handle costs.
| Query, | ||
| }; | ||
| use grovedb_version::version::GroveVersion; | ||
| use grovedb_version::version::{v2::GROVE_V2, GroveVersion}; |
Contributor
There was a problem hiding this comment.
Pin the V1 coverage to a concrete Grove version.
These V1-specific tests still run through GroveVersion::latest(), so the moment latest() moves again they stop validating the V1 dispatch this PR is adding. Please use the explicit version constant that maps operations.proof.prove_query to 1 instead of relying on latest().
Suggested pattern
-use grovedb_version::version::{v2::GROVE_V2, GroveVersion};
+use grovedb_version::version::{v2::GROVE_V2, v3::GROVE_V3, GroveVersion};
- let grove_version = GroveVersion::latest();
+ let grove_version = &GROVE_V3;Also applies to: 647-647, 1104-1104, 1732-1732, 2437-2437, 3323-3323, 4025-4025, 5002-5002, 5197-5197, 5376-5376
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@grovedb/src/tests/proof_coverage_tests.rs` at line 17, Tests that assert V1
dispatch currently call GroveVersion::latest(), which will break when latest()
advances; change those V1-specific tests to use the explicit V1 version constant
instead. Import and use the concrete V1 constant (e.g.
grovedb_version::version::v1::GROVE_V1) in place of GroveVersion::latest() in
the tests that validate operations.proof.prove_query mapping to 1 (references:
the test file's use of GroveVersion::latest() and the proof coverage assertions
that expect prove_query => 1).
Contributor
There was a problem hiding this comment.
Understood, acknowledged! 🐇
🧠 Learnings used
Learnt from: QuantumExplorer
Repo: dashpay/grovedb PR: 397
File: grovedb/src/tests/provable_count_sum_tree_tests.rs:82-90
Timestamp: 2025-12-17T18:39:16.131Z
Learning: In Rust test code for grovedb, when a function returns a cost-wrapped result (CostResult), using .unwrap().map_err(|e| e) is acceptable to discard cost tracking by unwrapping the inner Result and converting the error type for test helpers. This pattern should be applied across test files that interact with cost-wrapped results, not just a single test, to ensure consistency in how test helpers handle costs.
Comment on lines
+2656
to
+2673
| fn check_v1_layer_proof( | ||
| layer: &crate::operations::proof::LayerProof, | ||
| found_kvdigest_count: &mut bool, | ||
| ) { | ||
| let merk_bytes = match &layer.merk_proof { | ||
| ProofBytes::Merk(bytes) => bytes.as_slice(), | ||
| _ => return, | ||
| }; | ||
| let decoder = Decoder::new(merk_bytes); | ||
| let ops: Vec<Op> = decoder.collect::<Result<Vec<_>, _>>().unwrap_or_default(); | ||
|
|
||
| if has_kvdigest_count(&ops) { | ||
| *found_kvdigest_count = true; | ||
| } | ||
|
|
||
| for lower_layer in layer.lower_layers.values() { | ||
| check_v1_layer_proof(lower_layer, found_kvdigest_count); | ||
| } |
Contributor
There was a problem hiding this comment.
Don't stop descending when a V1 proof layer isn't ProofBytes::Merk.
V1 proofs can have non-Merk parent layers. Returning here skips lower_layers entirely, so this search misses counted child Merk layers under MMR/BulkAppend ancestors and can false-fail on a valid proof.
Suggested change
fn check_v1_layer_proof(
layer: &crate::operations::proof::LayerProof,
found_kvdigest_count: &mut bool,
) {
- let merk_bytes = match &layer.merk_proof {
- ProofBytes::Merk(bytes) => bytes.as_slice(),
- _ => return,
- };
- let decoder = Decoder::new(merk_bytes);
- let ops: Vec<Op> = decoder.collect::<Result<Vec<_>, _>>().unwrap_or_default();
-
- if has_kvdigest_count(&ops) {
- *found_kvdigest_count = true;
- }
+ if let ProofBytes::Merk(bytes) = &layer.merk_proof {
+ let decoder = Decoder::new(bytes.as_slice());
+ let ops: Vec<Op> =
+ decoder.collect::<Result<Vec<_>, _>>().unwrap_or_default();
+
+ if has_kvdigest_count(&ops) {
+ *found_kvdigest_count = true;
+ }
+ }
for lower_layer in layer.lower_layers.values() {
check_v1_layer_proof(lower_layer, found_kvdigest_count);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fn check_v1_layer_proof( | |
| layer: &crate::operations::proof::LayerProof, | |
| found_kvdigest_count: &mut bool, | |
| ) { | |
| let merk_bytes = match &layer.merk_proof { | |
| ProofBytes::Merk(bytes) => bytes.as_slice(), | |
| _ => return, | |
| }; | |
| let decoder = Decoder::new(merk_bytes); | |
| let ops: Vec<Op> = decoder.collect::<Result<Vec<_>, _>>().unwrap_or_default(); | |
| if has_kvdigest_count(&ops) { | |
| *found_kvdigest_count = true; | |
| } | |
| for lower_layer in layer.lower_layers.values() { | |
| check_v1_layer_proof(lower_layer, found_kvdigest_count); | |
| } | |
| fn check_v1_layer_proof( | |
| layer: &crate::operations::proof::LayerProof, | |
| found_kvdigest_count: &mut bool, | |
| ) { | |
| if let ProofBytes::Merk(bytes) = &layer.merk_proof { | |
| let decoder = Decoder::new(bytes.as_slice()); | |
| let ops: Vec<Op> = | |
| decoder.collect::<Result<Vec<_>, _>>().unwrap_or_default(); | |
| if has_kvdigest_count(&ops) { | |
| *found_kvdigest_count = true; | |
| } | |
| } | |
| for lower_layer in layer.lower_layers.values() { | |
| check_v1_layer_proof(lower_layer, found_kvdigest_count); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@grovedb/src/tests/provable_count_sum_tree_tests.rs` around lines 2656 - 2673,
The current check_v1_layer_proof returns early when layer.merk_proof is not
ProofBytes::Merk, which prevents descending into layer.lower_layers and misses
counted child Merk layers under non-Merk parents; update the function so that
non-Merk cases do not return but instead skip decoding and still
iterate/recursively call check_v1_layer_proof on layer.lower_layers (e.g.,
replace the '_ => return' behavior with a no-op or use if let
ProofBytes::Merk(...) { ... } and always recurse into lower_layers), preserving
the found_kvdigest_count update logic for Merk bytes.
- prove_query delegates to prove_query_non_serialized which dispatches to v0 or v1 based on grove_version - Versioned methods (prove_query_non_serialized_v0/v1) are pub(crate) - Add check_grovedb_v0_or_v1_with_cost! macro - Add grove version v3 with v1 proof generation enabled - Pin v0-specific tests to &GROVE_V2 with _for_version_2 suffix - Original test names use GroveVersion::latest() Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
QuantumExplorer
force-pushed
the
fix/A2-add-version-gating-to-proof-methods
branch
from
March 6, 2026 03:06
2c1fc47 to
faea2ca
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
grovedb/src/operations/proof/generate.rs (4)
1418-1428:⚠️ Potential issue | 🟠 MajorDon't hide commitment-frontier read or decode failures.
Falling back to
EMPTY_SINSEMILLA_ROOThere turns storage corruption or I/O failures into a valid-looking proof bound to the wrong anchor. OnlyOk(None)should use the empty root; read and deserialize errors should fail withCorruptedDataand context.As per coding guidelines, "Wrap errors with context using
.map_err(|e| Error::CorruptedData(format!("context: {}", e)))pattern in Rust source files".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@grovedb/src/operations/proof/generate.rs` around lines 1418 - 1428, The current match silently maps read or deserialize failures to grovedb_commitment_tree::EMPTY_SINSEMILLA_ROOT; change this so only Ok(None) returns EMPTY_SINSEMILLA_ROOT and any storage read error from storage_ctx.get(COMMITMENT_TREE_DATA_KEY).value or deserialize error from grovedb_commitment_tree::CommitmentFrontier::deserialize(frontier_bytes.as_ref()) is propagated as Error::CorruptedData with context (use .map_err(|e| Error::CorruptedData(format!("...: {}", e))) or equivalent), and adjust the surrounding function to return a Result so failures bubble up instead of producing a wrong proof root. Ensure references to COMMITMENT_TREE_DATA_KEY, CommitmentFrontier::deserialize, EMPTY_SINSEMILLA_ROOT, and Error::CorruptedData are used as described.
1330-1375:⚠️ Potential issue | 🟠 MajorDon't decrement
overall_limitby the full BulkAppend span.Lines 1331-1334 collapse arbitrary query items into a single
[start, end)interval, and Line 1373 subtracts that whole span fromoverall_limit. For sparse queries, this can consume far more slots than the proof actually returns; once the span exceedsu16::MAX, theas u16cast truncates beforesaturating_sub. Please base the decrement on the actual proved positions/results and clamp before converting tou16.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@grovedb/src/operations/proof/generate.rs` around lines 1330 - 1375, The code currently computes a span (start, end) via query_items_to_range and subtracts (end - start) cast to u16 from overall_limit, which overestimates for sparse queries and truncates when the span exceeds u16::MAX; instead, compute the actual number of returned/proved entries from the generated proof (use the result from BulkAppendTreeProof::generate, e.g. count the proved positions/values in bulk_proof), clamp that count to u16::MAX before casting, and then perform the saturating_sub on overall_limit using that clamped actual_count; update the code around the overall_limit handling (where start/end and the current count are used) to read the real proved count rather than the full [start,end) span.
1635-1662:⚠️ Potential issue | 🟠 MajorGuard the inclusive /
RangeAfterendpoint math against overflow.These branches do
start + 1/end + 1on decodedu16/u64bounds. A query containing0xFFFForu64::MAXwill overflow here, panicking in debug builds and wrapping in release. Please switch tochecked_add(1)and reject overflow withError::InvalidInput.Also applies to: 1759-1782, 1803-1854
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@grovedb/src/operations/proof/generate.rs` around lines 1635 - 1662, The code does unchecked start+1 / end+1 on decoded u16/u64 values (in QueryItem::RangeAfter, RangeAfterTo, RangeAfterToInclusive and similar blocks), which can overflow; update each occurrence where you do start + 1 or adjust end to use checked_add(1) (e.g. let start = Self::decode_be_u16(...)?.checked_add(1).ok_or(Error::InvalidInput("query range overflow"))? ) and similarly for end adjustments, and if checked_add returns None return Error::InvalidInput with a clear message; apply the same change to the other similar branches mentioned (the blocks around the other line ranges) and keep using MAX_INDICES and Error::InvalidInput for the overflow rejection.
993-1013:⚠️ Potential issue | 🟠 MajorEmpty MMR subqueries are treated as hits.
generate_mmr_layer_proofreturns an empty proof whenmmr_size == 0, but Line 1012 still marks the level as having a result. That bypasses the empty-subquery decrement at Lines 1187-1192, so limit behavior now depends on whether the subtree is Merk or MMR. Please mirror the Merk path here and only mark a hit when the lower proof actually consumes fromoverall_limit.Also applies to: 1187-1192, 1235-1254
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@grovedb/src/operations/proof/generate.rs` around lines 993 - 1013, generate_mmr_layer_proof currently returns an empty proof for mmr_size == 0, but the code unconditionally sets has_a_result_at_level and inserts into lower_layers; change this to mirror the Merk path by inspecting the returned layer_proof and only treating it as a hit when it actually consumed from overall_limit (e.g., proof is not empty or proof.consumed_count > 0 / !proof.is_empty()), so set has_a_result_at_level |= true and lower_layers.insert(key.clone(), layer_proof) only when that condition is true; apply the same conditional check to the other MMR handling sites referenced (the other occurrences analogous to the Merk path at the noted locations).
♻️ Duplicate comments (2)
grovedb/src/tests/provable_count_sum_tree_tests.rs (1)
2656-2673:⚠️ Potential issue | 🟠 MajorKeep descending through V1 non-
Merklayers.
_ => returnskipslower_layers, so this helper can miss counted childMerklayers under non-Merkparents and fail on a valid V1 proof.🩹 Suggested fix
fn check_v1_layer_proof( layer: &crate::operations::proof::LayerProof, found_kvdigest_count: &mut bool, ) { - let merk_bytes = match &layer.merk_proof { - ProofBytes::Merk(bytes) => bytes.as_slice(), - _ => return, - }; - let decoder = Decoder::new(merk_bytes); - let ops: Vec<Op> = decoder.collect::<Result<Vec<_>, _>>().unwrap_or_default(); - - if has_kvdigest_count(&ops) { - *found_kvdigest_count = true; + if let ProofBytes::Merk(bytes) = &layer.merk_proof { + let decoder = Decoder::new(bytes.as_slice()); + let ops: Vec<Op> = + decoder.collect::<Result<Vec<_>, _>>().unwrap_or_default(); + + if has_kvdigest_count(&ops) { + *found_kvdigest_count = true; + } } for lower_layer in layer.lower_layers.values() { check_v1_layer_proof(lower_layer, found_kvdigest_count); }Verify by confirming that
LayerProofalways haslower_layerswhileProofByteshas non-Merkvariants, so recursion must not depend on the byte kind:#!/bin/bash set -euo pipefail rg -n -C3 'fn check_v1_layer_proof|ProofBytes::Merk|lower_layers' grovedb/src/tests/provable_count_sum_tree_tests.rs rg -n -C2 'struct LayerProof|enum ProofBytes|ProofBytes::' --type rust🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@grovedb/src/tests/provable_count_sum_tree_tests.rs` around lines 2656 - 2673, The helper check_v1_layer_proof currently returns early on non-Merk ProofBytes (match arm `_ => return`), which skips recursing into layer.lower_layers and can miss child Merk layers; change it so recursion always descends into layer.lower_layers regardless of the ProofBytes variant and only attempt to decode/collect ops when ProofBytes::Merk is present (i.e., match to get merk_bytes and run Decoder there, but do not return from the function on other variants), keeping the recursion over lower_layers after/independent of the match; update check_v1_layer_proof (and any borrow usage) accordingly so lower_layers are always traversed.grovedb/src/tests/mod.rs (1)
52-52:⚠️ Potential issue | 🟠 MajorPin the proof-V1 dispatch tests to
GROVE_V3instead ofGroveVersion::latest().These are the V1-path counterparts to the new
_for_version_2tests. Usinglatest()makes them stop being a deterministic guard for the V1 dispatch path as soon aslatest()moves forward.🩹 Suggested fix
-use grovedb_version::version::{v2::GROVE_V2, GroveVersion}; +use grovedb_version::version::{v2::GROVE_V2, v3::GROVE_V3, GroveVersion}; ... #[test] fn test_path_query_proofs_without_subquery_with_reference() { - let grove_version = GroveVersion::latest(); + let grove_version = &GROVE_V3; ... #[test] fn test_path_query_proofs_without_subquery() { - let grove_version = GroveVersion::latest(); + let grove_version = &GROVE_V3;Verify by checking that
latest()currently resolves to the concrete V1-proof version and that these tests still callGroveVersion::latest()directly:#!/bin/bash set -euo pipefail rg -n -C2 'fn test_path_query_proofs_without_subquery_with_reference\b|fn test_path_query_proofs_without_subquery\b|GroveVersion::latest\(\)' grovedb/src/tests/mod.rs rg -n -C2 'fn latest\(|GROVE_V3|prove_query_non_serialized' --type rustAlso applies to: 2062-2234, 2436-2624
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@grovedb/src/tests/mod.rs` at line 52, The tests for the V1 dispatch path should be pinned to the concrete GROVE_V3 constant rather than calling GroveVersion::latest(); update the import and usages accordingly: change the use line to import GROVE_V3 (replace any GROVE_V2 import) and replace calls to GroveVersion::latest() in the test functions (e.g., test_path_query_proofs_without_subquery_with_reference, test_path_query_proofs_without_subquery and related prove_query_non_serialized tests) with the GROVE_V3 symbol so the tests deterministically target the V1-proof dispatch path; also scan the nearby test ranges noted (around the given function blocks) and replace any other latest() occurrences with GROVE_V3.
🧹 Nitpick comments (1)
grovedb/src/tests/provable_count_tree_comprehensive_test.rs (1)
719-834: Pin this proof-shape test to an explicit Grove version.This test uses
GroveVersion::latest()but then hard-codesGroveDBProof::V1. Sincelatest()is just the tail ofGROVE_VERSIONS, the next proof-format bump will break this test for the wrong reason. Use the explicit version constant that is supposed to emit V1 proofs here instead oflatest().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@grovedb/src/tests/provable_count_tree_comprehensive_test.rs` around lines 719 - 834, The test test_provable_count_tree_proof_contains_count_nodes currently uses GroveVersion::latest() but asserts on GroveDBProof::V1; replace the dynamic latest() with the explicit V1 grove version constant (e.g., GroveVersion::V1 or the project’s named V1 constant) where grove_version is set so the test is pinned to the proof format that produces GroveDBProof::V1; update the instantiation at the top of the test (the grove_version binding used in db creation, db.insert calls, and prove_query) to use that explicit V1 constant instead of GroveVersion::latest().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@grovedb/src/operations/proof/generate.rs`:
- Around line 1418-1428: The current match silently maps read or deserialize
failures to grovedb_commitment_tree::EMPTY_SINSEMILLA_ROOT; change this so only
Ok(None) returns EMPTY_SINSEMILLA_ROOT and any storage read error from
storage_ctx.get(COMMITMENT_TREE_DATA_KEY).value or deserialize error from
grovedb_commitment_tree::CommitmentFrontier::deserialize(frontier_bytes.as_ref())
is propagated as Error::CorruptedData with context (use .map_err(|e|
Error::CorruptedData(format!("...: {}", e))) or equivalent), and adjust the
surrounding function to return a Result so failures bubble up instead of
producing a wrong proof root. Ensure references to COMMITMENT_TREE_DATA_KEY,
CommitmentFrontier::deserialize, EMPTY_SINSEMILLA_ROOT, and Error::CorruptedData
are used as described.
- Around line 1330-1375: The code currently computes a span (start, end) via
query_items_to_range and subtracts (end - start) cast to u16 from overall_limit,
which overestimates for sparse queries and truncates when the span exceeds
u16::MAX; instead, compute the actual number of returned/proved entries from the
generated proof (use the result from BulkAppendTreeProof::generate, e.g. count
the proved positions/values in bulk_proof), clamp that count to u16::MAX before
casting, and then perform the saturating_sub on overall_limit using that clamped
actual_count; update the code around the overall_limit handling (where start/end
and the current count are used) to read the real proved count rather than the
full [start,end) span.
- Around line 1635-1662: The code does unchecked start+1 / end+1 on decoded
u16/u64 values (in QueryItem::RangeAfter, RangeAfterTo, RangeAfterToInclusive
and similar blocks), which can overflow; update each occurrence where you do
start + 1 or adjust end to use checked_add(1) (e.g. let start =
Self::decode_be_u16(...)?.checked_add(1).ok_or(Error::InvalidInput("query range
overflow"))? ) and similarly for end adjustments, and if checked_add returns
None return Error::InvalidInput with a clear message; apply the same change to
the other similar branches mentioned (the blocks around the other line ranges)
and keep using MAX_INDICES and Error::InvalidInput for the overflow rejection.
- Around line 993-1013: generate_mmr_layer_proof currently returns an empty
proof for mmr_size == 0, but the code unconditionally sets has_a_result_at_level
and inserts into lower_layers; change this to mirror the Merk path by inspecting
the returned layer_proof and only treating it as a hit when it actually consumed
from overall_limit (e.g., proof is not empty or proof.consumed_count > 0 /
!proof.is_empty()), so set has_a_result_at_level |= true and
lower_layers.insert(key.clone(), layer_proof) only when that condition is true;
apply the same conditional check to the other MMR handling sites referenced (the
other occurrences analogous to the Merk path at the noted locations).
---
Duplicate comments:
In `@grovedb/src/tests/mod.rs`:
- Line 52: The tests for the V1 dispatch path should be pinned to the concrete
GROVE_V3 constant rather than calling GroveVersion::latest(); update the import
and usages accordingly: change the use line to import GROVE_V3 (replace any
GROVE_V2 import) and replace calls to GroveVersion::latest() in the test
functions (e.g., test_path_query_proofs_without_subquery_with_reference,
test_path_query_proofs_without_subquery and related prove_query_non_serialized
tests) with the GROVE_V3 symbol so the tests deterministically target the
V1-proof dispatch path; also scan the nearby test ranges noted (around the given
function blocks) and replace any other latest() occurrences with GROVE_V3.
In `@grovedb/src/tests/provable_count_sum_tree_tests.rs`:
- Around line 2656-2673: The helper check_v1_layer_proof currently returns early
on non-Merk ProofBytes (match arm `_ => return`), which skips recursing into
layer.lower_layers and can miss child Merk layers; change it so recursion always
descends into layer.lower_layers regardless of the ProofBytes variant and only
attempt to decode/collect ops when ProofBytes::Merk is present (i.e., match to
get merk_bytes and run Decoder there, but do not return from the function on
other variants), keeping the recursion over lower_layers after/independent of
the match; update check_v1_layer_proof (and any borrow usage) accordingly so
lower_layers are always traversed.
---
Nitpick comments:
In `@grovedb/src/tests/provable_count_tree_comprehensive_test.rs`:
- Around line 719-834: The test
test_provable_count_tree_proof_contains_count_nodes currently uses
GroveVersion::latest() but asserts on GroveDBProof::V1; replace the dynamic
latest() with the explicit V1 grove version constant (e.g., GroveVersion::V1 or
the project’s named V1 constant) where grove_version is set so the test is
pinned to the proof format that produces GroveDBProof::V1; update the
instantiation at the top of the test (the grove_version binding used in db
creation, db.insert calls, and prove_query) to use that explicit V1 constant
instead of GroveVersion::latest().
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 09acd401-11bc-4b91-bae9-d0aba362b002
📒 Files selected for processing (15)
grovedb-version/src/lib.rsgrovedb-version/src/version/grovedb_versions.rsgrovedb-version/src/version/mod.rsgrovedb-version/src/version/v1.rsgrovedb-version/src/version/v2.rsgrovedb-version/src/version/v3.rsgrovedb/src/operations/proof/generate.rsgrovedb/src/tests/commitment_tree_tests.rsgrovedb/src/tests/dense_tree_tests.rsgrovedb/src/tests/mmr_tree_tests.rsgrovedb/src/tests/mod.rsgrovedb/src/tests/proof_coverage_tests.rsgrovedb/src/tests/provable_count_sum_tree_tests.rsgrovedb/src/tests/provable_count_tree_comprehensive_test.rsgrovedb/src/tests/v1_proof_tests.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- grovedb-version/src/version/mod.rs
- grovedb-version/src/version/v2.rs
- grovedb/src/tests/commitment_tree_tests.rs
- grovedb/src/tests/proof_coverage_tests.rs
QuantumExplorer
commented
Mar 6, 2026
| }; | ||
|
|
||
| use grovedb_version::version::GroveVersion; | ||
| use grovedb_version::version::{v2::GROVE_V2, GroveVersion}; |
| Query, | ||
| }; | ||
| use grovedb_version::version::GroveVersion; | ||
| use grovedb_version::version::{v2::GROVE_V2, GroveVersion}; |
5 tasks
6 tasks
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.
Summary
prove_queryandprove_query_non_serializedto use the version-dispatch pattern (matching Platform conventions)prove_querydelegates toprove_query_non_serializedwhich dispatches to v0 or v1 based ongrove_version.grovedb_versions.operations.proof.prove_query_non_serializedprove_query_non_serialized_v0,prove_query_non_serialized_v1) arepub(crate), notpubcheck_grovedb_v0_or_v1_with_cost!macro for methods that accept both v0 and v1prove_query_non_serialized: 1)&GROVE_V2with_for_version_2suffix, original names useGroveVersion::latest()Context
Audit finding A2: proof generation methods lacked version-based dispatch pattern used elsewhere in Platform.
Test plan
cargo buildpassescargo clippy -- -D warningspassescargo test -p grovedb --lib— 1265 passed, 0 failed🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Public API
Chores
Tests