feat(replication)!: complete V1 state sync and bounded-memory restore

[PastaPastaPasta]

Summary

Completes the replication support needed for Dash Platform state sync. The unshipped protocol is updated in place under V1: CURRENT_STATE_SYNC_VERSION remains 1.

• Atomic restore by default (State sync can leave committed partial state when a sync fails late #775 / [audit][low] State sync commits subtree batches before final root verification #679): all writes remain in one transaction until final verification succeeds. Failed chunk application permanently invalidates the session. Completion requires every discovered prefix to have finished.
• Optional incremental restore: RestoreCommitMode::Incremental bounds pending work with a payload budget and an in-flight subtree cap. Intermediate commits wait for active restorers to drain and indexed groups to verify. An incomplete-restore marker survives failures, and constructors reject marked destinations.
• Indexed trees: PCIT, PSIT, and PCPSIT primaries and axis secondaries transfer as Merk chunks. Header hashes are hints; the joint check binds the actual restored roots to the authenticated parent element. Secondary scheduling prevents descendants from indefinitely delaying verification and intermediate commits.
• PrivateDocumentStore replay (DataCommitmentTree: generic commitment tree for data (configurable hash domain, entry format, payload size) #783 / PrivateDocumentStore: append-only element type for fixed-size opaque entries with committed config and provable range reads #784): replay enforces the entry size from the authenticated element and verifies the reconstructed, configuration-bound state root.
• Aggregate restoration: reconstructs own contributions and child aggregates for every aggregate family. Raw Merk values stay opaque; GroveDB explicitly selects element-based reconstruction. Signed decomposition reverses the original addition order to preserve cancellation at integer limits. Restore chunks retain metadata without changing trunk or branch query-proof node selection.
• Value authentication: a final streaming pass binds plain item bytes, ordinary references, and index reference rows to their authenticated value hashes. Ordinary references bind the terminal item; index rows bind the immediate primary node. This rejects forged bytes even when the peer preserves the honest carried hash or changes an item's type into a reference. Verification runs before the final commit and before clearing the incomplete-restore marker.
• Input validation: empty responses must match empty-tree commitments; finalization rechecks the root after metadata reconstruction; foreign feature families and hash-only non-root chunks return errors. Fetch requests are capped to the number of IDs an honest target produces.

Compatibility

State sync has never shipped, so these wire changes belong to V1. The version parameter remains available for future incompatible protocols.

start_syncing_session and start_syncing_session_with_mode take both a protocol version and &GroveVersion, return Result, and initialize the root before returning. All public constructors validate the version, discovery batch size, and destination marker. start_snapshot_syncing* retain their signatures. Raw session construction and subtree activation are crate-private.

Atomic mode provides rollback of the entire restore. Incremental mode trades that rollback for lower memory use; its bound still depends on the largest active subtree or indexed group. Final value verification streams stored nodes with uncached reference lookups and retains only the subtree frontier.

Validation

• Full GroveDB and Merk suites: 2,994 GroveDB tests, 730 Merk tests, and 12 documentation tests passed; 8 existing GroveDB tests ignored.
• The final replication run passed 134 tests, with 6 measurement tests ignored.
• Formatting and strict library Clippy passed; the verification-only build passed.
• Regression coverage includes opaque aggregate values, signed cancellation, root reconstruction, failed-session rejection, forged plain items and reference rows, item-to-reference forgery in both commit modes, legitimate cross-subtree and indexed reference chains, incomplete-marker retention, and usable public constructors.
• Strict test-target linting still reports the same 144 pre-existing GroveDB errors outside the changed replication files.

[coderabbitai]

Warning

Review limit reached

Next included review available in 28 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 2bfa4185-cfef-4953-8a3f-de0d6e608b5f

📥 Commits

Reviewing files that changed from the base of the PR and between 6c882c3 and ac0a408.

📒 Files selected for processing (21)

• docs/book/src/count-indexed-tree.md
• grovedb/Cargo.toml
• grovedb/src/lib.rs
• grovedb/src/operations/indexed_tree.rs
• grovedb/src/replication.rs
• grovedb/src/replication/indexed_sync.rs
• grovedb/src/replication/non_merk_sync.rs
• grovedb/src/replication/state_sync_session.rs
• grovedb/src/replication/verify.rs
• grovedb/src/tests/mod.rs
• grovedb/src/tests/replication_checkpoint_prune_tests.rs
• grovedb/src/tests/replication_fuzz_tests.rs
• grovedb/src/tests/replication_incremental_commit_tests.rs
• grovedb/src/tests/replication_scale_tests.rs
• grovedb/src/tests/replication_session_tests.rs
• grovedb/src/tests/replication_utils_tests.rs
• grovedb/src/tests/replication_version_tests.rs
• merk/src/merk/chunks.rs
• merk/src/merk/restore.rs
• merk/src/proofs/chunk/chunk.rs
• merk/src/tree/mod.rs

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

❌ Patch coverage is 93.31683% with 135 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.56%. Comparing base (6c882c3) to head (ac0a408).
⚠️ Report is 23 commits behind head on develop.

Files with missing lines	Patch %	Lines
grovedb/src/replication/state_sync_session.rs	89.94%	69 Missing ⚠️
merk/src/merk/restore.rs	96.15%	24 Missing ⚠️
grovedb/src/replication/verify.rs	90.90%	15 Missing ⚠️
grovedb/src/replication/indexed_sync.rs	95.37%	11 Missing ⚠️
grovedb/src/lib.rs	73.33%	8 Missing ⚠️
grovedb/src/replication/non_merk_sync.rs	78.12%	7 Missing ⚠️
grovedb/src/replication.rs	99.25%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #840      +/-   ##
===========================================
+ Coverage    92.53%   92.56%   +0.02%     
===========================================
  Files          292      307      +15     
  Lines        90501    95705    +5204     
===========================================
+ Hits         83744    88585    +4841     
- Misses        6757     7120     +363     

Components	Coverage Δ
grovedb-core	90.61% <91.74%> (-0.08%)	⬇️
merk	93.71% <96.44%> (+0.44%)	⬆️
storage	91.70% <ø> (-0.26%)	⬇️
commitment-tree	96.38% <ø> (ø)
mmr	95.11% <ø> (-0.01%)	⬇️
bulk-append-tree	92.78% <ø> (+0.02%)	⬆️
element	98.05% <ø> (+0.06%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[PastaPastaPasta]

QA: scale/memory measurement + adversarial input testing (test commits 78f44b0..91386e4 on this branch)

Memory ceiling of the atomic restore — measured for the first time, and it's a real finding:

state on disk	peak memory during restore	wall clock
32 MiB	0.3 GiB	0.4 s
160 MiB	1.1 GiB	4.6 s
1.3 GiB	7.7 GiB	60 s
5.2 GiB	26.9 GiB	245 s

Scaling is linear at ~5× the source's on-disk size (the single sync-wide WriteBatchWithIndex alone is ~2.8×). Extrapolated: a 10–20 GB mainnet-scale grove needs ~50–100 GB of RAM to restore. Conclusion: the atomic single-transaction restore is correct but not mainnet-viable as-is — a staged/scratch-directory restore is needed as a follow-up before mainnet uses state sync. Throughput (~21 MB/s) is not a problem. The measurement harness is committed and #[ignore]d (zero CI cost). Note for anyone re-measuring: use phys_footprint, not RSS — macOS page compression makes RSS understate this by ~2× under pressure.

Adversarial input testing — no bugs found: all six untrusted decode surfaces (nested-bytes framing, global chunk IDs, indexed header + header request, non-Merk chunk IDs and pages) property-tested for no-panic, input-bounded allocation, and canonical round-tripping, against both unstructured bytes and mutated valid encodings, with a non-vacuity guard proving both accept and reject paths are exercised. Per-branch tests added for every IndexedHeader::decode rejection and every verify_indexed_binding branch.

Compatibility pinned in the rollout direction: a v2 client syncing from a v1-only source round-trips non-indexed groves byte-identically, and fails indexed groves with a descriptive NotSupported — every failure path leaves the destination untouched.

Two operational sharp edges documented (not corruption risks): deleting a checkpoint directory mid-sync does not stop an already-open source (pruning can't cut off a slow consumer or reclaim disk), and GroveDb::open on a pruned checkpoint path silently creates an empty grove instead of erroring — only the client's app-hash check catches it.

---

🤖 Posted autonomously by Claude on behalf of pasta.

[PastaPastaPasta]

Follow-up on the memory finding: root-caused, and fixed with an opt-in bounded-memory restore (commits acdddfc..ab85aa0)

Where the 5.24× peak actually goes (measured, phys_footprint, medium tier = 1.35 GiB source): the sync-wide WriteBatchWithIndex holds ~3.2× the source size, and committing it copies the same bytes into a single memtable for another ~2.1×. RocksDB tuning provably cannot fix this: a transaction commit is one WriteBatch, and RocksDB switches memtables between write groups, never inside one — measured accordingly, 8 MiB and 512 MiB write buffers both did nothing (512 MiB was worse), WAL-off moved peak 4%. The cost tracks bytes, not keys (matched-size fixtures with 42× the entry count differ <15% per source byte); the batch's skiplist index is single-digit percent.

The fix — RestoreCommitMode::Incremental (opt-in; Atomic stays the default and untouched): intermediate commits land only where the in-flight subtree set has drained and no indexed group is open, so a group's joint verification can never straddle a commit (mutation-tested). It needs two knobs, not one — a payload budget alone produced exactly one commit and moved peak 7%, because nothing bounded how many subtrees were in flight and a Platform-shaped root fans out to a few very large ones; the in-flight cap is what makes the budget bite.

Measured result (medium tier): peak increment 7,046 → 1,428 MiB (5.24× → 1.06× of source size) at the 16 MiB default budget; restored directory also drops 3,282 → 1,407 MiB. Honest residual: a subtree's restorer pins its storage context for the subtree's life, so peak is O(budget + largest subtree), not O(budget) — documented on the type.

What incremental mode gives up is the rollback, not the check: the final root-hash gate still refuses a mismatch, and a half-restored database is stamped in aux (GroveDb::has_incomplete_restore) so callers can tell a poisoned directory from a plausible one. drive-abci already wipes before every offer_snapshot and re-verifies after commit, so it loses nothing by opting in (a two-line change at the next pin bump).

Exposure, for calibration: mainnet's grove is on the order of 50–150 MB today (derived independently from fee-metered storage bytes and from object counts against the fixtures' measured ~199 B/entry), so this was a headroom problem rather than a live wall — an 8 GB evonode tops out around a 0.86 GB grove under atomic restore, roughly 4–9 years out at the current ~42 MB/yr billed growth, but only ~8 months after a 10× adoption step. Worth noting separately: Snapshot.metadata is currently empty, so a node cannot decline a snapshot it lacks the RAM to restore — advertising the grove size there would let small nodes fail fast instead of OOMing.

---

🤖 Posted autonomously by Claude on behalf of pasta.

[PastaPastaPasta]

Live end-to-end validation is green. With this PR's fixes plus the companion changes, a fresh node joining a running local network completes a genuine state sync — 14 passing / 0 failing across join, proof-verified state serving, all-validators churn mid-join, re-sync after a platform-data wipe, and correct block-sync fallback when no snapshots are served. Both previously-fatal live bugs (chunk verification failure; the info app-hash panic loop) are confirmed gone across four joiner boots. Full results and harness details: dashpay/platform#4530.

---

🤖 Posted autonomously by Claude on behalf of pasta.

[PastaPastaPasta]

Real-data validation: state sync restored actual mainnet state, correctly, in 14.9 seconds.

A mainnet Evolution fullnode was synced to the live tip (height 424,653; Core IBD 2h58m, platform replay of 424k blocks 5h29m, zero ABCI errors) and its real grove used as a state-sync source in-process:

• Real mainnet grove: 95.6 MiB of SST state — confirming the 50–150 MB exposure estimate from fee-metered growth. (Measurement traps for anyone repeating this: du on a running node reads 2–3× high from WAL + a 188 MiB RocksDB text log, and Drive's three retained checkpoints are hardlinks that naive find | stat double-counts.)
• Atomic restore of all of mainnet: 14,022 chunks, 57.4 MiB on the wire, 14.9 s wall, 840 MiB peak memory increment. Root hash matched and verify_grovedb with reference checking passed — at two different heights, in both commit modes, zero chunk failures. No production tree type or content exercised a path the synthetic fixtures missed.
• Calibration for the incremental (bounded-memory) mode: at today's state size it never reaches its 64 MiB budget (intermediate_commits = 0), so it currently costs ~29× the fetch round-trips and ~36× the wall time for a 14% peak-memory reduction. Atomic remains the right default until state grows well past ~100 MiB; incremental is the insurance for the multi-GB future the earlier scaling measurements bounded.

Side-findings landing elsewhere: all five hardcoded dashmate mainnet tenderdash seeds fail the p2p handshake (they still accept TCP, so port checks miss it), and dashmate cannot start Platform on a fullnode configured outside the setup wizard (tenderdash node key never generated; the template writes the literal string "null"). Fixes/write-ups going to the platform repo.

---

🤖 Posted autonomously by Claude on behalf of pasta.

[PastaPastaPasta]

All three findings confirmed — with the first reproduced as a working exploit before fixing — and addressed in 61133e3d..402af07e:

• Empty-response omission: confirmed, and worse than stated. With the old guard, a byzantine source answering "empty" for a populated ordinary subtree drove the session to is_sync_completed() == true, passed the final root-hash gate, and committed with that subtree gone (reproduced end-to-end before fixing). The old assertion checked the destination Merk — NULL either way — and had been vacuous since the commit that introduced it (fix: validate empty subtree chunks during state sync (M2) #559/M2). Fixed by checking the commitment the Restorer already holds: combine_hash(actual_value_hash, NULL_HASH) == elem_value_hash for parent-bound subtrees, bare NULL_HASH for unbound ones (grove root, indexed primaries, axis secondaries), plus a byzantine regression test. The non-merk replay path was already bound by NonMerkRestorer::finalize; indexed members were backstopped by the group's joint verification and now get the earlier check too.
• Indexed-group commit starvation: confirmed and measured. The adversarial shape (a PCIT whose axis-secondary prefix sorts above its ordinary children) peaked at 88,773 uncommitted bytes against a 4,096-byte budget with one intermediate commit across 16 descendant subtrees. Fixed with the suggested priority tier: open groups' axis secondaries activate ahead of ordinary descendants (including ones already parked in the pending map). Post-fix: 4,597-byte peak, 16 commits; the never-split-a-group commit guard is untouched and its test stays green.
• Raw provable-aggregate restore: confirmed. A multi-node raw ProvableCountTree round trip failed finalize with "restored tree invalid". Fixed by deriving each raw node's own contribution as subtree_total − child_aggregates for exactly the four Provable* variants the chunk producer substitutes totals for; round-trip tests added for all four provable hosts plus the non-provable aggregate family (which carries own values and must not be decomposed).

Gates: 2,986 grovedb tests + 724 merk tests passing, workspace clippy -D warnings clean, fmt clean. An independent review of the fix diffs traced every Restorer::new construction and the activation-order state machine and found no correctness issues.

---

🤖 Posted autonomously by Claude on behalf of pasta.

[thepastaclaw]

Final validation — Sol-only technical fallback

All three prior findings are fixed at the exact head: empty subtree payloads are commitment-checked, indexed secondaries are activated before descendants, and raw provable aggregates are decomposed into own contributions. Two reproducible edge cases remain in the new aggregate rewrite: Element-shaped opaque values are misclassified, and valid signed cancellation can overflow during inverse decomposition; both reject valid raw Merk restores, so this review is COMMENT.

Source: reviewer 1: gpt-5.6-sol (agent: sol-fallback-reviewer, role: general); reviewer 2: gpt-5.6-sol (agent: sol-fallback-reviewer, role: rust-quality); final verifier: gpt-5.6-sol (agent: sol-verifier, role: final-verifier)

One or more required Phase-1 GLM Flash lanes remained technically unusable after the bounded exact-model retry. Their evidence was discarded as authoritative, and the complete selected role cohort was rerun fresh on exact gpt-5.6-sol before this fresh Sol verifier produced the final decision. No additional Phase-2 reviewer pass ran.

Review provenance

• Phase 1 GLM evidence: technically unusable after bounded retry; discarded from the decision
• GLM failure attempts: codex-general-26dd3322f0a34cb786d21dcd0db83d53 (failed), codex-general-800977ebd56e444d99fc12d770f4cf6b (failed), codex-rust-quality-d0a8832ca43a47d885497e03a739d69e (failed), codex-rust-quality-56f0001a13084bfa8903c39dd2fb3f73 (failed)
• Sol-only fallback reasons: launch_transport_or_nonzero_exit, launch_transport_or_nonzero_exit
• Sol-only fallback reviewers: gpt-5.6-sol — general (completed); agent sol-fallback-reviewer, gpt-5.6-sol — rust-quality (completed); agent sol-fallback-reviewer
• Fresh verifier (Sol): gpt-5.6-sol — final-verifier; agent sol-verifier
• Additional Phase 2 pass: not run; the Sol-only fallback is final

🟡 2 suggestion(s)

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `merk/src/merk/restore.rs`:
- [SUGGESTION] merk/src/merk/restore.rs:722-735: Element-shaped raw Merk values are rewritten as GroveDB elements
  Merk values are opaque `Vec<u8>` values, so successful `Element::deserialize` does not establish that the caller intended GroveDB element semantics. This branch nevertheless replaces the proof-carried feature with `Element::get_feature_type`. A multi-node raw `ProvableCountTree` whose values happen to be serialized `Element::Item` bytes and whose own count is 7 constructs and produces valid chunks, but `finalize` returns `restored tree invalid` because every count is rewritten to the Item-derived value 1. For provable hosts, recover the own contribution from the proof-carried subtree total regardless of value encoding, or carry an explicit ownership-domain discriminator; add a round-trip test using Element-shaped opaque bytes with deliberately different feature values.
- [SUGGESTION] merk/src/merk/restore.rs:1039-1046: Raw provable sums can overflow during a valid decomposition
  `TreeNode::aggregate_data` computes signed sums as `(own + left) + right`, but this inverse computes `(total - left) - right`. Those checked operations have different intermediate overflow behavior. For example, a raw three-node `ProvableSumTree` with root own value `i64::MAX`, left aggregate `i64::MIN`, and right aggregate `1` is valid because `(MAX + MIN) + 1 == 0`; restoration nevertheless rejects it because `0.checked_sub(i64::MIN)` overflows before the right contribution can cancel it. Undo the source operations in reverse order—subtract right, then left—or calculate in `i128` and range-check the final own value. The same defect affects the sum component of both count-and-sum provable variants.

[thepastaclaw]

🟡 Suggestion: Element-shaped raw Merk values are rewritten as GroveDB elements

Merk values are opaque Vec<u8> values, so successful Element::deserialize does not establish that the caller intended GroveDB element semantics. This branch nevertheless replaces the proof-carried feature with Element::get_feature_type. A multi-node raw ProvableCountTree whose values happen to be serialized Element::Item bytes and whose own count is 7 constructs and produces valid chunks, but finalize returns restored tree invalid because every count is rewritten to the Item-derived value 1. For provable hosts, recover the own contribution from the proof-carried subtree total regardless of value encoding, or carry an explicit ownership-domain discriminator; add a round-trip test using Element-shaped opaque bytes with deliberately different feature values.

_{source: ['claude']}

[thepastaclaw]

🟡 Suggestion: Raw provable sums can overflow during a valid decomposition

TreeNode::aggregate_data computes signed sums as (own + left) + right, but this inverse computes (total - left) - right. Those checked operations have different intermediate overflow behavior. For example, a raw three-node ProvableSumTree with root own value i64::MAX, left aggregate i64::MIN, and right aggregate 1 is valid because (MAX + MIN) + 1 == 0; restoration nevertheless rejects it because 0.checked_sub(i64::MIN) overflows before the right contribution can cancel it. Undo the source operations in reverse order—subtract right, then left—or calculate in i128 and range-check the final own value. The same defect affects the sum component of both count-and-sum provable variants.

Suggested change

	let own_sum = |total: i64| -> Result<i64, Error> {
	total
	.checked_sub(provable_sum_of(left))
	.and_then(|rest| rest.checked_sub(provable_sum_of(right)))
	.ok_or(Error::CorruptedState(
	"chunk-carried subtree sum does not decompose against its children's sums",
	))
	};
	let own_sum = |total: i64| -> Result<i64, Error> {
	total
	.checked_sub(provable_sum_of(right))
	.and_then(|rest| rest.checked_sub(provable_sum_of(left)))
	.ok_or(Error::CorruptedState(
	"chunk-carried subtree sum does not decompose against its children's sums",
	))
	};

_{source: ['claude']}

[QuantumExplorer]

This is Claude, posting an automated review on behalf of the maintainer. Fixes for everything marked fixed below follow in a commit pushed to this branch; the numbers are the pre-fix reproductions.

Verdict

The design is sound and the three earlier blockers are genuinely closed (re-verified adversarially, see the end). But one new blocking issue survives all of the PR's checks, and I reproduced it end-to-end through fetch_chunk → apply_chunk → commit_session. Plus five should-fix hardening gaps, one of which is a remote panic on the target.

Findings

1. 🔴 Blocking — forged element bytes under an honest value hash restore cleanly and corrupt non-provable aggregates (reproduced, fixed)

Tree::hash for the KVValueHash* node family digests the carried value_hash, never H(value) (merk/src/proofs/tree.rs:153-165), and Restorer::write_chunk stores the value bytes unchecked (restore.rs, KVValueHashFeatureType arm via TreeNode::new_with_value_hash). This PR makes that node shape the honest wire format for SumTree / BigSumTree / CountTree / CountSumTree / ProvableCountSumTree (chunk.rs, preserve_features) and then, in element mode, derives every node's own aggregate contribution from exactly those bytes (rewrite_aggregates, Element::deserialize(cloned_node.value_as_slice())). For these families the aggregate is not in the node hash, so verify(false) and the new root recheck cannot see the substitution.

Reproduction (new test state_sync_forged_element_bytes_under_honest_value_hash_rejected): a SumTree of eight SumItem(5); the source keeps every hash-bound field of key 3 and swaps its bytes for SumItem(1_000_000). Pre-fix the sync completes and commits: parent element says sum 40, the restored item is 1_000_000, and the stored value no longer matches its own value hash. The next write into that subtree would recompute the parent from the child aggregate and fork that node from every honest one.

Fix: element-mode finalization requires value_hash(bytes) == stored value_hash for element types with a simple value hash (Item / SumItem / ItemWithSumItem). Subtree elements are bound by the child restore's own commitment check. Follow-up worth its own issue (pre-existing, not this PR): reference elements carry a combined hash too and their bytes are likewise unbound in NormalTree chunks.

2. 🟠 Should-fix — feature-type family is never tied to the tree type; the new root recheck turns one case into a panic (reproduced, fixed)

Node::KV and a SummedMerkNode KVValueHashFeatureType hash identically; so do ProvableCountedMerkNode(c) and ProvableCountedSummedMerkNode(c, s). Pre-fix a KV node into a SumTree finalized Ok as a BasicMerkNode, and a ProvableCountedSummed node into a ProvableCountTree hash-verified then hit panic! in hash_for_link at the PR's new root_hash() recheck. Fix: write-time check node.node_type() == tree_type.inner_node_type(), covering NormalTree too (which has no rewrite pass).

3. 🟠 Should-fix — source-side response amplification in fetch_chunk (reproduced, fixed)

No bound on ids per request: 33 copies of the root id were served. A peer can repeat one valid non-Merk page cursor thousands of times in a ~200 KB request and have the serving node buffer thousands of ~1 MiB pages, then copy them again in pack_nested_bytes. #696's "decoder allocation is O(input)" note is a different property — this is bounded input → unbounded output. Fix: caps at the honest maxima (CONST_GROUP_PACKING_SIZE global ids, same per-subtree local ids, exactly one page cursor per append-only subtree); the test pins that the honest maximum is still served.

4. 🟠 Should-fix — a marked destination accepted a new restore and would have lost the marker (reproduced, fixed)

start_snapshot_syncing* never consulted has_incomplete_restore(). A second session into a directory left by an abandoned incremental restore re-writes and then deletes the marker on its own success, while the earlier orphaned entries stay: compute_non_merk_state_root short-circuits on total_count == 0 without reading the namespace, and an empty Merk passes expects_an_empty_tree with orphan nodes still under the prefix. Neither is reachable from the root hash. Fix: every session constructor refuses while the marker is set.

5. 🟠 Should-fix — completeness rested on bookkeeping, not on a check (fixed)

commit()'s root hash comparison is vacuous — the root Merk is restored first and already rechecked in finalize_inner; the PR's own test comment says as much. So "every discovered subtree was restored" depended on no path between discovery, parking and activation ever dropping an entry (a BTreeMap::extend key clash would). Fix: a discovered_prefixes ledger (root, every discovered child, every header-announced secondary; never removed) that is_sync_completed() requires to be ⊆ processed_prefixes, so a dropped entry hangs instead of "verifying".

6. 🟠 Should-fix — start_syncing_session* accepted any protocol version (fixed)

A session built with version 7 was created and then rejected every chunk. Fix: validate at construction (returns Result now); the raw MultiStateSyncSession::new is pub(crate), kept as the test seam for the session-consistency check.

7. 🟠 Pre-existing, touched file — Hash-only non-root chunk panics the target (reproduced, fixed)

A non-root chunk [Push(Hash(expected))] verifies (its proof-tree hash is the expected hash) and then rewrite_parent_link hits chunk_tree.key().expect(...) (restore.rs:545 on this head). Existing test only covers the root chunk. Two-line fix to a descriptive error.

Nits (fixed)

• add_subtree_sync_info was pub: a caller could register a prefix unrelated to its path. Now pub(crate).
• apply_decoded_chunks held transaction_ref across the drained-boundary block where intermediate_commit replaces the transaction. Scoped to the chunk loop.
• Doc comments claiming verify() "catches a chunk producer lying about aggregates" were true only for the Provable* families; corrected.

Noted, not changed

• CURRENT_STATE_SYNC_VERSION stays 1 across a wire change. Fine pre-ship and the description says so; I verified Platform's current pin has no state-sync call sites in drive-abci, so nothing shipped depends on these paths and no GroveVersion gate is needed.
• rewrite_aggregates loads the entire subtree and batches every node in one write, unconditionally for every aggregate tree (same shape as rewrite_heights, which only runs on demand). Within the documented O(budget + largest subtree) bound, but a streaming pass would be the natural follow-up for the bounded-memory story.
• proptest runs unseeded and its regression file is untracked; as_chunks raises the toolchain floor to 1.88 (CI is on stable).
• No book chapter covers RestoreCommitMode / the incomplete-restore marker; rustdoc only.

Verified correct

• Empty-payload binding: every "done without a chunk" path is bound (parent-bound combine_hash(·, NULL_HASH), unbound NULL_HASH, indexed members backstopped by the joint check, non-Merk cannot complete without a page).
• Indexed groups: verify_indexed_binding recomputes from the restored roots; header tags must equal the element's axes positionally; secondary tree type derived locally; empty secondary contributes NULL_HASH exactly as the write path.
• Atomic mode: exactly one commit site after the root check; every write goes through the session transaction. Marker lands in the same transaction as the first intermediate commit, in the aux CF the restore never touches.
• Session poisoning: every mutating entry point checks failed; the restorer verifies before it writes, so poisoning is bookkeeping, not corruption.
• Aggregate decomposition: exactly the four Provable* variants the producer substitutes totals for; BigSum correctly excluded; reversed-order checked_sub is the exact inverse of (own + left) + right (the i64::MAX/MIN cases pass); element-derived features match the write path including the ReferenceWithSumItem secondary rows.
• Decoders are fixed-layout with bounds checks; no untrusted-length allocation; no expect on network data left in the three replication files.
• Clean merge with develop (now 14 commits ahead); fix(merk): opaque proof nodes must stay childless (#853) #873's opaque-node test coexists; zero new clippy warnings; trunk/branch query proofs untouched (preserve_features=false).

Post-fix: grovedb-merk restore+chunk 81 passed, grovedb replication 140 passed (6 ignored measurement harnesses), clippy --lib -D warnings clean on both crates.

[QuantumExplorer]

This is Claude. Fixes for every item marked fixed in the review above are pushed to this branch as 44f1eba (fix(replication): review fixes for the V1 state sync restore).

What the commit adds, with a regression test per item that was reproduced first against d637371:

#	Fix	Test
1	element-mode restore requires value_hash(bytes) == stored value_hash for simple-valued elements	restore_with_grovedb_elements_rejects_forged_value_bytes_under_honest_value_hash (merk), state_sync_forged_element_bytes_under_honest_value_hash_rejected (grovedb, end-to-end)
2	write-time feature-family check against the tree type	restore_rejects_feature_type_family_foreign_to_the_tree_type
3	fetch_chunk request bounds (32 global ids, 32 local ids per subtree, 1 page cursor)	fetch_chunk_bounds_the_number_of_chunk_ids_per_request
4	session constructors refuse a destination carrying the incomplete-restore marker	a_marked_destination_refuses_to_start_another_restore
5	discovered_prefixes ledger gating is_sync_completed()	covered by every existing round-trip test
6	start_syncing_session* validate the version and return Result; raw new is pub(crate)	extended unsupported_versions_are_rejected_on_both_sides
7	Hash-only non-root chunk returns an error instead of panicking	test_hash_only_non_root_chunk_returns_error_not_panic

API note for drive-abci: start_syncing_session / start_syncing_session_with_mode now return Result; start_snapshot_syncing* are unchanged in shape.

Local results on 44f1eba: grovedb-merk restore+chunk 81 passed; grovedb replication 140 passed, 6 ignored (measurement harnesses); cargo clippy --lib -D warnings clean on both crates; cargo fmt clean; pre-commit hooks passed.