Skip to content

Multi-key chunk → restore is broken for all Provable* tree types (OWN-vs-AGGREGATE mismatch) #671

Description

@QuantumExplorer

This is Claude. Surfaced during PR #670 (ProvableCountProvableSumTree) while writing chunk → restore round-trip tests.

Summary

Restorer::write_chunk (merk/src/merk/restore.rs) and the chunk producer's to_kv_*_node methods (merk/src/proofs/query/mod.rs) have a semantic mismatch on the count/sum field of KVCount / KVSum / KVCountSum proof nodes.

For single-leaf trees own == aggregate (no children to add) so chunk restoration works. For multi-key trees the restored merk's aggregate_data() recomputes as own + child_aggregates, yielding the wrong total — Restorer::finalize() then fails with ChunkRestoringError(InternalError(\"restored tree invalid\")) because the recomputed root hash doesn't match the source.

Affects every `Provable*` tree type:

This predates PR #670 — it affects ProvableCountTree identically. PR #670 only added a follow-up symptom for the new KVCountSum node type (now accepted at the chunk allowlist instead of rejected outright), but the underlying semantic mismatch is older.

Root cause

Producer side (merk/src/proofs/query/mod.rs:174-269): to_kv_count_node(), to_kv_sum_node(), to_kv_count_sum_node() all read self.tree().aggregate_data() and emit the AGGREGATE value into the proof node. That's correct for proof verification: the verifier reconstructs node_hash_with_count(kv, l, r, aggregate) and the aggregate must match what the prover committed.

Restorer side (merk/src/merk/restore.rs:353-…): write_chunk reads the field directly from the proof Node variant and writes it into TreeFeatureType::Provable*MerkNode(value). But by the TreeFeatureType contract, that value is the OWN value of the node — aggregate_data() adds children's aggregates on top via child_aggregate_*_data_as_*. So writing the AGGREGATE as if it were OWN double-counts the children.

Repro

Try a multi-key (≥ 2 keys) ProvableSumTree chunk round-trip — e.g. the test I wrote in PR #670 (deleted before merge because of this bug):

```rust
fn restore_chunk_round_trip_provable_sum_tree() {
// Source merk: ProvableSumTree with 15 keys, each with own sum=7.
// Build source, build chunks via ChunkProducer, restore via Restorer,
// call restorer.finalize().
// Expected: restored root == source root.
// Actual: ChunkRestoringError(InternalError("restored tree invalid"))
}
```

Single-leaf works because own == aggregate. This was the only end-to-end pattern I could land in PR #670 (see restore_single_leaf_kvsum_for_provable_sum_tree and the PCPS counterpart).

Why this never surfaced

The existing chunk-restore tests only run against NormalTree (make_batch_seq uses BasicMerkNode). The Provable* family was added later and chunk-restore was never tested end-to-end for these tree types. ProvableCountTree's KVCount write arm at restore.rs:353 has the same bug, just never triggered.

Candidate fixes

  1. New wire field for OWN values — add an OWN value alongside the aggregate in the chunk format. Requires a new chunk proof version (breaks wire compat).
  2. Reconstruct children-first + subtract — restore the leaf subtree first, then on the parent's write_chunk subtract child.aggregate from the parent's carried aggregate to derive OWN. Complex (depends on traversal order; the proof tree's children may be in any order; needs to read storage during restore).
  3. Separate node types for chunks vs proofs — cleanest semantically. Chunks use "KV with own value" variants, proofs use "KV with aggregate value" variants. Both can coexist in the wire format because chunk and proof flows are distinct entry points.

Option 3 is most aligned with the rest of the proof system (different proof-node types for different contexts — see e.g. KVCount for items vs HashWithCount for collapsed subtrees). I'd lean that direction.

Scope guidance for whoever picks this up

  • The bug is not specific to PCPS. The fix needs to cover all three Provable* types.
  • The existing restore.rs:353 KVCount arm has the same defect — fix it in the same PR.
  • Look at make_batch_rand / make_batch_seq in merk/src/test_utils/mod.rs — restoration tests likely want a make_provable_count_batch_seq etc. helper to systematically exercise this.
  • Add multi-key round-trip tests for all three Provable* types as the acceptance criterion.

Out of scope for this issue


September 2026 audit addendum — S07

Audit group: S07. Classification: correctness. Provisional severity: medium.

Audit addendum for #671, based on provisional static review of snapshot 2fa0f133. The preserved evidence confirms the reported mismatch between aggregates transported for proof reconstruction and the own-node contributions required by stored tree features. This is one existing reconstruction defect, with additional tree-family qualifications, rather than a new issue for every count or sum variant.

The expected contract is that an honest snapshot round trip preserves both element contents and aggregate semantics. Producer-side proof nodes can carry the aggregate of a whole subtree. Restoration installs that transported total as an individual node's own contribution and then attaches children, whose contributions are added again. Other host-family conversions can lose aggregate metadata or change schema. Link checks and expected-root comparison do not reconstruct the missing own-value semantics.

Outcomes depend on the family and tree shape. A single leaf avoids some own-versus-subtree ambiguity because there are no child contributions. Larger provable trees may fail descendant verification or finalization, rather than successfully returning incorrect state. Some nonprovable metadata damage need not change the cryptographic root. Accordingly this addendum does not claim universal successful root bypass or that every aggregate family fails identically.

Define restoration semantics that distinguish transported subtree totals from own contributions and preserve the trusted host schema. Reconstruct aggregates with child information, check link aggregates, and compare the finalized root with its expected commitment. Compatibility coverage should include ordinary and provable count, sum and combined families, with single-leaf and multi-key cases treated separately.

PR #557 fixed proof-node handling, while PR #840 explicitly proposes aggregate rewriting and broader round trips. Their descriptions provide remediation context but do not establish a fix in the audited snapshot. Issue #706 is related panic/error handling; correcting it alone does not restore aggregate values. No runtime honest-snapshot round trip was executed in this review. Medium severity is a correctness assessment; current develop status, deployment exposure and per-family runtime outcomes remain unverified.

Validation to complete

  • Compare honest single-leaf and multi-key round trips across ordinary/provable aggregate families.
  • Check restored own contributions, child totals, root and aggregate values independently.
  • Verify incompatible reconstruction returns an error while legitimate typed snapshots preserve schema.

Limits and existing work

Related tracking: issue #671 (open), issue #706 (open), PR #670 (merged), PR #557 (merged), PR #840 (open).

Scope: saved GroveDB worktree with revision context 2fa0f133877420a0d9c91ba7bc51b1775ab8c783. This report does not establish that current develop or any deployed application is affected. Focused runtime validation remains outstanding.

Audit source and canonical finding identifiers

Source status: snapshot-backed (git_worktree); plain source locations are used because this is not a sealed commit-only scan.

Audited revision context: 2fa0f133877420a0d9c91ba7bc51b1775ab8c783.

The findings were manually reconciled from a preserved scan bundle. The native scan ended before final completion; these are provisional source-review findings, not a completed native scan certification.

Canonical finding ID: csf_70eb6b24f46dd4f3c8b14904

Primary fingerprint: codex-security/v1:sha256:9eb9575c10b57a9aa6fa5950dc3be5c78732da42666e34b0ba005f9e10b87597

Source locations:

  • Location (root_control): grovedb-element/src/element_type.rs:612-687
  • Location (implementation): merk/src/proofs/query/mod.rs:185-215
  • Location (implementation): merk/src/merk/restore.rs:338-387
  • Location (implementation): merk/src/tree/mod.rs:880-891
  • Location (implementation): merk/src/merk/restore.rs:691-702

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:state-syncS groups: state restoration, synchronization and append-client transactions.audit:2026-09Reconciled September 2026 GroveDB audit; audited worktree at 2fa0f133.audit:needs-validationSaved source evidence reviewed; focused runtime and deployment validation outstanding.bugSomething isn't workingseverity:medium-provisionalMedium impact in the audited scenario; provisional static assessment.type:correctnessAudit reports incorrect library behavior or accounting without proven security impact.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions