fix(archiver): fully clean up removed blocks whose txs were re-included elsewhere#24732
Closed
benesjan wants to merge 1 commit into
Closed
fix(archiver): fully clean up removed blocks whose txs were re-included elsewhere#24732benesjan wants to merge 1 commit into
benesjan wants to merge 1 commit into
Conversation
…ed elsewhere BlockStore.removeBlocksAfter had two defects that corrupt the tx-effect index (txHash -> block position) when the same tx exists in two stored blocks, e.g. re-included after its original proposal expired: - deleteBlock removed #txEffects entries blindly by txHash, destroying the entry of a tx whose index already points at another stored block, which makes that block unreadable. - removeBlocksAfter skipped cleanup entirely for blocks it could not reconstruct, leaking their row, tx effects, and indices; a later insert at the same number then overwrites the row in place, leaving stale tx-effect entries pointing into the new chain. A stale entry makes getL2ToL1MembershipWitness resolve the tx at wrong coordinates and throw 'The L2ToL1Message you are trying to prove inclusion of does not exist' for a message in a proven block, and lets getTxReceipt report a proven position the chain no longer has. Cleanup now works from the raw storage row (so unreadable blocks are still fully released) and only deletes tx-effect entries still owned by the block being removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
Closed in favor of #24765 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
BlockStore's tx-effect index (txHash → owning block hash/position) can be corrupted when the same tx exists in two stored blocks at once — routine when a checkpoint proposal expires and the sequencer re-includes its txs:deleteBlockremoved#txEffectsentries by txHash without checking the entry still points at the block being deleted. Inserting the re-included block repoints the shared tx's entry at the new block; deleting the old block then destroys the new block's entry, making that block unreadable (Could not find tx effect for tx…).removeBlocksAfterdidgetBlock(bn) === undefined → warn + continue, so an unreadable block's row, tx effects, and indices were never released.addCheckpointslater overwrites the leaked row in place ("L1 data is authoritative"), leaving stale tx-effect entries pointing at coordinates now occupied by different txs.Downstream symptoms of a corrupted index:
getTxReceiptreporting positions the chain no longer has, unreadable blocks being silently dropped fromgetBlocks/getCheckpointsresponses (consumers see a checkpoint with missing blocks and no error), andgetL2ToL1MembershipWitnessresolving the wrong tx and throwingThe L2ToL1Message you are trying to prove inclusion of does not existfor a message that is on-chain and proven.Fix
removeBlocksAfternow cleans up from the raw storage row, so blocks whose bodies can no longer be fully loaded still release their row, tx effects (enumerated from#blockTxs), and hash/archive indices.deleteBlockonly deletes a tx-effect entry that is still owned by the block being removed (compares the entry's leading block hash), so a re-included tx's index survives the removal of its old block.The regression test adds two proposed blocks sharing a tx effect and asserts
removeBlocksAfterremoves both and leaves no residue. Before this change it fails: block 1 is removed, block 2 becomes unreadable and leaks.Verification
block_store.test.ts(fails on the previous implementation, passes now).Related observation while debugging downstream (can file separately if useful): during reorg ingestion there is a read window where
getCheckpoints(n, …, { includeBlocks: true })returns a checkpoint whose blocks are missing or empty with no warning — unreadable or not-yet-visible blocks are silently filtered out. Consumers that treat the returned payload as complete lose data; a loud error or a retryable signal would be safer.🤖 Generated with Claude Code