Skip to content

feat: paginated position-range reads with proofs at the BulkAppendTree layer - #786

Open
QuantumExplorer wants to merge 8 commits into
developfrom
feat/bulk-append-range-reads
Open

feat: paginated position-range reads with proofs at the BulkAppendTree layer#786
QuantumExplorer wants to merge 8 commits into
developfrom
feat/bulk-append-range-reads

Conversation

@QuantumExplorer

@QuantumExplorer QuantumExplorer commented Aug 3, 2026

Copy link
Copy Markdown
Member

Implements work-plan item 4 of #784: paginated position-range reads with proofs at the BulkAppendTree layer — the scanning hot path where clients of append-only stores walk "all entries since my cursor" in pages.

What's added

Crate layer (grovedb-bulk-append-tree) — the shared seam

  • BulkAppendTree::get_range(start, limit)RangePage { entries, total_count }. Chunk-aligned: each completed chunk overlapping the page is read and deserialized exactly once, so a page costs O(chunks touched) blob reads plus one read per buffer entry — not O(entries) random reads.
  • position_range_query(start, limit): the canonical query shape (8-byte big-endian position keys), derived identically by prover and verifier from (start, limit).
  • BulkAppendTreeProof::generate_for_range / verify_range: paginated proof round-trip over the existing chunk-MMR + dense-buffer proof, completeness enforced.
  • CommitmentTree::get_range pass-through for the shielded-pool scanning path.

GroveDB layer

  • Ops: GroveDb::bulk_get_range and GroveDb::commitment_tree_get_range.
  • PathQuery::new_bulk_position_range(path, key, start, limit): canonical page query (element key + position-range subquery).
  • GroveDb::prove_bulk_position_range: proves one page via the existing V1 proof layering (ProofBytes::BulkAppendTree / ProofBytes::CommitmentTree).
  • GroveDb::verify_bulk_position_range_proof: verifies the page entries (ascending, contiguous, complete) and extracts the authenticated total_count from the same proof bytes by subset-verifying the element itself. Absence beyond the end falls out of the provable count — position >= total_count does not exist — so ranges past the end need no per-position absence proofs; a page shorter than limit means the scan caught up with the tip.

The machinery lives at the shared BulkAppendTree layer and dispatches on element type, so the upcoming PrivateDocumentStore (#784) and DataCommitmentTree (#783) inherit it by adding their element variants to the prove/verify match arms.

Fix uncovered along the way

A V1 proof descending into an empty BulkAppendTree could never verify: insert commits the element with a NULL_HASH child hash (mirrored by verify_grovedb's integrity walk), but verify_bulk_append_lower_layer returned the domain-tagged blake3("bulk_state" || 0*32 || 0*32), so the combine_hash chain check always failed. The verifier now returns NULL_HASH for a zero-count BulkAppendTree element (sound: verify_and_compute_root already rejects any proof carrying data for a zero-count tree). CommitmentTree is untouched — EMPTY_COMMITMENT_TREE_STATE_ROOT deliberately folds in the computed empty bulk root.

Versioning / compatibility

  • No serialized proof-format change — the proofs reuse the existing V1 layering, so no new GROVE_V* flag is introduced.
  • No write-path changes: no state root moves by a byte. The full pre-existing CommitmentTree suites pass unchanged (the canary).
  • Under GROVE_V1/V2 the V0 prover already rejects descending into these (unreleased) tree types; nothing widens there.

Tests & benches

  • Crate-level: get_range and proof round-trips across chunk boundaries, empty ranges (limit 0), ranges past the end, single-entry pages, large multi-chunk pages, empty trees, saturating-overflow starts, tampered-root and missing-chunk rejection, and a cursor-walk covering the whole tree.
  • GroveDB-level: the same matrix for both BulkAppendTree and CommitmentTree through prove_bulk_position_range / verify_bulk_position_range_proof, plus nested paths, wrong-element errors, and narrower-proof rejection — every round-trip asserts the proof root equals the live DB root.
  • New bench bulk_range_scan_benchmark: paged read / per-page prove / per-page verify at page sizes 16/256/1024, and a full proved cursor-walk (4096 entries, chunk size 64, 96-byte entries).

Validation

  • cargo nextest run --workspace --all-features: 4536 passed, 0 failed, 4 skipped
  • cargo clippy --workspace --all-features -- -D warnings: clean
  • cargo check --workspace --all-features --all-targets: clean (bench compiles)
  • cargo fmt --all --check: clean
  • Bench smoke-run in --test mode: all sections pass

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added paginated range reads for BulkAppendTree and CommitmentTree data.
    • Added APIs to retrieve page entries alongside the tree’s total count.
    • Added generation and verification of authenticated position-range proofs.
    • Added query support for bounded position ranges.
  • Bug Fixes

    • Improved handling of empty, out-of-range, incomplete, and invalid range proofs.
  • Tests

    • Added extensive coverage for pagination, chunk boundaries, buffering, empty trees, limits, and proof validation.

QuantumExplorer and others added 4 commits August 3, 2026 07:05
…f helpers

Add the position-range read seam at the shared BulkAppendTree layer so
every BulkAppendTree-backed element type (CommitmentTree today, the
planned PrivateDocumentStore / DataCommitmentTree) inherits it:

- BulkAppendTree::get_range(start, limit): fetch entries for
  [start, start + limit) clamped to total_count, returned as a RangePage
  (entries + total_count). Chunk-aligned: each completed chunk
  overlapping the range is read and deserialized exactly once, so a page
  costs O(chunks touched) blob reads instead of O(entries) random reads.
- position_range_query(start, limit): the canonical Query (8-byte
  big-endian position keys) shared by prover and verifier.
- BulkAppendTreeProof::generate_for_range / verify_range: paginated
  proof round-trip over the existing chunk-MMR + dense-buffer proof,
  with completeness enforced; absence past the end falls out of the
  authenticated total_count (position >= count), not per-position
  absence proofs.
- CommitmentTree::get_range pass-through for the shielded-pool scanning
  path.

Part of work-plan item 4 of #784.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…_HASH

A V1 proof descending into an empty BulkAppendTree could never verify:
insert commits the element with a NULL_HASH child hash (there is no
bulk state until the first append) and verify_grovedb's integrity walk
mirrors that, but verify_bulk_append_lower_layer returned the
domain-tagged empty state root blake3("bulk_state" || 0*32 || 0*32),
so the combine_hash chain check always failed.

Return NULL_HASH for a zero-count BulkAppendTree element instead,
matching what the writer commits. This is sound because
verify_and_compute_root has already rejected any proof carrying chunk
or buffer data for a zero-count tree. CommitmentTree keeps the computed
value: its insert commits EMPTY_COMMITMENT_TREE_STATE_ROOT, which folds
in the computed empty bulk root.

Verification-only change on an element type not reachable in any
shipped grove version's state; no wire format or state root changes,
so no GROVE_V* gate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nly trees

Wire the BulkAppendTree range-read seam through GroveDB, per work-plan
item 4 of #784 — clients of append-only stores walk
"all entries since my cursor" in pages:

- GroveDb::bulk_get_range / GroveDb::commitment_tree_get_range:
  chunk-aligned page fetch returning a RangePage (entries +
  total_count), O(chunks touched) instead of O(entries) random reads.
- PathQuery::new_bulk_position_range: the canonical query shape for a
  page — element key plus an 8-byte big-endian position-range subquery
  — derived identically by prover and verifier from (start, limit).
- GroveDb::prove_bulk_position_range: proves one page via the existing
  V1 proof layering (ProofBytes::BulkAppendTree / CommitmentTree); no
  wire-format change, so no new GROVE_V* gate.
- GroveDb::verify_bulk_position_range_proof: verifies the page entries
  (ascending, contiguous, complete) and extracts the authenticated
  total_count from the same proof bytes by subset-verifying the element
  itself. Absence beyond the end falls out of the provable count
  (position >= total_count) — no per-position absence proofs.

The seam lives at the shared BulkAppendTree layer and dispatches on the
element type, so the planned PrivateDocumentStore (#784) and
DataCommitmentTree (#783) pick it up by adding their element variants
to the prove/verify match arms.

Tests cover round-trips across chunk boundaries, empty ranges, ranges
past the end, single-entry pages, large multi-chunk pages, empty trees,
nested paths, wrong-element errors, narrower-proof rejection, and the
cursor-walk scan pattern for both BulkAppendTree and CommitmentTree.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Benchmark the append-only scanning hot path on a 4096-entry
BulkAppendTree (chunk size 64, 96-byte entries): single-page reads,
per-page proof generation and verification at page sizes 16/256/1024,
and a full proved cursor-walk of the tree at page size 256.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@QuantumExplorer, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 25 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d7264a77-cefe-427e-a5c8-55ff09056b5d

📥 Commits

Reviewing files that changed from the base of the PR and between 3f3efe2 and acb99bd.

📒 Files selected for processing (15)
  • grovedb-bulk-append-tree/src/test_utils.rs
  • grovedb-bulk-append-tree/src/tree/fetch.rs
  • grovedb-bulk-append-tree/src/tree/tests.rs
  • grovedb-commitment-tree/src/commitment_tree/mod.rs
  • grovedb-version/src/version/grovedb_versions.rs
  • grovedb-version/src/version/v1.rs
  • grovedb-version/src/version/v2.rs
  • grovedb-version/src/version/v3.rs
  • grovedb-version/src/version/v4.rs
  • grovedb/src/operations/bulk_append_tree.rs
  • grovedb/src/operations/commitment_tree.rs
  • grovedb/src/operations/proof/generate.rs
  • grovedb/src/operations/proof/verify.rs
  • grovedb/src/tests/bulk_append_tree_tests.rs
  • grovedb/src/tests/commitment_tree_tests.rs
📝 Walkthrough

Walkthrough

Added paginated range reads and range proofs for BulkAppendTree and CommitmentTree. GroveDB now exposes range-read, proof-generation, and proof-verification APIs, with tests and a benchmark for paged scans.

Changes

Position-range tree access

Layer / File(s) Summary
Range result and tree reads
grovedb-bulk-append-tree/src/tree/..., grovedb-commitment-tree/src/commitment_tree/mod.rs
RangePage represents ordered entries and total_count. Tree reads clamp ranges, handle chunks and buffer entries, and report corrupted storage data.
Range read validation
grovedb-bulk-append-tree/src/tree/tests.rs
Tests cover boundaries, empty pages, overflow inputs, and complete paginated scans.

Canonical range proofs

Layer / File(s) Summary
Query and proof helpers
grovedb-bulk-append-tree/src/proof/mod.rs, grovedb/src/query/mod.rs
Canonical position queries use big-endian keys and saturating bounds. Proof APIs generate and verify requested ranges.
Proof helper validation
grovedb-bulk-append-tree/src/proof/tests.rs
Tests cover chunk and buffer boundaries, empty trees, paginated scans, root rejection, missing chunks, and saturated bounds.

GroveDB integration

Layer / File(s) Summary
Range-read operations
grovedb/src/operations/bulk_append_tree.rs, grovedb/src/operations/commitment_tree.rs, grovedb/src/lib.rs, grovedb-commitment-tree/src/lib.rs
GroveDB exposes bounded range reads with element validation, transaction storage, cost propagation, and mapped errors.
Range proof operations
grovedb/src/operations/proof/..., grovedb/src/tests/...
GroveDB generates and verifies position-range proofs, validates authenticated counts and contiguous entries, and supports nested BulkAppendTree and CommitmentTree cases.
Range scan benchmark
grovedb/Cargo.toml, grovedb/benches/bulk_range_scan_benchmark.rs
Added benchmarks for paged reads, proof generation, proof verification, and full proof-backed scans.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related issues

Possibly related PRs

  • dashpay/grovedb#782 — Both modify GroveDB proof generation and verification for BulkAppendTree and CommitmentTree.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant GroveDb
  participant BulkAppendTree
  participant ProofVerifier
  Client->>GroveDb: request position range
  GroveDb->>BulkAppendTree: read or generate range proof
  BulkAppendTree-->>GroveDb: RangePage or serialized proof
  GroveDb->>ProofVerifier: verify range proof and total_count
  ProofVerifier-->>GroveDb: verified RangePage
  GroveDb-->>Client: range entries and total_count
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: paginated position-range reads with proofs for BulkAppendTree.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/bulk-append-range-reads

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (4)
grovedb-bulk-append-tree/src/tree/fetch.rs (1)

96-115: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Hoist MMR construction out of the chunk loop.

get_chunk_value rebuilds MmrStore and MMR::new_with_overlay (which clones mmr_overlay) on every call. For a page that touches N chunks, this loop pays N MMR reconstructions and N overlay clones instead of one. BulkAppendTreeProof::generate in grovedb-bulk-append-tree/src/proof/mod.rs already avoids this: it builds the MMR once and reuses it across all queried chunk indices through a get_node closure.

Build the MMR once before the loop and read each chunk's leaf from that single instance, so a wide page (as exercised by test_range_roundtrip_large_multi_chunk_page and bulk_range_scan_benchmark.rs) does not pay a repeated overlay clone per chunk.

♻️ Proposed fix: build the MMR once before the chunk loop
         if start < chunk_end {
             let first_chunk = start / epoch_size;
             let last_chunk = (chunk_end - 1) / epoch_size;
+            let mmr_store = MmrStore::with_key_size(&self.dense_tree.storage, MmrKeySize::U32);
+            let mmr = MMR::new_with_overlay(self.mmr_size(), &mmr_store, self.mmr_overlay.clone());
             for chunk_idx in first_chunk..=last_chunk {
-                let blob = self.get_chunk_value(chunk_idx)?.ok_or_else(|| {
-                    BulkAppendError::CorruptedData(format!(
-                        "missing chunk blob for index {}",
-                        chunk_idx
-                    ))
-                })?;
+                let mmr_pos = leaf_to_pos(chunk_idx);
+                let node = mmr.batch.element_at_position(mmr_pos).unwrap().map_err(|e| {
+                    BulkAppendError::MmrError(format!(
+                        "failed to read MMR node for chunk {}: {}",
+                        chunk_idx, e
+                    ))
+                })?;
+                let blob = node.and_then(|n| n.into_value()).ok_or_else(|| {
+                    BulkAppendError::CorruptedData(format!(
+                        "missing chunk blob for index {}",
+                        chunk_idx
+                    ))
+                })?;
                 let chunk_entries = deserialize_chunk_blob(&blob)?;
                 let chunk_start = chunk_idx * epoch_size;
                 for (i, value) in chunk_entries.into_iter().enumerate() {
                     let pos = chunk_start + i as u64;
                     if pos >= start && pos < chunk_end {
                         entries.push((pos, value));
                     }
                 }
             }
         }
🤖 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-bulk-append-tree/src/tree/fetch.rs` around lines 96 - 115, Refactor
the range-fetch logic around the chunk loop to construct the MmrStore and
MMR::new_with_overlay once before iterating chunks, then reuse that instance to
retrieve each chunk leaf instead of calling get_chunk_value per chunk. Follow
the reuse pattern in BulkAppendTreeProof::generate while preserving
missing-chunk corruption errors and existing deserialization and filtering
behavior.
grovedb/src/tests/bulk_append_tree_tests.rs (1)

2411-2432: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider adding a chunk-shared disjoint-range case.

This test covers a narrow proof rejected for a wider range. A second case is worth pinning: prove [0, 2) and then verify [2, 4). Both ranges sit inside chunk 0, and the proof carries the whole chunk blob, so the completeness check may accept the second request. The behavior is defensible, but a test makes the intended semantics explicit and prevents an accidental change later.

🤖 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/tests/bulk_append_tree_tests.rs` around lines 2411 - 2432, Extend
test_bulk_position_range_proof_wrong_range_rejected with a shared-chunk
disjoint-range case: generate a proof for [0, 2) and verify it against [2, 4),
asserting the result matches the intended semantics that the whole chunk blob
permits this request. Keep the existing wider-range rejection assertion
unchanged.
grovedb/src/tests/commitment_tree_tests.rs (1)

3015-3035: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a CommitmentTree wrong-range rejection test.

grovedb/src/tests/bulk_append_tree_tests.rs has test_bulk_position_range_proof_wrong_range_rejected, but no CommitmentTree equivalent exists here. The CommitmentTree path uses a different proof envelope (ProofBytes::CommitmentTree with the sinsemilla prefix) and a different child-hash derivation, so the bulk test does not cover it. Add a test that proves a narrow range and expects verification of a wider range to fail.

💚 Proposed test
#[test]
fn test_commitment_tree_position_range_proof_wrong_range_rejected() {
    let grove_version = GroveVersion::latest();
    let (db, _) = make_ct_db_with_notes(10);

    // Proof generated for [0, 2) (inside chunk 0) must not verify a request
    // for [0, 6), which also needs chunk 1.
    let narrow_proof = db
        .prove_bulk_position_range(
            vec![b"root".to_vec()],
            b"pool",
            0,
            2,
            None,
            grove_version,
        )
        .unwrap()
        .expect("prove narrow range");

    GroveDb::verify_bulk_position_range_proof(
        &narrow_proof,
        vec![b"root".to_vec()],
        b"pool",
        0,
        6,
        grove_version,
    )
    .expect_err("proof for a narrower range must be rejected");
}
🤖 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/tests/commitment_tree_tests.rs` around lines 3015 - 3035, Add a
CommitmentTree wrong-range rejection test near
test_commitment_tree_position_range_proof_across_chunk_boundary. Generate a
proof for positions 0 through 2 using prove_bulk_position_range, then verify it
against the wider range 0 through 6 with
GroveDb::verify_bulk_position_range_proof and assert verification fails.
grovedb/src/operations/proof/generate.rs (1)

225-234: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider adding a version gate to this new public entry point.

Every other public proof entry point in this file starts with check_grovedb_v0_with_cost! (prove_query_many, prove_query, prove_trunk_chunk, prove_branch_chunk). prove_bulk_position_range has none. The delegated prove_query call still enforces its own gate, so the method is not unguarded in practice. Adding an explicit gate keeps the new API in the version table and lets it be disabled independently later. The same applies to GroveDb::verify_bulk_position_range_proof in grovedb/src/operations/proof/verify.rs.

Based on coding guidelines: "When adding functionality, check GroveDB version compatibility".

🤖 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 225 - 234, Add an
explicit GroveDB version gate at the start of both prove_bulk_position_range and
GroveDb::verify_bulk_position_range_proof, matching the
check_grovedb_v0_with_cost! pattern used by the other public proof entry points.
Ensure each new API is independently represented in the version compatibility
checks before executing its existing logic.

Source: Coding guidelines

🤖 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 `@grovedb-bulk-append-tree/src/tree/fetch.rs`:
- Around line 96-115: Refactor the range-fetch logic around the chunk loop to
construct the MmrStore and MMR::new_with_overlay once before iterating chunks,
then reuse that instance to retrieve each chunk leaf instead of calling
get_chunk_value per chunk. Follow the reuse pattern in
BulkAppendTreeProof::generate while preserving missing-chunk corruption errors
and existing deserialization and filtering behavior.

In `@grovedb/src/operations/proof/generate.rs`:
- Around line 225-234: Add an explicit GroveDB version gate at the start of both
prove_bulk_position_range and GroveDb::verify_bulk_position_range_proof,
matching the check_grovedb_v0_with_cost! pattern used by the other public proof
entry points. Ensure each new API is independently represented in the version
compatibility checks before executing its existing logic.

In `@grovedb/src/tests/bulk_append_tree_tests.rs`:
- Around line 2411-2432: Extend
test_bulk_position_range_proof_wrong_range_rejected with a shared-chunk
disjoint-range case: generate a proof for [0, 2) and verify it against [2, 4),
asserting the result matches the intended semantics that the whole chunk blob
permits this request. Keep the existing wider-range rejection assertion
unchanged.

In `@grovedb/src/tests/commitment_tree_tests.rs`:
- Around line 3015-3035: Add a CommitmentTree wrong-range rejection test near
test_commitment_tree_position_range_proof_across_chunk_boundary. Generate a
proof for positions 0 through 2 using prove_bulk_position_range, then verify it
against the wider range 0 through 6 with
GroveDb::verify_bulk_position_range_proof and assert verification fails.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 178317cb-131d-4b2b-82d6-99500b180279

📥 Commits

Reviewing files that changed from the base of the PR and between d473818 and 3f3efe2.

📒 Files selected for processing (18)
  • grovedb-bulk-append-tree/src/lib.rs
  • grovedb-bulk-append-tree/src/proof/mod.rs
  • grovedb-bulk-append-tree/src/proof/tests.rs
  • grovedb-bulk-append-tree/src/tree/fetch.rs
  • grovedb-bulk-append-tree/src/tree/mod.rs
  • grovedb-bulk-append-tree/src/tree/tests.rs
  • grovedb-commitment-tree/src/commitment_tree/mod.rs
  • grovedb-commitment-tree/src/lib.rs
  • grovedb/Cargo.toml
  • grovedb/benches/bulk_range_scan_benchmark.rs
  • grovedb/src/lib.rs
  • grovedb/src/operations/bulk_append_tree.rs
  • grovedb/src/operations/commitment_tree.rs
  • grovedb/src/operations/proof/generate.rs
  • grovedb/src/operations/proof/verify.rs
  • grovedb/src/query/mod.rs
  • grovedb/src/tests/bulk_append_tree_tests.rs
  • grovedb/src/tests/commitment_tree_tests.rs

@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.48991% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.13%. Comparing base (d473818) to head (acb99bd).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
grovedb/src/operations/proof/verify.rs 74.03% 27 Missing ⚠️
grovedb-bulk-append-tree/src/tree/fetch.rs 92.40% 6 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #786      +/-   ##
===========================================
+ Coverage    92.11%   92.13%   +0.01%     
===========================================
  Files          257      257              
  Lines        77936    78642     +706     
===========================================
+ Hits         71794    72459     +665     
- Misses        6142     6183      +41     
Components Coverage Δ
grovedb-core 90.42% <88.36%> (+0.06%) ⬆️
merk 92.89% <ø> (ø)
storage 87.00% <ø> (ø)
commitment-tree 96.07% <100.00%> (+0.01%) ⬆️
mmr 96.79% <ø> (ø)
bulk-append-tree 90.33% <94.54%> (+0.50%) ⬆️
element 97.95% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

- Build the chunk MMR once per get_range call instead of once per chunk:
  going through get_chunk_value re-cloned the MMR overlay for every
  chunk in the page, exactly the repeated work chunk-alignment is meant
  to avoid. Reuses the same single-MMR pattern as
  BulkAppendTreeProof::generate.
- Add explicit version gates (prove_bulk_position_range,
  verify_bulk_position_range_proof) matching the other public proof
  entry points; 0 across GROVE_V1..V4, so no behavior change — the
  delegated prove_query/verify_query gates still apply transitively.
- Pin the shared-chunk disjoint-range semantics with a test: a proof
  generated for [0, 2) verifies a request for [2, 4) inside the same
  chunk, because chunk-aligned proofs carry the whole authenticated
  blob. Rejection of ranges needing an unproved chunk is unchanged.
- Add the CommitmentTree-envelope wrong-range rejection test mirroring
  the BulkAppendTree one (different proof envelope and child-hash
  derivation, so the bulk test did not cover it).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@QuantumExplorer

Copy link
Copy Markdown
Member Author

This is Claude. Addressed all four review comments in ad8c8a7:

  1. MMR hoisted out of the chunk loop (fetch.rs): get_range now builds the MmrStore + MMR (and its overlay clone) once per call and reads each chunk leaf from that single instance, matching the reuse pattern in BulkAppendTreeProof::generate. Missing-chunk corruption errors and deserialization/filtering behavior are preserved.

  2. Shared-chunk disjoint-range case pinned (bulk_append_tree_tests.rs): extended test_bulk_position_range_proof_wrong_range_rejected — a proof generated for [0, 2) does verify a request for [2, 4) inside the same chunk, and the test asserts the returned entries and root. This is intended: chunk-aligned proofs carry the whole authenticated blob, so every entry in it is bound to the root; the wider-range rejection (needing an unproved chunk) is unchanged.

  3. CommitmentTree wrong-range rejection test added (commitment_tree_tests.rs): test_commitment_tree_position_range_proof_wrong_range_rejected, exercising the ProofBytes::CommitmentTree envelope (sinsemilla prefix + different child-hash derivation) as suggested.

  4. Version gates added: prove_bulk_position_range and verify_bulk_position_range_proof now start with the standard check_grovedb_v0* gate and are represented in the version table (0 across GROVE_V1..V4 — no behavior change; the delegated prove_query/verify_query_with_options gates still apply transitively, and under V1/V2 the V0 prover rejects bulk descent as before).

Validation after the changes: cargo nextest run --workspace --all-features — 4557 passed, 0 failed; clippy (--workspace --all-features -- -D warnings), --all-targets check, verify-only build, and fmt --check all clean.

QuantumExplorer and others added 2 commits August 3, 2026 07:51
codecov/patch flagged the new code at 80.55% against the 90% bar. Cover
the arms that are reachable without contrived proofs:

- get_range corruption paths: storage claiming chunks / buffered entries
  it does not hold must error, not silently skip entries.
- Version-gate rejections for prove_bulk_position_range and
  verify_bulk_position_range_proof under an unknown feature version.
- verify_bulk_position_range_proof adversarial inputs: a canonical range
  proof over a plain Tree with 8-byte item keys (element type must be
  rejected when extracting total_count) and over a nonexistent key (a
  proof binding no element must not invent a total_count).

The remaining uncovered lines are defense-in-depth InvalidProof arms
(root-mismatch between sub-proofs of the same bytes, non-8-byte /
non-item / non-contiguous rows under a genuine bulk element) that
cannot be reached through any proof that survives the lower layer's
own checks.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
MemStorageContext (codecov-ignored test util) grows a fail_gets switch
that makes every read error, so the chunk-lookup MmrError arm in
get_range is exercised: a broken backing store must surface as an error,
not a panic or a silent empty page.

The buffer-side ok_or_else arm stays uncovered by design: the dense
tree's get() errors (never returns Ok(None)) for a missing value below
its count, so that arm is unreachable defensive depth, same as the
remaining InvalidProof arms in verify_bulk_position_range_proof.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@QuantumExplorer

QuantumExplorer commented Aug 3, 2026

Copy link
Copy Markdown
Member Author

This is Claude. Follow-up on the coverage gate: codecov/patch flagged the new code at 80.55% against the repo's 90% bar, now green at ~90.1% after two commits:

  • 9326581 — tests for every error arm reachable without contrived proofs: get_range corruption paths (storage claiming chunks/buffer entries it doesn't hold), version-gate rejections for both new entry points under an unknown feature version, and adversarial verify_bulk_position_range_proof inputs (a canonical range proof over a plain Tree with 8-byte item keys must be rejected when extracting total_count; a proof for a nonexistent key must not invent one).
  • eac3480 — the MMR storage-error path: MemStorageContext (codecov-ignored test util) grew a fail_gets switch so a broken backing store is exercised end-to-end and surfaces as MmrError, not a panic or a silent empty page.

The remaining uncovered lines are deliberately left as-is: they are defense-in-depth InvalidProof/CorruptedData arms that cannot be reached by any proof that survives the lower layer's own completeness checks (root-mismatch between sub-proofs of the same bytes, non-8-byte / non-item / non-contiguous rows under a genuine bulk element, and the buffer ok_or_else arm — the dense tree's get() errors rather than returning Ok(None) for a missing value below its count). Reaching them would mean weakening the checks they back up.

All 12 checks are green: full suite (4559 tests run, 4 skipped), clippy -D warnings, all-targets check, verify-only build, formatting, and both codecov statuses.

@QuantumExplorer QuantumExplorer left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found two actionable issues in the new range-read path. The focused range/proof tests pass, but neither issue is covered by the current tests.

///
/// Absence needs no lookup: positions `>= total_count` do not exist, so
/// a page shorter than `limit` means the end of the tree was reached.
pub fn get_range(&self, start: u64, limit: u16) -> Result<RangePage, BulkAppendError> {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Preserve range-read storage costs

get_range returns a plain Result, so the MMR element_at_position(...).unwrap() and every buffer get discard their CostResult.cost. Consequently, bulk_get_range and commitment_tree_get_range report none of the page's seeks or loaded bytes—up to 65,535 uncharged buffer reads. Please return and aggregate a CostResult through both GroveDB wrappers so cost limits reflect the actual work.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is Claude. Fixed in acb99bd: get_range now returns a CostResult — the chunk-MMR node reads and every dense-buffer read charge their seeks and loaded bytes (buffer reads go through dense_tree.get directly so nothing is discarded), and the costs aggregate through CommitmentTree::get_range, bulk_get_range, and commitment_tree_get_range. Added test_bulk_get_range_reports_storage_costs, which asserts nonzero seek/loaded-byte costs on a chunk+buffer-spanning page and that an 11-entry page loads more bytes than a 1-entry page.

chunk_idx
))
})?;
let chunk_entries = deserialize_chunk_blob(&blob)?;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Validate completed chunk length

deserialize_chunk_blob accepts any valid encoded entry count, but this loop assumes each completed chunk contains exactly epoch_size entries. A short chunk silently omits positions; an oversized chunk can overlap positions from the next chunk. The returned page then violates its contiguous-page contract and cursor scans can stall before total_count. Please reject the chunk as corrupted unless chunk_entries.len() == epoch_size as usize.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is Claude. Fixed in acb99bd: get_range now rejects a completed chunk as CorruptedData unless it deserializes to exactly epoch_size entries, with a test that tampers the MMR overlay with both a short and an oversized blob. One note on scope: the check is deliberately only on this raw read path — in proof verification the chunk bytes are bound to the state root (a wrong-length blob changes the root and fails the comparison), and the audit NOTE in proof/mod.rs documents that adding a length check there is redundant by design, so that side is unchanged.

Address review on paginated range reads:

- get_range now returns a CostResult: the chunk-MMR node reads and each
  dense-buffer read charge their seeks and loaded bytes, aggregated
  through CommitmentTree::get_range, bulk_get_range, and
  commitment_tree_get_range, so cost limits reflect a page's actual
  work instead of treating up to 65,535 buffer reads as free. The
  buffer reads go through dense_tree.get directly so their costs are
  captured.
- get_range rejects a completed chunk as corrupted unless it holds
  exactly epoch_size entries: a short blob would silently omit
  positions and an oversized one would overlap the next chunk, breaking
  the contiguous-page contract and stalling cursor scans. Unlike proof
  verification — where chunk bytes are bound to the state root and a
  length check is redundant (see the NOTE in proof/mod.rs) — this raw
  read path has no root comparison backing it.

Tests: a tampered-overlay chunk with the wrong entry count (short and
oversized) is rejected with CorruptedData, and bulk_get_range reports
nonzero seek/loaded-byte costs that grow with page size.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.

1 participant