fix(archiver): validate checkpoint attestations from calldata before fetching blobs (A-1252)#24247
Open
spalladino wants to merge 2 commits into
Open
Conversation
…fetching blobs (A-1252)
74113a0 to
57a2f09
Compare
…s (A-1252) Replace the separate epochs_reject_invalid_attestations_from_calldata suite (which deleted blob files on disk and spun up a special late observer) with a single test reusing runInvalidationTest. The bad checkpoint is made reachable only from L1 calldata: skipCollectingAttestations makes it invalid, skipBroadcastProposals withholds the p2p proposal, skipPushProposedBlocksToArchiver denies the proposer's own archiver a local copy, and a jest spy drops every node's blob store. A proposer can then only invalidate it by rejecting from calldata before fetching the (withheld) blob.
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.
Summary
Re-scoped replacement for #24183. During L1 sync the archiver fetched and decoded a checkpoint's blob data before validating its committee attestations. A checkpoint published with invalid/insufficient attestations and malformed-but-hash-matching blob data threw
BlobDeserializationErrorduring decode before the invalid-attestation rejection path ran — so it was never recorded as rejected, the L1 sync point never advanced past it, and the archiver re-queried the same L1 blocks every poll and re-threw forever, taking any valid checkpoints in the same batch down with it.Fixes A-1252
Fix
Validate attestations from L1 calldata first. The signed consensus payload (header, archive root, fee asset price modifier) is fully available from calldata without any blob, so a checkpoint with invalid attestations (or one descending from an already-rejected ancestor) is rejected — emitting the same events and persisting the same rejected entries as before — without fetching its blob. Blobs are then fetched only for the surviving (attestation-valid, non-descendant) checkpoints.
validation.ts: addvalidateCheckpointAttestationsFromCalldataand extract the shared core validator (validateCheckpointAttestationsnow delegates to it). The calldata-builtConsensusPayloadis identical to the blob-decoded one, so no accept/reject verdict changes.l1_synchronizer.ts: reorderhandleCheckpointsto validate-then-fetch;lastSeenCheckpointis tracked from calldata since rejected checkpoints are no longer built intoPublishedCheckpoints.What changed vs #24183
This PR is narrower than #24183. It drops all of #24183's blob-failure skipping machinery (the "rows 4/5" work: the
BlobFetchOutcomesentinel, thecanPrune-gated skip, and thestopAfterBatchearly loop break).A blob fetch/decode failure for a checkpoint that has valid attestations stays fatal — it throws and propagates, rolling back the L1 sync point so the fetch is retried on the next iteration, exactly as before #24183. We do not skip such checkpoints.
Deferred to A-1260
Rather than skipping checkpoints if they are prunable, which would lead to the proposer trying to build on top of an incorrect chain tip from the rollup's point of view (since the validators' archive skips checkpoints that the rollup on L1 still considers valid until it's actually pruned), we keep the archiver stuck in them to prevent advancing past an invalid point. To ensure they get pruned, we add a code path in the sequencer so it attempts a prune even when syncing fails, piggybacking on the "vote if propose failed" path.
Tests
archiver-sync.test.ts: regression test — a checkpoint with invalid attestations and malformed blob data is rejected without aBlobDeserializationErrorretry loop, and re-polling the same L1 state is stable. Plus a test that a malformed blob with valid attestations still throws (stays fatal), and that a matching local proposed checkpoint is promoted even when its on-chain blob is malformed (blob fetch skipped).epochs_invalidate_block.parallel.test.ts: a canonical invalid-attestations checkpoint is rejected from calldata before any blob fetch — a late observer (blob withheld, promotion disabled) syncs past it without ever ingesting it, and once an honest proposer invalidates and replaces it, the on-chain archive at that number differs from the bad checkpoint's and every node progresses past it.Full
archiver-sync.test.ts(56) andvalidation.test.ts(14) pass;yarn build/format/lintclean.