Don't treat a block the storage never persisted as committed

[claude]

Problem

CallbackStorage.Index returned nil — success — for a block it deliberately did not persist: a Telock, the block built after the sealing block purely to extend the dying epoch until that sealing block finalizes. Telocks are never indexed.

A nil error from Storage.Index is the consensus engine's only signal that a block is durable at Storage.NumBlocks()-1, so Epoch.indexFinalization treated the skip as a real commit: it set e.lastBlock to the block and advanced e.round via progressRoundsDueToCommit, while nextSeqToCommit() — which is Storage.NumBlocks() — stayed behind.

The two cursors can never reconcile afterwards. The node:

• builds proposals on a phantom parent (metadata() prefers e.lastBlock), which honest nodes reject;
• advertises the never-persisted block to replicating peers as its latest finalized sequence (handleReplicationRequest, which is exempt from post-seal send suppression);
• rejects the proposal that does belong at that sequence, because its round already advanced.

Two ways in:

1. No attacker needed, in the dying epoch. A Telock legitimately collects a finalization whenever the sealing block's finalization lags — that is exactly what Telocks are for. If the Telock finalization for round R+1 is stored before the sealing finalization for round R is processed, the indexFinalizations loop commits the sealing block and then phantom-commits the Telock in the same iteration.
2. Replay, in the new epoch. Old-epoch artifacts reached the commit path with no epoch binding. VerifyQC only attests that the signers form a quorum of the current validator set, which a quorum of the previous epoch still satisfies whenever the sets overlap, and the replication verification path uses OnlyVMVerifyOpt, which accepts a Telock vacuously (its InnerBlock is nil). A replayed Telock chains onto the last block of its epoch, so it claims the very round and sequence the first block of the new epoch belongs to. Its finalization also occupies rounds[R+1], which blocks the real finalization for that round (storeFinalization: "already has a finalization") and so blocks the healing path.

If a quorum is poisoned during the transition window, no real block at seq s+1 is ever finalized and indexFinalizations can never commit again — a halt that survives restarts, since no node holds a committable block at the stuck sequence.

Fix

The skip is no longer reported as success

• common.ErrBlockNotIndexed is added and the Storage.Index contract is documented: a nil error means the block is stored at its sequence; an implementation that intentionally does not persist a block must say so.
• CallbackStorage.Index returns it for a Telock. The block is deliberately left in the CachedStorage cache — a Telock must stay retrievable by digest while its epoch is being extended.

The engine no longer commits state for a block that is not in storage

• indexFinalization reports whether the block was committed, and leaves e.lastBlock and the round untouched when it was not — both for a reported skip and, defensively, for a storage that reports success without advancing NumBlocks().
• indexFinalizations stops the commit loop and leaves the round in place, so the block that does belong at that sequence can still be committed.
• The replication path restores the rounds map to its previous state, so a declined block neither looks finalized to peers nor blocks the real block for that round.
• The non-validator no longer halts, nor treats the sequence as accepted, when the storage declines to persist a block.

Old-epoch artifacts are rejected before they reach the commit path

• verifyQuorumRound rejects quorum rounds whose block, or whose finalization, belongs to an earlier epoch; handleFinalizationMessage rejects such finalizations too. Every sequence an epoch still has to commit belongs to that epoch or a later one, so this cannot reject anything a lagging node needs — a node catches up epoch by epoch, sealing and transitioning as it commits each sealing block.
• A sealed epoch stops accepting finalization messages: nothing beyond its sealing block belongs to it, and it already refuses to verify or build blocks.

Testing

go test -race ./... passes, with no new ERR/WARN output.

New regression tests, each verified to fail against the unpatched code:

• TestCallbackStorageReportsTelockSkip — the adapter reports the skip rather than returning nil, does not persist the Telock, does not run the post-index callback, and keeps the Telock retrievable by digest.
• TestEpochDoesNotCommitBlockTheStorageDeclinedToIndex — with a storage that declines the seq-2 block, both when it reports the skip and when it hides it behind a nil error: nothing at or after that sequence is committed, the round does not advance past it, and a replicating peer is told the last committed block is the latest finalized sequence. (The nil-error case reproduces the original desync: the round advanced past an uncommitted block.)
• TestEpochRejectsFinalizationsFromPreviousEpochs — replaying a previous-epoch block that claims the current epoch's first sequence, through a notarized quorum round, a finalized quorum round, a proposal, and a finalization message, leaves storage, the commit cursor and the round unchanged.

Follow-up after adversarial review

A fresh-context review found that refusing to commit is not enough on its own — whatever the engine still holds for that sequence has to go too, or it takes the place of the block that does belong there:

• The non-validator would have halted permanently. The first cut returned early from newFinalizedBlockTask, skipping removeOldSequencesAndEpochs — the only thing that ever clears an uncommitted entry from incompleteSequences. handleBlock then drops the real block for that sequence as a duplicate, and handleFinalization treats its finalization as conflicting and sets haltedError. Ironically the pre-fix false commit cleared that state as a side effect, so the first cut turned a self-healing state into a halt. It now drops the entry and re-requests the sequence.
• indexFinalizations kept the round. That let the refused block be served to replicating peers as a finalized quorum round for its sequence (locateQuorumRecord), be used as the parent of the next proposal (metadata via getHighestRound), and make storeProposal refuse the real block for that round. It now drops the round, as the replication path already did — and the test asserts the replication response never carries it.
• The replication path now re-requests the sequence, since the caller removes it from the replication state before handing it over.

Two review points I checked and did not act on:

• The reviewer believed the notarized replication path used full MSM verification, making the block-level epoch check redundant. It does not — createNotarizedBlockVerificationTask also verifies with OnlyVMVerifyOpt, so the check is load-bearing and stays.
• setMetadataFromRecords can lower e.Epoch back to the previous epoch when a WAL segment holding post-sealing-block rounds survives garbage collection, which would disable the epoch checks after a crash-restart-after-seal. That is a separate pre-existing bug (it also mis-derives e.round and restores stale rounds, so such a node already rejects every real proposal); the storage-level guard still holds there, which is why it is the boundary and the epoch checks are documented as a layer above it. Flagged rather than half-fixed here.

New tests, each verified to fail without its corresponding fix:

• TestNonValidatorRecoversFromBlockTheStorageDeclinedToIndex — the refused block is dropped rather than held, and the block that belongs at that sequence is still committed without halting.
• TestEpochDoesNotCommitBlockTheStorageDeclinedToIndex now also asserts that no replication response serves the refused block as finalized.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GpZGbLPZnb9sV7EVcYi3dB

---

Generated by Claude Code