Stop a rollback moving chunk metadata forward

[Amaury Chamayou (achamayou)]

Stacked on #8243. Review the top commit only; the base PRs must merge first.

Why this is necessary

Store::rollback() has two paths. If the target is below the store''s version it truncates; if the target is at or beyond it, nothing local is discarded and it returns early. Both paths reset the chunker to the rollback target.

For the truncating path that is right. For the early-return path it is not, because the chunker is allowed to lag the store. An entry is assigned a version by next_version() well before Store::commit() records its size, so at any moment the chunker can legitimately be behind. Resetting it to a target at or beyond the store''s version therefore moves it forward, past entries whose sizes were never recorded.

The chunker''s counter is independent of the store''s version and only ever advances, so the offset is permanent. Every subsequent entry is then recorded against the wrong version, transaction_sizes acquires a hole, and get_unchunked_size() silently under-counts - chunk boundaries drift from what the ledger contains, and compacted_to() can prune entries that were never accounted for.

A rollback to exactly the current version is routine: it is how a view change is communicated when there is nothing to truncate.

What changes

Clamp the rollback target to the store''s own version, so a rollback can only ever move chunk metadata back:

chunker->rolled_back_to(std::min<Version>(tx_id.seqno, version));

Why this is minimal

One std::min on the non-truncating path. The truncating path is untouched - there the target is below version by construction, so the clamp would be a no-op.

The alternative, keeping the chunker eagerly in step with allocation rather than with commit, would mean recording sizes for entries that may never be replicated, which is the resurrection problem the previous PR in this stack fixes.

Performance

None. One comparison on a path taken only during a view change.

Testing

kv_test gains "A rollback never moves chunk metadata past the store''s version", covering both a rollback to exactly the current version and one beyond it, then asserting later entries are still recorded against their own version. Single-threaded and deterministic.

Verified that the test fails without the clamp - the chunker ends ahead of the store, and stays ahead - and passes with it.

Labelled run-long-test.

---

Review stack

These five PRs come from one investigation and are stacked; review and merge in order.

PR	Change
#8242	Reject stale-view writes before local commit
#8243	Order chunk metadata and snapshot scheduling with rollback
#8244	Stop a rollback moving chunk metadata forward
#8245	Guard rollback-sensitive transaction flags
#8246	Guard reserved signature side effects

All were found by an interleaving-exploration harness built over the real KV, consensus and history stack (draft #8238). The harness itself is deliberately not included here; these PRs carry only the fixes and the single-threaded regression tests that pin them.

[Copilot]

🔵 Needs a closer look

It touches subtle rollback/ledger-chunking invariants in the KV store where correctness depends on concurrency/interleaving behavior that warrants final human review.

Pull request overview

Fixes an edge case in ccf::kv::Store::rollback() where a non-truncating rollback (target at/beyond the Store’s current version) could incorrectly advance ledger chunk metadata, potentially desynchronising chunk boundaries from the actual ledger contents.

Changes:

• Clamp the chunker rollback target on the non-truncating rollback path to min(tx_id.seqno, version) so chunk metadata cannot be moved forward beyond the Store’s current version.
• Add a kv_test regression asserting that rollbacks to the current version and beyond it do not advance chunk metadata, and that subsequent entries are still chunked against their own versions.
• Add a corresponding CHANGELOG.md “Fixed” entry for this behavior change.

Custom instructions used:

• .github/copilot-instructions.md
• .github/instructions/changelog.instructions.md
• .github/instructions/reviewing.instructions.md

File summaries

File	Description
src/kv/store.h	Clamp chunker rollback target on the early-return (non-truncating) rollback path to prevent advancing chunk metadata past the Store’s version.
src/kv/test/kv_test.cpp	Add regression test covering rollbacks to current/beyond-current version and verifying chunker stays aligned.
CHANGELOG.md	Document the fix in the current release’s “Fixed” section with PR reference.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.