Skip to content

Conversation

@QuantumExplorer
Copy link
Member

@QuantumExplorer QuantumExplorer commented Dec 29, 2025

Issue being fixed or feature implemented

Add tests to verify the functionality of absence proofs in the ProvableCountSumTree.

What was done?

  • Implemented multiple test cases to validate the behavior of absence proofs when querying existing and non-existent keys in a ProvableCountSumTree.
  • Enhanced the query verification process to include checks for keys that may or may not exist.

How Has This Been Tested?

New tests have been added to ensure that absence proofs correctly handle scenarios with a mix of existing and non-existent keys. All tests pass successfully.

Breaking Changes

None

Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

Summary by CodeRabbit

Release Notes

  • New Features

    • Exposed maximum tree depth calculation utility when minimal or verify features are enabled.
  • Tests

    • Added comprehensive tests for ProvableCountSumTree absence proofs with mixed existing and non-existent keys in various positions.
  • Bug Fixes

    • Enhanced proof verification to properly handle additional node variants, reducing false negatives in tree verification scenarios.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 29, 2025

📝 Walkthrough

Walkthrough

The PR adds a public re-export of calculate_max_tree_depth_from_count from grovedb_merk under specific feature gates, introduces three new absence-proof tests for ProvableCountSumTree with mixed existing and non-existent keys, and extends proof verification logic to recognize and process three additional Node variants without raising false errors.

Changes

Cohort / File(s) Summary
Library Exports
grovedb/src/lib.rs
Added public re-export of calculate_max_tree_depth_from_count from grovedb_merk conditionally under cfg(any(feature = "minimal", feature = "verify")) feature gates.
Test Suite Additions
grovedb/src/tests/provable_count_sum_tree_tests.rs
Added three new absence-proof tests for ProvableCountSumTree covering mixed key scenarios (non-existent keys on right, left, and middle). Each test creates trees, inserts items, builds SizedQuery, proves, and verifies with root hash and presence/absence assertions. Imported SizedQuery for test usage.
Verification Logic Expansion
merk/src/proofs/query/verify.rs
Extended proof verification to recognize three new Node variants—KVValueHashFeatureType, KVRefValueHashCount, and KVCount—as valid conditions in bound-verification branches and main node-processing loop, treating them equivalently to existing counterparts to prevent false "Cannot verify" errors in abridged/ProvableCountTree contexts.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A feature gate, a test or three,
New node types now verified free,
Absence proofs with keys all mixed,
No false errors left unfixed,
GroveDB grows with trusted sight! 🌲✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'feat: add absence proof tests for ProvableCountSumTree' clearly and directly summarizes the main change: adding new test cases for absence proofs in ProvableCountSumTree, which is the primary focus of the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch featadd-absence-proof-tests

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4c5fbe1 and 9efcfbb.

📒 Files selected for processing (3)
  • grovedb/src/lib.rs
  • grovedb/src/tests/provable_count_sum_tree_tests.rs
  • merk/src/proofs/query/verify.rs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Format Rust code with cargo fmt
Run cargo clippy with -D warnings and fix all lints

Files:

  • grovedb/src/lib.rs
  • grovedb/src/tests/provable_count_sum_tree_tests.rs
  • merk/src/proofs/query/verify.rs
{grovedb,merk,storage,costs}/src/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

{grovedb,merk,storage,costs}/src/**/*.rs: Use cost_return_on_error! for early returns while accumulating cost
Wrap errors with context (e.g., .map_err(|e| Error::CorruptedData(format!("context: {}", e))))

Files:

  • grovedb/src/lib.rs
  • grovedb/src/tests/provable_count_sum_tree_tests.rs
  • merk/src/proofs/query/verify.rs
**/tests/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/tests/**/*.rs: Every state-modifying operation must be testable with proofs
Verify that cost calculations in tests match actual operations
Ensure reference integrity in tests (no cycles)

Files:

  • grovedb/src/tests/provable_count_sum_tree_tests.rs
{grovedb/src/operations/proof,merk/src/proofs}/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Never trust unverified proofs; always verify before use

Files:

  • merk/src/proofs/query/verify.rs
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to {grovedb/src/operations/proof,merk/src/proofs}/**/*.rs : Never trust unverified proofs; always verify before use
📚 Learning: 2025-09-06T12:46:28.692Z
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to merk/src/tree/**/*.rs : Maintain AVL balance factor in {-1, 0, 1} using single/double rotations

Applied to files:

  • grovedb/src/lib.rs
📚 Learning: 2025-09-06T12:46:28.692Z
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to merk/src/tree/walk/**/*.rs : Use lazy loading to avoid loading unnecessary data

Applied to files:

  • grovedb/src/lib.rs
📚 Learning: 2025-09-06T12:46:28.692Z
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to {grovedb,merk,storage,costs}/src/**/*.rs : Use cost_return_on_error! for early returns while accumulating cost

Applied to files:

  • grovedb/src/lib.rs
📚 Learning: 2025-12-17T18:39:20.910Z
Learnt from: QuantumExplorer
Repo: dashpay/grovedb PR: 397
File: grovedb/src/tests/provable_count_sum_tree_tests.rs:82-90
Timestamp: 2025-12-17T18:39:20.910Z
Learning: In grovedb_merk test code, the `execute()` function returns a cost-wrapped result (e.g., tuple or CostResult). The pattern `.unwrap().map_err(|e| e)` is correct when `.unwrap()` extracts the inner Result from the cost wrapper, and `.map_err()` then converts the error type. This is commonly used in test helpers to discard cost tracking.

Applied to files:

  • grovedb/src/lib.rs
📚 Learning: 2025-09-06T12:46:28.692Z
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to {grovedb,merk,storage,costs}/src/**/*.rs : Wrap errors with context (e.g., .map_err(|e| Error::CorruptedData(format!("context: {}", e))))

Applied to files:

  • grovedb/src/lib.rs
📚 Learning: 2025-09-06T12:46:28.692Z
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to **/tests/**/*.rs : Verify that cost calculations in tests match actual operations

Applied to files:

  • grovedb/src/lib.rs
  • grovedb/src/tests/provable_count_sum_tree_tests.rs
📚 Learning: 2025-04-18T06:12:25.554Z
Learnt from: pauldelucia
Repo: dashpay/grovedb PR: 366
File: grovedb/src/error.rs:7-7
Timestamp: 2025-04-18T06:12:25.554Z
Learning: The `Error::InvalidProofError` in grovedb/src/operations/proof/util.rs is from the merk error handling system, distinct from grovedb's own `Error::InvalidProof` variant. These are separate error types from different crates.

Applied to files:

  • grovedb/src/lib.rs
  • grovedb/src/tests/provable_count_sum_tree_tests.rs
  • merk/src/proofs/query/verify.rs
📚 Learning: 2025-09-06T12:46:28.692Z
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to **/tests/**/*.rs : Every state-modifying operation must be testable with proofs

Applied to files:

  • grovedb/src/tests/provable_count_sum_tree_tests.rs
📚 Learning: 2025-09-06T12:46:28.692Z
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to **/tests/**/*.rs : Ensure reference integrity in tests (no cycles)

Applied to files:

  • grovedb/src/tests/provable_count_sum_tree_tests.rs
📚 Learning: 2025-09-06T12:46:28.692Z
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to {grovedb/src/operations/proof,merk/src/proofs}/**/*.rs : Never trust unverified proofs; always verify before use

Applied to files:

  • grovedb/src/tests/provable_count_sum_tree_tests.rs
  • merk/src/proofs/query/verify.rs
📚 Learning: 2025-09-06T12:46:28.692Z
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to grovedb/src/batch/**/*.rs : Prefer batch operations for multiple related changes

Applied to files:

  • grovedb/src/tests/provable_count_sum_tree_tests.rs
📚 Learning: 2025-09-06T12:46:28.692Z
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to {storage/src/**/*.rs,grovedb/src/batch/**/*.rs} : Use transactions for multi-step operations to ensure safety

Applied to files:

  • grovedb/src/tests/provable_count_sum_tree_tests.rs
📚 Learning: 2025-12-17T18:39:16.131Z
Learnt from: QuantumExplorer
Repo: dashpay/grovedb PR: 397
File: grovedb/src/tests/provable_count_sum_tree_tests.rs:82-90
Timestamp: 2025-12-17T18:39:16.131Z
Learning: In Rust test code for grovedb, when a function returns a cost-wrapped result (CostResult), using .unwrap().map_err(|e| e) is acceptable to discard cost tracking by unwrapping the inner Result and converting the error type for test helpers. This pattern should be applied across test files that interact with cost-wrapped results, not just a single test, to ensure consistency in how test helpers handle costs.

Applied to files:

  • grovedb/src/tests/provable_count_sum_tree_tests.rs
🧬 Code graph analysis (1)
grovedb/src/lib.rs (1)
merk/src/proofs/branch/depth.rs (1)
  • calculate_max_tree_depth_from_count (38-71)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Linting
  • GitHub Check: Compilation errors
  • GitHub Check: Tests
  • GitHub Check: Code Coverage
🔇 Additional comments (8)
grovedb/src/lib.rs (1)

175-176: LGTM! Clean feature-gated re-export.

The re-export of calculate_max_tree_depth_from_count from grovedb_merk is appropriately feature-gated and aligns with the PR's goal of supporting absence-proof verification that may need to calculate tree depth constraints.

grovedb/src/tests/provable_count_sum_tree_tests.rs (4)

24-24: LGTM! Necessary import for absence proof tests.

The SizedQuery import is required for the new absence-proof tests below and is correctly sourced from the query module.


1748-1749: LGTM! Updated section header reflects expanded test coverage.

The plural "ABSENCE PROOF TESTS" header appropriately describes the multiple absence-proof scenarios now covered.


1750-1875: LGTM! Thorough absence-proof test for non-existent key on the right.

This test properly exercises absence-proof verification when querying a mix of existing and non-existent keys where the non-existent key is lexicographically greater (on the right). The test correctly:

  • Sets up a ProvableCountSumTree with known items
  • Uses SizedQuery (required for absence proof verification)
  • Calls verify_query_with_absence_proof (the appropriate verification method)
  • Asserts root hash consistency and validates presence/absence signals

1877-2129: LGTM! Comprehensive absence-proof test coverage for left and middle positions.

These two tests complete the absence-proof coverage by testing scenarios where the non-existent key is:

  1. Lexicographically smaller (on the left) using aa_unknown_address_xxxxxxxxx
  2. Between two existing keys (in the middle) using address_1, address_2 (non-existent), and address_3

Together with the "on right" test above, these provide thorough validation of absence-proof behavior across all tree positions. The consistent test structure and verification approach ensures reliability.

merk/src/proofs/query/verify.rs (3)

166-168: LGTM! Properly extends lower bound verification to ProvableCount node variants.

The three new node variants (KVValueHashFeatureType, KVRefValueHashCount, KVCount) are correctly treated as valid proof nodes for lower bound verification, consistent with their non-feature-type counterparts. This prevents false "Cannot verify lower bound" errors when processing ProvableCountTree/ProvableCountSumTree proofs.


198-200: LGTM! Symmetric upper bound verification for ProvableCount node variants.

The same three node variants are correctly added to the upper bound verification logic (right-to-left direction), maintaining symmetry with the lower bound additions and preventing false "Cannot verify upper bound" errors.


335-360: LGTM! Proper execution handling for ProvableCount node variants.

The three new node variants are correctly integrated into the execution flow:

  1. KVCount (lines 335-341): Handles nodes with count metadata, computing value hash locally
  2. KVValueHashFeatureType (lines 342-351): Handles nodes with feature type metadata and pre-computed value hash
  3. KVRefValueHashCount (lines 352-360): Handles reference nodes with count metadata

All three correctly call execute_node to add matching key-value pairs to the output. The count and feature_type parameters are appropriately ignored here, as they're used by the tree hash verification logic in the execute function (tree.rs). The comments clearly explain that hash verification security comes from the final merkle root check, not from local verification of value hashes.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@QuantumExplorer
Copy link
Member Author

Self Reviewed

@QuantumExplorer QuantumExplorer merged commit 83a8fe5 into develop Dec 29, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants