Skip to content

fix(sequencer): invalidate malicious yParity attestation checkpoints (A-1401)#24537

Merged
PhilWindle merged 5 commits into
merge-train/spartan-v5from
spl/a-1401-malicious-proposer-yparity
Jul 6, 2026
Merged

fix(sequencer): invalidate malicious yParity attestation checkpoints (A-1401)#24537
PhilWindle merged 5 commits into
merge-train/spartan-v5from
spl/a-1401-malicious-proposer-yparity

Conversation

@spalladino

@spalladino spalladino commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Context

A-1351 (#24515) fixed the honest-proposer and relay path for non-canonical yParity attestation signatures, but a malicious selected proposer can still hand-craft propose() calldata carrying a non-proposer signature slot with v ∈ {0,1} (or an all-zero (r,s,v) in a bitmap-marked signature slot). L1 accepts this at propose time (it only recovers the proposer's own slot) and stores the attestations hash over the raw bytes. Afterwards, either:

  • the slot recovers to the correct member, so honest nodes deem the checkpoint valid and nobody invalidates it — but epoch proving reverts at ECDSA.recover (which rejects v ∉ {27,28}), a silent stall; or
  • the slot is detected as invalid, but the invalidation repack via packAttestations is not byte-faithful, so the recomputed hash diverges from the stored one and InvalidateLib reverts — wedging the chain until prune.

This is the malicious-proposer sibling of A-1351; it survives that fix. Tracks aztec-claude#650.

Approach

TS-only — no L1 changes, since this targets a fresh deployment. Two parts:

  • DetectiongetAttestationInfoFromPayload no longer passes allowYParityAsV: true, so TS signature validity matches L1's ECDSA.recover. A non-canonical slot is now classified as an invalid attestation, so honest nodes detect the bad checkpoint instead of silently accepting it.
  • Byte-faithful invalidation — the raw packed CommitteeAttestations tuple from the checkpoint calldata is threaded verbatim (as verbatimAttestations) through the negative ValidateCheckpointResult into buildInvalidateCheckpointRequest, which submits those exact bytes rather than repacking. With the original bytes, invalidateBadAttestation reproduces the stored hash and L1's tryRecover returns address(0) ≠ member, so invalidation succeeds.

verbatimAttestations is a required field on the negative result and is serialized unconditionally — a repack fallback is intentionally absent, since silently repacking would re-introduce the wedge.

The gossip sender-recovery path (recoverCoordinationSigner) deliberately stays lenient (allowYParityAsV) so an honest node can still attribute a yParity-encoded message and canonicalize it on ingress; only the on-chain checkpoint validation is made strict. A comment documents the split so it is not accidentally unified.

Tests

An e2e regression under the invalidation suite (invalidate_block.parallel.test.ts, "proposer invalidates checkpoint with a yParity attestation slot") demonstrates the attack red→green: a malicious proposer lands a checkpoint whose non-proposer attestation slots carry raw yParity bytes, and the honest next proposer invalidates it — which only succeeds when the invalidation submits the verbatim calldata bytes (a repack diverges from the on-chain attestationsHash and reverts). Plus unit coverage for the strict attestation-info classification, the byte-faithful passthrough, and the result round-trip.

An attester-side variant (a committee member emitting yParity attestations) is not separately tested: the honest proposer's orderAttestations normalization (A-1351) canonicalizes the byte before the L1 bundle, so it never reaches L1 and produces the same on-chain outcome as — and is subsumed by — the malicious-proposer case above.

Fixes A-1401

…ty/all-zero attestation slots (A-1401)

A malicious selected proposer can hand-craft propose() calldata with a non-proposer
attestation slot carrying a yParity recovery byte (v in {0,1}) or an all-zero (r,s,v)
in a bitmap signature slot. L1 accepts it at propose time, but the checkpoint can
never be proven (ValidatorSelectionLib.verifyAttestations -> ECDSA.recover rejects
v not in {27,28}) nor automatically invalidated, because the invalidation evidence was
rebuilt via packAttestations, which is not byte-faithful to the stored tuple, so its
keccak256 diverged from the on-chain attestationsHash and InvalidateLib reverted.

Detection: getAttestationInfoFromPayload now recovers with default opts (no
allowYParityAsV), so a slot with v not in {27,28} is judged invalid off-chain, matching
L1 proving, and is routed to invalid-attestation. The gossip receipt path
(recoverCoordinationSigner) stays lenient; A-1351 normalizes on pool ingress.

Remediation: thread the exact packed CommitteeAttestations tuple from L1 calldata
(calldata_retriever -> data_retrieval -> validation) onto the negative
ValidateCheckpointResult, and use it verbatim in buildInvalidateCheckpointRequest. The
builder throws rather than repacking if the tuple is missing, so the wedge can never be
silently reintroduced. packedAttestations is a required field, serialized and schema-
validated alongside the rest of the result.

Adds test-only injection knobs (injectYParityAttestation on the proposer,
injectYParityOwnAttestation on the attester) and YParityCommitteeAttestationsAndSigners
to exercise both the proposer- and attester-side vectors end to end.
…ehavior

On the A-1401 branch, reverting A-1351 normalization makes A-1401's own detection invalidate the non-canonical checkpoint (proven tip stays at 0) rather than the proof reverting at ECDSA.recover; both keep the epoch unproven.
… L1 validation

recoverCoordinationSigner keeps allowYParityAsV deliberately, so A-1351's
accept-and-normalize path can attribute yParity-encoded gossip before
canonicalizing it. Document that this is intentionally looser than the strict
on-chain checkpoint validation (getAttestationInfoFromPayload) so a future
consistency refactor does not reopen A-1351 or A-1401.
- rename the verbatim-tuple field to verbatimAttestations to make its purpose clear
- move the test-only blob-path validateCheckpointAttestations helper into its test,
  exporting the shared validateAttestations core from the archiver validation module
- flip every non-proposer signed slot to yParity in the malicious-proposer model
  (rename YParity... to MaliciousYParityCommitteeAttestationsAndSigners, keyed on the
  proposer's own slot so propose() still passes verifyProposer)
- drop the dead missing-tuple guard in buildInvalidateCheckpointRequest (field is required)
- remove plan-relative "Part A/Part B" references from comments
…injection

The prover-backed yparity_attestation_proving test only showed invalidation in
the red state (A-1351 reverted): with the honest proposer's orderAttestations
normalization in place, a committee member's yParity recovery byte is
canonicalized before the L1 bundle and never reaches L1, so there is nothing to
invalidate. The malicious-proposer case (injectYParityAttestation, covered by
invalidate_block Test 2) already lands a bad-v attestation on L1 and has the next
proposer invalidate it -- the same on-chain outcome. Remove the test and the now
orphaned attester-side injectYParityOwnAttestation config flag.
@PhilWindle PhilWindle merged commit 5e0e53f into merge-train/spartan-v5 Jul 6, 2026
15 checks passed
@PhilWindle PhilWindle deleted the spl/a-1401-malicious-proposer-yparity branch July 6, 2026 16:46
spalladino pushed a commit that referenced this pull request Jul 6, 2026
…test (#24543)

## Flake

`multi-node/block-production/multi_validator_node.parallel.test.ts`,
case "should build blocks & attest with multiple validator keys", failed
on [#24537](#24537)
(unrelated PR) with:

```
TypeError: Cannot read properties of undefined (reading 'checkpoint')
    at checkpoint (multi-node/block-production/multi_validator_node.parallel.test.ts:92:73)
```

CI log: http://ci.aztec-labs.com/0f321ffb9fa923af

## Root cause

`deployContractAndGetAttestedCheckpoint` reads the published checkpoint
from the archiver immediately after the deploy tx receipt turns mined:

```ts
const [publishedCheckpoint] = await dataStore.getCheckpoints({ from: blockData!.checkpointNumber, limit: 1 });
const payload = ConsensusPayload.fromCheckpoint(publishedCheckpoint.checkpoint, ...); // undefined here
```

The receipt turns mined as soon as the block is built locally, but the
archiver only stores the *published* checkpoint (with its attestations,
which come from the L1 `propose` calldata) once its L1 sync downloads
that tx. In the window between the two, `getCheckpoints` returns `[]`
and `publishedCheckpoint` is `undefined`.

The CI log shows the race directly: the test node published checkpoint 1
to L1 at `15:17:57.694` (`sequencer:publisher ... Published checkpoint 1
at slot 14`), and its archiver logged `Downloaded checkpoint 1` at
`15:17:57.795` — by which point the test had already failed and teardown
was running.

This is a test-side race, not a production bug: the archiver API is
behaving as documented for a not-yet-synced checkpoint.

## Fix

Poll for the checkpoint with `retryUntil` instead of reading it once,
mirroring the existing pattern in
`single-node/cross-chain/cross_chain_messaging_test.ts`
(`advanceToEpochProven`), which wraps the same lookup in
`retryUntil(..., 'archiver indexes checkpoint N', 120, 0.5)`.

Note: the `next`-line variant of this test
(`e2e_multi_validator/e2e_multi_validator_node.test.ts`) has the same
unguarded lookup in two places and could flake the same way; that file
is on a different base branch, so it is not touched here.

Based on `merge-train/spartan-v5` because the flaking file only exists
on the v5 line (it was consolidated into `multi-node/` there), and the
failing run was on a PR targeting that branch.

---
*Created by
[claudebox](https://claudebox.work/v2/sessions/0959caa9f03f59fa) ·
group: `slackbot`*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants