Skip to content

State sync cannot transfer the append-only tree family — a single populated CommitmentTree makes snapshots from that node unusable #785

Description

@QuantumExplorer

Summary

The replication module (grovedb/src/replication/) has zero support for the append-only (non-Merk) tree family — CommitmentTree, MmrTree, BulkAppendTree, DenseAppendOnlyFixedSizeTree. The verified behavior today:

A single populated CommitmentTree anywhere in the grove makes state sync from that node impossible. The source's fetch_chunk fails with an opaque CorruptedData("... cannot create chunk producer for empty Merk") when the syncing peer requests the CT subtree's chunk, so no peer can ever complete a snapshot sync from any node that holds one.

Since CommitmentTree (element discriminant 11) is the Dash Platform shielded-pool notes tree and is live on mainnet/testnet, this is a live operational gap independent of any new feature work: from the moment the first shielded note lands, every node holding it can no longer serve a usable state-sync snapshot, and new nodes can only bootstrap by replaying blocks. The failure mode is "fail hard, fail safe" — no corruption is produced — but availability of state sync is gone network-wide.

Reproduction tests (3 tests, all passing against develop) are on branch claude/eager-blackwell-838b49 in grovedb/src/tests/replication_session_tests.rs:

  • state_sync_populated_commitment_tree_fails_on_source_fetch
  • state_sync_populated_non_merk_trees_all_fail_on_source_fetch (MmrTree / BulkAppendTree / DenseAppendOnlyFixedSizeTree — same failure)
  • state_sync_empty_commitment_tree_succeeds (empty CT syncs fine; see "why the naive fix is dangerous")

Trace

  1. Discovery enqueues the CT like a normal Merk subtree. discover_new_subtrees_metadata (grovedb/src/replication/state_sync_session.rs) iterates parent elements and uses value.is_any_tree(), which includes all four non-Merk types. Only indexed trees get the up-front NotSupported rejection (State sync does not support indexed trees (PCIT/PSIT/PCPSIT) #778); non-Merk trees fall through and are scheduled for Merk-chunk restore.
  2. Source-side fetch_chunk breaks on the payload. fetch_chunk (grovedb/src/replication.rs) opens the CT's prefix and calls merk.is_empty_tree(), which raw-iterates the namespace (merk/src/merk/mod.rs:534). A populated CT's namespace contains its non-Merk payload entries (frontier, buffer, MMR nodes), so the check says "not empty" — but the CT's Merk is rootless by design (root_key = None), so ChunkProducer::new then fails:
    CorruptedData("failed to create chunk producer by prefix tx:<prefix> with:
    chunking error chunk from empty tree: cannot create chunk producer for empty Merk")
    
  3. The syncing peer gets this error for the CT chunk request and the session can never reach is_sync_completed().

Why the naive fix is dangerous

An empty CommitmentTree state-syncs successfully today: no payload entries exist yet, is_empty_tree() is true, the source returns an empty chunk, the destination verifies the NULL Merk root and moves on — and the final app-hash check passes, because the parent Merk (which embeds the CT element bytes and the payload binding) restores byte-for-byte.

That passing path demonstrates the trap: nothing in the restore pipeline ever recomputes a non-Merk state root from payload. If fetch_chunk were "fixed" to skip these subtrees or return empty chunks, a sync of a populated CT would complete, pass the app-hash check at commit, and silently produce a destination whose CT element claims total_count = N while its frontier and note data are missing — detected only later by verify_grovedb or the first commitment_tree_anchor / commitment_tree_get_value call. Any fix must actually transfer and re-verify the payload.

Affected element types

Discriminant Element Status
11 (+139 wrapped) CommitmentTree Live — shielded-pool notes tree
12 (+140) MmrTree Same failure, verified
13 (+141) BulkAppendTree Same failure, verified
14 (+142) DenseAppendOnlyFixedSizeTree Same failure, verified
15 (+143) PrivateDocumentStore (#784) Will inherit the gap — same storage model
16 (+144) DataCommitmentTree (#783) Will inherit the gap — same storage model

What a correct transfer needs (per subtree)

Everything for these tree types lives as raw KV entries in the subtree's single blake3(path)-prefixed data namespace; the tree's own Merk is always empty and there are never child subtrees beneath it:

  • CommitmentTree = BulkAppendTree + Sinsemilla frontier:
    • serialized frontier at COMMITMENT_TREE_DATA_KEY = b"__ct_data__" (grovedb-commitment-tree/src/commitment_tree/mod.rs:28)
    • dense-buffer entries at 2-byte BE position keys (grovedb-dense-fixed-sized-merkle-tree, position_key(u16))
    • chunk-level MMR nodes at 4-byte tagged BE position keys (MmrKeySize::U32, MSB set for namespace separation); compacted chunk blobs are the MMR leaves
  • BulkAppendTree: the same, minus the frontier
  • MmrTree: MMR nodes at 8-byte BE position keys (MmrKeySize::U64)
  • DenseAppendOnlyFixedSizeTree: entries at 2-byte BE position keys

So the transfer unit is simply "all KV pairs under the prefix", paged.

Receiving-side verification — no new trust assumptions needed

The parent leaf's value_hash is already a commitment to the payload: inserts write combine_hash(value_hash(element_bytes), state_root) via insert_subtree(..., combined_root, ...) (grovedb/src/operations/commitment_tree.rs), where for CT

ct_state_root   = blake3("ct_state"   || sinsemilla_root || bulk_state_root)
bulk_state_root = blake3("bulk_state" || mmr_root        || dense_tree_root)

(#782 recently strengthened exactly this terminal binding.) The parent Merk restore already hash-verifies that leaf up the chain to the app hash, so the receiver holds a trusted 32-byte commitment to the payload before fetching a single payload byte. Verification is: write the raw KVs, recompute the type-specific state root, and require

combine_hash(value_hash(element_bytes), recomputed_state_root) == elem_value_hash_from_parent

The recompute dispatch already existscompute_non_merk_child_hash (grovedb/src/lib.rs:2379), used by verify_grovedb for precisely this check. The future pds_state (#784) and dct_state (#783) config-committed roots re-verify identically, with the committed config participating in the element bytes / domain tag.

One caveat vs. Merk restore: raw KV paging is not incrementally verifiable per chunk the way Merk chunk proofs are — verification lands at subtree completion (a malicious peer is caught before the subtree is finalized, same transaction-rollback story as other restore failures). If incremental verification is wanted later, the BulkAppendTree's immutable chunk blobs are individually MMR-authenticated and could be verified per-page against the MMR peaks.

Proposed approach

Phase 0 — immediate guard (~20 lines + tests). Reject non-Merk tree types up-front with a descriptive Error::NotSupported in both discover_new_subtrees_metadata and fetch_chunk, exactly like the indexed-tree guards from #778. Sync fails either way today; this replaces an opaque CorruptedData with an actionable error and protects against any future refactor that would turn the hard failure into the silent-skip trap above.

Phase 1 — real support (one PR, est. 500–800 lines incl. tests).

  1. fetch_chunk: branch on tree_type.uses_non_merk_data_storage() → stream raw KV pages from the prefixed storage context (raw_iter) in bounded-size chunks; the local chunk id encodes the resume key. The global chunk id already carries the TreeType discriminant (7–10 cover this family), so the ID format extends naturally.
  2. state_sync_session: for these subtrees, use a raw-restore path instead of a Merk Restorer — write KVs through the immediate storage context; on subtree completion recompute the state root via the compute_non_merk_child_hash dispatch and enforce the parent binding; reject the subtree on mismatch; skip child discovery (there are no children).
  3. Bump / gate on the state-sync protocol version as appropriate. Replication is node-local wire behavior, not a consensus state transition — no committed hash changes — so this should not need GROVE_V4 gating (consistent with State sync does not support indexed trees (PCIT/PSIT/PCPSIT) #778's guards landing ungated); a CURRENT_STATE_SYNC_VERSION bump is the right lever if wire compatibility matters.
  4. Tests: round-trip populated CT/MMR/bulk/dense including a multi-epoch CT (buffer + compacted chunks + frontier); tamper tests (bit-flip one payload byte, drop the frontier key → subtree must be rejected); interaction with subtrees_batch_size batching.

Type 11 first? Possible — CT is the only live exposure, and each type is an independent dispatch arm. But types 12–14 reuse the identical raw-page + recompute path, so the family costs barely more than CT alone; recommend shipping all four together, with #784/#783 (types 15/16) required to plug into the same path when they land.

Cross-references

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions