feat: add Element::ProvableCountProvableSumTree + dual-axis crossover proofs - #670
Conversation
Adds the new tree variant that bakes BOTH the per-node count AND the per-node sum into the cryptographic state via node_hash_with_count_and_sum. Enables both AggregateCountOnRange AND AggregateSumOnRange proofs against the same root hash. Foundation pieces (this commit): - Element / ElementType variants (slot 20; twins 148/178/194) - TreeType::ProvableCountProvableSumTree (discriminant 12) - AggregateData::ProvableCountAndProvableSum(u64, i64) - TreeFeatureType::ProvableCountedAndProvableSummedMerkNode(u64, i64) - NodeType::ProvableCountProvableSumNode (tag byte 8) - merk::tree::node_hash_with_count_and_sum - merk::tree::hash_for_link arm with fail-closed gate - Commit-time aggregate-data dispatch arm - 5 new proof Node variants: KVCountSum / KVHashCountSum / KVRefValueHashCountSum / KVDigestCountSum / HashWithCountAndSum - Tag bytes 0x40-0x4D in encoding.rs - ProofNodeType::KvCountSum + KvRefValueHashCountSum - aggregate_count / aggregate_sum allowlists extended - GroveDB terminal-type gates accept the new variant - grovedbg-types Element + TreeFeatureType variants - Insert / batch / query / proof generation+verification wired Workspace + tests compile clean. Tests-as-code not yet written; follow-up commits will add the dedicated test suite plus the headline crossover test (one tree, both proofs against same root hash). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Update the discriminant exhaustiveness tests in grovedb-element/src/element_type.rs and merk/src/tree_type/mod.rs to acknowledge the new ProvableCountProvableSumTree base discriminant 20, the four new wrapper twins (148/178/194), and the new TreeType variant 12. All workspace tests pass (cargo test --workspace --all-features): 41 test binaries green, 0 failures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the test suite for the new variant + wires the remaining merk
integration sites surfaced during testing.
Tests added (grovedb/src/tests/provable_count_provable_sum_tree_tests.rs):
1. Round-trip insert/get tracks (count, sum)
2. Negative/zero/extreme aggregates propagate correctly
3. Root hash diverges from ProvableCountSumTree AND ProvableSumTree
over identical content
4. Crossover proof test - IGNORED with detailed docstring on the
protocol gap (aggregate emit.rs needs dual-axis Node dispatch
for PCPS trees; Node variant scaffolding is in place)
5. NonCounted(PCPS) suppresses count contribution
6. NotSummed(PCPS) suppresses sum
7. NotCountedOrSummed(PCPS) suppresses both axes
Hash function tests for node_hash_with_count_and_sum:
- Determinism, distinctness, sensitivity, axis-swap, extremes
Integration sites: get_specialized_cost, layered_value_defined_cost,
value_defined_cost, specialized_costs_for_key_value, tree_type,
tree_feature_type, root_key_and_tree_type{,_owned}, tree_flags_and_type,
reconstruct_with_root_key.
Workspace test summary: 0 failures, 1 ignored (crossover).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both the AggregateCountOnRange and AggregateSumOnRange emitters now dispatch on the host tree's TreeType. When the host is ProvableCountProvableSumTree, they emit the dual-axis Node variants (HashWithCountAndSum, KVDigestCountSum) instead of the single-axis ones (HashWithCount/HashWithSum, KVDigestCount/KVDigestSum), carrying BOTH aggregates so the verifier can reconstruct node_hash_with_count_and_sum. This unblocks the headline crossover test: - A single ProvableCountProvableSumTree produces verifiable count AND verifiable sum proofs against the SAME root hash. Changes: merk/src/proofs/query/aggregate_count/emit.rs - emit_count_proof now takes tree_type: TreeType - binds_sum_into_hash(tree_type) → true for ProvableCountProvableSumTree - Disjoint/Contained branch dispatches HashWithCount vs HashWithCountAndSum - Boundary branch dispatches KVDigestCount vs KVDigestCountSum - Recursive calls thread tree_type through merk/src/proofs/query/aggregate_count/prove.rs - create_aggregate_count_on_range_proof passes tree_type through - Updated error message + doc to mention ProvableCountProvableSumTree merk/src/proofs/query/aggregate_count/verify.rs - Phase 1 allowlist accepts HashWithCountAndSum, KVDigestCountSum - verify_count_shape pulls count from either single- or dual-axis variant at each classification position - Error messages updated to mention both variant flavors merk/src/proofs/query/aggregate_sum/emit.rs - Symmetric: emit_sum_proof now takes tree_type - binds_count_into_hash(tree_type) → true for ProvableCountProvableSumTree - Disjoint/Contained: HashWithSum vs HashWithCountAndSum - Boundary: KVDigestSum vs KVDigestCountSum merk/src/proofs/query/aggregate_sum/prove.rs - create_aggregate_sum_on_range_proof passes tree_type through - Updated error message + doc merk/src/proofs/query/aggregate_sum/verify.rs - Phase 1 allowlist accepts HashWithCountAndSum, KVDigestCountSum - verify_sum_shape pulls sum from either variant flavor grovedb/src/tests/provable_count_provable_sum_tree_tests.rs - Removed #[ignore] from pcps_supports_both_count_and_sum_proofs_against_same_root - Test now passes — count proof returns 5, sum proof returns 150, both verify against the same GroveDB root hash Workspace test summary: 0 failures across 41 binaries. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
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:
📝 WalkthroughWalkthroughAdds ProvableCountProvableSumTree: a dual-axis tree variant binding u64 count and i64 sum into node hashes and integrates it across element types, proof node variants and encoding, merk hashing/linking, proof emission and verification (count and sum), GroveDB query/insert/batch paths, debugger/wire types, and extensive tests. ChangesProvableCountProvableSumTree implementation
Estimated code review effort 🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #670 +/- ##
===========================================
+ Coverage 91.21% 91.38% +0.17%
===========================================
Files 210 224 +14
Lines 61072 65404 +4332
===========================================
+ Hits 55706 59771 +4065
- Misses 5366 5633 +267
🚀 New features to boost your workflow:
|
clippy's doc_lazy_continuation rule rejects multi-line list items where continuation lines aren't indented under the bullet. Fixes 3 errors on the NotCountedOrSummedProvableCountProvableSumTree doc comment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…edicates Codecov flagged the new PCPS encoding/decoding/hash-reconstruction paths at low patch coverage. Adds focused tests across: grovedb-query/src/proofs/mod.rs - Display tests for each of the 5 new Node variants (KVCountSum, KVHashCountSum, KVRefValueHashCountSum, KVDigestCountSum, HashWithCountAndSum) so the Display match arms register as covered - Encoding round-trip tests for Push + PushInverted of each variant, covering the small-value and large-value tag-byte branches (0x40..=0x4D), and HashWithCountAndSum extremes (count=u64::MAX/sum=i64::MIN/etc) grovedb-query/src/proofs/tree_feature_type.rs - Round-trip test for ProvableCountedAndProvableSummedMerkNode (tag 8) with extreme value combinations - ProvableCountProvableSumNode layout mirrors CountSumNode - count() / zero_count / zero_sum coverage for the dual-axis variant - decoder rejects unknown tag byte 9 merk/src/proofs/tree.rs - Hash reconstruction tests for each of the 5 Node variants — each test forges either the count or sum and asserts the recomputed node hash changes (binding both axes to the hash chain) - aggregate_data() returns ProvableCountAndProvableSum for KVCountSum and HashWithCountAndSum - key() returns the right key for keyed variants and None for keyless merk/src/tree/tree_feature_type.rs - AggregateData::ProvableCountAndProvableSum coverage for parent_tree_type, as_sum_i64, as_count_u64, as_summed_i128 (incl. extremes) - From<TreeFeatureType> coverage for the new ProvableCountedAndProvableSummedMerkNode variant merk/src/tree/mod.rs - #[should_panic] regression test for the fail-closed hash_for_link arm on ProvableCountProvableSumTree merk/src/tree_type/mod.rs - ProvableCountProvableSumTree added to uses_non_merk_data_storage, is_count_bearing, is_sum_bearing, is_count_and_sum_bearing, allows_sum_item, empty_tree_feature_type, to_element_type, tree_type_discriminant_roundtrip, and Display tests Workspace tests: 0 failures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds direct coverage for the new variant's constructor family and helper accessors. Mirrors the existing provable_sum_tree_constructors_and_helpers test: - Constructors: empty/with_flags/with_root_key/with_flags_and_sum_and_count_value - Type predicates including is_provable_count_provable_sum_tree - Value accessors borrowed + owned - Wrong-element error paths - Negative-sum + positive-count round-trip - Boundary value combinations (u64::MAX count + i64::MIN sum) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous run had all 3 sharded coverage shards complete cleanly, but codecov merged them with 30% fewer covered lines than the prior run that touched the same code paths (79% → 49% with no code regression). That points at a codecov merge race, not a real coverage drop. Force a fresh CI run to settle the metric. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
grovedb/src/batch/mod.rs (2)
3147-3335:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing propagation branch for
ProvableCountProvableSumTreebreaks parent update conversion.Line 3147 onward converts tree insert ops into
GroveOp::InsertTreeWithRootHash, butElement::ProvableCountProvableSumTree(..)is not handled. For that variant, execution falls into the final error path ("insertion of element under a non tree"), so upward propagation can fail for valid dual-axis subtree inserts.💡 Suggested patch
@@ - } else if let Element::ProvableSumTree( + } else if let Element::ProvableSumTree( .., flags, ) = element { *mutable_occupied_entry = GroveOp::InsertTreeWithRootHash { hash: root_hash, root_key: calculated_root_key, flags: flags.clone(), aggregate_data, non_counted, not_summed, not_counted_or_summed, } + } else if let + Element::ProvableCountProvableSumTree( + .., + flags, + ) = element + { + *mutable_occupied_entry = + GroveOp::InsertTreeWithRootHash { + hash: root_hash, + root_key: calculated_root_key, + flags: flags.clone(), + aggregate_data, + non_counted, + not_summed, + not_counted_or_summed, + } // Non-Merk trees → InsertNonMerkTree🤖 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 `@grovedb/src/batch/mod.rs` around lines 3147 - 3335, The code misses a match arm for Element::ProvableCountProvableSumTree(..) so valid dual-axis trees fall through to the final error; add an else if branch matching Element::ProvableCountProvableSumTree(.., flags) (similar to the existing ProvableCountSumTree/ProvableSumTree arms) and set *mutable_occupied_entry = GroveOp::InsertTreeWithRootHash { hash: root_hash, root_key: calculated_root_key, flags: flags.clone(), aggregate_data, non_counted, not_summed, not_counted_or_summed } so this variant is converted to InsertTreeWithRootHash like the other provable tree variants.
2905-2917:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDual-axis tree is missing from value-defined-cost recalculation after flags updates.
In the flags-update closure, tree variants are enumerated to return
LayeredValueDefinedCost(...), butElement::ProvableCountProvableSumTree(..)is missing. That drops into_ => Ok((true, None)), which can produce incorrect cost accounting for this new tree type.💡 Suggested patch
@@ Element::ProvableCountTree(..) | Element::ProvableCountSumTree(..) | Element::ProvableSumTree(..) + | Element::ProvableCountProvableSumTree(..) | Element::CommitmentTree(..) | Element::MmrTree(..) | Element::BulkAppendTree(..)🤖 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 `@grovedb/src/batch/mod.rs` around lines 2905 - 2917, The match arm that maps updated element variants to LayeredValueDefinedCost is missing the Element::ProvableCountProvableSumTree(..) variant, causing it to fall through to the default (_ => Ok((true, None))) and miscompute costs; update the flags-update closure's match (the arm that currently lists Element::Tree, Element::SumTree, ..., Element::DenseAppendOnlyFixedSizeTree) to also include Element::ProvableCountProvableSumTree(..) so it returns LayeredValueDefinedCost(...) for new_element (the same handling as the other layered/provable trees) instead of falling back to the default.grovedb/src/operations/proof/generate.rs (1)
1594-1633:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd empty-tree descent path for aggregate-sum on
ProvableCountProvableSumTree.The new empty-tree special-case only handles
is_aggregate_count_query. For aggregate-sum queries, an emptyProvableCountProvableSumTreefalls through to the generic empty-tree branch (Line 1624+), so no lower-layer ASOR proof gets emitted.Suggested fix
@@ - let is_aggregate_count_query = path_query + let is_aggregate_count_query = path_query .query .query .has_aggregate_count_on_range_anywhere(); + let is_aggregate_sum_query = path_query + .query + .query + .has_aggregate_sum_on_range_anywhere(); @@ Ok(Element::ProvableCountTree(None, ..)) | Ok(Element::ProvableCountSumTree(None, ..)) | Ok(Element::ProvableCountProvableSumTree(None, ..)) @@ lower_layers.insert(key.clone(), layer_proof); } + Ok(Element::ProvableSumTree(None, ..)) + | Ok(Element::ProvableCountProvableSumTree(None, ..)) + if !done_with_results + && is_aggregate_sum_query + && query.has_subquery_or_matching_in_path_on_key(key) => + { + let mut lower_path = path.clone(); + lower_path.push(key.as_slice()); + let previous_limit = *overall_limit; + let layer_proof = cost_return_on_error!( + &mut cost, + self.prove_subqueries_v1( + lower_path, + path_query, + overall_limit, + prove_options, + current_depth + 1, + grove_version, + ) + ); + if previous_limit != *overall_limit { + has_a_result_at_level |= true; + } + lower_layers.insert(key.clone(), layer_proof); + }🤖 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 `@grovedb/src/operations/proof/generate.rs` around lines 1594 - 1633, When encountering an empty ProvableCountProvableSumTree the logic only descends for is_aggregate_count_query; add a matching branch that also descends for aggregate-sum queries so an ASOR proof for lower layers is produced. Concretely, mirror the existing branch that matches Ok(Element::ProvableCountProvableSumTree(None, ..)) under the condition is_aggregate_count_query && query.has_subquery_or_matching_in_path_on_key(key) and add the equivalent condition for the aggregate-sum case (e.g. is_aggregate_sum_query or the appropriate query.is_aggregate_sum check), calling self.prove_subqueries_v1(lower_path, path_query, overall_limit, prove_options, current_depth + 1, grove_version), updating previous_limit/overall_limit, setting has_a_result_at_level, and inserting into lower_layers just like the count branch so empty ProvableCountProvableSumTree emits lower-layer ASOR proofs.grovedb-element/src/element/serialize.rs (1)
22-25:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale wrapper allowlist docs in
serialize.The doc block still says
NotSummed/NotCountedOrSummedaccept four sum-tree variants, but the implementation now accepts six (includingProvableSumTreeandProvableCountProvableSumTree).🤖 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 `@grovedb-element/src/element/serialize.rs` around lines 22 - 25, The documentation for the serialize module's wrapper allowlist is stale: update the doc comment that lists accepted sum-tree variants for `NotSummed` / `NotCountedOrSummed` to include the two new variants `ProvableSumTree` and `ProvableCountProvableSumTree` (in addition to the existing `SumTree`, `BigSumTree`, `CountSumTree`, and `ProvableCountSumTree`), so the comment matches the current implementation in serialize.rs referencing `NotSummed` and `NotCountedOrSummed`.merk/src/proofs/query/aggregate_count/mod.rs (1)
64-78:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
provable_count_from_aggregateis using a verifier error class on prover-only code.This helper is only compiled for the local proof-generation/read path, so an unexpected aggregate variant means the merk state is inconsistent with its declared
TreeType. ReturningInvalidProofErrorhere makes local corruption look like a bad remote proof.Suggested change
- other => Err(Error::InvalidProofError(format!( + other => Err(Error::CorruptedData(format!( "expected ProvableCount aggregate data on a provable count tree, got {:?}", other ))),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 current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@merk/src/proofs/query/aggregate_count/mod.rs` around lines 64 - 78, provable_count_from_aggregate is returning Error::InvalidProofError for an unexpected AggregateData variant (prover-only code); change the returned error to Error::CorruptedData and include contextual information about the unexpected variant so local corruption isn't misclassified as a verifier error — update the match arm that currently returns Err(Error::InvalidProofError(...)) to return Err(Error::CorruptedData(format!("provable_count_from_aggregate: unexpected aggregate variant: {:?}", other))) so the function (provable_count_from_aggregate) reports corruption with context rather than a verifier/InvalidProofError.
🧹 Nitpick comments (2)
grovedb-element/src/element_type.rs (1)
941-976: ⚡ Quick winExtend the serialization pinning test to cover the new base variant.
These
TryFrom(20)assertions are useful, but they do not pin the actual on-disk byte emitted byElement::ProvableCountProvableSumTree.test_element_serialization_discriminants_match_element_typestill stops at base19, so a futureElementenum reorder could slip through for this new variant.🤖 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 `@grovedb-element/src/element_type.rs` around lines 941 - 976, The serialization pinning test needs to include the new Element::ProvableCountProvableSumTree variant so its on-disk discriminant is pinned; update test_element_serialization_discriminants_match_element_type to assert that the serialized discriminant for Element::ProvableCountProvableSumTree equals the base 20 (and that its non-counted twin serializes to 0x80|20 == 148), similar to the existing assertions for other variants—locate the assertions comparing ElementType::try_from(20) / try_from(148) and add corresponding checks that the Element enum variant serializes to the same byte values so future reorders will fail the test.grovedb-query/src/proofs/encoding.rs (1)
484-628: ⚡ Quick winAdd dedicated round-trip coverage for the
0x40..=0x4Dwire family.These branches introduce a new protocol family across
encode_into,encoding_length, anddecode, andDecoderadvances byencoding_length()rather than bytes consumed. A small helper-based test matrix like the existing sum-family coverage would catch tag drift and offset desyncs here quickly.Also applies to: 766-819, 1633-1851
🤖 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 `@grovedb-query/src/proofs/encoding.rs` around lines 484 - 628, Add focused round-trip tests that exercise the new 0x40..=0x4D wire-family variants through encode_into, encoding_length, and decode to catch tag/offset drift: create a test matrix that builds each Node variant used in Op::Push and Op::PushInverted (KVCountSum, KVHashCountSum, KVRefValueHashCountSum, KVDigestCountSum, HashWithCountAndSum), encode them, check encoding_length matches actual bytes written, then feed the exact bytes to Decoder and assert decode returns the original Op and the Decoder advances by the number of bytes actually consumed (not by encoding_length()). Ensure tests mirror the existing sum-family coverage pattern so tag values (0x40..0x4D) and varint boundaries are validated.
🤖 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/lib.rs`:
- Around line 972-973: aggregate_consistency_labels is missing a match arm for
the enum variant Element::ProvableCountProvableSumTree, so PCPS trees fall into
the catch-all as mismatches; update the match in aggregate_consistency_labels to
include Element::ProvableCountProvableSumTree(..) and handle it the same way as
the existing CommitmentTree/other provable tree branches (i.e., produce the same
aggregate consistency label or call the same helper used for CommitmentTree),
referencing the aggregate_consistency_labels function and the
Element::ProvableCountProvableSumTree variant to locate where to add the new
arm.
In `@grovedb/src/operations/proof/verify.rs`:
- Around line 2695-2710: get_key_value_from_node() was updated to accept
KVRefValueHashCountSum but extract_elements_and_leaf_keys() still only rejects
KVRefValueHash, KVRefValueHashCount, and KVRefValueHashSum, allowing an
unauthenticated deref via KVRefValueHashCountSum; update the opaque-hash
rejection guard inside extract_elements_and_leaf_keys() to also reject
Node::KVRefValueHashCountSum (add it alongside KVRefValueHash,
KVRefValueHashCount and KVRefValueHashSum), ensuring both functions treat
KVRefValueHashCountSum as opaque and preventing smuggling of dereferenced
values.
In `@grovedb/src/tests/provable_count_sum_tree_tests.rs`:
- Around line 84-86: get_node_count() (and the equivalent match used around
lines ~93) currently doesn't recognize the dual-axis node variants so the
branches handling Node::KVCountSum, Node::KVDigestCountSum, and
Node::KVRefValueHashCountSum never run; update the match arms in get_node_count
(and the other count-extraction helper used at the second site) to include
Node::KVCountSum(...), Node::KVDigestCountSum(...), and
Node::KVRefValueHashCountSum(...) and return the stored count (the count field
inside those variants) as Some(count) so those key-collection branches (the
Node::KVCountSum / KVDigestCountSum / KVRefValueHashCountSum clones) become
reachable and tests assert the correct counts.
In `@merk/src/merk/get.rs`:
- Around line 384-391: The rustdoc for the aggregate methods (e.g.,
AggregateCountOnRange / aggregate methods in merk/src/merk/get.rs) is out of
sync with the runtime checks: the code now allows
crate::TreeType::ProvableCountProvableSumTree but the docs still list only
ProvableCountTree and ProvableCountSumTree; update the method-level rustdoc
comments above those aggregate methods (also the second occurrence around the
lines noted) to include ProvableCountProvableSumTree in the allowed tree-type
list and adjust wording to match the expanded allowlist so the docs reflect the
runtime behavior.
In `@merk/src/merk/prove.rs`:
- Around line 166-173: Update the rustdoc for the proof methods whose runtime
guard now allows crate::TreeType::ProvableCountProvableSumTree so the docs match
the code: locate the rustdoc blocks for the AggregateCountOnRange-related
methods (the docs adjacent to the runtime check that accepts ProvableCountTree,
ProvableCountSumTree, and ProvableCountProvableSumTree) and add
ProvableCountProvableSumTree to the list of valid tree types in both docs (the
two doc blocks corresponding to the checks around the AggregateCountOnRange
logic), ensuring the doc text enumerates all three accepted TreeType variants.
In `@merk/src/proofs/query/aggregate_count/emit.rs`:
- Around line 132-143: The branches that run while building local proofs (e.g.,
the binds_sum_into_hash check and matching on aggregate inside emit.rs using
symbols like binds_sum_into_hash, aggregate, and
AggregateData::ProvableCountAndProvableSum for ProvableCountProvableSumTree)
should treat failures as local corruption, not invalid peer proofs: replace the
Err(Error::InvalidProofError(...)).wrap_with_cost(cost) returns with
Err(Error::CorruptedData(format!(...))).wrap_with_cost(cost) and any failed
reads from aggregate_data() should be converted using the `.map_err(|e|
Error::CorruptedData(format!("context: {}", e)))` pattern. Apply the same change
to the other similar branches noted (around the match at the other locations
referenced, e.g., the blocks at the 178-185 and 259-269 regions) so all
prover-side invariant failures produce Error::CorruptedData with contextual
messages.
In `@merk/src/proofs/query/mod.rs`:
- Around line 483-487: ProofNodeType::KvRefValueHashCountSum currently delegates
to to_kv_value_hash_feature_type_node(), but that helper doesn't map
AggregateData::ProvableCountAndProvableSum(...) into the aggregated
TreeFeatureType; it falls back to self.tree().feature_type(). Update
to_kv_value_hash_feature_type_node() (the function used by
KvRefValueHashCountSum) to detect when the node's AggregateData is
ProvableCountAndProvableSum(...) and return
TreeFeatureType::ProvableCountedAndProvableSummedMerkNode(...) (constructed with
the correct inner feature info), instead of returning
self.tree().feature_type(); ensure the pattern match/branch uses the
AggregateData enum variant and constructs the aggregated feature type
consistently with how node hashes are committed.
In `@merk/src/proofs/query/verify.rs`:
- Around line 561-580: The match arms for Node::KVCountSum,
Node::KVDigestCountSum, and Node::KVRefValueHashCountSum are being executed but
those variants are not being included in the lower/upper-bound checks and the
KVDigest* boundary helper functions, causing valid ProvableCountProvableSumTree
proofs to be rejected; update the boundary/range logic (the last_push
lower/upper-bound checks) and the helper functions boundaries_in_proof and
key_exists_as_boundary_in_proof to handle these three variants just like the
existing KV*/KVDigest* branches—i.e., add match arms for KVCountSum,
KVDigestCountSum, and KVRefValueHashCountSum, propagate their key and
value/value_hash semantics consistently (Some(value)/None and using the passed
value_hash) when calling the same boundary helpers and when computing last_push
bounds, so they are treated equivalently to the other KV/KVDigest variants.
In `@merk/src/proofs/tree.rs`:
- Around line 541-553: In execute_with_options() update the BST-order key
validation to include the new keyed dual-axis node variants
(Node::KVRefValueHashSum, Node::KVCountSum, Node::KVDigestCountSum,
Node::KVRefValueHashCountSum) wherever keyed nodes are matched for Op::Push so
their keys participate in the monotonic-key checks, and make the exact same
addition to the mirrored Op::PushInverted match arm so both Push and
PushInverted enforce the same ordering invariant.
---
Outside diff comments:
In `@grovedb-element/src/element/serialize.rs`:
- Around line 22-25: The documentation for the serialize module's wrapper
allowlist is stale: update the doc comment that lists accepted sum-tree variants
for `NotSummed` / `NotCountedOrSummed` to include the two new variants
`ProvableSumTree` and `ProvableCountProvableSumTree` (in addition to the
existing `SumTree`, `BigSumTree`, `CountSumTree`, and `ProvableCountSumTree`),
so the comment matches the current implementation in serialize.rs referencing
`NotSummed` and `NotCountedOrSummed`.
In `@grovedb/src/batch/mod.rs`:
- Around line 3147-3335: The code misses a match arm for
Element::ProvableCountProvableSumTree(..) so valid dual-axis trees fall through
to the final error; add an else if branch matching
Element::ProvableCountProvableSumTree(.., flags) (similar to the existing
ProvableCountSumTree/ProvableSumTree arms) and set *mutable_occupied_entry =
GroveOp::InsertTreeWithRootHash { hash: root_hash, root_key:
calculated_root_key, flags: flags.clone(), aggregate_data, non_counted,
not_summed, not_counted_or_summed } so this variant is converted to
InsertTreeWithRootHash like the other provable tree variants.
- Around line 2905-2917: The match arm that maps updated element variants to
LayeredValueDefinedCost is missing the Element::ProvableCountProvableSumTree(..)
variant, causing it to fall through to the default (_ => Ok((true, None))) and
miscompute costs; update the flags-update closure's match (the arm that
currently lists Element::Tree, Element::SumTree, ...,
Element::DenseAppendOnlyFixedSizeTree) to also include
Element::ProvableCountProvableSumTree(..) so it returns
LayeredValueDefinedCost(...) for new_element (the same handling as the other
layered/provable trees) instead of falling back to the default.
In `@grovedb/src/operations/proof/generate.rs`:
- Around line 1594-1633: When encountering an empty ProvableCountProvableSumTree
the logic only descends for is_aggregate_count_query; add a matching branch that
also descends for aggregate-sum queries so an ASOR proof for lower layers is
produced. Concretely, mirror the existing branch that matches
Ok(Element::ProvableCountProvableSumTree(None, ..)) under the condition
is_aggregate_count_query && query.has_subquery_or_matching_in_path_on_key(key)
and add the equivalent condition for the aggregate-sum case (e.g.
is_aggregate_sum_query or the appropriate query.is_aggregate_sum check), calling
self.prove_subqueries_v1(lower_path, path_query, overall_limit, prove_options,
current_depth + 1, grove_version), updating previous_limit/overall_limit,
setting has_a_result_at_level, and inserting into lower_layers just like the
count branch so empty ProvableCountProvableSumTree emits lower-layer ASOR
proofs.
In `@merk/src/proofs/query/aggregate_count/mod.rs`:
- Around line 64-78: provable_count_from_aggregate is returning
Error::InvalidProofError for an unexpected AggregateData variant (prover-only
code); change the returned error to Error::CorruptedData and include contextual
information about the unexpected variant so local corruption isn't misclassified
as a verifier error — update the match arm that currently returns
Err(Error::InvalidProofError(...)) to return
Err(Error::CorruptedData(format!("provable_count_from_aggregate: unexpected
aggregate variant: {:?}", other))) so the function
(provable_count_from_aggregate) reports corruption with context rather than a
verifier/InvalidProofError.
---
Nitpick comments:
In `@grovedb-element/src/element_type.rs`:
- Around line 941-976: The serialization pinning test needs to include the new
Element::ProvableCountProvableSumTree variant so its on-disk discriminant is
pinned; update test_element_serialization_discriminants_match_element_type to
assert that the serialized discriminant for
Element::ProvableCountProvableSumTree equals the base 20 (and that its
non-counted twin serializes to 0x80|20 == 148), similar to the existing
assertions for other variants—locate the assertions comparing
ElementType::try_from(20) / try_from(148) and add corresponding checks that the
Element enum variant serializes to the same byte values so future reorders will
fail the test.
In `@grovedb-query/src/proofs/encoding.rs`:
- Around line 484-628: Add focused round-trip tests that exercise the new
0x40..=0x4D wire-family variants through encode_into, encoding_length, and
decode to catch tag/offset drift: create a test matrix that builds each Node
variant used in Op::Push and Op::PushInverted (KVCountSum, KVHashCountSum,
KVRefValueHashCountSum, KVDigestCountSum, HashWithCountAndSum), encode them,
check encoding_length matches actual bytes written, then feed the exact bytes to
Decoder and assert decode returns the original Op and the Decoder advances by
the number of bytes actually consumed (not by encoding_length()). Ensure tests
mirror the existing sum-family coverage pattern so tag values (0x40..0x4D) and
varint boundaries are validated.
🪄 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: c2e58d15-70df-4629-8b10-27ed1995eabd
📒 Files selected for processing (52)
docs/PROVABLE_COUNT_PROVABLE_SUM_TREE_IMPLEMENTATION.mdgrovedb-element/src/element/constructor.rsgrovedb-element/src/element/helpers.rsgrovedb-element/src/element/mod.rsgrovedb-element/src/element/serialize.rsgrovedb-element/src/element/visualize.rsgrovedb-element/src/element_type.rsgrovedb-element/tests/element_constructors_helpers.rsgrovedb-query/src/proofs/encoding.rsgrovedb-query/src/proofs/mod.rsgrovedb-query/src/proofs/tree_feature_type.rsgrovedb/src/batch/mod.rsgrovedb/src/debugger.rsgrovedb/src/lib.rsgrovedb/src/operations/get/query.rsgrovedb/src/operations/insert/mod.rsgrovedb/src/operations/proof/aggregate_count/helpers.rsgrovedb/src/operations/proof/aggregate_sum/helpers.rsgrovedb/src/operations/proof/generate.rsgrovedb/src/operations/proof/mod.rsgrovedb/src/operations/proof/verify.rsgrovedb/src/tests/mod.rsgrovedb/src/tests/provable_count_provable_sum_tree_tests.rsgrovedb/src/tests/provable_count_sum_tree_tests.rsgrovedbg-types/src/lib.rsmerk/src/element/costs.rsmerk/src/element/delete.rsmerk/src/element/get.rsmerk/src/element/reconstruct.rsmerk/src/element/tree_type.rsmerk/src/merk/chunks.rsmerk/src/merk/get.rsmerk/src/merk/prove.rsmerk/src/proofs/branch/mod.rsmerk/src/proofs/chunk/chunk.rsmerk/src/proofs/query/aggregate_count/emit.rsmerk/src/proofs/query/aggregate_count/mod.rsmerk/src/proofs/query/aggregate_count/prove.rsmerk/src/proofs/query/aggregate_count/verify.rsmerk/src/proofs/query/aggregate_sum/emit.rsmerk/src/proofs/query/aggregate_sum/mod.rsmerk/src/proofs/query/aggregate_sum/prove.rsmerk/src/proofs/query/aggregate_sum/verify.rsmerk/src/proofs/query/mod.rsmerk/src/proofs/query/verify.rsmerk/src/proofs/tree.rsmerk/src/tree/hash.rsmerk/src/tree/link.rsmerk/src/tree/mod.rsmerk/src/tree/tree_feature_type.rsmerk/src/tree_type/costs.rsmerk/src/tree_type/mod.rs
Coverage additions: merk/src/proofs/query/aggregate_count/tests.rs - make_15_key_provable_count_provable_sum_tree builder - integration_count_proof_against_pcps_round_trips: count proof against a PCPS host tree round-trips correctly via the dual-axis emitter - shape_walk_rejects_disjoint_hashwithcountandsum_with_children_pcps: malformed-proof rejection covers the new variant arms in verify_count_shape - integration_pcps_count_forgery_changes_root_hash: forging count on a HashWithCountAndSum makes the reconstructed root diverge merk/src/proofs/query/aggregate_sum/tests.rs - provable_sum_from_aggregate_accepts_dual_axis_variant - is_provable_sum_bearing_for_provable_sum_tree_and_pcps (replaces the old single-tree gate test now that both flavors are valid hosts) - make_15_key_provable_count_provable_sum_tree builder - integration_sum_proof_against_pcps_round_trips (c..=l → 75) - shape_walk_rejects_disjoint_hashwithcountandsum_with_children_pcps grovedb-element/src/element_type.rs - test_proof_node_type_provable_count_provable_sum_tree: every base-element-type / wrapper combination dispatches to the right dual-axis ProofNodeType (KvCountSum, KvRefValueHashCountSum, KvValueHashFeatureType) Workspace tests: 0 failures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
grovedb-query/src/proofs/mod.rs - encoding_length_matches_actual_byte_length_for_dual_axis: cover the encoding_length match arms for the 5 new Node variants by asserting the predicted length equals the actual encoded byte count, for both Push and PushInverted, with small + large value sizes where applicable merk/src/proofs/query/aggregate_count/tests.rs - regular_query_verifier_rejects_hash_with_count_and_sum_node: mirror of the existing HashWithCount rejection test for the dual-axis variant - regular_query_verifier_rejects_kv_hash_count_sum_node: rejects the path-hash dual-axis variant when used in a regular query proof These hit the new arms in merk/src/proofs/query/verify.rs that the regular query verifier exposes for the dual-axis Nodes (KVCountSum / KVHashCountSum / etc — all rejected on sight outside aggregate proofs). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Local builds happen to import the Encode trait method via auto-resolution that CI doesn't see. Using <Op as Encode>::encode_into / encoding_length disambiguates. Also remove the attempted assert_terminated marker (not a real Terminated API). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The to_kv_count_sum_node, to_kvhash_count_sum_node, and to_kvdigest_count_sum_node helpers in merk/src/proofs/query/mod.rs are called only inside create_proof_internal — which the aggregate proof crossover test bypasses (it uses prove_aggregate_*_on_range instead). Mirroring the sum-side regular_prove_on_provable_sum_tree_emits_* tests, these two new tests: - regular_prove_on_pcps_emits_dual_axis_helpers: queries a few keys out of 15 against a PCPS host; asserts the proof contains both KVCountSum (queried-item path → to_kv_count_sum_node) and KVHashCountSum (non-queried path → to_kvhash_count_sum_node). - regular_prove_on_pcps_absent_key_emits_kvdigestcountsum: queries an absent key against a single-key PCPS host; asserts the boundary node is a KVDigestCountSum (via to_kvdigest_count_sum_node). Closes the biggest remaining patch-coverage gap (~34 uncovered lines in merk/src/proofs/query/mod.rs). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add ~825 lines of negative-path tests to push codecov/patch toward the 90% target. These exercise rejection arms in the aggregate count/sum verifiers that the happy-path round-trips don't reach, plus the dual-axis kv-typed Node arms in branch/mod.rs. aggregate_count/verify.rs (Phase-2 rejection arms): - non-HashWithCount at Contained position - Contained HashWithCount leaf with attached children (leaf check) - Contained HashWithCountAndSum leaf with attached children (PCPS dual-axis) - KVDigestCountSum outside its inherited subtree bounds (PCPS dual-axis) - non-KVDigestCount at Boundary position - own_count underflow via tampered KVDigestCount (checked_sub arm) aggregate_sum/verify.rs (Phase-2 rejection arms): - non-HashWithSum at Contained position - non-HashWithSum at Disjoint position (multi-level handcrafted proof) - KVDigestCountSum at Contained (dual-axis wrong-type) - Contained HashWithSum leaf with attached children - Contained HashWithCountAndSum leaf with attached children (PCPS) - non-KVDigestSum at Boundary position - KVDigestSum / KVDigestCountSum outside inherited bounds - i128→i64 narrow at boundary value (i64::MAX two-key tree round-trip) Direct unit coverage for the count-side predicates that previously had no direct tests (the sum side already had them): - provable_count_from_aggregate accept arms (ProvableCount, ProvableCountAndSum, ProvableCountAndProvableSum) plus reject arms (NoAggregateData, Sum, BigSum, ProvableSum) - is_provable_count_bearing true-set (all three count-bearing types, including the new PCPS host) and false-set branch/mod.rs dual-axis Node arms (previously uncovered): - terminal_keys with KVDigestCountSum, KVCountSum, KVRefValueHashCountSum (covers the dual-axis kv-key arms in get_key_from_node) - terminal_keys with HashWithCountAndSum + KVHashCountSum returns empty (negative-side: confirms these are returned as None — no phantom keys) All 825 added lines exercise pure happy-path assertions or shape-walk InvalidProofError rejections; no test depends on prover internals, so the suite remains robust to proof encoding changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses every actionable finding from the CodeRabbit review on PR #670. **Critical security fix** - verify.rs::extract_elements_and_leaf_keys: reject Node::KVRefValueHashCountSum in the opaque-hash guard alongside KVRefValueHash{,Count,Sum}. Without this, a forged trunk/branch proof could smuggle an unauthenticated dereferenced value through the new dual-axis reference node — the merk-level hash chain would still appear valid because the embedded opaque hash is treated as authoritative, but the verifier never gets to see the referenced value at this layer. Also extend leaf-count extraction to include KVCountSum. **Major correctness fixes** - grovedb/src/lib.rs::aggregate_consistency_labels: add explicit arm for (ProvableCountProvableSumTree, ProvableCountAndProvableSum) + empty-merk identity case. Without this, valid PCPS trees fell into the catch-all and were reported as aggregate mismatches. - merk/src/proofs/tree.rs::execute_with_options: include KVCountSum / KVDigestCountSum / KVRefValueHashCountSum in the Op::Push and Op::PushInverted BST-order key checks so dual-axis proofs enforce the same monotonic-key invariant as every other keyed node type. - merk/src/proofs/query/verify.rs: thread dual-axis nodes through the lower/upper-bound `last_push` matches, the absence-proof last-push match, the `boundaries_in_proof` helper, and `key_exists_as_boundary_in_proof`. Without these, `Key + Range` queries against ProvableCountProvableSumTree could be wrongly rejected with "Cannot verify lower bound of queried range" or miss legitimate boundaries entirely. - merk/src/proofs/query/mod.rs::to_kv_value_hash_feature_type_node: recognize ProvableCountAndProvableSum aggregates and surface TreeFeatureType::ProvableCountedAndProvableSummedMerkNode (was falling through to self.tree().feature_type(), which would carry the local feature instead of the aggregated (count, sum) that is actually committed in the node hash for KvRefValueHashCountSum reference proofs). - grovedb/src/batch/mod.rs: add ProvableCountProvableSumTree arms in two sites — the LayeredValueDefinedCost match for flag updates and the InsertTreeWithRootHash propagation branch. Without these, valid dual-axis batch inserts fell into the "insertion of element under a non tree" error path during upward propagation. - grovedb/src/operations/proof/generate.rs: add an `is_aggregate_sum_query` short-circuit for empty ProvableSumTree / ProvableCountProvableSumTree under an AggregateSumOnRange carrier. Without this, empty sum-bearing hosts fell through to the generic empty-tree branch and no lower-layer ASOR proof got emitted. - grovedb/src/tests/provable_count_sum_tree_tests.rs::get_node_count: recognize the dual-axis KVCountSum / KVDigestCountSum / KVRefValueHashCountSum variants so rotation/stress proof-tests no longer silently skip dual-axis nodes when verifying counts. Also recognize ProvableCountedAndProvableSummedMerkNode feature types in KVValueHashFeatureType nodes. **Minor error-classification fixes** - merk/src/proofs/query/aggregate_count/{mod,emit}.rs: change all prover-side aggregate invariant failures from InvalidProofError (verifier-class) to CorruptedData (local-corruption-class) per the repo error-handling convention. The corresponding unit test now expects CorruptedData. The sum side already used CorruptedData; this brings the count side into alignment. **Doc updates** - merk/src/merk/{get,prove}.rs: include ProvableCountProvableSumTree in the rustdoc allow-lists for count_aggregate_on_range / sum_aggregate_on_range / prove_aggregate_*_on_range so the contracts match the runtime guards. - grovedb-element/src/element/serialize.rs: list the ProvableSumTree and ProvableCountProvableSumTree variants in the serialize() doc — the wrapper allowlist grew from four to six sum-bearing variants. **Test fixups** - Two existing tests (shape_walk_rejects_kvdigestcountsum_outside_inherited_bounds_pcps, same for the sum side) were relaxed to accept any InvalidProofError message — adding KVDigestCountSum to the BST-order check means Phase 1's key-ordering catches the malformed key before Phase 2's inherited-bounds check does. Either rejection is correct; the goal is that an out-of-bounds boundary key never produces a successful verify. All 1720 grovedb + 540 merk tests pass; cargo fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
This is Claude — pushed 979d1c06 addressing every actionable CodeRabbit finding (inline + outside-diff). Per-comment replies posted on each thread. Critical
Major
Minor
All 1720 grovedb + 540 merk tests pass; |
Previous CI run reported 75.64% patch coverage but the codecov comment explicitly notes 'HEAD has 1 upload less than BASE' — BASE has 3 shards uploaded, HEAD only 2. All three Test Ubuntu shards reported SUCCESS so this is the same codecov shard-merge race observed earlier on this PR. Triggering a fresh run to get a complete 3-shard upload merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previous run reported 85.92% patch coverage. Targeted the three biggest gaps with focused tests, ~400 lines added. **lib.rs::aggregate_consistency_labels — 6 PCPS unit tests** The new `(ProvableCountProvableSumTree, ProvableCountAndProvableSum)` arm and its empty-merk identity case were uncovered (16 lines @ 0%): - equal recorded+actual → None - count mismatch → labels with "recorded count N" / "ProvableCountAndProvableSum count N" - sum mismatch → labels with "sum N" / "sum -N" - empty-merk identity (0, 0) + NoAggregateData → None - non-zero + NoAggregateData → catch-all variant mismatch - PCPS paired with wrong aggregate kind (ProvableCountAndSum) → catch-all variant-mismatch **merk/src/proofs/query/verify.rs — 4 dual-axis regression tests** Parallel of the existing `provable_sum_tree_bound_regression_tests`: - `key_plus_range_on_pcps_left_to_right_verifies` - `key_plus_range_on_pcps_right_to_left_verifies` - `full_range_round_trips_through_dual_axis_verify_arms` — exercises every dual-axis Node variant in `execute_proof` (KVCountSum for queried Items, KVHashCountSum for path nodes, KVDigestCountSum for boundaries) end-to-end via merk.prove + Query::execute_proof - `kv_digest_count_sum_appears_in_both_boundary_helpers` — pins consistency between `boundaries_in_proof` and `key_exists_as_boundary_in_proof` on PCPS boundary nodes **merk/src/proofs/tree.rs — 7 BST-order tests** Pins the dual-axis arm added to `execute_with_options`'s monotonic-key check for both `Op::Push` and `Op::PushInverted`: - Push rejects decreasing KVCountSum / KVDigestCountSum / KVRefValueHashCountSum keys - PushInverted rejects increasing KVCountSum / KVDigestCountSum / KVRefValueHashCountSum keys - Push accepts monotonically-increasing dual-axis keys (positive side) All 551 merk lib + 1727 grovedb tests pass; cargo fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ery gate User-reported PCPS findings missed by the prior CodeRabbit pass. All three are real bugs in the code paths that handle the new dual-axis ProvableCountProvableSumTree. **P1 (Reference proof downgrade) — grovedb/src/operations/proof/generate.rs** `Element::proof_node_type()` can produce `ProofNodeType::KvRefValueHashCountSum` for References under a `ProvableCountProvableSumTree` parent, and the merk layer emits `KVValueHashFeatureType(_, _, _, ProvableCountedAndProvableSummedMerkNode(count, sum))` for it. But both GroveDB post-processing loops in `generate.rs` (the v1 and v0 ref rewrite sites at lines ~447 and ~1285) extracted only the count-only (`ProvableCountedMerkNode`) and sum-only (`ProvableSummedMerkNode`) features. A PCPS-host reference would therefore fall through to the plain `KVRefValueHash` arm and be DOWNGRADED — losing both hash-bound aggregate axes from the wire proof. The verifier then reconstructs `node_hash_with_count_and_sum` with wrong (zero or guessed) aggregates, producing a root-hash mismatch. Fix: extract `count_sum_for_ref` from `ProvableCountedAndProvableSummedMerkNode(count, sum)` and emit `Node::KVRefValueHashCountSum` first (strictest invariant) in both loops' dispatch ladders. Reproduction confirmed: temporarily reverting the fix surfaces "V1 mismatch in lower layer hash" on the new pcps_reference_proof_round_trips_against_same_root test. **P2 (PCPS chunks emitted but not restorable) — merk/src/merk/restore.rs** `create_proof_node_for_chunk` (merk/src/proofs/chunk/chunk.rs) dispatches via `ProofNodeType::KvSum` → `to_kv_sum_node()` → `Node::KVSum`, and via `ProofNodeType::KvCountSum` → `to_kv_count_sum_node()` → `Node::KVCountSum`. So a `ProvableSumTree` chunk contains `KVSum` nodes and a PCPS chunk contains `KVCountSum` nodes. But `Restorer::verify_chunk`'s allowlist only accepts `KVValueHashFeatureType | KV | KVValueHash | KVCount`, so both kinds of chunk get rejected immediately with "expected chunk proof to contain only kv or hash nodes". `Restorer::write_chunk` has the same gap in its `match &proof_node.node` dispatch. Fix: add `KVSum` and `KVCountSum` to both the `verify_chunk` allowlist and the `write_chunk` Node match. New write arms produce `TreeFeatureType::ProvableSummedMerkNode(sum)` and `ProvableCountedAndProvableSummedMerkNode(count, sum)` entries respectively. Pinned with two new tests: `KVSum` chunks pass the allowlist; `KVCountSum` chunks pass the allowlist. (NOTE: the existing `KVCount` restore arm has a latent OWN-vs-AGGREGATE semantic issue that prevents end-to-end chunk → restore round-trips on any Provable* host — that's a separate pre-existing bug that affects `ProvableCountTree` too, and is out of scope here.) **P2 (trunk_query rejects PCPS) — merk/src/merk/mod.rs** `Merk::trunk_query`'s `supports_count` guard hard-coded `CountTree | CountSumTree | ProvableCountTree | ProvableCountSumTree` and rejected PCPS with `InvalidOperation`, even though `TreeType::is_count_bearing()` already correctly reports PCPS as count-bearing. The error message even listed only the four old types. The privacy-clamping branch (`is_provable_count_tree`) had the same omission, so a PCPS trunk query with `min_depth` set would silently bypass `calculate_chunk_depths_with_minimum` and leak small-subtree information. Fix: delegate the `supports_count` check to `TreeType::is_count_bearing()` (the canonical predicate that already includes PCPS) and add PCPS to the privacy-path match. Updated the error message + rustdoc to enumerate all five count-bearing types. Pinned with two new tests: - `test_trunk_query_on_provable_count_provable_sum_tree` — trunk query succeeds on PCPS and produces a non-empty proof - `test_trunk_query_with_min_depth_engages_privacy_path_for_pcps` — trunk query with `min_depth` succeeds on PCPS All 1727 grovedb + 555 merk tests pass; cargo fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Patch coverage on 6825810 regressed to 89.32% (target 90%) because the previous push added production code for the three P1/P2 fixes but the corresponding tests only exercised the v1 ref-rewrite loop and the verify_chunk allowlist. The actual write_chunk arms and the v0 ref-rewrite loop's PCPS arm were uncovered. This adds 4 targeted tests covering all three branches the previous commit added: **v0 ref-rewrite loop coverage** (grovedb/src/operations/proof/generate.rs line ~1285): - `pcps_reference_proof_round_trips_against_same_root_v0_envelope` runs the same PCPS-reference proof round-trip but against `GROVE_V2`, which dispatches `prove_query_non_serialized` to the v0 path. Both v0 and v1 ref-rewrite loops have the same defect; the v1 test landed in the previous commit, this is the v0 mirror. Parametrized the existing test body (`pcps_reference_proof_round_trip_with(grove_version)`) so both v0 and v1 share the same setup + assertions. **write_chunk arm coverage** (merk/src/merk/restore.rs): - `restore_single_leaf_kvsum_for_provable_sum_tree` — single-leaf `ProvableSumTree` chunk round-trip. The chunk emits `Node::KVSum` for the leaf; restoration runs the new KVSum write arm to produce a `TreeFeatureType::ProvableSummedMerkNode(7)` entry. For a single leaf, own == aggregate, so the restored root hash matches the source root exactly. (Multi-key chunks on Provable* trees have a separate pre-existing OWN-vs-AGGREGATE issue affecting `ProvableCountTree` too — that's out of scope for this PR.) - `restore_single_leaf_kvcountsum_for_provable_count_provable_sum_tree` — same shape but on PCPS, exercising the new KVCountSum write arm that produces `ProvableCountedAndProvableSummedMerkNode(1, 7)`. Replaced the prior allowlist-only tests (`write_chunk_kvsum_node_produces_provable_summed_feature_type` and `write_chunk_kvcountsum_node_produces_dual_axis_feature_type`) with these because the allowlist tests only covered ~5 lines (the verify_chunk allowlist arm) while the new tests exercise the full write_chunk arm (~14 lines each) end-to-end. All 555 merk + 1728 grovedb tests pass; cargo fmt clean. Expected patch coverage: ≥ 90% (was 89.32% with ~22 new lines covered). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
merk/src/merk/mod.rs (1)
1886-1918: ⚡ Quick winStrengthen the min-depth PCPS test to assert privacy-path behavior.
This test only checks that
trunk_querysucceeds. The non-privacy path can also succeed, so a regression could still pass here.Proposed tightening
fn test_trunk_query_with_min_depth_engages_privacy_path_for_pcps() { + use crate::proofs::branch::calculate_chunk_depths_with_minimum; use crate::TreeFeatureType::ProvableCountedAndProvableSummedMerkNode; let grove_version = GroveVersion::latest(); @@ - let result = merk - .trunk_query(8, Some(5), grove_version) + let max_depth = 2; + let min_depth = 5; + let result = merk + .trunk_query(max_depth, Some(min_depth), grove_version) .unwrap() .expect("trunk_query with min_depth on PCPS must succeed"); assert!(!result.proof.is_empty()); + let expected = calculate_chunk_depths_with_minimum(result.tree_depth, max_depth, min_depth) + .expect("expected chunk-depth calculation to succeed"); + assert_eq!(result.chunk_depths, expected); }🤖 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 `@merk/src/merk/mod.rs` around lines 1886 - 1918, The test test_trunk_query_with_min_depth_engages_privacy_path_for_pcps currently only checks success; strengthen it by asserting that the privacy-path behaviour is actually used: after calling merk.trunk_query(8, Some(5), grove_version) keep the existing non-empty proof assertion and add an assertion that the returned result.chunk_depths reflects the minimum-clamped first chunk depth (e.g. assert that result.chunk_depths.first() == Some(5) or equivalent), so the test verifies calculate_chunk_depths_with_minimum / is_provable_count_tree actually engaged the privacy path for the PCPS tree.merk/src/proofs/query/aggregate_count/tests.rs (1)
1767-1794: ⚡ Quick winNarrow this mutation to one boundary node.
This rewrites every
KVDigestCountto0, so the proof can fail on an earlier, unrelated shape error before it ever reaches thechecked_subunderflow path the test claims to cover. Mutating just the intended parent node will make the regression deterministic.♻️ Proposed tightening
- for op in ops.iter_mut() { + for op in ops.iter_mut().rev() { if let ProofOp::Push(Node::KVDigestCount(_, _, c)) = op { *c = 0; rewrote = true; + break; } }🤖 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 `@merk/src/proofs/query/aggregate_count/tests.rs` around lines 1767 - 1794, The loop currently zeroes every KVDigestCount in ops, which can trigger unrelated shape errors first; instead locate and mutate only the intended parent boundary KVDigestCount (the last one encountered) — e.g., iterate through ops tracking the index of the last ProofOp::Push(Node::KVDigestCount(...)) and after the loop set only that entry's count to 0 (or break immediately when found while iterating from the end), keep the rewrote assertion and leave the subsequent encode_proof/verify_aggregate_count_on_range_proof logic unchanged so the test deterministically hits the checked_sub underflow path.
🤖 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 `@merk/src/merk/mod.rs`:
- Around line 1886-1918: The test
test_trunk_query_with_min_depth_engages_privacy_path_for_pcps currently only
checks success; strengthen it by asserting that the privacy-path behaviour is
actually used: after calling merk.trunk_query(8, Some(5), grove_version) keep
the existing non-empty proof assertion and add an assertion that the returned
result.chunk_depths reflects the minimum-clamped first chunk depth (e.g. assert
that result.chunk_depths.first() == Some(5) or equivalent), so the test verifies
calculate_chunk_depths_with_minimum / is_provable_count_tree actually engaged
the privacy path for the PCPS tree.
In `@merk/src/proofs/query/aggregate_count/tests.rs`:
- Around line 1767-1794: The loop currently zeroes every KVDigestCount in ops,
which can trigger unrelated shape errors first; instead locate and mutate only
the intended parent boundary KVDigestCount (the last one encountered) — e.g.,
iterate through ops tracking the index of the last
ProofOp::Push(Node::KVDigestCount(...)) and after the loop set only that entry's
count to 0 (or break immediately when found while iterating from the end), keep
the rewrote assertion and leave the subsequent
encode_proof/verify_aggregate_count_on_range_proof logic unchanged so the test
deterministically hits the checked_sub underflow path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 861fc733-04f5-4493-8a46-92f63a93c5d6
📒 Files selected for processing (21)
grovedb-element/src/element/serialize.rsgrovedb-element/src/element_type.rsgrovedb-query/src/proofs/mod.rsgrovedb/src/batch/mod.rsgrovedb/src/lib.rsgrovedb/src/operations/proof/generate.rsgrovedb/src/operations/proof/verify.rsgrovedb/src/tests/provable_count_provable_sum_tree_tests.rsgrovedb/src/tests/provable_count_sum_tree_tests.rsmerk/src/merk/get.rsmerk/src/merk/mod.rsmerk/src/merk/prove.rsmerk/src/merk/restore.rsmerk/src/proofs/branch/tests.rsmerk/src/proofs/query/aggregate_count/emit.rsmerk/src/proofs/query/aggregate_count/mod.rsmerk/src/proofs/query/aggregate_count/tests.rsmerk/src/proofs/query/aggregate_sum/tests.rsmerk/src/proofs/query/mod.rsmerk/src/proofs/query/verify.rsmerk/src/proofs/tree.rs
🚧 Files skipped from review as they are similar to previous changes (7)
- merk/src/merk/prove.rs
- grovedb-query/src/proofs/mod.rs
- merk/src/merk/get.rs
- grovedb/src/batch/mod.rs
- merk/src/proofs/query/aggregate_count/emit.rs
- merk/src/proofs/tree.rs
- grovedb-element/src/element_type.rs
Addresses the 2 nitpick suggestions from CodeRabbit's latest review on PR #670. Both are valid quality improvements that make existing tests more deterministic / observable. **mod.rs: trunk_query min_depth privacy-path assertion** The previous `test_trunk_query_with_min_depth_engages_privacy_path_for_pcps` only checked that `trunk_query` returned `Ok` — but the non-privacy path also returns `Ok`, so a regression that silently fell back to `calculate_chunk_depths` would still pass the test. Tightened to: - Use 25 keys (AVL tree depth ≥ 6) with `max_depth=4`, `min_depth=4` — parameters where the two depth-split functions return *different* vectors: `calculate_chunk_depths(6, 4)` → `[3, 3]` (natural even split) vs `calculate_chunk_depths_with_minimum(6, 4, 4)` → `[4, 2]` (front chunk clamped up to min_depth). - Assert `result.chunk_depths == calculate_chunk_depths_with_minimum(tree_depth, max_depth, min_depth)` — pins the privacy-path output as the headline check. - Assert the non-privacy output differs — guards against test-vacuity if a future change accidentally makes the two functions return the same output for these inputs. A regression where the PCPS arm in `is_provable_count_tree` is dropped would now fail the privacy-path equality assertion. **aggregate_count/tests.rs: narrow shape_walk_rejects_own_count_underflow** The previous version of this test zeroed *every* `KVDigestCount` in the proof op stream, which could trip an earlier shape error before the verifier ever reached the `checked_sub` underflow arm — making the test non-deterministic (the rejection message might come from an unrelated arm). Per CodeRabbit suggestion: iterate `ops.iter_mut().rev()` and `break` on the first match, mutating only the *last* `KVDigestCount` op (the deepest parent boundary node in the walk, whose children are already on the proof stack). This makes the `checked_sub` underflow the deterministic target of the rejection. All 555 merk lib tests pass; cargo fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
This is Claude — pushed ceb52cbd addressing both CodeRabbit nitpicks from the latest review. mod.rs:1886 — aggregate_count/tests.rs:1767 — All 555 merk + 1728 grovedb tests pass; cargo fmt clean. |
Merges develop (which brings in PR #672 — rejection of NonCounted/ NotCountedOrSummed in ProvableCountTree/ProvableCountSumTree). PR #672 was scoped to the two count-bearing Provable* hosts that existed at the time; this commit extends the same rule to the dual-axis `ProvableCountProvableSumTree` host introduced by this PR. **Why PCPS rejects the same wrappers** PCPS commits BOTH count AND sum into every node hash via `node_hash_with_count_and_sum`. A `NonCounted` child contributes own_count = 0 to the parent, so the cryptographically-committed count would diverge from the actual number of stored elements — the same footgun PR #672 closed for the single-axis count hosts. By identical reasoning, `NotCountedOrSummed` is also rejected (it would diverge on BOTH committed axes). `NotSummed` is intentionally left accepted under PCPS, matching PR #672's deferral of the NotSummed-in-Provable* question. **No new predicate logic needed** The existing `TreeType::accepts_non_counted_children()` and `accepts_not_counted_or_summed_children()` predicates from PR #672 already exclude PCPS (they `matches!(self, CountTree | CountSumTree)` which doesn't include ProvableCountProvableSumTree). The merk insert guards (`merk/src/element/insert.rs`) and the GroveDB batch guard (`grovedb/src/batch/mod.rs`) call these predicates, so PCPS rejection is automatic after the merge. **Test changes** - `non_counted_pcps_inserts_into_pcps_parent_without_incrementing_count` → `non_counted_rejected_under_provable_count_provable_sum_tree_parent` Flipped from acceptance to rejection assertion. - `not_counted_or_summed_pcps_inserts_into_pcps_parent_and_zeros_both_axes` → `not_counted_or_summed_rejected_under_provable_count_provable_sum_tree_parent` Flipped from acceptance to rejection assertion. - `not_summed_pcps_inserts_into_pcps_parent_and_zeros_sum_contribution` Unchanged — still asserts NotSummed acceptance (PR #672 pattern). - `tree_type::accepts_non_counted_children` test extended to assert PCPS is in the rejection set (also adds PCPS to the count-bearing implication loop). - `tree_type::accepts_not_counted_or_summed_children` test extended to assert PCPS is in the rejection set. **Doc updates** - `grovedb-element/src/element/mod.rs` and `grovedb-element/src/element/constructor.rs`: doc comments for `NotCountedOrSummed` now enumerate both `ProvableCountSumTree` and `ProvableCountProvableSumTree` as parents that reject the wrapper. - `grovedb/src/tests/provable_count_provable_sum_tree_tests.rs` module docstring updated to describe the new rejection rule. **Merge conflict resolution** Both conflicts were in doc comments only (element/mod.rs:174-188 and element/constructor.rs:616-625). Resolved by extending develop's strict-rejection wording to also cover PCPS. All 1733 grovedb + 559 merk tests pass; cargo fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…t-provable-sum-tree
Merges develop (brings in PR #669 — offset-paginated proofs for ProvableCountTree / ProvableCountSumTree single-range queries) and extends the same prove/verify flow to the new dual-axis ProvableCountProvableSumTree (PCPS) host. ## Why PCPS needs special handling PR #669's call-out (in `count_offset/mod.rs`): > Why ProvableCountSumTree only commits the count (not the sum): > ProvableCountSumTree nodes hash via node_hash_with_count — the sum > is stored on the node but is not bound to the node hash. That property is what lets PR #669 emit a count-only `HashWithCount` collapse op for both ProvableCountTree and ProvableCountSumTree. PCPS is **different**: it hashes via `node_hash_with_count_and_sum`, so BOTH count AND sum are committed into every node hash. A `HashWithCount` collapse op for a PCPS host would not let the verifier reconstruct the right hash function — root hash mismatch. ## What this adds The same `binds_sum_into_hash(tree_type)` dispatch pattern I used when extending aggregate_count/aggregate_sum to PCPS earlier in this PR: **merk/src/proofs/query/count_offset/mod.rs** - `is_provable_count_bearing` now matches PCPS too. - New `binds_sum_into_hash(tree_type)` predicate (`true` for PCPS). - New `provable_sum_from_dual_axis_aggregate(data)` helper used by the emit path to populate the sum field of the dual-axis Node variants. - `provable_count_from_aggregate` now accepts `AggregateData::ProvableCountAndProvableSum`. **merk/src/proofs/query/count_offset/emit.rs** - `emit_count_offset_proof` now takes a `tree_type: TreeType` parameter and threads it through both recursive descents. - Collapse-arm emission dispatches: - PCPS host → `Node::HashWithCountAndSum(kv, l, r, count, sum)` - Single-axis host → `Node::HashWithCount(kv, l, r, count)` - Boundary emission dispatches: - PCPS host → `Node::KVDigestCountSum(key, value_hash, count, sum)` - Single-axis host → `Node::KVDigestCount(key, value_hash, count)` - `emit_returned_node` now takes `tree_type` and uses it to drive the `ElementType::proof_node_type(parent_tree_type)` dispatch (previously hardcoded `ProvableCountTree`). Handles the new `ProofNodeType::KvCountSum` (PCPS Item-flavored returns → `walker.to_kv_count_sum_node()`) and `KvRefValueHashCountSum` (delegates to `to_kv_value_hash_feature_type_node` which already carries the dual-axis aggregate via `ProvableCountedAndProvableSummedMerkNode`). **merk/src/proofs/query/count_offset/verify.rs** - Phase-1 allowlist accepts dual-axis Node variants (`HashWithCountAndSum`, `KVDigestCountSum`, `KVCountSum`) alongside the existing single-axis ones. - `aggregate_of_proof_tree_node` reads count out of all three new variants + the `ProvableCountedAndProvableSummedMerkNode` feature type in `KVValueHashFeatureType`. - Collapse arm in `verify_count_offset_shape` matches both `HashWithCount` and `HashWithCountAndSum`; per-element key extractor recognizes `KVDigestCountSum` and `KVCountSum`. - `classify_self` accepts the dual-axis variants in the appropriate boundary roles (KVDigestCountSum at path/skipped/past-limit, KVCountSum as value-returned for Item-flavored entries). **merk/src/merk/prove_count_offset.rs** - `Merk::prove_count_offset_on_range` tree-type gate now includes PCPS; error messages updated. **grovedb/src/operations/proof/generate.rs** - Both prover-side tree-type gates (top-level + leaf-dispatch) include PCPS. **grovedb/src/query/mod.rs** - Doc comments + error message for the syntactic `validate_count_offset_paginated` validators updated to mention PCPS as a third accepted host. ## Tests **merk-level (5 new in `count_offset/tests.rs`)**: - `pcps_round_trip_offset_0_limit_none_full_range_ascending` — returns all 15 keys end-to-end, exercising every dual-axis Node variant. - `pcps_round_trip_offset_5_limit_3_ascending` — offset+limit composition through the dual-axis collapse op. - `pcps_round_trip_offset_5_limit_3_descending` — inverted op family with dual-axis variants. - `pcps_round_trip_offset_in_middle_of_partial_range` — Boundary classifications + dual-axis variants on partial-range queries. - `pcps_count_offset_root_hash_diverges_from_single_axis` — proves the same query on a ProvableCountSumTree and a PCPS over identical content and asserts the reconstructed root hashes differ. This pins the dual-axis dispatch: without it the verifier would reconstruct `node_hash_with_count` (wrong for PCPS) and the assertion fails. **grovedb-level (1 new in `count_offset_paginated_tests.rs`)**: - `end_to_end_offset_on_provable_count_provable_sum_tree` — parallel of `end_to_end_offset_on_provable_count_sum_tree`, goes through the full path-query stack on a PCPS host. 601 merk + 1765 grovedb tests pass; cargo fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
User-reported finding:
> KVCountSum is still missing from GroveDB proof result/limit
> accounting. PCPS item queries are emitted as Node::KVCountSum but
> both GroveDB post-processing loops only match/preserve KV, KVCount,
> KVSum, and feature-type nodes. That means PCPS item results can
> verify cryptographically while skipping overall_limit decrement
> and has_a_result_at_level, so limited or multi-layer GroveDB
> queries can over-prove results or treat a non-empty PCPS subquery
> as empty.
Confirmed and fixed in both the v0 and v1 post-processing loops in
`grovedb/src/operations/proof/generate.rs`:
**should_preserve_node_type** (lines 538, 1518) — adds `KVCountSum`
to the allowlist alongside `KVCount` / `KVSum` / `KVValueHashFeatureType`.
Without this, if `KVCountSum` ever reached the "rewrite to Node::KV"
fallback in the Item-class branch, the dual-axis count+sum binding
would be destroyed (the verifier's hash chain wouldn't reconstruct
`node_hash_with_count_and_sum`). This is parallel to how `KVCount`
preserves count and `KVSum` preserves sum for the single-axis hosts.
**Item-class outer match** (lines 590, 1562) — adds
`Node::KVCountSum(key, value, ..)` to the alternative pattern that
controls whether the Item-class branch fires for a given op.
Without this, a PCPS Item arriving as `Node::KVCountSum` falls
through to the loop's `_ => continue` arm:
- `overall_limit` doesn't decrement.
- `has_a_result_at_level` doesn't get set.
The consequence: for multi-layer queries with PCPS as a subquery
target, the outer layer's `prove_subqueries` would see the PCPS
layer's prove_subqueries return with `overall_limit` unchanged,
treat the layer as if it had no results, and (under the default
`ProveOptions { decrease_limit_on_empty_sub_query_result: true }`)
erroneously decrement an extra slot at the empty-subquery handling
arm (line 867). Cascading wrong-limit accounting follows.
**Tests added**:
- `pcps_regular_query_with_limit_round_trips` — smoke check that a
regular `prove_query` with a SizedQuery::limit on a PCPS host
round-trips and returns exactly `limit` items in sorted order.
Pins the new code path; the prove-side bookkeeping fix is
transparent to the verifier so this passes both with and
without the fix, but it confirms the dual-axis hash binding
stays intact (the `should_preserve_node_type` half of the fix).
- `pcps_subquery_items_surface_in_verified_result` — multi-layer
query (outer Tree → subquery into PCPS host) where the PCPS
layer holds 3 Items. Without the fix, the outer post-processor
fails to track that the PCPS layer had results; with the fix,
the 3 items surface correctly in the verified result set.
1767 grovedb + 607 merk tests pass; cargo fmt clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
User flag: "Why are we touching v0 proofs?" V0 proofs are LOCKED — the per-project memory rule is explicit: "never modify the V0 prover/verifier". Earlier commits in this PR (6825810, 745672b) extended both the v0 ref-rewrite loop AND the v0 should_preserve_node_type / item-class arm to handle the new dual-axis PCPS Node variants. That's a behavior change to the v0 prover — disallowed by the contract — even though it would never fire in production (v0 grove versions predate PCPS, so v0 deployments have no PCPS subtrees in their data). This commit reverts those v0 modifications and routes the unsupported combination through the existing v0 rejection pattern (same shape used for `MmrTree` / `BulkAppendTree` / `DenseAppendOnlyFixedSizeTree` at line ~778 of `prove_subqueries`). **Reverted from `prove_subqueries` (v0)**: - KVCountSum arms in `should_preserve_node_type`. - KVCountSum arm in the item-class outer match pattern. - `count_sum_for_ref` extraction logic. - `KVRefValueHashCountSum` dispatch arm in the reference-rewrite ladder. **Added** — a tree-type rejection at the v0 entry point in `prove_subqueries` (parallel of the existing `MmrTree` / `BulkAppendTree` / `DenseAppendOnlyFixedSizeTree` rejection): ```rust if matches!(subtree.tree_type, MerkTreeType::ProvableCountProvableSumTree) { return Err(Error::NotSupported( "ProvableCountProvableSumTree hosts require V1 proof envelopes; \ upgrade the grove version producing the proof to v3 or later" .to_string(), )) .wrap_with_cost(cost); } ``` Detection requires the open merk (PCPS isn't syntactically distinguishable from a regular Tree at the dispatcher level), so the gate lives at the v0 leaf-merk-open site rather than in the higher- up `prove_query_non_serialized` dispatcher (which is where the syntactic ACOR/ASOR v0 rejections live). **Doc comments updated** in both fix sites to explain the V0 lock: the `should_preserve_node_type` allowlist comment block now notes that PCPS handling intentionally lives in V1 only and that V0 + PCPS is rejected at dispatch time. Same in the reference-dispatch ladder. **V1 changes preserved** (commit context): - `prove_subqueries_v1`: dual-axis dispatch for PCPS Items + refs (commit 745672b) — STAYS. - `verify.rs` chunk-verification arms (KVRefValueHashCountSum opaque guard, KVCountSum leaf-count extraction) — STAYS (they live in V1-only `verify_trunk_chunk_proof_v1` / `verify_branch_chunk_proof` callers, not in the v0 verify path). **Test changes**: - `pcps_reference_proof_round_trips_against_same_root_v0_envelope` was a round-trip test on v0; renamed to `pcps_proof_rejected_on_v0_envelope` and flipped to assert rejection with the new `Error::NotSupported` message. - The v1 ref proof test (`pcps_reference_proof_round_trips_against_same_root`) is unchanged. - The multi-layer subquery test (`pcps_subquery_items_surface_in_verified_result`) is unchanged. **CodeRabbit nitpick on matches!(fetched, …)** — not actionable. `matches!(fetched, Element::ProvableCountProvableSumTree(_, _, _, _))` uses all `_` wildcards and doesn't bind any fields; `_` is a non-binding wildcard so no move happens. The test compiles and passes (the suite has been green across many CI runs). The CodeRabbit suggestion is based on a general statement about matches! that doesn't apply to all-wildcard patterns. All 1767 grovedb + 607 merk tests pass; cargo fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previous CI on 3b28a0e reported patch coverage at 89.92% — 0.08% short of the 90% target. Largest uncovered chunk is `grovedb/src/batch/mod.rs` at 5.55% (17 missing lines). The PCPS arms I added in earlier commits to the batch path's `LayeredValueDefinedCost` flag-update match and the `InsertTreeWithRootHash` propagation branch never fired in any test — single-element insert tests bypass the batch propagation; only a real batch op that inserts a PCPS subtree + children triggers the propagation rewrite. New test `pcps_batch_apply_propagates_aggregate`: - Applies a 4-op batch (PCPS subtree insert + 3 sum_item child inserts) in one `apply_batch` call. - The batch executor rewrites the original PCPS insert op into `GroveOp::InsertTreeWithRootHash` during propagation, triggering the new arm at `batch/mod.rs:3264`. - Verifies the PCPS root's aggregate after the batch reflects the 3 children's count (3) and the sum of their values (10+20+30=60). This exercises ~15+ lines of previously-uncovered production code in `batch/mod.rs`, which should push patch coverage from 89.92% above the 90% target. 1768 grovedb tests pass; cargo fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…bined proof Add a new QueryItem variant that returns BOTH the u64 count AND the signed i64 sum of children with keys in a range, from a single proof against a ProvableCountProvableSumTree (PCPS) host. The proof shape is byte-identical to AggregateCountOnRange against a PCPS host — both emitters write HashWithCountAndSum / KVDigestCountSum ops because PCPS binds both axes into the node hash via node_hash_with_count_and_sum. The combined variant ships a dedicated prover that tracks both axes in one walk and a verifier that walks both axes in parallel. PCPS-only enforcement - Merk-level prover (Merk::prove_aggregate_count_and_sum_on_range) rejects every non-PCPS tree type up front with InvalidProofError. - GroveDB-level verifier (GroveDb::verify_aggregate_count_and_sum_query) rejects non-PCPS terminal elements via the leaf-chain enforce step. - V0 envelopes are rejected at prove_query_non_serialized with Error::NotSupported (V1-required message). V0 PROOFS ARE LOCKED — the new feature lives entirely on V1. Validators - PathQuery / SizedQuery / Query::validate_aggregate_count_and_sum_on_range enforce: single combined item, inner range that isn't Key / RangeFull / any aggregate, no subqueries, no pagination, non-root path. - Bincode decoder rejects nested aggregate-in-aggregate combinations for all three aggregate variants (orthogonality). - Serde Deserialize uses NonAggregateInner so the inner field set rejects all aggregate tags before any recursion can happen. New code - grovedb-query/src/query_item/mod.rs: variant 12, encode/decode, serde, helpers (is_aggregate_count_and_sum_on_range, aggregate_count_and_sum_inner). - grovedb-query/src/query.rs: new_aggregate_count_and_sum_on_range, aggregate_count_and_sum_on_range, has_aggregate_count_and_sum_on_range_anywhere, validate_aggregate_count_and_sum_on_range. - grovedb-query/src/query_item/intersect.rs: delegating range-set arms. - grovedb/src/query/mod.rs: PathQuery / SizedQuery mirrors. - merk/src/proofs/query/aggregate_count_and_sum/: new module with emit.rs (dual-axis walker), prove.rs (RefWalker entry), verify.rs (two-phase verifier with i128 sum accumulator), tests.rs (round-trip, PCPS-only, empty, forged-count, forged-sum, forged-KVDigest variants, cross-axis substitution). - merk/src/merk/prove.rs: prove_aggregate_count_and_sum_on_range. - grovedb/src/operations/proof/generate.rs: V1 short-circuit branch + empty-PCPS-host carrier descent + V0 NotSupported gate. V0 path untouched. - grovedb/src/operations/proof/aggregate_count_and_sum/: new envelope module mirroring aggregate_sum (mod, helpers, leaf_chain). Tests - 8 new merk-level tests cover round-trip, PCPS-only rejection on every other tree type, empty merk, count/sum forgery on both HashWithCountAndSum and KVDigestCountSum, and cross-axis node substitution. - 3 new grovedb-level tests: pcps_combined_count_and_sum_proof_returns_both_axes_from_one_proof, combined_aggregate_query_rejected_on_provable_count_sum_tree, combined_aggregate_query_rejected_on_v0_envelope. - 8 new grovedb-query encoding tests (round-trip, nested rejection, orthogonality with both other aggregates, helpers/bounds, display, serde round-trip). - 7 new validator unit tests covering happy path, extra items, inner Key / RangeFull / aggregate rejection, subquery rejection, conditional-branch rejection, and walker detection. All workspace tests pass (1771 grovedb, 615 merk, 188 grovedb-query). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
grovedb-query/src/query_item/mod.rs (1)
1301-1356:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAggregate-wrapper delegation currently drops already-collected iterator cost.
iter_is_valid_for_typecollectsOperationCostand reads iterator state before thematch, but wrapper branches (Aggregate*OnRange) returninner.iter_is_valid_for_type(...)directly. The newAggregateCountAndSumOnRangearm inherits this path, which can under-account cost and duplicate iterator reads.Suggested fix
pub fn iter_is_valid_for_type<I: RawIterator>( &self, iter: &I, limit: Option<u16>, aggregate_limit: Option<i64>, left_to_right: bool, ) -> CostContext<bool> { + if let QueryItem::AggregateCountOnRange(inner) + | QueryItem::AggregateSumOnRange(inner) + | QueryItem::AggregateCountAndSumOnRange(inner) = self + { + return inner.iter_is_valid_for_type(iter, limit, aggregate_limit, left_to_right); + } + let mut cost = OperationCost::default(); @@ - QueryItem::AggregateCountOnRange(inner) => { - return inner.iter_is_valid_for_type(iter, limit, aggregate_limit, left_to_right); - } - QueryItem::AggregateSumOnRange(inner) => { - return inner.iter_is_valid_for_type(iter, limit, aggregate_limit, left_to_right); - } - QueryItem::AggregateCountAndSumOnRange(inner) => { - return inner.iter_is_valid_for_type(iter, limit, aggregate_limit, left_to_right); - } + QueryItem::AggregateCountOnRange(_) + | QueryItem::AggregateSumOnRange(_) + | QueryItem::AggregateCountAndSumOnRange(_) => unreachable!(), };As per coding guidelines, "Use
cost_return_on_error!macro for early returns with cost accumulation in Rust source files".🤖 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 `@grovedb-query/src/query_item/mod.rs` around lines 1301 - 1356, The Aggregate wrapper arms (QueryItem::AggregateCountOnRange, ::AggregateSumOnRange, ::AggregateCountAndSumOnRange) directly return inner.iter_is_valid_for_type(...) which bypasses the already-collected OperationCost and duplicates iterator reads; update these arms to accumulate and return the pre-computed cost on early return using the cost_return_on_error! macro instead of direct return, i.e., call inner.iter_is_valid_for_type(...) but wrap its early-return path with cost_return_on_error!(collected_cost, ...) so iter_is_valid_for_type(iter, limit, aggregate_limit, left_to_right) preserves the collected cost and avoids duplicate iterator access.grovedb/src/operations/proof/generate.rs (1)
815-824:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winReject PCPS elements from V0 proofs entirely.
These new V0 arms still let
prove_subqueriesserializeElement::ProvableCountProvableSumTreewhen the query returns the tree itself without descending into it. That widens the shipped V0 wire format even though the block comment above says V0 must keep rejecting every input v1/v2 rejected. Please fail closed on any PCPS element in the V0 path, not just when the currently-open subtree host is PCPS.Suggested fix
- | Ok(Element::ProvableCountProvableSumTree(..)) | Ok(Element::CommitmentTree(..)) | Ok(Element::MmrTree(..)) | Ok(Element::BulkAppendTree(..)) | Ok(Element::DenseAppendOnlyFixedSizeTree(..)) if !done_with_results => { #[cfg(feature = "proof_debug")] { println!( "found tree {}, no subquery query is {:?}", hex_to_ascii(key), query ); } if let Some(limit) = overall_limit.as_mut() { *limit -= 1; } has_a_result_at_level |= true; } + Ok(Element::ProvableCountProvableSumTree(..)) => { + return Err(Error::NotSupported( + "ProvableCountProvableSumTree elements require V1 proof envelopes; \ + upgrade the grove version producing the proof to v3 or later" + .to_string(), + )) + .wrap_with_cost(cost); + }Also applies to: 853-864
🤖 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 `@grovedb/src/operations/proof/generate.rs` around lines 815 - 824, The V0 proof path is currently allowing PCPS elements (e.g., Element::ProvableCountProvableSumTree, Element::ProvableCountSumTree, Element::ProvableSumTree, Element::ProvableCountTree) to be serialized when a query returns the tree itself; change the match arms in generate.rs (the match that lists Element::Tree, Element::SumTree, Element::BigSumTree, Element::CountTree, Element::ProvableCountTree, Element::CountSumTree, Element::ProvableCountSumTree, Element::ProvableSumTree, Element::ProvableCountProvableSumTree, Element::CommitmentTree) so that any PCPS-related variants (all Element::Provable* and Element::ProvableCountProvableSumTree) are rejected unconditionally in the V0 path (return an Err/early fail) instead of being treated like non-PCPS trees; ensure the same change is applied to the other similar match block later (the one around the 853-864 region) so V0 never accepts any Provable* elements.
🤖 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/operations/proof/aggregate_count_and_sum/helpers.rs`:
- Around line 87-99: The call to level_query.execute_proof currently uses
.unwrap(), which will panic on outer CostResult errors and lose cost context;
replace the .unwrap() by handling the CostResult properly (e.g., call
.map_err(...) on the outer error or match the Result to convert/propagate the
cost error) and then apply the existing .map_err(...) on the inner verification
result before ?-unwrapping; specifically modify the chain around
level_query.execute_proof(merk_bytes, None, true, 0) so that outer errors are
mapped into a suitable Error (or propagated) instead of unwrapping, keeping
references to level_query, merk_bytes, path_query, target_key and
Error::InvalidProof intact.
---
Outside diff comments:
In `@grovedb-query/src/query_item/mod.rs`:
- Around line 1301-1356: The Aggregate wrapper arms
(QueryItem::AggregateCountOnRange, ::AggregateSumOnRange,
::AggregateCountAndSumOnRange) directly return inner.iter_is_valid_for_type(...)
which bypasses the already-collected OperationCost and duplicates iterator
reads; update these arms to accumulate and return the pre-computed cost on early
return using the cost_return_on_error! macro instead of direct return, i.e.,
call inner.iter_is_valid_for_type(...) but wrap its early-return path with
cost_return_on_error!(collected_cost, ...) so iter_is_valid_for_type(iter,
limit, aggregate_limit, left_to_right) preserves the collected cost and avoids
duplicate iterator access.
In `@grovedb/src/operations/proof/generate.rs`:
- Around line 815-824: The V0 proof path is currently allowing PCPS elements
(e.g., Element::ProvableCountProvableSumTree, Element::ProvableCountSumTree,
Element::ProvableSumTree, Element::ProvableCountTree) to be serialized when a
query returns the tree itself; change the match arms in generate.rs (the match
that lists Element::Tree, Element::SumTree, Element::BigSumTree,
Element::CountTree, Element::ProvableCountTree, Element::CountSumTree,
Element::ProvableCountSumTree, Element::ProvableSumTree,
Element::ProvableCountProvableSumTree, Element::CommitmentTree) so that any
PCPS-related variants (all Element::Provable* and
Element::ProvableCountProvableSumTree) are rejected unconditionally in the V0
path (return an Err/early fail) instead of being treated like non-PCPS trees;
ensure the same change is applied to the other similar match block later (the
one around the 853-864 region) so V0 never accepts any Provable* elements.
🪄 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: 39c6491a-de80-4806-af76-4d477444747e
📒 Files selected for processing (22)
grovedb-bulk-append-tree/src/proof/mod.rsgrovedb-dense-fixed-sized-merkle-tree/src/proof/mod.rsgrovedb-query/src/aggregate_count.rsgrovedb-query/src/query.rsgrovedb-query/src/query_item/intersect.rsgrovedb-query/src/query_item/mod.rsgrovedb/src/operations/proof/aggregate_count_and_sum/helpers.rsgrovedb/src/operations/proof/aggregate_count_and_sum/leaf_chain.rsgrovedb/src/operations/proof/aggregate_count_and_sum/mod.rsgrovedb/src/operations/proof/generate.rsgrovedb/src/operations/proof/mod.rsgrovedb/src/operations/proof/verify.rsgrovedb/src/query/mod.rsgrovedb/src/tests/provable_count_provable_sum_tree_tests.rsmerk/src/merk/prove.rsmerk/src/proofs/query/aggregate_count_and_sum/emit.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/verify.rsmerk/src/proofs/query/count_offset/tests.rsmerk/src/proofs/query/mod.rs
✅ Files skipped from review due to trivial changes (1)
- grovedb-bulk-append-tree/src/proof/mod.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- merk/src/proofs/query/mod.rs
…erage above 90% The previous combined-aggregate PR (commit 79d45a7) added ~600 lines of production code but only happy-path tests, dropping patch coverage from 90.245% to 83.78%. This commit adds targeted tests for the major unreached error/validator/edge arms. merk-side (`merk/src/proofs/query/aggregate_count_and_sum/tests.rs`, +12 tests): * `combined_verifier_rejects_non_dual_axis_at_disjoint` — Phase 1 allowlist rejection of plain Hash op * `combined_verifier_rejects_non_hashwithcountandsum_at_contained` — Phase 2 type rejection at Contained position * `combined_verifier_rejects_disjoint_leaf_with_children` / `combined_verifier_rejects_contained_leaf_with_children` — "must be a leaf" assertion at Disjoint / Contained positions * `combined_verifier_rejects_non_kvdigestcountsum_at_boundary` — Phase 2 type rejection at Boundary position * `combined_verifier_rejects_boundary_key_outside_bounds` — bound enforcement on KVDigestCountSum keys * `combined_verifier_rejects_own_count_underflow` — `checked_sub` underflow when children claim more than parent * `combined_verifier_rejects_i64_sum_narrow_overflow` — i128→i64 narrow gate * `combined_fuzz_byte_mutation_no_silent_forgery` — fuzzer asserting no silent count/sum forgery on byte mutations * `provable_count_and_sum_from_aggregate_*` — predicate accept/reject arms * `is_provable_count_and_sum_bearing_only_for_pcps` — PCPS-only predicate grovedb-side (`grovedb/src/tests/provable_count_provable_sum_tree_tests.rs`, +20 tests): PathQuery-level validator coverage: * `empty_path_combined_aggregate_rejected_at_validation` * `combined_aggregate_rejects_limit_at_validation` * `combined_aggregate_rejects_offset_at_validation` * `combined_aggregate_rejects_nested_aggregate_at_validation` * `combined_aggregate_rejects_inner_key_at_validation` * `combined_aggregate_rejects_inner_range_full_at_validation` * `path_query_has_aggregate_count_and_sum_on_range_present_and_absent` Envelope-level rejection (V1 strict-shape gates, helpers.rs + leaf_chain.rs): * `combined_v1_envelope_with_non_merk_proof_bytes_is_rejected` * `combined_v1_envelope_with_missing_lower_layer_is_rejected` * `combined_v1_envelope_with_extra_lower_layer_is_rejected` * `combined_v1_envelope_with_wrong_keyed_lower_layer_is_rejected` * `combined_v1_envelope_with_lower_layers_under_leaf_is_rejected` * `combined_v1_envelope_with_malformed_leaf_proof_is_rejected` * `combined_v1_envelope_with_corrupted_non_leaf_merk_bytes_is_rejected` * `combined_proof_with_trailing_bytes_is_rejected` * `combined_unparsable_envelope_is_rejected` * `combined_v0_envelope_rejected_at_verifier_gate` — V1-only gate at verifier side * `combined_v1_envelope_non_pcps_terminal_rejected_by_type_gate` — terminal-type gate in enforce_lower_chain * `combined_v1_envelope_non_tree_intermediate_rejected` — intermediate `is_any_tree()` gate * `combined_aggregate_carrier_descends_into_empty_pcps` — empty-PCPS subquery descent branch in prove_subqueries_v1 All 32 new tests pass alongside the existing 615+1771 baseline; no production code touched. V0 prover/verifier untouched. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CodeRabbit flagged that the three aggregate-wrapper arms in `QueryItem::iter_is_valid_for_type` (AggregateCountOnRange, AggregateSumOnRange, AggregateCountAndSumOnRange) early-return from the inner match BEFORE the outer `cost` accumulator is folded into the result, dropping the already-collected `iter.key()` cost AND making the recursive call re-read the iterator. Fix: short-circuit wrapper variants at the TOP of the function, before any `iter.key()` read. The inner item does its own read + cost accumulation, so the outer-level read becomes redundant. The in-match arms for wrapper variants become `unreachable!()` since the early return covers them all. Skipped CodeRabbit findings (decline with reasons): - `helpers.rs:99` `.unwrap()` on `CostResult` — false alarm. `.unwrap()` on `CostContext<T>` (which is what `CostResult<T>` is — `CostContext` is a wrapper struct, not `Result`) returns the inner `T` while discarding the cost. No panic risk. The `Result` is then handled by `.map_err(...)?`. Same idiom used by sibling `aggregate_count/helpers.rs` and `aggregate_sum/helpers.rs`. - V0 PCPS rejection at `generate.rs:815-823` — declining as inconsistent with the established V0 pattern. The match arm groups PCPS with `MmrTree`, `BulkAppendTree`, `DenseAppendOnlyFixedSizeTree`, and the other `Provable*` variants as "tree-without-subquery" emissions. CodeRabbit's suggestion would single-out PCPS for hard rejection while leaving sibling new-Element-variants behaving as tree-passthrough — that's the inconsistency. V0 already rejects PCPS at descend-time via the leaf-merk-open guard at `prove_subqueries:425`; the surface that remains (returning a PCPS Element as a query result without descending) follows the same passthrough pattern as MmrTree/BulkAppendTree and is consistent with how V0 has historically dealt with new Element variants it doesn't fully understand. 627 merk + 1791 grovedb tests pass; cargo fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
This is Claude — pushed 2d675c09 addressing the latest CodeRabbit review. Fixed
Declined
|
…regate paths Adds ~15 targeted tests to cover the remaining rejection arms and happy paths added in this PR: merk/src/proofs/query/aggregate_count_and_sum/tests.rs - Phase-2 Disjoint-position type-shape rejection (KVDigestCountSum where HashWithCountAndSum is required) - Phase-2 Boundary-position type-shape rejection (HashWithCountAndSum where KVDigestCountSum is required) - Phase-2 boundary key outside inherited subtree bounds rejection (the `key_strictly_inside` gate, distinct from the upstream ordering check) - Crafted-proof i128->i64 narrow-gate rejection for sums that overflow during the parallel-axis walk grovedb/src/tests/provable_count_provable_sum_tree_tests.rs - Non-leaf merk proof rewritten so its result_set does not contain the expected path key (helpers.rs key-not-found rejection arm) - Intermediate-tree value bytes rewritten to a different-flagged tree (helpers.rs chain-mismatch rejection arm, deserializes-as-tree but hash diverges) - Intermediate-tree value bytes rewritten to garbage (helpers.rs deserialize-error rejection arm) - Three-layer happy-path TEST_LEAF -> outer -> pcps end-to-end verify exercises non-leaf helper happy paths across multiple chain hops - Empty-PCPS-host carrier descents under ACOR / ASOR (and an updated ACAS) carriers — sized queries so the post-recursion limit-tracking arm sees a real limit transition - count-offset paginated against a NormalTree at proof-generation time pins the InvalidQuery rejection wording grovedb-query/src/aggregate_count.rs - Inner-Sum and inner-ACASOR rejection arms inside validate_leaf_aggregate_count_on_range (new orthogonality rule) - Carrier ACASOR-outer rejection arm inside validate_carrier_aggregate_count_on_range grovedb-query/src/query.rs - ASOR-wrapping-ACASOR rejection arm inside validate_aggregate_sum_on_range - Dispatcher "not a combined query" error wording pinned Coverage uplift on this PR's new files: - aggregate_count_and_sum/helpers.rs: 76.6% -> 94.3% - aggregate_count_and_sum/verify.rs: 82.9% -> 94.6% - aggregate_count_and_sum/emit.rs: 89.7% -> 91.9% - aggregate_count_and_sum/mod.rs: 75% -> 100% - grovedb-query/src/aggregate_count.rs: new arms now hit (was 0% on patch lines) - grovedb-query/src/query.rs: new ACASOR arms now hit All existing tests continue to pass (1798 grovedb / 631 grovedb-merk / 184 grovedb-query lib tests). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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 `@merk/src/proofs/query/aggregate_count_and_sum/tests.rs`:
- Around line 251-279: The test forged_kvdigest_count_changes_root_or_fails
mutates a KVDigestCountSum but never fails if no mutation occurred; add an
assertion that the tampering actually happened (e.g., assert!(tampered, "no
KVDigestCountSum found to tamper")) immediately after the loop and before
encoding/verifying the proof, so the test fails fast if the proof shape changed;
apply the same assert addition to the sibling sum-tampering test that mutates
the sum variant.
🪄 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: 31a072e2-4aee-4b66-bc1e-a7e8e47d3e5f
📒 Files selected for processing (5)
grovedb-query/src/aggregate_count.rsgrovedb-query/src/query.rsgrovedb-query/src/query_item/mod.rsgrovedb/src/tests/provable_count_provable_sum_tree_tests.rsmerk/src/proofs/query/aggregate_count_and_sum/tests.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- grovedb-query/src/aggregate_count.rs
- grovedb-query/src/query_item/mod.rs
Add 7 targeted tests that fill in patch-coverage gaps across the ProvableCountProvableSumTree (PCPS) feature surface. Total: 91.17% patch coverage (up from 89.88%), comfortably above the 90% target. Per-file impact: grovedb/src/operations/proof/generate.rs (+12 lines covered) - dense_tree_rejects_aggregate_count_and_sum_on_range - mmr_tree_rejects_aggregate_count_and_sum_on_range - bulk_append_tree_rejects_aggregate_count_and_sum_on_range Mirror the existing ACOR / ASOR rejection tests for the dual-axis AggregateCountAndSumOnRange variant. Each pins one of the three index-resolution helpers' new ACAS arms (query_items_to_positions / query_items_to_leaf_indices / query_items_to_range). grovedb/src/operations/proof/mod.rs (+17 lines covered, now 100%) - combined_aggregate_proof_display_includes_pcps_node_variants - regular_prove_on_pcps_formats_kv_count_sum_nodes - pcps_reference_proof_display_includes_kv_ref_value_hash_count_sum Drive the Display arms for the dual-axis Node variants (KVCountSum, KVHashCountSum, KVDigestCountSum, HashWithCountAndSum, KVRefValueHashCountSum) in node_to_string. Mirror of the sum_proof_display_includes_sum_node_variants pattern from aggregate_sum_query_tests.rs. grovedb-element/tests/element_constructors_helpers.rs - flag_accessors_handle_provable_count_provable_sum_tree Cover the PCPS arms in get_flags / get_flags_owned / get_flags_mut / set_flags. Mirror of flag_accessors_handle_reference_with_sum_item. grovedb-element/tests/element_display_and_serialization.rs - Extend serialize_deserialize_round_trip_all_element_types_and_errors Add ProvableSumTree (disc 19) and ProvableCountProvableSumTree (disc 20) to the round-trip loop. Add end-of-test cases for the three wrapper-twin discriminants (NonCounted=148, NotSummed=178, NotCountedOrSummed=194) of PCPS plus negative cases for invalid wrapper-inner pairings. Also fix a pre-existing typo comment caught by pre-commit typos check. The ASOR / ACAS carrier-descent arms in prove_subqueries_v1 (lines 1986-2045) remain uncovered — they're guarded by is_aggregate_sum_query / is_aggregate_count_and_sum_query but the top-level validate_aggregate_sum_on_range / validate_aggregate_count_and_sum_on_range rejects any limit-bearing or non-leaf-shaped query before the recursive prover sees it, so those arms are effectively defense-in-depth / unreachable from the public prove_query API. All 1804 grovedb lib tests + 127 grovedb-element tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CodeRabbit nitpick: the two `forged_kvdigest_*_changes_root_or_fails` tests guarded the verify step behind `if tampered` and silently passed if the fixture stopped producing `KVDigestCountSum` ops. A future proof-shape change could drop this coverage without failing CI. Mirror the sibling `HashWithCountAndSum` test (lines 232-235): assert the mutation actually happened before encoding, and add the same descriptive `assert_ne!` message to the verify branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…m out of query.rs
Mirror the existing `aggregate_count.rs` layout — each aggregate
variant's Query helpers and validation live in their own sibling
module, keeping the `Query` core in `query.rs` focused on
general-purpose query plumbing.
**Moved out of `query.rs`:**
- `grovedb-query/src/aggregate_sum.rs` (new):
`new_aggregate_sum_on_range`, `aggregate_sum_on_range`,
`has_aggregate_sum_on_range_anywhere`, `validate_aggregate_sum_on_range`
plus their tests (selector-walking + cross-aggregate orthogonality
arm for the ACASOR inner rejection).
- `grovedb-query/src/aggregate_count_and_sum.rs` (new):
`new_aggregate_count_and_sum_on_range`,
`aggregate_count_and_sum_on_range`,
`has_aggregate_count_and_sum_on_range_anywhere`,
`validate_aggregate_count_and_sum_on_range` plus their tests
(happy path, all rejection arms, subquery walking, dispatch error).
`query.rs` shrinks substantially as a result. Module declarations
registered in `grovedb-query/src/lib.rs` next to the existing
`mod aggregate_count`. The methods are still on `Query` via
`impl Query { ... }` blocks so no caller change is needed.
`cargo test -p grovedb-query --lib` — all 184 tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ntAndSumOnRange Mirrors the count-side carrier shape (PR #670 precedent) onto the sum and PCPS-combined dual-axis aggregate variants. Drive's compound "sum X per outer-key Y" queries can now resolve through a single proof, matching the existing count-side primitive. Query layer (grovedb-query): - Split `Query::validate_aggregate_sum_on_range` into `_leaf_` + `_carrier_` validators with auto-dispatch (mirrors `aggregate_count.rs`). - Same split for `validate_aggregate_count_and_sum_on_range`. - Comprehensive validator tests on both axes covering happy paths, RangeFull rejection, nested carrier rejection, conditional-branch rejection, empty subquery_path key rejection, every Range* outer variant, and the direct-validator-only branches that the dispatcher masks. SizedQuery / PathQuery layer (grovedb/src/query/mod.rs): - Per-shape size-constraint checks: leaf rejects both limit and offset; carrier accepts limit, still rejects offset. - Auto-dispatch in `SizedQuery::validate_aggregate_{sum,count_and_sum}_on_range`. - Strict-leaf entry points `validate_leaf_aggregate_{sum,count_and_sum}_on_range` for callers that produce a single i64 / (u64, i64) and must reject carrier. - SizedQuery-level leaf+carrier limit/offset regression tests. Verifier layer (grovedb/src/operations/proof): - aggregate_sum/{classification,per_key}.rs + matching OuterMatch + execute_carrier_layer_proof helpers. - aggregate_count_and_sum/{classification,per_key}.rs likewise. - New entry points `verify_aggregate_sum_query_per_key` (returns Vec<(Vec<u8>, i64)>) and `verify_aggregate_count_and_sum_query_per_key` (returns Vec<(Vec<u8>, u64, i64)>). - Existing `verify_aggregate_sum_query` / `verify_aggregate_count_and_sum_query` switched to strict-leaf validation so they keep returning (root, i64) / (root, u64, i64) for leaf queries and a clear InvalidQuery for carrier queries. - Dual-axis invariant: combined per_key rejects every non-PCPS terminal merk via the existing `enforce_lower_chain` terminal-type gate. Integration tests: - grovedb/src/tests/aggregate_sum_carrier_query_tests.rs (10 tests) - grovedb/src/tests/aggregate_count_and_sum_carrier_query_tests.rs (10 tests) V0 proofs untouched. Existing leaf-shape entry points are byte-compatible — same proof bytes, same return shapes; carrier shape is purely additive through the new `_per_key` entry points. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two bugs introduced by the carrier-aggregate extension (e69df59) that escaped review: **Fix #1 (P2): `query_aggregate_sum` accepts carrier shape** `GroveDb::query_aggregate_sum` returns a single `i64` but called the broadened `PathQuery::validate_aggregate_sum_on_range()` (the auto- dispatcher), which accepts both leaf and carrier shapes after the carrier work landed. For a carrier-shape query `path_query.path` points to the outer fan-out tree, not the leaf sum tree — the function would then open the WRONG tree and call `sum_aggregate_on_range` against it, producing `CorruptedData` instead of a clear `InvalidQuery`. Switched to `validate_leaf_aggregate_sum_on_range()` (strict-leaf), matching the count side at `query_aggregate_count` (which already had the same pattern with a clear leaf-only doc comment). Added `no_proof_sum_rejects_carrier_shape` regression test mirroring count's `no_proof_rejects_carrier_shape`. **Fix #2 (P2): empty-path rejection blocks root-carrier queries** `PathQuery::validate_aggregate_{count,sum,count_and_sum}_on_range` rejected `path.is_empty()` unconditionally. That's correct for leaf shapes (the GroveDB root is always a NormalTree, never a count/sum/PCPS tree), but wrong for carriers — a carrier query may legitimately fan out across the root's top-level keys and descend via `subquery_path` to a leaf merk at lower depth. The per-key verifier was already structurally ready to execute the carrier layer at depth 0; the upstream PathQuery validator was the only blocker. Made the empty-path check shape-aware: only rejects when the query itself owns an aggregate item at the top level (leaf shape). Added explicit empty-path checks to all three strict-leaf validators (`validate_leaf_aggregate_{count,sum,count_and_sum}_on_range`) so the leaf consumers (`verify_*_query`, `query_aggregate_*`) still reject root queries with the same clear message. Reworded the rejection messages to clarify "leaf queries may not target the root merk... Carrier queries may target the root merk; use verify_*_query_per_key" — guides callers to the right entry point. Tests: - `root_carrier_{count,sum,combined}_with_empty_path_succeeds` — round trip a real carrier proof rooted at the GroveDB root, verifying both count, sum, and combined axes. - `root_leaf_{count,sum,combined}_with_empty_path_still_rejected` — confirm the leaf-shape rejection still fires ("leaf" in the error message + verifier surface still errors). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…te_common The three aggregate-axis verifier modules each carried byte-identical copies of four helpers (modulo a single axis-label substring in the diagnostic strings). This was 4 x 3 = 12 copies of essentially the same logic, and `expect_merk_bytes` was even called out by a reviewer as obvious duplication. Centralized into a new `grovedb/src/operations/proof/aggregate_common.rs`: - `OuterMatch` — pure type, identical across axes. - `verify_single_key_layer_proof_v0` — identical across axes. - `expect_merk_bytes` — takes an `axis_label: &'static str` so the per-axis prefix in the rejection message is preserved. - `execute_carrier_layer_proof` — same axis-label parameterization. Each axis's `helpers.rs` now: - Re-exports `OuterMatch` + `verify_single_key_layer_proof_v0` so existing callers in `leaf_chain.rs` / `per_key.rs` keep their `use super::helpers::*` imports working with zero changes. - Defines a one-line `const AXIS_LABEL: &str` and thin wrappers around the shared `expect_merk_bytes` / `execute_carrier_layer_proof` that pass the label. Caller signatures and error strings are byte-for-byte preserved. - Keeps the genuinely axis-specific helpers (`verify_*_leaf` with different return types per axis, `enforce_lower_chain` with different terminal-type acceptance sets). Net diff: -158 lines, but the real win is structural — future changes to the shared logic propagate automatically rather than requiring three parallel updates. Behavior preserved: all 272 existing aggregate/query tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…lope Second pass of aggregate-proof dedup after the helpers.rs round (f376510). Two more cross-axis duplicates moved into `aggregate_common`: **AggregateClassification struct + classify_aggregate_path_query** Each axis had its own struct (`AggregateCountClassification`, `AggregateSumClassification`, `AggregateCountAndSumClassification`) with identical 4-field layout, and an identical `classify_*` function body differing only in the validator method called and the "top-level owns aggregate-X item" predicate. Now: - Shared `AggregateClassification` struct in `aggregate_common`. - Shared `classify_aggregate_path_query` generic over closures supplying the axis-specific `validate` and `is_leaf` callbacks. - Each axis's `classification.rs` shrinks to ~36 lines: a type alias (`pub type Aggregate*Classification = ...AggregateClassification`) so existing callers in `per_key.rs` need no changes, plus a thin classify wrapper that supplies the two closures. **require_v1_envelope** Each axis had a near-identical V0-envelope rejection (only the type name and axis label differed). Now: - Shared `require_v1_envelope` in `aggregate_common` taking `query_type_name: &'static str`. - Each axis's `mod.rs` keeps a thin wrapper supplying just the type name (e.g. `"AggregateCountOnRange"`). - Removed redundant axis-label from the error string ("such a proof" instead of "an aggregate-count proof") — tests only check for "require V1 proof envelopes" substring; no callers depended on the trailing axis label. Caller surface preserved end-to-end: the type aliases keep the axis-specific names (`AggregateCountClassification` etc.) working, so per_key.rs / mod.rs imports need no changes. Net diff: -25 lines (133 added to aggregate_common, ~158 removed from the three axes). Bigger structural win: future tweaks to the classification descriptor or envelope check propagate automatically. Behavior preserved: all 272 aggregate + SizedQuery tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Removed references that read as stale dev-process noise to a reader encountering the code months later, with no information loss about the code's current behavior or rationale: **PR # references (8 sites)** - generate.rs (combined-aggregate gate): "PR #670 / grove v3+ feature" → "grove v3+ feature". - count_offset_paginated_tests.rs (assert message + 2 docs): "PR #672 closes the P1 finding" / "the P1 finding's root cause" → describe the actual rule (NonCounted inserts into ProvableCountTree rejected). - provable_count_provable_sum_tree_tests.rs (4 docs): "PR #672 for ProvableCountTree…" → "Same rejection rule that applies to…". - aggregate_count_query_tests.rs (forgery test branches): drop "added in PR #663" / "added in this PR" qualifiers from documentation of two error-branches. - not_counted_or_summed_tests.rs: drop "PR #666's contract" qualifier. - reference_with_sum_item_tests.rs (2 sites): drop "added in PR #667" / "added in this PR" / "PR #667 already covers" — rephrase as factual statements. - aggregate_sum_query_tests.rs (test-section header): drop "PR #662's no-proof query_aggregate_count" → "Sum-side mirror of the no-proof query_aggregate_count tests." - query.rs (query_aggregate_sum doc): drop "Mirrors PR #662's". - non_counted_tests.rs (module header): drop "Codex review of PR #654". **CodeRabbit references (3 sites)** - count_offset_paginated_tests.rs (2 sites): drop "(CodeRabbit review on grovedb#669)" parenthetical. - merk/mod.rs (test doc): drop "Tightened (per CodeRabbit review)". - merk/proofs/query/aggregate_count/tests.rs (comment): drop "(per CodeRabbit review)". **Temporal markers — "Before this fix" / "in this PR" framing (4 sites)** - merk/mod.rs (2 docs): "Before this fix the supports_count match…" → "the support check delegates to is_count_bearing(), so any hand-rolled match here would be a drift-risk regression." / "previously omitted from this manual match" → drop the temporal qualifier. - aggregate_sum_carrier_query_tests.rs (test doc): "previously blanket — it blocked legitimate root-carrier queries" → describe current shape-aware behavior. - provable_count_provable_sum_tree_tests.rs (2 sites): "fixed in this PR — without KVRefValueHashCountSum" → "rely on the KVRefValueHashCountSum dispatch arm — without it". "both gained Element::ProvableCountProvableSumTree arms in this PR" → "both carry Element::ProvableCountProvableSumTree arms". **"before this feature" comments (2 sites)** - aggregate_count_query_tests.rs + aggregate_sum_carrier_query_tests.rs (per-key symmetry tests): "same proof bytes it did before this feature" → "same proof bytes whether the caller verifies via verify_aggregate_X_query or the per-key entry point." **"Before this module existed" / "forthcoming" framing (2 sites)** - aggregate_common.rs: "Before this module existed each axis carried its own private copy" → "These items would otherwise be byte-identical copies across each axis. Centralizing them here…" - aggregate_count/mod.rs: "The same leaf/carrier shape will apply to forthcoming aggregate variants (sum, average)" → "The same leaf/carrier shape applies to the sum and combined axes — see the sibling … modules." - grovedb-query/src/aggregate_count.rs: "Forthcoming aggregate variants (sum, average) will live in sibling modules" → "The sum and combined axes live in sibling modules." **Renamed:** `p1_noncounted_in_provable_count_tree_rejected_at_insert` → `noncounted_in_provable_count_tree_rejected_at_insert` to drop the "P1" audit-finding prefix from the test name itself. No behavior changes. All tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves conflicts with develop's PR #670 (Element::ProvableCountProvableSumTree + dual-axis crossover proofs). **Fourth on-disk discriminant collision shifted.** Develop assigns `ElementType::ProvableCountProvableSumTree = 20`; this PR's prior post-merge `ElementType::CountIndexedTree = 20`. Resolution shifts cidx discriminants by one (the same pattern as the previous three develop-side enum landings). | Symbol | Before | After | |----------------------------------------------|--------|-------| | `ElementType::ProvableCountProvableSumTree` | — | **20** (from develop) | | `ElementType::CountIndexedTree` | 20 | **21** | | `ElementType::ProvableCountIndexedTree` | 21 | **22** | | `ElementType::NonCountedCountIndexedTree` | 148 | **149** (= `0x80 \| 21`) | | `ElementType::NonCountedProvableCountIndexedTree` | 149 | **150** (= `0x80 \| 22`) | | `TreeType::ProvableCountProvableSumTree` | — | **12** (from develop) | | `TreeType::CountIndexedTree` | 12 | **13** | | `TreeType::ProvableCountIndexedTree` | 13 | **14** | `Element` enum variant order: `…ProvableSumTree, ProvableCountProvableSumTree, CountIndexedTree, ProvableCountIndexedTree`. ElementShadow serde-decoder mirrors. NonCounted inner-byte allowlist becomes `0..=14 | 18 | 19 | 20 | 21 | 22`. NonCounted twin range expands to `[128, 150]`. Base-variant canonical round-trip test grew 19→20 cases. Other resolutions union match arms in the same files as the prior merge rounds: - `merk/tree_type/mod.rs` (10 conflicts): TreeType variant + 7 dispatch methods + 2 tests. PCPS slotted into `is_sum_bearing` and `is_count_and_sum_bearing` (it's count-and-sum) and into the implication-loop test; the wrapper-acceptance predicates correctly reject PCPS (provable → no NonCounted, single Provable*CountSum* doesn't carry both axes for NotCountedOrSummed). - `merk/tree_type/costs.rs` (1 conflict): PCPS reuses `COUNT_SUM_TREE_COST_SIZE`. - `merk/element/costs.rs` (2 conflicts): get_specialized_cost + value_defined_cost both gain PCPS. - `merk/element/tree_type.rs` (1 conflict): get_feature_type's PCPS arm uses `ProvableCountedAndProvableSummedMerkNode`. - `merk/tree/mod.rs` (1 conflict): re-export now includes `node_hash_with_count_and_sum`. - `grovedb-element/element/helpers.rs` (2 conflicts): count_value_or_default + is_non_empty_merk_tree handle PCPS. - `grovedb-element/element/mod.rs` (3 conflicts): Element enum, ElementShadow, From<ElementShadow> dispatch. - `grovedb-element/element_type.rs` (13 conflicts): byte assignments + all dispatch + range tests + test fixtures. - `grovedb/operations/get/query.rs` (2 conflicts): both QueryItemOrSumReturnType dispatches gain a PCPS → CountSumValue arm. - `grovedb/operations/proof/generate.rs` (1 conflict): empty-Element match arm list. Post-merge non-exhaustive-match fix: - `grovedb/operations/count_indexed_tree.rs`: subtree-insert arm gains `ProvableCountProvableSumTree(..)`. Verified: - `cargo test --workspace --lib`: 3500+ tests, 0 failures. - `cargo fmt --all` applied. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ers (#673) * feat(element): add required_{item,reference}_with_sum_item_space helpers Adds two worst-case storage-sizing helpers paralleling Element::required_item_space for the sum-bearing variants introduced in #670 / #667: - Element::required_item_with_sum_item_space - Element::required_reference_with_sum_item_space Both reserve 10 bytes for the i64 sum_value as an upper bound (bincode 2.x maxes at 9 bytes for a zigzag-encoded u64; 10 is a safety margin) so dry-run / stateless-cost callers in dash-platform never undercharge on summable-index writes. Wires required_item_with_sum_item_space and required_reference_with_sum_item_space FeatureVersion fields through GroveDBElementMethodVersions (initialized to 0 in v1, v2, v3). Tests cover the manual-formula contract and exhaustively sweep boundary sum values (0, +-1, +-250, i64::MAX, i64::MIN), payload sizes, max_hop, and flag variants to assert helper >= serialize().len() for every combination. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(element): cover version-mismatch path for new sum_item helpers Adds required_with_sum_item_space_helpers_reject_unknown_version to exercise the check_grovedb_v0! mismatch arm on both new helpers, lifting diff coverage above the 90% codecov gate (the macro's error branch was the only uncovered region in the previous diff). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Adds
Element::ProvableCountProvableSumTree— the dual-axis cousin ofProvableCountTree(count hash-bound) andProvableSumTree(sum hash-bound). Both aggregates land in every node's hash vianode_hash_with_count_and_sum, so a single tree supports BOTHAggregateCountOnRangeANDAggregateSumOnRangeproofs against the same root hash.PR #661 introduced
ProvableSumTreeand theAggregateSumOnRangeproof; this PR is its natural completion — combining count + sum hash-binding in a single tree variant.What's new
Type machinery
ElementProvableCountProvableSumTree(Option<Vec<u8>>, CountValue, SumValue, Option<ElementFlags>)ElementTypeProvableCountProvableSumTreebaseElementTypeNonCountedProvableCountProvableSumTree0x80 | 20)ElementTypeNotSummedProvableCountProvableSumTree0xB2, hand-assigned)ElementTypeNotCountedOrSummedProvableCountProvableSumTree0xC2, hand-assigned)TreeTypeProvableCountProvableSumTreeAggregateDataProvableCountAndProvableSum(u64, i64)TreeFeatureTypeProvableCountedAndProvableSummedMerkNode(u64, i64)NodeTypeProvableCountProvableSumNodeHash function
merk::tree::node_hash_with_count_and_sum(kv, l, r, count, sum)—Blake3(kv \|\| l \|\| r \|\| count_be8 \|\| sum_be8). Fixed 8-byte big-endian encoding for each aggregate (not varint) so the hash is independent of the prover's choice of size.Tree::hash_for_linkarm dispatches to the new function with the fail-closed gate (panics onfeature_type↔tree_typemismatch, never silently falls through to plainnode_hash).New proof Node variants (grovedb-query)
Five new
Nodevariants for proof emission againstProvableCountProvableSumTree:KVCountSum(key, value, count, sum)KVHashCountSum(kv_hash, count, sum)KVRefValueHashCountSum(key, ref_value, ref_hash, count, sum)KVDigestCountSum(key, value_hash, count, sum)HashWithCountAndSum(kv_hash, left, right, count, sum)All carry both aggregates so the verifier can reconstruct
node_hash_with_count_and_sum. Encoded with tag bytes0x40..=0x4D(Push + PushInverted pairs).Dual-axis emitter dispatch (the headline)
The
aggregate_count/emit.rsandaggregate_sum/emit.rsemitters now dispatch on the host tree'sTreeType:ProvableCountProvableSumTree: emit dual-axis variants (HashWithCountAndSum,KVDigestCountSum) — both aggregates included so the verifier can reconstruct the right hash function.HashWithCount/KVDigestCountresp.HashWithSum/KVDigestSum).Both verifiers accept the dual-axis variants at every classification position (Disjoint, Contained, Boundary) and extract the right aggregate from each.
Wrapper compatibility
All three wrappers (
NonCounted,NotSummed,NotCountedOrSummed) acceptProvableCountProvableSumTreeas their inner element and behave per their existing contracts.GroveDB integration
grovedbg-typesdebug enums extendedHeadline test
Test summary
cargo test --workspace --all-features: 0 failures across 41 test binaries.Tests added:
node_hash_with_count_and_sumunit tests inmerk/src/tree/hash.rs(determinism, distinctness from sibling hash flavors, sensitivity to each of 5 inputs, axis-swap, extremes)grovedb/src/tests/provable_count_provable_sum_tree_tests.rs:ProvableCountSumTreeANDProvableSumTreeover identical contentNonCounted(PCPS)suppresses count contributionNotSummed(PCPS)suppresses sum but propagates countNotCountedOrSummed(PCPS)suppresses both axesFiles changed
Foundation:
grovedb-element/,grovedb-query/proofs/,merk/,grovedbg-types/Integration:
grovedb/src/{batch,operations/insert,operations/get,operations/proof,lib,debugger}.rsTests:
merk/src/tree/hash.rs,grovedb/src/tests/provable_count_provable_sum_tree_tests.rsReference
ProvableSumTree)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests
Bug Fixes / Compatibility