feat(archiver): decode robustness against out-of-range checkpoint header/archive fields - #24204
Closed
spalladino wants to merge 7 commits into
Closed
feat(archiver): decode robustness against out-of-range checkpoint header/archive fields#24204spalladino wants to merge 7 commits into
spalladino wants to merge 7 commits into
Conversation
…der/archive fields Part of A-1254. Archiver-side defense-in-depth (Fix 1) complementing the merged L1 range checks (#24199): a checkpoint with an out-of-range field is treated as an invalid checkpoint that is skipped/invalidated, instead of throwing during decode and permanently stalling the L1 sync point. - Add a single raw-header type `L1CheckpointHeader` (plain type + free helpers in stdlib) that replaces `CheckpointHeader.fromViem`/`toViem`. It holds the viem wire shape, can carry out-of-range values without throwing, and hashes to the same value as a `CheckpointHeader` for in-range inputs (pinned by a test). - Carry the archive root as raw `Buffer32` on the whole L1-read sync path (`status`, `getCheckpointProposedEvents`, `archiveAt`), converting to `Fr` only at the single checkpoint ingestion boundary. - `CalldataRetriever` decodes a raw header + raw archive and does no validation; it verifies the propose candidate via a raw EIP-712 digest helper so decode never falls through to the trace-fallback "hash mismatch" throw for a malicious header. - Always run `validateCheckpointAttestations` (now consuming the raw header) before building a published checkpoint; reject via the existing insufficient/invalid-attestation reasons, and fail loudly only in the catastrophic case where a header is out of range yet a quorum signed it (which Fix 2 makes unreachable on a patched chain). - Widen `CheckpointInfo.archive`, `RejectedCheckpoint.archiveRoot`, and the affected events to `Fr | Buffer32`; on-disk format is unchanged. - Tests: unit tests pinning the header-hash layout and out-of-range detection per field; an archiver integration test asserting an out-of-range header is skipped and the sync point advances; and a `setStorageAt`-based e2e exercising the out-of-range archive-root read path. Co-Authored-By: Santiago Palladino <santiago@aztec-labs.com>
Base automatically changed from
mitch/gk-722-protect-fees-against-unsound-verifier-v5
to
v5-next
June 19, 2026 21:33
…ue ids Rebuild the out-of-range checkpoint-header e2e to exercise full recovery: a lone proposer lands an under-attested checkpoint, its stored archive root is corrupted to a value above the BN254 modulus via setStorageAt, and the honest validators plus an archiver-only observer keep syncing through the hardened status()/archiveAt() reads, invalidate the under-attested checkpoint, and build past it. The corruption is injected post-propose because the L1 propose path now reverts on out-of-range fields (#24199). Also strip Linear issue-id mentions from the decode-robustness comments across archiver, ethereum, stdlib, and sequencer-client, keeping the technical rationale and the #24199 PR cross-reference.
…-of-range root RollupContract.getCheckpoint eagerly converted the on-chain archive root to Fr, which throws when the stored value is outside the BN254 field. A malicious proposer can land such a value, bricking any honest node that reads the checkpoint on a sync/startup path (e.g. the tx-pool fee provider booting against the pending tip). Carry the archive as Buffer32 on the read path and compare archive roots as bytes in the prover-node publisher so a mismatching out-of-range value reports a mismatch instead of throwing. Refs #24199
… survive out-of-range roots
…under-attestation The epochs_out_of_range_header test corrupted archives[1] immediately after waitUntilCheckpointNumber(1), which only reads the on-chain checkpoint number directly from L1, not any node's archiver. The honest archivers run on an independent poll loop, so the corruption raced their ingestion of checkpoint 1. When corruption won the race, the archiver dropped checkpoint 1 on an archive-root mismatch (real calldata root != corrupted stored root) before running attestation validation, so the honest nodes never learned the pending tip was under-attested and never invalidated it; the chain wedged and the test timed out waiting for invalidation. Wait for all honest archivers to ingest checkpoint 1 and flag it as under-attested before corrupting the stored slot, closing the race deterministically while preserving the corrupted-read robustness still exercised by the observer-sync and archiveAt/status assertions.
…uffer32 unions Archive roots read from L1 are fundamentally raw bytes (a rejected checkpoint can carry an out-of-range root that cannot be an Fr), so carry them uniformly as Buffer32 on every checkpoint/rejected-checkpoint struct and event, converting to Fr only at the validated ingestion boundary. - CheckpointInfo, RejectedCheckpoint, RawCheckpointEntry, and the equivocation / descendant events now use Buffer32 instead of Fr | Buffer32. - Drop the modulus-conditional conversions: archiveFromBuffer, the block_store rehydration guards, and validation's lastArchiveRootToFr (which was silently reducing out-of-range values). - Delete the dead archiveRootToBuffer32 helper. - Narrow computeCheckpointPayloadDigest / encodeCheckpointPayloadToSign to Buffer32. - Dedup the two rejected-checkpoint records into toRejectedCheckpoint(reason) and drop a redundant toL1CheckpointHeader round-trip in the divergence check. Keep Fr | Buffer32 only on archiveRootsEqual and the rejected-checkpoint key lookups, which are polymorphic comparators/keys with genuine Fr and Buffer32 callers, not structs.
Contributor
Author
|
Closing again per request; work continues on branch spl/a-1254-archiver-robustness (now includes the Fr | Buffer32 -> Buffer32 simplification, commit c7349b4). |
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
1 participant
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.
Part of A-1254.
A malicious proposer can post a checkpoint whose header (or archive root) carries a
uint256value above the BN254 scalar field modulus. Before this change the archiver eagerly converted those L1-read values intoFrwhile decoding, which throws for an out-of-range value; the throw was uncaught and the node's L1 sync point stalled permanently (a brick). This PR is the archiver-side defense-in-depth that complements the merged L1 range checks (#24199): an out-of-range checkpoint is now treated as an invalid checkpoint that is skipped/invalidated, exactly like one with bad attestations, instead of throwing and stalling the sync point.What this fix does:
L1CheckpointHeader(plain type + free helpers instdlib) that fully replacesCheckpointHeader.fromViem/toViem. It keeps the viem wire shape, can hold out-of-range values without throwing, and hashes to the same value asCheckpointHeaderfor in-range inputs (pinned by a test).Buffer32across the whole L1-read sync path (RollupContract.status,getCheckpointProposedEvents,archiveAt), converting toFronly at the single checkpoint-ingestion boundary.CalldataRetrievernow returns a raw header + raw archive and performs no field-range validation; it verifies the propose candidate using a raw EIP-712 digest helper, so decode no longer falls through to the trace-fallback "hash mismatch" throw for a malicious header.validateCheckpointAttestations(now consuming the raw header) before building a published checkpoint; rejects via the existing insufficient/invalid-attestation reasons and advances the sync point. Fails loudly only in the catastrophic case where a header is out of range yet a committee quorum signed it — which the L1 fix makes unreachable on a patched chain.CheckpointInfo.archive,RejectedCheckpoint.archiveRoot, and the affectedL2BlockSourceevents toFr | Buffer32. The on-disk storage format is unchanged.Tests:
L1CheckpointHeaderhash byte layout againstCheckpointHeader.hash, and verifying out-of-range detection for each exploitable field at exactlyMODULUSand2^256 - 1.setStorageAt-based e2e (epochs_out_of_range_header.test.ts) that overwrites a checkpoint's archive root in L1 storage with an out-of-range value and asserts the honest node keeps syncing. This test was implemented and compiles but was not run to completion locally due to e2e runtime.The L1 range checks (#24199) is already merged into the base branch and prevents new out-of-range fields from landing via
propose. This fix here is the archiver's defense for pre-upgrade chains and future-added fields.