From f45c85edd049845c8bcf36fd5aa9a95b305de7e6 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Mon, 6 Jul 2026 10:46:16 -0300 Subject: [PATCH 1/5] fix(sequencer): detect and byte-faithfully invalidate malicious yParity/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. --- .../src/l1/calldata_retriever.test.ts | 1 + .../archiver/src/l1/calldata_retriever.ts | 7 ++ .../archiver/src/l1/data_retrieval.ts | 6 ++ .../archiver/src/modules/validation.test.ts | 24 +++++ .../archiver/src/modules/validation.ts | 29 ++++- .../archiver/src/store/block_store.test.ts | 4 + .../node_public_calls_simulator.test.ts | 1 + .../invalidate_block.parallel.test.ts | 13 +++ ...arity_attestation_proving.parallel.test.ts | 101 +++++++++++++++++ yarn-project/sequencer-client/src/config.ts | 5 + .../l1_publisher.integration.test.ts | 1 + .../src/publisher/sequencer-publisher.test.ts | 58 +++++++++- .../src/publisher/sequencer-publisher.ts | 11 +- .../sequencer/checkpoint_proposal_job.test.ts | 3 + .../src/sequencer/checkpoint_proposal_job.ts | 20 ++++ .../src/sequencer/sequencer.test.ts | 2 + .../attestations_block_watcher.test.ts | 6 ++ .../stdlib/src/block/attestation_info.test.ts | 33 ++++++ .../stdlib/src/block/attestation_info.ts | 9 +- .../proposal/attestations_and_signers.test.ts | 102 +++++++++++++++++- .../proposal/attestations_and_signers.ts | 47 +++++++- .../src/block/validate_block_result.test.ts | 30 ++++++ .../stdlib/src/block/validate_block_result.ts | 50 ++++++++- yarn-project/stdlib/src/interfaces/configs.ts | 3 + .../stdlib/src/interfaces/validator.ts | 7 ++ yarn-project/validator-client/src/config.ts | 4 + .../validator-client/src/validator.ts | 25 ++++- 27 files changed, 588 insertions(+), 14 deletions(-) create mode 100644 yarn-project/end-to-end/src/multi-node/invalid-attestations/yparity_attestation_proving.parallel.test.ts create mode 100644 yarn-project/stdlib/src/block/attestation_info.test.ts diff --git a/yarn-project/archiver/src/l1/calldata_retriever.test.ts b/yarn-project/archiver/src/l1/calldata_retriever.test.ts index 3f6e43fba6f4..624b50592324 100644 --- a/yarn-project/archiver/src/l1/calldata_retriever.test.ts +++ b/yarn-project/archiver/src/l1/calldata_retriever.test.ts @@ -505,6 +505,7 @@ describe('CalldataRetriever', () => { archiveRoot: Fr.random(), header: CheckpointHeader.random(), attestations: [], + packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, blockHash, feeAssetPriceModifier: 0n, }; diff --git a/yarn-project/archiver/src/l1/calldata_retriever.ts b/yarn-project/archiver/src/l1/calldata_retriever.ts index 0c2f79f74a7f..6d435ebfa2df 100644 --- a/yarn-project/archiver/src/l1/calldata_retriever.ts +++ b/yarn-project/archiver/src/l1/calldata_retriever.ts @@ -34,6 +34,12 @@ type CheckpointData = { archiveRoot: Fr; header: CheckpointHeader; attestations: CommitteeAttestation[]; + /** + * The exact packed `CommitteeAttestations` tuple as it appears in the propose calldata, preserved + * verbatim (never re-derived from {@link attestations}) so invalidation evidence stays byte-faithful to + * the on-chain `attestationsHash`. + */ + packedAttestations: ViemCommitteeAttestations; blockHash: string; feeAssetPriceModifier: bigint; }; @@ -458,6 +464,7 @@ export class CalldataRetriever { archiveRoot, header, attestations, + packedAttestations, blockHash, feeAssetPriceModifier, }; diff --git a/yarn-project/archiver/src/l1/data_retrieval.ts b/yarn-project/archiver/src/l1/data_retrieval.ts index bc27eb545fbc..c5203276b17c 100644 --- a/yarn-project/archiver/src/l1/data_retrieval.ts +++ b/yarn-project/archiver/src/l1/data_retrieval.ts @@ -12,6 +12,7 @@ import type { InboxContract, MessageSentLog, RollupContract, + ViemCommitteeAttestations, ViemHeader, } from '@aztec/ethereum/contracts'; import type { ViemPublicClient, ViemPublicDebugClient } from '@aztec/ethereum/types'; @@ -56,6 +57,11 @@ export type RetrievedCheckpointFromCalldata = RetrievedCheckpointBase & { blobHashes: Buffer[]; /** Parent beacon block root from the L1 block, used for blob fetching. */ parentBeaconBlockRoot: string | undefined; + /** + * The exact packed `CommitteeAttestations` tuple from the propose calldata, carried verbatim so that + * attestation validation can attach byte-faithful invalidation evidence to a negative result. + */ + packedAttestations: ViemCommitteeAttestations; }; export async function retrievedToPublishedCheckpoint({ diff --git a/yarn-project/archiver/src/modules/validation.test.ts b/yarn-project/archiver/src/modules/validation.test.ts index 50fa30fc5efa..4c49d9cdf2eb 100644 --- a/yarn-project/archiver/src/modules/validation.test.ts +++ b/yarn-project/archiver/src/modules/validation.test.ts @@ -232,6 +232,30 @@ describe('validateCheckpointAttestations', () => { expect(result.invalidIndex).toBe(2); }); + it('fails if an attestation is in yParity (v in {0, 1}) form even though it recovers to the right member', async () => { + const checkpoint = await makeCheckpoint(signers.slice(0, 4), committee); + + // Rewrite index 2's canonical signature to its yParity form: same (r, s), recovery byte 0/1. It still + // recovers to the same committee member off-chain, but L1 ECDSA.recover rejects v not in {27, 28}. + const original = checkpoint.attestations[2]; + const yParity = new Signature(original.signature.r, original.signature.s, original.signature.v - 27); + checkpoint.attestations[2] = new CommitteeAttestation(original.address, yParity); + + const attestations = getAttestationInfoFromPublishedCheckpoint(checkpoint, TEST_COORDINATION_SIGNATURE_CONTEXT); + expect(attestations[2].status).toBe('invalid-signature'); + + const result = await validateCheckpointAttestations( + checkpoint, + epochCache, + constants, + TEST_COORDINATION_SIGNATURE_CONTEXT, + logger, + ); + assert(!result.valid); + assert(result.reason === 'invalid-attestation'); + expect(result.invalidIndex).toBe(2); + }); + it('reports correct index when invalid attestation follows provided address', async () => { const checkpoint = await makeCheckpoint(signers.slice(0, 3), committee); diff --git a/yarn-project/archiver/src/modules/validation.ts b/yarn-project/archiver/src/modules/validation.ts index 529dceea8ebf..1204ed87e9f8 100644 --- a/yarn-project/archiver/src/modules/validation.ts +++ b/yarn-project/archiver/src/modules/validation.ts @@ -1,4 +1,5 @@ import type { EpochCache } from '@aztec/epoch-cache'; +import type { ViemCommitteeAttestations } from '@aztec/ethereum/contracts'; import { type CheckpointNumber, EpochNumber } from '@aztec/foundation/branded-types'; import { compactArray } from '@aztec/foundation/collection'; import type { Fr } from '@aztec/foundation/curves/bn254'; @@ -6,6 +7,7 @@ import type { Logger } from '@aztec/foundation/log'; import { type AttestationInfo, type CommitteeAttestation, + CommitteeAttestationsAndSigners, type ValidateCheckpointNegativeResult, type ValidateCheckpointResult, getAttestationInfoFromPayload, @@ -42,7 +44,18 @@ export function validateCheckpointAttestations( ): Promise { const { checkpoint, attestations } = publishedCheckpoint; const payload = ConsensusPayload.fromCheckpoint(checkpoint, signatureContext); - return validateAttestations(payload, attestations, checkpoint.toCheckpointInfo(), epochCache, constants, logger); + // The blob path has no production callers (invalidation always originates from the calldata path), so a + // repack is acceptable here to satisfy the required packedAttestations field on a negative result. + const packedAttestations = CommitteeAttestationsAndSigners.packAttestations(attestations); + return validateAttestations( + payload, + attestations, + packedAttestations, + checkpoint.toCheckpointInfo(), + epochCache, + constants, + logger, + ); } /** The subset of a calldata-only checkpoint needed to validate its committee attestations. */ @@ -52,6 +65,8 @@ export type CalldataCheckpointForAttestations = { feeAssetPriceModifier: bigint; header: CheckpointHeader; attestations: CommitteeAttestation[]; + /** The exact packed attestations tuple from L1 calldata, carried verbatim for byte-faithful invalidation. */ + packedAttestations: ViemCommitteeAttestations; }; /** @@ -80,7 +95,15 @@ export function validateCheckpointAttestationsFromCalldata( checkpointNumber: checkpoint.checkpointNumber, timestamp: checkpoint.header.timestamp, }; - return validateAttestations(payload, checkpoint.attestations, checkpointInfo, epochCache, constants, logger); + return validateAttestations( + payload, + checkpoint.attestations, + checkpoint.packedAttestations, + checkpointInfo, + epochCache, + constants, + logger, + ); } /** @@ -91,6 +114,7 @@ export function validateCheckpointAttestationsFromCalldata( async function validateAttestations( payload: ConsensusPayload, attestations: CommitteeAttestation[], + packedAttestations: ViemCommitteeAttestations, checkpointInfo: CheckpointInfo, epochCache: EpochCache, constants: Pick, @@ -137,6 +161,7 @@ async function validateAttestations( epoch, attestors, attestations, + packedAttestations, }); for (let i = 0; i < attestorInfos.length; i++) { diff --git a/yarn-project/archiver/src/store/block_store.test.ts b/yarn-project/archiver/src/store/block_store.test.ts index 39b7efebc43e..175553eb67fe 100644 --- a/yarn-project/archiver/src/store/block_store.test.ts +++ b/yarn-project/archiver/src/store/block_store.test.ts @@ -1989,6 +1989,7 @@ describe('BlockStore', () => { seed: 456n, attestors: [EthAddress.random()], attestations: [CommitteeAttestation.random()], + packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, reason: 'insufficient-attestations', }; @@ -2007,6 +2008,7 @@ describe('BlockStore', () => { epoch: EpochNumber(789), seed: 101n, attestations: [CommitteeAttestation.random()], + packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, reason: 'invalid-attestation', invalidIndex: 5, }; @@ -2027,6 +2029,7 @@ describe('BlockStore', () => { seed: 888n, attestors: [EthAddress.random()], attestations: [CommitteeAttestation.random()], + packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, reason: 'insufficient-attestations', }; @@ -2046,6 +2049,7 @@ describe('BlockStore', () => { seed: 0n, attestors: [], attestations: [], + packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, reason: 'insufficient-attestations', }; diff --git a/yarn-project/aztec-node/src/aztec-node/node_public_calls_simulator.test.ts b/yarn-project/aztec-node/src/aztec-node/node_public_calls_simulator.test.ts index 618d90163c1e..402eea9b1860 100644 --- a/yarn-project/aztec-node/src/aztec-node/node_public_calls_simulator.test.ts +++ b/yarn-project/aztec-node/src/aztec-node/node_public_calls_simulator.test.ts @@ -452,6 +452,7 @@ function makeInvalidStatus(firstInvalid: CheckpointNumber): ValidateCheckpointRe seed: 0n, attestors: [], attestations: [], + packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, reason: 'insufficient-attestations', }; } diff --git a/yarn-project/end-to-end/src/multi-node/invalid-attestations/invalidate_block.parallel.test.ts b/yarn-project/end-to-end/src/multi-node/invalid-attestations/invalidate_block.parallel.test.ts index 76d2e0b73db2..4fdfca2c8517 100644 --- a/yarn-project/end-to-end/src/multi-node/invalid-attestations/invalidate_block.parallel.test.ts +++ b/yarn-project/end-to-end/src/multi-node/invalid-attestations/invalidate_block.parallel.test.ts @@ -818,6 +818,19 @@ describe('multi-node/invalid-attestations/invalidate_block', () => { }); }); + // Injects a non-proposer attestation slot in yParity (v ∈ {0, 1}) form in the packed L1 tuple. L1 accepts + // it at propose() (attestation signatures are not verified there), but the checkpoint can never be proven + // (ValidatorSelectionLib.verifyAttestations → ECDSA.recover rejects v ∉ {27, 28}). A repack of the + // invalidation evidence would canonicalize the byte back to 27/28 and diverge from the on-chain + // attestationsHash, reverting the invalidation. Verifies a good proposer detects it (Part A) and + // invalidates it byte-faithfully from the raw calldata (Part B). Regression for A-1401. + it('proposer invalidates checkpoint with a yParity attestation slot', async () => { + await runInvalidationTest({ + attackConfig: { injectYParityAttestation: true }, + disableConfig: { injectYParityAttestation: false }, + }); + }); + // Injects shuffled attestation ordering (accepted offchain but rejected by L1 which requires the // committee order). Waits for the bad checkpoint then verifies a good proposer invalidates it. // Regression for the node accepting attestations that did not conform to the committee order, diff --git a/yarn-project/end-to-end/src/multi-node/invalid-attestations/yparity_attestation_proving.parallel.test.ts b/yarn-project/end-to-end/src/multi-node/invalid-attestations/yparity_attestation_proving.parallel.test.ts new file mode 100644 index 000000000000..7dc0ab157fca --- /dev/null +++ b/yarn-project/end-to-end/src/multi-node/invalid-attestations/yparity_attestation_proving.parallel.test.ts @@ -0,0 +1,101 @@ +import type { AztecNodeService } from '@aztec/aztec-node'; +import type { Logger } from '@aztec/aztec.js/log'; +import type { Delayer } from '@aztec/ethereum/l1-tx-utils'; +import { asyncMap } from '@aztec/foundation/async-map'; +import { CheckpointNumber } from '@aztec/foundation/branded-types'; +import { retryUntil } from '@aztec/foundation/retry'; + +import { jest } from '@jest/globals'; + +import type { EndToEndContext } from '../../fixtures/utils.js'; +import { + MOCK_GOSSIP_MULTI_VALIDATOR_OPTS, + MULTI_VALIDATOR_BLOCK_PRODUCTION_TIMING, + MultiNodeTestContext, + type RegisteredValidator, + buildMockGossipValidators, +} from '../multi_node_test_context.js'; + +jest.setTimeout(1000 * 60 * 10); + +const NODE_COUNT = 3; + +// A committee member that emits its own attestations in yParity (v ∈ {0, 1}) form models the A-1351 +// attester-side DoS. Without normalization the non-canonical recovery byte reaches L1 in the honest +// proposer's bundle and epoch proving reverts (ValidatorSelectionLib.verifyAttestations → ECDSA.recover +// rejects v ∉ {27, 28}), so the on-chain proven tip stalls. With A-1351 applied the honest proposer +// canonicalizes the byte on pool ingress and again in orderAttestations before the L1 bundle, so the +// checkpoint lands canonical, the epoch proves, and the proven tip advances. +// +// This is a prover-backed regression: the base invalidation suite starts no prover node and A-1351 +// produces no on-chain event, so proven-tip advancement is the only observable signal. Modelled on +// block-production/proof_boundary.parallel.test.ts (createProverNode + Delayer). To reproduce the red +// state, revert the three A-1351 normalization layers (orderAttestations, the attestation pool ingress, +// CheckpointAttestation.withNormalizedSignature) and the packAttestations v-canonicalization: the proof +// tx is still sent but reverts on L1 and the proven tip stays at 0. +describe('multi-node/invalid-attestations/yparity_attestation_proving', () => { + let context: EndToEndContext; + let logger: Logger; + let test: MultiNodeTestContext; + let validators: RegisteredValidator[]; + let proverNode: AztecNodeService; + + afterEach(async () => { + jest.restoreAllMocks(); + await test?.teardown(); + }); + + it('proves an epoch when a committee member emits yParity attestations', async () => { + validators = buildMockGossipValidators(NODE_COUNT); + test = await MultiNodeTestContext.setup({ + ...MOCK_GOSSIP_MULTI_VALIDATOR_OPTS, + ...MULTI_VALIDATOR_BLOCK_PRODUCTION_TIMING, + initialValidators: validators, + aztecProofSubmissionEpochs: 1, + }); + ({ context, logger } = test); + + // node 0 is the malicious committee member (rewrites its own attestations to yParity form); the rest + // are honest. One malicious member out of three still leaves quorum, so checkpoints keep landing. + logger.warn(`Starting ${NODE_COUNT} validator nodes (node 0 emits yParity attestations)`); + await asyncMap(validators, ({ privateKey }, i) => + test.createValidatorNode([privateKey], { + minTxsPerBlock: 0, + maxTxsPerBlock: 1, + injectYParityOwnAttestation: i === 0, + }), + ); + + proverNode = await test.createProverNode({ cancelTxOnTimeout: false, maxSpeedUpAttempts: 0, dontStart: true }); + context.proverNode = proverNode; + await proverNode.getProverNode()!.start(); + const proverDelayer: Delayer = proverNode.getProverNode()!.getDelayer()!; + + // Let the chain build the first checkpoint, produced while the malicious member is gossiping. + await test.waitUntilCheckpointNumber(CheckpointNumber(1), test.L2_SLOT_DURATION_IN_S * 6); + + // Advance the L1 clock by a full epoch so epoch 0 becomes provable, keeping the interval miner running. + const block = await test.l1Client.getBlock({ includeTransactions: false }); + const warpTo = Number(block.timestamp) + test.epochDuration * test.L2_SLOT_DURATION_IN_S; + logger.warn(`Warping L1 to ${warpTo} to make epoch 0 provable`); + await test.context.cheatCodes.eth.warp(warpTo, { resetBlockInterval: true }); + + // The prover must submit a proof AND the on-chain proven tip must advance past genesis. In the red + // state (A-1351 reverted) the proof tx is still sent but reverts, so the proven tip stays at 0. + await retryUntil( + async () => { + await test.monitor.run(true); + const provenNumber = await test.rollup.getProvenCheckpointNumber(); + return proverDelayer.getSentTxHashes().length > 0 && Number(provenNumber) >= 1; + }, + 'prover advances L1 proven checkpoint despite yParity attestations', + test.L2_SLOT_DURATION_IN_S * 16, + 1, + ); + + expect(proverDelayer.getSentTxHashes().length).toBeGreaterThan(0); + expect(Number(await test.rollup.getProvenCheckpointNumber())).toBeGreaterThanOrEqual(1); + + logger.warn(`Test succeeded '${expect.getState().currentTestName}'`); + }); +}); diff --git a/yarn-project/sequencer-client/src/config.ts b/yarn-project/sequencer-client/src/config.ts index 2dd82b2b0d1a..4601145119ba 100644 --- a/yarn-project/sequencer-client/src/config.ts +++ b/yarn-project/sequencer-client/src/config.ts @@ -62,6 +62,7 @@ export const DefaultSequencerConfig = { injectFakeAttestation: false, injectHighSValueAttestation: false, injectUnrecoverableSignatureAttestation: false, + injectYParityAttestation: false, fishermanMode: false, shuffleAttestationOrdering: false, skipPushProposedBlocksToArchiver: false, @@ -225,6 +226,10 @@ export const sequencerConfigMappings: ConfigMappingsType = { description: 'Inject an attestation with an unrecoverable signature (for testing only)', ...booleanConfigHelper(DefaultSequencerConfig.injectUnrecoverableSignatureAttestation), }, + injectYParityAttestation: { + description: 'Inject a non-proposer attestation slot in yParity form in the packed L1 tuple (for testing only)', + ...booleanConfigHelper(DefaultSequencerConfig.injectYParityAttestation), + }, fishermanMode: { env: 'FISHERMAN_MODE', description: diff --git a/yarn-project/sequencer-client/src/publisher/l1_publisher.integration.test.ts b/yarn-project/sequencer-client/src/publisher/l1_publisher.integration.test.ts index 3f861bb270c6..bbca7125e7ae 100644 --- a/yarn-project/sequencer-client/src/publisher/l1_publisher.integration.test.ts +++ b/yarn-project/sequencer-client/src/publisher/l1_publisher.integration.test.ts @@ -893,6 +893,7 @@ describe('L1Publisher integration', () => { checkpoint: checkpoint.toCheckpointInfo(), attestors: [], attestations: badAttestations, + packedAttestations: CommitteeAttestationsAndSigners.packAttestations(badAttestations), epoch: EpochNumber(1), seed: 1n, reason: 'insufficient-attestations', diff --git a/yarn-project/sequencer-client/src/publisher/sequencer-publisher.test.ts b/yarn-project/sequencer-client/src/publisher/sequencer-publisher.test.ts index a6857f87c304..1b162a02ffc2 100644 --- a/yarn-project/sequencer-client/src/publisher/sequencer-publisher.test.ts +++ b/yarn-project/sequencer-client/src/publisher/sequencer-publisher.test.ts @@ -8,6 +8,7 @@ import { MulticallForwarderRevertedError, type RollupContract, type SlashingProposerContract, + type ViemCommitteeAttestations, } from '@aztec/ethereum/contracts'; import { type L1TxUtils, @@ -15,13 +16,20 @@ import { MAX_L1_TX_LIMIT, defaultL1TxUtilsConfig, } from '@aztec/ethereum/l1-tx-utils'; -import { BlockNumber, EpochNumber, SlotNumber } from '@aztec/foundation/branded-types'; +import { BlockNumber, CheckpointNumber, EpochNumber, SlotNumber } from '@aztec/foundation/branded-types'; +import { Fr } from '@aztec/foundation/curves/bn254'; import { TimeoutError } from '@aztec/foundation/error'; import { EthAddress } from '@aztec/foundation/eth-address'; import { sleep } from '@aztec/foundation/sleep'; +import { bufferToHex } from '@aztec/foundation/string'; import { TestDateProvider } from '@aztec/foundation/timer'; import { EmpireBaseAbi, RollupAbi } from '@aztec/l1-artifacts'; -import { CommitteeAttestationsAndSigners, L2Block, Signature } from '@aztec/stdlib/block'; +import { + CommitteeAttestationsAndSigners, + L2Block, + Signature, + type ValidateCheckpointResult, +} from '@aztec/stdlib/block'; import { Checkpoint } from '@aztec/stdlib/checkpoint'; import { EmptyL1RollupConstants } from '@aztec/stdlib/epoch-helpers'; import { CheckpointHeader } from '@aztec/stdlib/rollup'; @@ -1103,4 +1111,50 @@ describe('SequencerPublisher', () => { expect(forwardSpy).not.toHaveBeenCalled(); }); }); + + describe('buildInvalidateCheckpointRequest', () => { + const checkpointNumber = CheckpointNumber(5); + const committee = [EthAddress.random(), EthAddress.random()]; + const checkpoint = { + archive: Fr.random(), + lastArchive: Fr.random(), + slotNumber: SlotNumber(1), + checkpointNumber, + timestamp: 0n, + }; + + const makeInvalidResult = (packedAttestations: ViemCommitteeAttestations): ValidateCheckpointResult => ({ + valid: false, + reason: 'invalid-attestation', + checkpoint, + committee, + epoch: EpochNumber(1), + seed: 0n, + attestors: [], + invalidIndex: 1, + attestations: [], + packedAttestations, + }); + + beforeEach(() => { + rollup.getCheckpointNumber.mockResolvedValue(checkpointNumber); + rollup.buildInvalidateBadAttestationRequest.mockReturnValue({ + to: mockRollupAddress, + data: '0x', + abi: [], + } as any); + }); + + it('passes the raw packed attestations tuple verbatim to invalidateBadAttestation', async () => { + const packed = { signatureIndices: '0x80', signaturesOrAddresses: bufferToHex(Buffer.alloc(65, 7)) } as const; + await publisher.simulateInvalidateCheckpoint(makeInvalidResult(packed)); + expect(rollup.buildInvalidateBadAttestationRequest).toHaveBeenCalledWith(checkpointNumber, packed, committee, 1); + }); + + it('throws rather than repacking when the raw packed tuple is missing', async () => { + await expect(publisher.simulateInvalidateCheckpoint(makeInvalidResult(undefined as any))).rejects.toThrow( + /missing packed attestations/, + ); + }); + }); }); diff --git a/yarn-project/sequencer-client/src/publisher/sequencer-publisher.ts b/yarn-project/sequencer-client/src/publisher/sequencer-publisher.ts index 8770950743f4..f7746e9a160d 100644 --- a/yarn-project/sequencer-client/src/publisher/sequencer-publisher.ts +++ b/yarn-project/sequencer-client/src/publisher/sequencer-publisher.ts @@ -912,7 +912,16 @@ export class SequencerPublisher { const logData = { ...checkpoint, reason }; this.log.debug(`Building invalidate checkpoint ${checkpoint.checkpointNumber} request`, logData); - const attestationsAndSigners = CommitteeAttestationsAndSigners.packAttestations(validationResult.attestations); + // Use the exact packed tuple posted to L1 verbatim. A repack via `packAttestations` is not a + // byte-faithful inverse of `fromPacked` (a canonicalized yParity byte or an all-zero signature slot + // round-trips differently), so it would diverge from the stored `attestationsHash` and revert the + // invalidation. Fail loud rather than fall back to a repack, which would silently re-wedge the chain. + const attestationsAndSigners = validationResult.packedAttestations; + if (!attestationsAndSigners) { + throw new Error( + `Cannot build invalidation for checkpoint ${checkpoint.checkpointNumber}: missing packed attestations from L1 calldata`, + ); + } if (reason === 'invalid-attestation') { return this.rollupContract.buildInvalidateBadAttestationRequest( diff --git a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts index fd0c3090a76f..f4f3c6cdc715 100644 --- a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts +++ b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts @@ -1014,6 +1014,7 @@ describe('CheckpointProposalJob', () => { attestors: [EthAddress.random()], invalidIndex: 0, attestations: [CommitteeAttestation.random()], + packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, }; l2BlockSource.getPendingChainValidationStatus.mockResolvedValue(invalidValidation); @@ -1051,6 +1052,7 @@ describe('CheckpointProposalJob', () => { attestors: [EthAddress.random()], invalidIndex: 0, attestations: [CommitteeAttestation.random()], + packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, }); await pipelinedJob.executeAndAwait(); @@ -1084,6 +1086,7 @@ describe('CheckpointProposalJob', () => { attestors: [EthAddress.random()], invalidIndex: 0, attestations: [CommitteeAttestation.random()], + packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, }; l2BlockSource.getPendingChainValidationStatus.mockResolvedValue(invalidValidation); diff --git a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts index a5517823c817..dd1a6f5dcd6e 100644 --- a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts +++ b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts @@ -33,6 +33,7 @@ import { MaliciousCommitteeAttestationsAndSigners, type ProposedCheckpointSink, type ValidateCheckpointResult, + YParityCommitteeAttestationsAndSigners, } from '@aztec/stdlib/block'; import { type Checkpoint, @@ -1383,6 +1384,7 @@ export class CheckpointProposalJob implements Traceable { this.config.injectFakeAttestation || this.config.injectHighSValueAttestation || this.config.injectUnrecoverableSignatureAttestation || + this.config.injectYParityAttestation || this.config.shuffleAttestationOrdering ) { return this.manipulateAttestations(proposal.slotNumber, epoch, seed, committee, sorted); @@ -1484,6 +1486,24 @@ export class CheckpointProposalJob implements Traceable { return new CommitteeAttestationsAndSigners(attestations, this.getSignatureContext()); } + if (this.config.injectYParityAttestation) { + // Pick a non-proposer signed slot and force its recovery byte to yParity (v ∈ {0, 1}) form in the + // packed L1 tuple, after packAttestations has canonicalized it. Models a malicious proposer landing + // a slot L1 accepts at propose() but that can never be proven (ECDSA.recover rejects v ∉ {27, 28}). + const nonProposerIndices: number[] = []; + for (let i = 0; i < attestations.length; i++) { + if (!attestations[i].signature.isEmpty() && i !== proposerIndex) { + nonProposerIndices.push(i); + } + } + if (nonProposerIndices.length > 0) { + const targetIndex = nonProposerIndices[randomInt(nonProposerIndices.length)]; + this.log.warn(`Injecting yParity attestation in checkpoint for slot ${slotNumber} at index ${targetIndex}`); + return new YParityCommitteeAttestationsAndSigners(attestations, targetIndex, this.getSignatureContext()); + } + return new CommitteeAttestationsAndSigners(attestations, this.getSignatureContext()); + } + if (this.config.shuffleAttestationOrdering) { this.log.warn(`Shuffling attestation ordering in checkpoint for slot ${slotNumber} (proposer #${proposerIndex})`); diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts index 05894143248f..3f58fae13e4e 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts @@ -124,6 +124,7 @@ describe('sequencer', () => { seed: 0n, attestors: [], attestations: [], + packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, reason: 'insufficient-attestations', }; @@ -1184,6 +1185,7 @@ describe('sequencer', () => { seed: 123n, attestors: [], attestations: [], + packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, reason: 'insufficient-attestations', }; diff --git a/yarn-project/slasher/src/watchers/attestations_block_watcher.test.ts b/yarn-project/slasher/src/watchers/attestations_block_watcher.test.ts index 54a26f58d642..43b7e9499415 100644 --- a/yarn-project/slasher/src/watchers/attestations_block_watcher.test.ts +++ b/yarn-project/slasher/src/watchers/attestations_block_watcher.test.ts @@ -68,6 +68,7 @@ describe('AttestationsBlockWatcher', () => { seed: 0n, attestors: [], attestations: [], + packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, }; const event: InvalidCheckpointDetectedEvent = { @@ -99,6 +100,7 @@ describe('AttestationsBlockWatcher', () => { attestors: [], invalidIndex: 0, attestations: [], + packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, }; const event: InvalidCheckpointDetectedEvent = { @@ -146,6 +148,7 @@ describe('AttestationsBlockWatcher', () => { seed: 0n, attestors: [attestor1, attestor2], attestations: [], + packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, }; // First event: slash the proposer for the invalid attestations on its own checkpoint. @@ -193,6 +196,7 @@ describe('AttestationsBlockWatcher', () => { seed: 0n, attestors: [], attestations: [], + packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, }; watcher.handleInvalidCheckpoint({ type: 'invalidCheckpointDetected', @@ -257,6 +261,7 @@ describe('AttestationsBlockWatcher', () => { seed: 0n, attestors: [], attestations: [], + packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, }, }); @@ -320,6 +325,7 @@ describe('AttestationsBlockWatcher', () => { seed: 0n, attestors: [], attestations: [], + packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, }; const event: InvalidCheckpointDetectedEvent = { diff --git a/yarn-project/stdlib/src/block/attestation_info.test.ts b/yarn-project/stdlib/src/block/attestation_info.test.ts new file mode 100644 index 000000000000..7617613f3aca --- /dev/null +++ b/yarn-project/stdlib/src/block/attestation_info.test.ts @@ -0,0 +1,33 @@ +import { Secp256k1Signer } from '@aztec/foundation/crypto/secp256k1-signer'; +import { Signature } from '@aztec/foundation/eth-signature'; + +import { ConsensusPayload } from '../p2p/consensus_payload.js'; +import { getHashedSignaturePayloadTypedData } from '../p2p/signature_utils.js'; +import { getAttestationInfoFromPayload } from './attestation_info.js'; +import { CommitteeAttestation } from './proposal/committee_attestation.js'; + +describe('getAttestationInfoFromPayload', () => { + const payload = ConsensusPayload.random(); + const signer = Secp256k1Signer.random(); + const digest = getHashedSignaturePayloadTypedData(payload); + const canonicalSignature = signer.sign(digest); + + it('recovers a canonical (v in {27, 28}) signature to its signer', () => { + expect([27, 28]).toContain(canonicalSignature.v); + const [info] = getAttestationInfoFromPayload(payload, [CommitteeAttestation.fromSignature(canonicalSignature)]); + expect(info).toEqual({ address: signer.address, status: 'recovered-from-signature' }); + }); + + it('rejects a yParity (v in {0, 1}) signature that still recovers off-chain to a committee member', () => { + // The yParity form recovers to the same member with allowYParityAsV, but L1 ECDSA.recover only accepts + // v in {27, 28}, so it must be classified invalid here to match L1 proving. + const yParitySignature = new Signature(canonicalSignature.r, canonicalSignature.s, canonicalSignature.v - 27); + const [info] = getAttestationInfoFromPayload(payload, [CommitteeAttestation.fromSignature(yParitySignature)]); + expect(info).toEqual({ status: 'invalid-signature' }); + }); + + it('reports an empty signature slot as empty', () => { + const [info] = getAttestationInfoFromPayload(payload, [CommitteeAttestation.empty()]); + expect(info).toEqual({ status: 'empty' }); + }); +}); diff --git a/yarn-project/stdlib/src/block/attestation_info.ts b/yarn-project/stdlib/src/block/attestation_info.ts index 64657db48b89..f2d70232ba95 100644 --- a/yarn-project/stdlib/src/block/attestation_info.ts +++ b/yarn-project/stdlib/src/block/attestation_info.ts @@ -60,9 +60,14 @@ export function getAttestationInfoFromPayload( return { address: attestation.address, status: 'provided-as-address' as const }; } - // Try to recover address from signature + // Try to recover address from signature. Recover with default opts (no allowYParityAsV): a slot + // whose recovery byte is in yParity form (v ∈ {0, 1}) is judged invalid here, matching L1 proving + // (ValidatorSelectionLib.verifyAttestations → ECDSA.recover only accepts v ∈ {27, 28}). A malicious + // proposer that lands such a slot cannot have it silently treated as valid off-chain; it is routed to + // invalid-attestation and invalidated. The gossip receipt path (recoverCoordinationSigner) stays + // lenient with allowYParityAsV; A-1351 normalizes on pool ingress and before the L1 bundle. try { - const recoveredAddress = recoverAddress(hashedPayload, attestation.signature, { allowYParityAsV: true }); + const recoveredAddress = recoverAddress(hashedPayload, attestation.signature); return { address: recoveredAddress, status: 'recovered-from-signature' as const }; } catch { // Signature present but recovery failed diff --git a/yarn-project/stdlib/src/block/proposal/attestations_and_signers.test.ts b/yarn-project/stdlib/src/block/proposal/attestations_and_signers.test.ts index a53d78cc23d6..bda879638062 100644 --- a/yarn-project/stdlib/src/block/proposal/attestations_and_signers.test.ts +++ b/yarn-project/stdlib/src/block/proposal/attestations_and_signers.test.ts @@ -1,11 +1,32 @@ +import type { ViemCommitteeAttestations } from '@aztec/ethereum/contracts'; import { Buffer32 } from '@aztec/foundation/buffer'; import { EthAddress } from '@aztec/foundation/eth-address'; import { Signature } from '@aztec/foundation/eth-signature'; +import { bufferToHex } from '@aztec/foundation/string'; + +import { type AbiParameter, encodeAbiParameters, keccak256 } from 'viem'; import { TEST_COORDINATION_SIGNATURE_CONTEXT } from '../../tests/mocks.js'; -import { CommitteeAttestationsAndSigners } from './attestations_and_signers.js'; +import { CommitteeAttestationsAndSigners, YParityCommitteeAttestationsAndSigners } from './attestations_and_signers.js'; import { CommitteeAttestation } from './committee_attestation.js'; +const committeeAttestationsStruct: AbiParameter = { + type: 'tuple', + components: [ + { name: 'signatureIndices', type: 'bytes' }, + { name: 'signaturesOrAddresses', type: 'bytes' }, + ], +}; + +/** keccak256 of the ABI-encoded CommitteeAttestations tuple, matching the on-chain attestationsHash. */ +function attestationsHash(packed: ViemCommitteeAttestations): string { + return keccak256(encodeAbiParameters([committeeAttestationsStruct], [packed])); +} + +function repack(packed: ViemCommitteeAttestations, committeeSize: number): ViemCommitteeAttestations { + return CommitteeAttestationsAndSigners.packAttestations(CommitteeAttestation.fromPacked(packed, committeeSize)); +} + function popcount(hex: `0x${string}`): number { let count = 0; for (const byte of Buffer.from(hex.slice(2), 'hex')) { @@ -59,3 +80,82 @@ describe('CommitteeAttestationsAndSigners.packAttestations', () => { expect(popcount(bundle.getPackedAttestations().signatureIndices)).toEqual(1); }); }); + +// A repack (packAttestations ∘ fromPacked) is not a byte-faithful inverse of the raw L1 tuple, so the +// invalidation evidence must carry the raw bytes verbatim rather than re-derive them. These cases document +// exactly where the repacked bytes (and thus the attestationsHash) diverge from a maliciously-crafted tuple. +describe('packAttestations is not a byte-faithful inverse of fromPacked', () => { + it('diverges for a yParity (v=0) signature slot: repack canonicalizes v to 27', () => { + const raw: ViemCommitteeAttestations = { + signatureIndices: '0x80', // bit 7 set -> slot 0 is a signature + signaturesOrAddresses: bufferToHex( + Buffer.concat([Buffer.from([0]), Buffer32.random().toBuffer(), Buffer32.random().toBuffer()]), + ), + }; + + const repacked = repack(raw, 1); + + expect(repacked).not.toEqual(raw); + expect(attestationsHash(repacked)).not.toEqual(attestationsHash(raw)); + }); + + it('diverges for an all-zero signature slot: repack clears the bit and packs it as an address', () => { + const raw: ViemCommitteeAttestations = { + signatureIndices: '0x80', // bit set, but the 65-byte payload is all zero (v, r, s) + signaturesOrAddresses: bufferToHex(Buffer.alloc(65)), + }; + + const repacked = repack(raw, 1); + + // fromPacked reads it as an empty signature, so the repack drops the bit and emits a 20-byte address. + expect(repacked.signatureIndices).not.toEqual(raw.signatureIndices); + expect(attestationsHash(repacked)).not.toEqual(attestationsHash(raw)); + }); +}); + +describe('YParityCommitteeAttestationsAndSigners', () => { + it('rewrites only the target slot to yParity form while keeping getSigners and the bitmap consistent', () => { + const attestations = [signing(27), signing(28), CommitteeAttestation.fromAddress(EthAddress.random()), signing(27)]; + const targetIndex = 1; + const bundle = new YParityCommitteeAttestationsAndSigners( + attestations, + targetIndex, + TEST_COORDINATION_SIGNATURE_CONTEXT, + ); + + const packed = bundle.getPackedAttestations(); + const unpacked = CommitteeAttestation.fromPacked(packed, attestations.length); + + // The target slot carries a non-canonical yParity recovery byte; all other signed slots stay canonical. + expect([0, 1]).toContain(unpacked[targetIndex].signature.v); + expect([27, 28]).toContain(unpacked[0].signature.v); + expect([27, 28]).toContain(unpacked[3].signature.v); + + // The bitmap and signers are untouched, so propose() would not revert SignersSizeMismatch. + const honest = new CommitteeAttestationsAndSigners(attestations, TEST_COORDINATION_SIGNATURE_CONTEXT); + expect(packed.signatureIndices).toEqual(honest.getPackedAttestations().signatureIndices); + expect(popcount(packed.signatureIndices)).toEqual(bundle.getSigners().length); + }); + + it('preserves (r, s) of the target slot so it still recovers to the same signer', () => { + const attestations = [signing(27), signing(28)]; + const targetIndex = 1; + const honest = new CommitteeAttestationsAndSigners(attestations, TEST_COORDINATION_SIGNATURE_CONTEXT); + const bundle = new YParityCommitteeAttestationsAndSigners( + attestations, + targetIndex, + TEST_COORDINATION_SIGNATURE_CONTEXT, + ); + + const honestTarget = CommitteeAttestation.fromPacked(honest.getPackedAttestations(), attestations.length)[ + targetIndex + ]; + const maliciousTarget = CommitteeAttestation.fromPacked(bundle.getPackedAttestations(), attestations.length)[ + targetIndex + ]; + + expect(maliciousTarget.signature.r).toEqual(honestTarget.signature.r); + expect(maliciousTarget.signature.s).toEqual(honestTarget.signature.s); + expect(maliciousTarget.signature.v).toEqual(honestTarget.signature.v - 27); + }); +}); diff --git a/yarn-project/stdlib/src/block/proposal/attestations_and_signers.ts b/yarn-project/stdlib/src/block/proposal/attestations_and_signers.ts index c6ee4a5f8dc9..6b8a3c37b046 100644 --- a/yarn-project/stdlib/src/block/proposal/attestations_and_signers.ts +++ b/yarn-project/stdlib/src/block/proposal/attestations_and_signers.ts @@ -1,5 +1,5 @@ import type { ViemCommitteeAttestations } from '@aztec/ethereum/contracts'; -import { hexToBuffer } from '@aztec/foundation/string'; +import { bufferToHex, hexToBuffer } from '@aztec/foundation/string'; import { encodeAbiParameters, parseAbiParameters } from 'viem'; import { z } from 'zod'; @@ -157,3 +157,48 @@ export class MaliciousCommitteeAttestationsAndSigners extends CommitteeAttestati return this.signers; } } + +/** + * Malicious extension of CommitteeAttestationsAndSigners that rewrites one non-empty signature slot's + * recovery byte to yParity form (v ∈ {0, 1}) in the packed output, after the honest `packAttestations` + * has already canonicalized it to v ∈ {27, 28}. Models a malicious selected proposer that hand-crafts + * `propose()` calldata L1 accepts but no honest node can byte-replay: the signature still recovers to the + * same member (r, s and the recovery parity are preserved), the bitmap bit stays set, and `getSigners()` + * stays consistent, so `propose()` does not revert `SignersSizeMismatch` -- yet the checkpoint can never + * be proven (`ECDSA.recover` rejects v ∉ {27, 28}). For testing only. + */ +export class YParityCommitteeAttestationsAndSigners extends CommitteeAttestationsAndSigners { + constructor( + attestations: CommitteeAttestation[], + /** Committee index of the (non-empty, non-proposer) signature slot whose recovery byte to force to yParity. */ + private targetIndex: number, + signatureContext: CoordinationSignatureContext, + ) { + super(attestations, signatureContext); + } + + override getPackedAttestations(): ViemCommitteeAttestations { + const packed = super.getPackedAttestations(); + const data = hexToBuffer(packed.signaturesOrAddresses); + + // Walk the packed byte-vector to find the v-byte offset of the target slot. A signed slot occupies + // 65 bytes (v, r, s); an empty slot occupies 20 bytes (address only). + let offset = 0; + for (let i = 0; i < this.attestations.length; i++) { + const isSigned = !this.attestations[i].signature.isEmpty(); + if (i === this.targetIndex) { + if (!isSigned) { + throw new Error(`Target slot ${i} is not a signature slot; cannot force a yParity recovery byte`); + } + // `packAttestations` canonicalized v to 27/28; rewrite back to the equivalent yParity byte (0/1), + // preserving the recovery parity so the signature still recovers to the same member. + const v = data[offset]; + data[offset] = v >= 27 ? v - 27 : v; + break; + } + offset += isSigned ? 65 : 20; + } + + return { signatureIndices: packed.signatureIndices, signaturesOrAddresses: bufferToHex(data) }; + } +} diff --git a/yarn-project/stdlib/src/block/validate_block_result.test.ts b/yarn-project/stdlib/src/block/validate_block_result.test.ts index 386449143608..5b12380f2b5c 100644 --- a/yarn-project/stdlib/src/block/validate_block_result.test.ts +++ b/yarn-project/stdlib/src/block/validate_block_result.test.ts @@ -1,7 +1,10 @@ import { EpochNumber } from '@aztec/foundation/branded-types'; +import { Buffer32 } from '@aztec/foundation/buffer'; import { EthAddress } from '@aztec/foundation/eth-address'; +import { Signature } from '@aztec/foundation/eth-signature'; import { randomCheckpointInfo } from '../checkpoint/checkpoint_info.js'; +import { CommitteeAttestationsAndSigners } from './proposal/attestations_and_signers.js'; import { CommitteeAttestation } from './proposal/committee_attestation.js'; import { type ValidateCheckpointResult, @@ -10,6 +13,13 @@ import { } from './validate_block_result.js'; describe('ValidateCheckpointResult', () => { + // A non-trivial packed tuple with a signature slot and an address-only slot, so the round-trip exercises + // both packed segments rather than the empty (0x, 0x) tuple. + const packedAttestations = CommitteeAttestationsAndSigners.packAttestations([ + new CommitteeAttestation(EthAddress.ZERO, new Signature(Buffer32.random(), Buffer32.random(), 27)), + CommitteeAttestation.fromAddress(EthAddress.random()), + ]); + describe('serialization to buffer', () => { it('valid result', () => { const result: ValidateCheckpointResult = { valid: true }; @@ -29,6 +39,7 @@ describe('ValidateCheckpointResult', () => { attestors: [EthAddress.random(), EthAddress.random()], invalidIndex: 4, attestations: [CommitteeAttestation.random(), CommitteeAttestation.random()], + packedAttestations, }; const serialized = serializeValidateCheckpointResult(result); const deserialized = deserializeValidateCheckpointResult(serialized); @@ -45,10 +56,29 @@ describe('ValidateCheckpointResult', () => { seed: 2n, attestors: [EthAddress.random(), EthAddress.random()], attestations: [CommitteeAttestation.random(), CommitteeAttestation.random()], + packedAttestations, }; const serialized = serializeValidateCheckpointResult(result); const deserialized = deserializeValidateCheckpointResult(serialized); expect(deserialized).toEqual(result); }); + + it('preserves the packed attestations tuple byte-for-byte through a round-trip', () => { + const result: ValidateCheckpointResult = { + valid: false, + reason: 'invalid-attestation', + checkpoint: randomCheckpointInfo(), + committee: [EthAddress.random()], + epoch: EpochNumber(1), + seed: 2n, + attestors: [EthAddress.random()], + invalidIndex: 0, + attestations: [CommitteeAttestation.random()], + packedAttestations, + }; + const deserialized = deserializeValidateCheckpointResult(serializeValidateCheckpointResult(result)); + expect(deserialized.valid).toBe(false); + expect((deserialized as typeof result).packedAttestations).toEqual(packedAttestations); + }); }); }); diff --git a/yarn-project/stdlib/src/block/validate_block_result.ts b/yarn-project/stdlib/src/block/validate_block_result.ts index 62b760ebac7a..88aa22bb918f 100644 --- a/yarn-project/stdlib/src/block/validate_block_result.ts +++ b/yarn-project/stdlib/src/block/validate_block_result.ts @@ -1,7 +1,9 @@ +import type { ViemCommitteeAttestations } from '@aztec/ethereum/contracts'; import { EpochNumber, EpochNumberSchema } from '@aztec/foundation/branded-types'; import { EthAddress } from '@aztec/foundation/eth-address'; import { type ZodFor, schemas } from '@aztec/foundation/schemas'; import { BufferReader, serializeToBuffer } from '@aztec/foundation/serialize'; +import { bufferToHex, hexToBuffer } from '@aztec/foundation/string'; import { z } from 'zod'; @@ -30,6 +32,14 @@ export type ValidateCheckpointNegativeResult = attestors: EthAddress[]; /** Committee attestations for this checkpoint as they were posted to L1 */ attestations: CommitteeAttestation[]; + /** + * The exact packed `CommitteeAttestations` tuple as posted to L1 calldata, carried verbatim so the + * invalidation evidence is byte-faithful to the stored `attestationsHash`. A repack via + * `packAttestations` is not a round-trip inverse of `fromPacked`, so it would diverge from the hash + * and revert `invalidateBadAttestation`/`invalidateInsufficientAttestations`. Always populated on the + * calldata validation path, the only production source of invalidation evidence. + */ + packedAttestations: ViemCommitteeAttestations; /** Reason for the checkpoint being invalid: not enough attestations were posted */ reason: 'insufficient-attestations'; } @@ -47,6 +57,12 @@ export type ValidateCheckpointNegativeResult = attestors: EthAddress[]; /** Committee attestations for this checkpoint as they were posted to L1 */ attestations: CommitteeAttestation[]; + /** + * The exact packed `CommitteeAttestations` tuple as posted to L1 calldata, carried verbatim so the + * invalidation evidence is byte-faithful to the stored `attestationsHash`. See the same field on the + * insufficient-attestations variant for why a repack cannot be used. + */ + packedAttestations: ViemCommitteeAttestations; /** Reason for the checkpoint being invalid: an invalid attestation was posted */ reason: 'invalid-attestation'; /** Index in the attestations array of the invalid attestation posted */ @@ -56,6 +72,12 @@ export type ValidateCheckpointNegativeResult = /** Result type for validating checkpoint attestations */ export type ValidateCheckpointResult = { valid: true } | ValidateCheckpointNegativeResult; +/** Zod schema for the raw packed `CommitteeAttestations` viem tuple (two 0x-prefixed hex strings). */ +const ViemCommitteeAttestationsSchema: ZodFor = z.object({ + signatureIndices: schemas.HexStringWith0x, + signaturesOrAddresses: schemas.HexStringWith0x, +}); + export const ValidateCheckpointResultSchema: ZodFor = z.union([ z.object({ valid: z.literal(true) }), z.object({ @@ -66,6 +88,7 @@ export const ValidateCheckpointResultSchema: ZodFor = seed: schemas.BigInt, attestors: z.array(schemas.EthAddress), attestations: z.array(CommitteeAttestation.schema), + packedAttestations: ViemCommitteeAttestationsSchema, reason: z.literal('insufficient-attestations'), }), z.object({ @@ -76,6 +99,7 @@ export const ValidateCheckpointResultSchema: ZodFor = seed: schemas.BigInt, attestors: z.array(schemas.EthAddress), attestations: z.array(CommitteeAttestation.schema), + packedAttestations: ViemCommitteeAttestationsSchema, reason: z.literal('invalid-attestation'), invalidIndex: z.number(), }), @@ -87,6 +111,8 @@ export function serializeValidateCheckpointResult(result: ValidateCheckpointResu } const checkpointBuffer = serializeCheckpointInfo(result.checkpoint); + const signatureIndices = hexToBuffer(result.packedAttestations.signatureIndices); + const signaturesOrAddresses = hexToBuffer(result.packedAttestations.signaturesOrAddresses); return serializeToBuffer( result.valid, result.reason, @@ -101,6 +127,10 @@ export function serializeValidateCheckpointResult(result: ValidateCheckpointResu result.attestations.length, result.attestations, result.reason === 'invalid-attestation' ? result.invalidIndex : 0, + signatureIndices.length, + signatureIndices, + signaturesOrAddresses.length, + signaturesOrAddresses, ); } @@ -118,10 +148,26 @@ export function deserializeValidateCheckpointResult(bufferOrReader: Buffer | Buf const attestors = reader.readVector(EthAddress, MAX_COMMITTEE_SIZE); const attestations = reader.readVector(CommitteeAttestation, MAX_COMMITTEE_SIZE); const invalidIndex = reader.readNumber(); + const packedAttestations: ViemCommitteeAttestations = { + signatureIndices: bufferToHex(reader.readBuffer()), + signaturesOrAddresses: bufferToHex(reader.readBuffer()), + }; + if (reason === 'insufficient-attestations') { - return { valid, reason, checkpoint, committee, epoch, seed, attestors, attestations }; + return { valid, reason, checkpoint, committee, epoch, seed, attestors, attestations, packedAttestations }; } else if (reason === 'invalid-attestation') { - return { valid, reason, checkpoint, committee, epoch, seed, attestors, invalidIndex, attestations }; + return { + valid, + reason, + checkpoint, + committee, + epoch, + seed, + attestors, + invalidIndex, + attestations, + packedAttestations, + }; } else { const _: never = reason; throw new Error(`Unknown reason: ${reason}`); diff --git a/yarn-project/stdlib/src/interfaces/configs.ts b/yarn-project/stdlib/src/interfaces/configs.ts index a806a31e0354..14602960be57 100644 --- a/yarn-project/stdlib/src/interfaces/configs.ts +++ b/yarn-project/stdlib/src/interfaces/configs.ts @@ -96,6 +96,8 @@ export interface SequencerConfig { injectHighSValueAttestation?: boolean; /** Inject an attestation with an unrecoverable signature (for testing only) */ injectUnrecoverableSignatureAttestation?: boolean; + /** Inject a non-proposer attestation slot in yParity (v ∈ {0, 1}) form in the packed L1 tuple (for testing only) */ + injectYParityAttestation?: boolean; /** Whether to run in fisherman mode: builds blocks on every slot for validation without publishing */ fishermanMode?: boolean; /** Shuffle attestation ordering to create invalid ordering (for testing only) */ @@ -165,6 +167,7 @@ export const SequencerConfigSchema = zodFor()( injectFakeAttestation: z.boolean().optional(), injectHighSValueAttestation: z.boolean().optional(), injectUnrecoverableSignatureAttestation: z.boolean().optional(), + injectYParityAttestation: z.boolean().optional(), fishermanMode: z.boolean().optional(), shuffleAttestationOrdering: z.boolean().optional(), blockDurationMs: z.number().positive().optional(), diff --git a/yarn-project/stdlib/src/interfaces/validator.ts b/yarn-project/stdlib/src/interfaces/validator.ts index f17e96e2a475..62b2521d2a14 100644 --- a/yarn-project/stdlib/src/interfaces/validator.ts +++ b/yarn-project/stdlib/src/interfaces/validator.ts @@ -66,6 +66,12 @@ export type ValidatorClientConfig = ValidatorHASignerConfig & /** Agree to attest to equivocated checkpoint proposals (for testing purposes only) */ attestToEquivocatedProposals?: boolean; + /** + * Rewrite this validator's own attestation signature to yParity (v ∈ {0, 1}) form after signing, + * modelling a malicious committee member (for testing purposes only). + */ + injectYParityOwnAttestation?: boolean; + /** Accept proposal validation regardless of slot timing (for testing only) */ skipProposalSlotValidation?: boolean; @@ -120,6 +126,7 @@ export const ValidatorClientConfigSchema = zodFor WatcherEmitter) return undefined; } - const attestations = await this.validationService.attestToCheckpointProposal(proposal, attestors, checkpointNumber); + let attestations = await this.validationService.attestToCheckpointProposal(proposal, attestors, checkpointNumber); + + if (this.config.injectYParityOwnAttestation) { + this.log.warn(`Injecting yParity form into own attestations for slot ${proposal.slotNumber}`); + attestations = attestations.map(a => toYParityAttestation(a)); + } // Track the proposal we attested to (to prevent equivocation) this.lastAttestedProposal = proposal; @@ -1128,3 +1133,17 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) return authResponse.toBuffer(); } } + +/** + * Rewrites an attestation's recovery byte to yParity (v ∈ {0, 1}) form, preserving (r, s) and the recovery + * parity so it still recovers to the same signer. Models a malicious committee member; for testing only. + */ +function toYParityAttestation(attestation: CheckpointAttestation): CheckpointAttestation { + const signature = attestation.signature; + const yParityV = signature.v >= 27 ? signature.v - 27 : signature.v; + return new CheckpointAttestation( + attestation.payload, + new Signature(signature.r, signature.s, yParityV), + attestation.proposerSignature, + ); +} From c008424496372bb9bbe412d4fab5f7d551753b51 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Mon, 6 Jul 2026 11:18:53 -0300 Subject: [PATCH 2/5] docs(test): correct A-1351 yParity red-repro note to match observed behavior 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. --- .../yparity_attestation_proving.parallel.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/yarn-project/end-to-end/src/multi-node/invalid-attestations/yparity_attestation_proving.parallel.test.ts b/yarn-project/end-to-end/src/multi-node/invalid-attestations/yparity_attestation_proving.parallel.test.ts index 7dc0ab157fca..1a0ec3e188e4 100644 --- a/yarn-project/end-to-end/src/multi-node/invalid-attestations/yparity_attestation_proving.parallel.test.ts +++ b/yarn-project/end-to-end/src/multi-node/invalid-attestations/yparity_attestation_proving.parallel.test.ts @@ -31,8 +31,10 @@ const NODE_COUNT = 3; // produces no on-chain event, so proven-tip advancement is the only observable signal. Modelled on // block-production/proof_boundary.parallel.test.ts (createProverNode + Delayer). To reproduce the red // state, revert the three A-1351 normalization layers (orderAttestations, the attestation pool ingress, -// CheckpointAttestation.withNormalizedSignature) and the packAttestations v-canonicalization: the proof -// tx is still sent but reverts on L1 and the proven tip stays at 0. +// CheckpointAttestation.withNormalizedSignature) and the packAttestations v-canonicalization. The proven +// tip then stays at 0 and the test times out: on a pre-A-1401 tree the proof tx is sent but reverts on L1 +// (ECDSA.recover), while on this branch A-1401's own detection invalidates the non-canonical checkpoint +// before it can be proven. Either way, without A-1351 the epoch never proves. describe('multi-node/invalid-attestations/yparity_attestation_proving', () => { let context: EndToEndContext; let logger: Logger; From 2a6ba68be31e33b4f43b5ab8cf4acafcb08cb550 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Mon, 6 Jul 2026 11:33:39 -0300 Subject: [PATCH 3/5] docs(stdlib): note why gossip sender recovery stays lenient vs strict 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. --- yarn-project/stdlib/src/p2p/signature_utils.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/yarn-project/stdlib/src/p2p/signature_utils.ts b/yarn-project/stdlib/src/p2p/signature_utils.ts index b71c9a8b89e3..fbb5cffe27c9 100644 --- a/yarn-project/stdlib/src/p2p/signature_utils.ts +++ b/yarn-project/stdlib/src/p2p/signature_utils.ts @@ -112,6 +112,15 @@ export function getHashedSignaturePayloadTypedData(signable: Signable): Buffer32 return Buffer32.fromString(hashTypedData(getCoordinationSignatureTypedData(signable))); } +/** + * Recovers the sender of a gossiped coordination message. Deliberately lenient (allowYParityAsV): a + * signature whose recovery byte is in yParity form (v ∈ {0, 1}) still resolves to its sender, so the P2P + * layer can attribute and accept it and then canonicalize on ingress (CheckpointAttestation. + * withNormalizedSignature, A-1351) before it reaches the L1 bundle. This is intentionally looser than + * on-chain checkpoint validation (getAttestationInfoFromPayload), which recovers strictly to mirror L1's + * ECDSA.recover — do not unify the two. These proposal/attestation signatures are P2P-only and never reach + * L1 verbatim, so leniency here cannot leak a non-canonical byte onto L1. + */ export function recoverCoordinationSigner(signable: Signable, signature: Signature): EthAddress | undefined { const digest = getHashedSignaturePayloadTypedData(signable); return tryRecoverAddress(digest, signature, { allowYParityAsV: true }); From d21af9c763cb34cec3bebd14da82c7adefc98596 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Mon, 6 Jul 2026 12:10:38 -0300 Subject: [PATCH 4/5] refactor(sequencer): address review feedback on A-1401 yParity fix - 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 --- .../src/l1/calldata_retriever.test.ts | 2 +- .../archiver/src/l1/calldata_retriever.ts | 16 +++---- .../archiver/src/l1/data_retrieval.ts | 2 +- .../archiver/src/modules/validation.test.ts | 39 ++++++++++++++-- .../archiver/src/modules/validation.ts | 38 ++------------- .../archiver/src/store/block_store.test.ts | 8 ++-- .../archiver/src/test/fake_l1_state.ts | 6 +-- .../node_public_calls_simulator.test.ts | 2 +- .../invalidate_block.parallel.test.ts | 12 ++--- .../l1_publisher.integration.test.ts | 2 +- .../src/publisher/sequencer-publisher.test.ts | 10 +--- .../src/publisher/sequencer-publisher.ts | 9 +--- .../sequencer/checkpoint_proposal_job.test.ts | 6 +-- .../src/sequencer/checkpoint_proposal_job.ts | 27 +++++------ .../src/sequencer/sequencer.test.ts | 4 +- .../attestations_block_watcher.test.ts | 12 ++--- .../proposal/attestations_and_signers.test.ts | 46 ++++++++++--------- .../proposal/attestations_and_signers.ts | 31 ++++++------- .../src/block/validate_block_result.test.ts | 10 ++-- .../stdlib/src/block/validate_block_result.ts | 18 ++++---- 20 files changed, 145 insertions(+), 155 deletions(-) diff --git a/yarn-project/archiver/src/l1/calldata_retriever.test.ts b/yarn-project/archiver/src/l1/calldata_retriever.test.ts index 624b50592324..716c3cb23b22 100644 --- a/yarn-project/archiver/src/l1/calldata_retriever.test.ts +++ b/yarn-project/archiver/src/l1/calldata_retriever.test.ts @@ -505,7 +505,7 @@ describe('CalldataRetriever', () => { archiveRoot: Fr.random(), header: CheckpointHeader.random(), attestations: [], - packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, + verbatimAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, blockHash, feeAssetPriceModifier: 0n, }; diff --git a/yarn-project/archiver/src/l1/calldata_retriever.ts b/yarn-project/archiver/src/l1/calldata_retriever.ts index 6d435ebfa2df..da2085b973d8 100644 --- a/yarn-project/archiver/src/l1/calldata_retriever.ts +++ b/yarn-project/archiver/src/l1/calldata_retriever.ts @@ -39,7 +39,7 @@ type CheckpointData = { * verbatim (never re-derived from {@link attestations}) so invalidation evidence stays byte-faithful to * the on-chain `attestationsHash`. */ - packedAttestations: ViemCommitteeAttestations; + verbatimAttestations: ViemCommitteeAttestations; blockHash: string; feeAssetPriceModifier: bigint; }; @@ -412,14 +412,14 @@ export class CalldataRetriever { return undefined; } - const [decodedArgs, packedAttestations] = args! as readonly [ + const [decodedArgs, verbatimAttestations] = args! as readonly [ { archive: Hex; oracleInput: { feeAssetPriceModifier: bigint }; header: ViemHeader }, ViemCommitteeAttestations, ...unknown[], ]; // Verify attestationsHash - const computedAttestationsHash = this.computeAttestationsHash(packedAttestations); + const computedAttestationsHash = this.computeAttestationsHash(verbatimAttestations); if ( !Buffer.from(hexToBytes(computedAttestationsHash)).equals( Buffer.from(hexToBytes(expectedHashes.attestationsHash)), @@ -447,7 +447,7 @@ export class CalldataRetriever { return undefined; } - const attestations = CommitteeAttestation.fromPacked(packedAttestations, this.targetCommitteeSize); + const attestations = CommitteeAttestation.fromPacked(verbatimAttestations, this.targetCommitteeSize); this.logger.trace(`Validated and decoded propose calldata for checkpoint ${checkpointNumber}`, { checkpointNumber, @@ -455,7 +455,7 @@ export class CalldataRetriever { header: decodedArgs.header, l1BlockHash: blockHash, attestations, - packedAttestations, + verbatimAttestations, targetCommitteeSize: this.targetCommitteeSize, }); @@ -464,7 +464,7 @@ export class CalldataRetriever { archiveRoot, header, attestations, - packedAttestations, + verbatimAttestations, blockHash, feeAssetPriceModifier, }; @@ -474,8 +474,8 @@ export class CalldataRetriever { } /** Computes the keccak256 hash of ABI-encoded CommitteeAttestations. */ - private computeAttestationsHash(packedAttestations: ViemCommitteeAttestations): Hex { - return keccak256(encodeAbiParameters([this.getCommitteeAttestationsStructDef()], [packedAttestations])); + private computeAttestationsHash(verbatimAttestations: ViemCommitteeAttestations): Hex { + return keccak256(encodeAbiParameters([this.getCommitteeAttestationsStructDef()], [verbatimAttestations])); } /** Computes the keccak256 payload digest from the checkpoint header, archive root, and fee asset price modifier. */ diff --git a/yarn-project/archiver/src/l1/data_retrieval.ts b/yarn-project/archiver/src/l1/data_retrieval.ts index c5203276b17c..18282920db60 100644 --- a/yarn-project/archiver/src/l1/data_retrieval.ts +++ b/yarn-project/archiver/src/l1/data_retrieval.ts @@ -61,7 +61,7 @@ export type RetrievedCheckpointFromCalldata = RetrievedCheckpointBase & { * The exact packed `CommitteeAttestations` tuple from the propose calldata, carried verbatim so that * attestation validation can attach byte-faithful invalidation evidence to a negative result. */ - packedAttestations: ViemCommitteeAttestations; + verbatimAttestations: ViemCommitteeAttestations; }; export async function retrievedToPublishedCheckpoint({ diff --git a/yarn-project/archiver/src/modules/validation.test.ts b/yarn-project/archiver/src/modules/validation.test.ts index 4c49d9cdf2eb..4c44691b264a 100644 --- a/yarn-project/archiver/src/modules/validation.test.ts +++ b/yarn-project/archiver/src/modules/validation.test.ts @@ -5,15 +5,48 @@ import { times } from '@aztec/foundation/collection'; import { Secp256k1Signer, flipSignature } from '@aztec/foundation/crypto/secp256k1-signer'; import { Signature } from '@aztec/foundation/eth-signature'; import { type Logger, createLogger } from '@aztec/foundation/log'; -import { CommitteeAttestation, EthAddress } from '@aztec/stdlib/block'; -import { Checkpoint } from '@aztec/stdlib/checkpoint'; +import { + CommitteeAttestation, + CommitteeAttestationsAndSigners, + EthAddress, + type ValidateCheckpointResult, +} from '@aztec/stdlib/block'; +import { Checkpoint, type PublishedCheckpoint } from '@aztec/stdlib/checkpoint'; +import type { L1RollupConstants } from '@aztec/stdlib/epoch-helpers'; +import { ConsensusPayload, type CoordinationSignatureContext } from '@aztec/stdlib/p2p'; import { TEST_COORDINATION_SIGNATURE_CONTEXT } from '@aztec/stdlib/testing'; import { type MockProxy, mock } from 'jest-mock-extended'; import assert from 'node:assert'; import { makeSignedPublishedCheckpoint } from '../test/mock_structs.js'; -import { getAttestationInfoFromPublishedCheckpoint, validateCheckpointAttestations } from './validation.js'; +import { getAttestationInfoFromPublishedCheckpoint, validateAttestations } from './validation.js'; + +/** + * Blob-path wrapper over the shared attestation validator, used only by these tests. Production + * invalidation always originates from the calldata path (validateCheckpointAttestationsFromCalldata); + * this repacks a fully decoded checkpoint's attestations to supply the verbatimAttestations field. + */ +function validateCheckpointAttestations( + publishedCheckpoint: PublishedCheckpoint, + epochCache: EpochCache, + constants: Pick, + signatureContext: CoordinationSignatureContext, + logger?: Logger, +): Promise { + const { checkpoint, attestations } = publishedCheckpoint; + const payload = ConsensusPayload.fromCheckpoint(checkpoint, signatureContext); + const verbatimAttestations = CommitteeAttestationsAndSigners.packAttestations(attestations); + return validateAttestations( + payload, + attestations, + verbatimAttestations, + checkpoint.toCheckpointInfo(), + epochCache, + constants, + logger, + ); +} describe('validateCheckpointAttestations', () => { let epochCache: MockProxy; diff --git a/yarn-project/archiver/src/modules/validation.ts b/yarn-project/archiver/src/modules/validation.ts index 1204ed87e9f8..43ca05790ebd 100644 --- a/yarn-project/archiver/src/modules/validation.ts +++ b/yarn-project/archiver/src/modules/validation.ts @@ -7,7 +7,6 @@ import type { Logger } from '@aztec/foundation/log'; import { type AttestationInfo, type CommitteeAttestation, - CommitteeAttestationsAndSigners, type ValidateCheckpointNegativeResult, type ValidateCheckpointResult, getAttestationInfoFromPayload, @@ -31,33 +30,6 @@ export function getAttestationInfoFromPublishedCheckpoint( return getAttestationInfoFromPayload(payload, attestations); } -/** - * Validates the attestations of a checkpoint already retrieved (with its blocks) from blobs. - * Returns true if the attestations are valid and sufficient, false otherwise. - */ -export function validateCheckpointAttestations( - publishedCheckpoint: PublishedCheckpoint, - epochCache: EpochCache, - constants: Pick, - signatureContext: CoordinationSignatureContext, - logger?: Logger, -): Promise { - const { checkpoint, attestations } = publishedCheckpoint; - const payload = ConsensusPayload.fromCheckpoint(checkpoint, signatureContext); - // The blob path has no production callers (invalidation always originates from the calldata path), so a - // repack is acceptable here to satisfy the required packedAttestations field on a negative result. - const packedAttestations = CommitteeAttestationsAndSigners.packAttestations(attestations); - return validateAttestations( - payload, - attestations, - packedAttestations, - checkpoint.toCheckpointInfo(), - epochCache, - constants, - logger, - ); -} - /** The subset of a calldata-only checkpoint needed to validate its committee attestations. */ export type CalldataCheckpointForAttestations = { checkpointNumber: CheckpointNumber; @@ -66,7 +38,7 @@ export type CalldataCheckpointForAttestations = { header: CheckpointHeader; attestations: CommitteeAttestation[]; /** The exact packed attestations tuple from L1 calldata, carried verbatim for byte-faithful invalidation. */ - packedAttestations: ViemCommitteeAttestations; + verbatimAttestations: ViemCommitteeAttestations; }; /** @@ -98,7 +70,7 @@ export function validateCheckpointAttestationsFromCalldata( return validateAttestations( payload, checkpoint.attestations, - checkpoint.packedAttestations, + checkpoint.verbatimAttestations, checkpointInfo, epochCache, constants, @@ -111,10 +83,10 @@ export function validateCheckpointAttestationsFromCalldata( * independent of whether the checkpoint's blocks have been decoded from blobs. Returns true if the * attestations are valid and sufficient, false otherwise. */ -async function validateAttestations( +export async function validateAttestations( payload: ConsensusPayload, attestations: CommitteeAttestation[], - packedAttestations: ViemCommitteeAttestations, + verbatimAttestations: ViemCommitteeAttestations, checkpointInfo: CheckpointInfo, epochCache: EpochCache, constants: Pick, @@ -161,7 +133,7 @@ async function validateAttestations( epoch, attestors, attestations, - packedAttestations, + verbatimAttestations, }); for (let i = 0; i < attestorInfos.length; i++) { diff --git a/yarn-project/archiver/src/store/block_store.test.ts b/yarn-project/archiver/src/store/block_store.test.ts index 175553eb67fe..0d4a4a7ec69a 100644 --- a/yarn-project/archiver/src/store/block_store.test.ts +++ b/yarn-project/archiver/src/store/block_store.test.ts @@ -1989,7 +1989,7 @@ describe('BlockStore', () => { seed: 456n, attestors: [EthAddress.random()], attestations: [CommitteeAttestation.random()], - packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, + verbatimAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, reason: 'insufficient-attestations', }; @@ -2008,7 +2008,7 @@ describe('BlockStore', () => { epoch: EpochNumber(789), seed: 101n, attestations: [CommitteeAttestation.random()], - packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, + verbatimAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, reason: 'invalid-attestation', invalidIndex: 5, }; @@ -2029,7 +2029,7 @@ describe('BlockStore', () => { seed: 888n, attestors: [EthAddress.random()], attestations: [CommitteeAttestation.random()], - packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, + verbatimAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, reason: 'insufficient-attestations', }; @@ -2049,7 +2049,7 @@ describe('BlockStore', () => { seed: 0n, attestors: [], attestations: [], - packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, + verbatimAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, reason: 'insufficient-attestations', }; diff --git a/yarn-project/archiver/src/test/fake_l1_state.ts b/yarn-project/archiver/src/test/fake_l1_state.ts index 005cb8577c73..0b9cbcb177f2 100644 --- a/yarn-project/archiver/src/test/fake_l1_state.ts +++ b/yarn-project/archiver/src/test/fake_l1_state.ts @@ -682,7 +682,7 @@ export class FakeL1State { getHashedSignaturePayloadTypedData(attestationsAndSigners), ); - const packedAttestations = attestationsAndSigners.getPackedAttestations(); + const verbatimAttestations = attestationsAndSigners.getPackedAttestations(); const rollupInput = encodeFunctionData({ abi: RollupAbi, @@ -693,7 +693,7 @@ export class FakeL1State { archive, oracleInput: { feeAssetPriceModifier: 0n }, }, - packedAttestations, + verbatimAttestations, attestationsAndSigners.getSigners().map(signer => signer.toString()), attestationsAndSignersSignature.toViemSignature(), blobInput, @@ -716,7 +716,7 @@ export class FakeL1State { // Compute attestationsHash (same logic as CalldataRetriever) const attestationsHash = Buffer32.fromString( - keccak256(encodeAbiParameters([this.getCommitteeAttestationsStructDef()], [packedAttestations])), + keccak256(encodeAbiParameters([this.getCommitteeAttestationsStructDef()], [verbatimAttestations])), ); // Compute payloadDigest (same logic as CalldataRetriever) diff --git a/yarn-project/aztec-node/src/aztec-node/node_public_calls_simulator.test.ts b/yarn-project/aztec-node/src/aztec-node/node_public_calls_simulator.test.ts index 402eea9b1860..9dd3e089a1c4 100644 --- a/yarn-project/aztec-node/src/aztec-node/node_public_calls_simulator.test.ts +++ b/yarn-project/aztec-node/src/aztec-node/node_public_calls_simulator.test.ts @@ -452,7 +452,7 @@ function makeInvalidStatus(firstInvalid: CheckpointNumber): ValidateCheckpointRe seed: 0n, attestors: [], attestations: [], - packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, + verbatimAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, reason: 'insufficient-attestations', }; } diff --git a/yarn-project/end-to-end/src/multi-node/invalid-attestations/invalidate_block.parallel.test.ts b/yarn-project/end-to-end/src/multi-node/invalid-attestations/invalidate_block.parallel.test.ts index 4fdfca2c8517..83870322cd25 100644 --- a/yarn-project/end-to-end/src/multi-node/invalid-attestations/invalidate_block.parallel.test.ts +++ b/yarn-project/end-to-end/src/multi-node/invalid-attestations/invalidate_block.parallel.test.ts @@ -818,12 +818,12 @@ describe('multi-node/invalid-attestations/invalidate_block', () => { }); }); - // Injects a non-proposer attestation slot in yParity (v ∈ {0, 1}) form in the packed L1 tuple. L1 accepts - // it at propose() (attestation signatures are not verified there), but the checkpoint can never be proven - // (ValidatorSelectionLib.verifyAttestations → ECDSA.recover rejects v ∉ {27, 28}). A repack of the - // invalidation evidence would canonicalize the byte back to 27/28 and diverge from the on-chain - // attestationsHash, reverting the invalidation. Verifies a good proposer detects it (Part A) and - // invalidates it byte-faithfully from the raw calldata (Part B). Regression for A-1401. + // Injects every non-proposer attestation slot in yParity (v ∈ {0, 1}) form in the packed L1 tuple. L1 + // accepts it at propose() (attestation signatures are not verified there), but the checkpoint can never be + // proven (ValidatorSelectionLib.verifyAttestations → ECDSA.recover rejects v ∉ {27, 28}). A repack of the + // invalidation evidence would canonicalize the bytes back to 27/28 and diverge from the on-chain + // attestationsHash, reverting the invalidation. Verifies an honest proposer detects the bad checkpoint and + // invalidates it byte-faithfully from the raw calldata. Regression for A-1401. it('proposer invalidates checkpoint with a yParity attestation slot', async () => { await runInvalidationTest({ attackConfig: { injectYParityAttestation: true }, diff --git a/yarn-project/sequencer-client/src/publisher/l1_publisher.integration.test.ts b/yarn-project/sequencer-client/src/publisher/l1_publisher.integration.test.ts index bbca7125e7ae..475f61c76b9d 100644 --- a/yarn-project/sequencer-client/src/publisher/l1_publisher.integration.test.ts +++ b/yarn-project/sequencer-client/src/publisher/l1_publisher.integration.test.ts @@ -893,7 +893,7 @@ describe('L1Publisher integration', () => { checkpoint: checkpoint.toCheckpointInfo(), attestors: [], attestations: badAttestations, - packedAttestations: CommitteeAttestationsAndSigners.packAttestations(badAttestations), + verbatimAttestations: CommitteeAttestationsAndSigners.packAttestations(badAttestations), epoch: EpochNumber(1), seed: 1n, reason: 'insufficient-attestations', diff --git a/yarn-project/sequencer-client/src/publisher/sequencer-publisher.test.ts b/yarn-project/sequencer-client/src/publisher/sequencer-publisher.test.ts index 1b162a02ffc2..363a60540308 100644 --- a/yarn-project/sequencer-client/src/publisher/sequencer-publisher.test.ts +++ b/yarn-project/sequencer-client/src/publisher/sequencer-publisher.test.ts @@ -1123,7 +1123,7 @@ describe('SequencerPublisher', () => { timestamp: 0n, }; - const makeInvalidResult = (packedAttestations: ViemCommitteeAttestations): ValidateCheckpointResult => ({ + const makeInvalidResult = (verbatimAttestations: ViemCommitteeAttestations): ValidateCheckpointResult => ({ valid: false, reason: 'invalid-attestation', checkpoint, @@ -1133,7 +1133,7 @@ describe('SequencerPublisher', () => { attestors: [], invalidIndex: 1, attestations: [], - packedAttestations, + verbatimAttestations, }); beforeEach(() => { @@ -1150,11 +1150,5 @@ describe('SequencerPublisher', () => { await publisher.simulateInvalidateCheckpoint(makeInvalidResult(packed)); expect(rollup.buildInvalidateBadAttestationRequest).toHaveBeenCalledWith(checkpointNumber, packed, committee, 1); }); - - it('throws rather than repacking when the raw packed tuple is missing', async () => { - await expect(publisher.simulateInvalidateCheckpoint(makeInvalidResult(undefined as any))).rejects.toThrow( - /missing packed attestations/, - ); - }); }); }); diff --git a/yarn-project/sequencer-client/src/publisher/sequencer-publisher.ts b/yarn-project/sequencer-client/src/publisher/sequencer-publisher.ts index f7746e9a160d..0b8f9f0a6b04 100644 --- a/yarn-project/sequencer-client/src/publisher/sequencer-publisher.ts +++ b/yarn-project/sequencer-client/src/publisher/sequencer-publisher.ts @@ -915,13 +915,8 @@ export class SequencerPublisher { // Use the exact packed tuple posted to L1 verbatim. A repack via `packAttestations` is not a // byte-faithful inverse of `fromPacked` (a canonicalized yParity byte or an all-zero signature slot // round-trips differently), so it would diverge from the stored `attestationsHash` and revert the - // invalidation. Fail loud rather than fall back to a repack, which would silently re-wedge the chain. - const attestationsAndSigners = validationResult.packedAttestations; - if (!attestationsAndSigners) { - throw new Error( - `Cannot build invalidation for checkpoint ${checkpoint.checkpointNumber}: missing packed attestations from L1 calldata`, - ); - } + // invalidation. + const attestationsAndSigners = validationResult.verbatimAttestations; if (reason === 'invalid-attestation') { return this.rollupContract.buildInvalidateBadAttestationRequest( diff --git a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts index f4f3c6cdc715..bac6571e1b66 100644 --- a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts +++ b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts @@ -1014,7 +1014,7 @@ describe('CheckpointProposalJob', () => { attestors: [EthAddress.random()], invalidIndex: 0, attestations: [CommitteeAttestation.random()], - packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, + verbatimAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, }; l2BlockSource.getPendingChainValidationStatus.mockResolvedValue(invalidValidation); @@ -1052,7 +1052,7 @@ describe('CheckpointProposalJob', () => { attestors: [EthAddress.random()], invalidIndex: 0, attestations: [CommitteeAttestation.random()], - packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, + verbatimAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, }); await pipelinedJob.executeAndAwait(); @@ -1086,7 +1086,7 @@ describe('CheckpointProposalJob', () => { attestors: [EthAddress.random()], invalidIndex: 0, attestations: [CommitteeAttestation.random()], - packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, + verbatimAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, }; l2BlockSource.getPendingChainValidationStatus.mockResolvedValue(invalidValidation); diff --git a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts index dd1a6f5dcd6e..729aa8039b87 100644 --- a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts +++ b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts @@ -31,9 +31,9 @@ import { type L2BlockSink, type L2BlockSource, MaliciousCommitteeAttestationsAndSigners, + MaliciousYParityCommitteeAttestationsAndSigners, type ProposedCheckpointSink, type ValidateCheckpointResult, - YParityCommitteeAttestationsAndSigners, } from '@aztec/stdlib/block'; import { type Checkpoint, @@ -1487,21 +1487,16 @@ export class CheckpointProposalJob implements Traceable { } if (this.config.injectYParityAttestation) { - // Pick a non-proposer signed slot and force its recovery byte to yParity (v ∈ {0, 1}) form in the - // packed L1 tuple, after packAttestations has canonicalized it. Models a malicious proposer landing - // a slot L1 accepts at propose() but that can never be proven (ECDSA.recover rejects v ∉ {27, 28}). - const nonProposerIndices: number[] = []; - for (let i = 0; i < attestations.length; i++) { - if (!attestations[i].signature.isEmpty() && i !== proposerIndex) { - nonProposerIndices.push(i); - } - } - if (nonProposerIndices.length > 0) { - const targetIndex = nonProposerIndices[randomInt(nonProposerIndices.length)]; - this.log.warn(`Injecting yParity attestation in checkpoint for slot ${slotNumber} at index ${targetIndex}`); - return new YParityCommitteeAttestationsAndSigners(attestations, targetIndex, this.getSignatureContext()); - } - return new CommitteeAttestationsAndSigners(attestations, this.getSignatureContext()); + // Force every non-proposer signed slot's recovery byte to yParity (v ∈ {0, 1}) form in the packed L1 + // tuple, after packAttestations has canonicalized it. The proposer's own slot is left canonical so + // propose() still passes verifyProposer. Models a malicious proposer landing a checkpoint L1 accepts + // but that can never be proven (ECDSA.recover rejects v ∉ {27, 28}). + this.log.warn(`Injecting yParity attestations in checkpoint for slot ${slotNumber} (proposer #${proposerIndex})`); + return new MaliciousYParityCommitteeAttestationsAndSigners( + attestations, + proposerIndex, + this.getSignatureContext(), + ); } if (this.config.shuffleAttestationOrdering) { diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts index 3f58fae13e4e..234114eeb189 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts @@ -124,7 +124,7 @@ describe('sequencer', () => { seed: 0n, attestors: [], attestations: [], - packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, + verbatimAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, reason: 'insufficient-attestations', }; @@ -1185,7 +1185,7 @@ describe('sequencer', () => { seed: 123n, attestors: [], attestations: [], - packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, + verbatimAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, reason: 'insufficient-attestations', }; diff --git a/yarn-project/slasher/src/watchers/attestations_block_watcher.test.ts b/yarn-project/slasher/src/watchers/attestations_block_watcher.test.ts index 43b7e9499415..6dfe8e0dabcd 100644 --- a/yarn-project/slasher/src/watchers/attestations_block_watcher.test.ts +++ b/yarn-project/slasher/src/watchers/attestations_block_watcher.test.ts @@ -68,7 +68,7 @@ describe('AttestationsBlockWatcher', () => { seed: 0n, attestors: [], attestations: [], - packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, + verbatimAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, }; const event: InvalidCheckpointDetectedEvent = { @@ -100,7 +100,7 @@ describe('AttestationsBlockWatcher', () => { attestors: [], invalidIndex: 0, attestations: [], - packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, + verbatimAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, }; const event: InvalidCheckpointDetectedEvent = { @@ -148,7 +148,7 @@ describe('AttestationsBlockWatcher', () => { seed: 0n, attestors: [attestor1, attestor2], attestations: [], - packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, + verbatimAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, }; // First event: slash the proposer for the invalid attestations on its own checkpoint. @@ -196,7 +196,7 @@ describe('AttestationsBlockWatcher', () => { seed: 0n, attestors: [], attestations: [], - packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, + verbatimAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, }; watcher.handleInvalidCheckpoint({ type: 'invalidCheckpointDetected', @@ -261,7 +261,7 @@ describe('AttestationsBlockWatcher', () => { seed: 0n, attestors: [], attestations: [], - packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, + verbatimAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, }, }); @@ -325,7 +325,7 @@ describe('AttestationsBlockWatcher', () => { seed: 0n, attestors: [], attestations: [], - packedAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, + verbatimAttestations: { signatureIndices: '0x', signaturesOrAddresses: '0x' }, }; const event: InvalidCheckpointDetectedEvent = { diff --git a/yarn-project/stdlib/src/block/proposal/attestations_and_signers.test.ts b/yarn-project/stdlib/src/block/proposal/attestations_and_signers.test.ts index bda879638062..79a11b1a4315 100644 --- a/yarn-project/stdlib/src/block/proposal/attestations_and_signers.test.ts +++ b/yarn-project/stdlib/src/block/proposal/attestations_and_signers.test.ts @@ -7,7 +7,10 @@ import { bufferToHex } from '@aztec/foundation/string'; import { type AbiParameter, encodeAbiParameters, keccak256 } from 'viem'; import { TEST_COORDINATION_SIGNATURE_CONTEXT } from '../../tests/mocks.js'; -import { CommitteeAttestationsAndSigners, YParityCommitteeAttestationsAndSigners } from './attestations_and_signers.js'; +import { + CommitteeAttestationsAndSigners, + MaliciousYParityCommitteeAttestationsAndSigners, +} from './attestations_and_signers.js'; import { CommitteeAttestation } from './committee_attestation.js'; const committeeAttestationsStruct: AbiParameter = { @@ -113,23 +116,23 @@ describe('packAttestations is not a byte-faithful inverse of fromPacked', () => }); }); -describe('YParityCommitteeAttestationsAndSigners', () => { - it('rewrites only the target slot to yParity form while keeping getSigners and the bitmap consistent', () => { +describe('MaliciousYParityCommitteeAttestationsAndSigners', () => { + it('rewrites every non-proposer signed slot to yParity form while keeping getSigners and the bitmap consistent', () => { const attestations = [signing(27), signing(28), CommitteeAttestation.fromAddress(EthAddress.random()), signing(27)]; - const targetIndex = 1; - const bundle = new YParityCommitteeAttestationsAndSigners( + const proposerIndex = 0; + const bundle = new MaliciousYParityCommitteeAttestationsAndSigners( attestations, - targetIndex, + proposerIndex, TEST_COORDINATION_SIGNATURE_CONTEXT, ); const packed = bundle.getPackedAttestations(); const unpacked = CommitteeAttestation.fromPacked(packed, attestations.length); - // The target slot carries a non-canonical yParity recovery byte; all other signed slots stay canonical. - expect([0, 1]).toContain(unpacked[targetIndex].signature.v); - expect([27, 28]).toContain(unpacked[0].signature.v); - expect([27, 28]).toContain(unpacked[3].signature.v); + // The proposer's own slot stays canonical; every other signed slot carries a yParity recovery byte. + expect([27, 28]).toContain(unpacked[proposerIndex].signature.v); + expect([0, 1]).toContain(unpacked[1].signature.v); + expect([0, 1]).toContain(unpacked[3].signature.v); // The bitmap and signers are untouched, so propose() would not revert SignersSizeMismatch. const honest = new CommitteeAttestationsAndSigners(attestations, TEST_COORDINATION_SIGNATURE_CONTEXT); @@ -137,25 +140,26 @@ describe('YParityCommitteeAttestationsAndSigners', () => { expect(popcount(packed.signatureIndices)).toEqual(bundle.getSigners().length); }); - it('preserves (r, s) of the target slot so it still recovers to the same signer', () => { + it('preserves (r, s) of the rewritten slots so they still recover to the same signer', () => { const attestations = [signing(27), signing(28)]; - const targetIndex = 1; + const proposerIndex = 0; + const flippedIndex = 1; const honest = new CommitteeAttestationsAndSigners(attestations, TEST_COORDINATION_SIGNATURE_CONTEXT); - const bundle = new YParityCommitteeAttestationsAndSigners( + const bundle = new MaliciousYParityCommitteeAttestationsAndSigners( attestations, - targetIndex, + proposerIndex, TEST_COORDINATION_SIGNATURE_CONTEXT, ); - const honestTarget = CommitteeAttestation.fromPacked(honest.getPackedAttestations(), attestations.length)[ - targetIndex + const honestSlot = CommitteeAttestation.fromPacked(honest.getPackedAttestations(), attestations.length)[ + flippedIndex ]; - const maliciousTarget = CommitteeAttestation.fromPacked(bundle.getPackedAttestations(), attestations.length)[ - targetIndex + const maliciousSlot = CommitteeAttestation.fromPacked(bundle.getPackedAttestations(), attestations.length)[ + flippedIndex ]; - expect(maliciousTarget.signature.r).toEqual(honestTarget.signature.r); - expect(maliciousTarget.signature.s).toEqual(honestTarget.signature.s); - expect(maliciousTarget.signature.v).toEqual(honestTarget.signature.v - 27); + expect(maliciousSlot.signature.r).toEqual(honestSlot.signature.r); + expect(maliciousSlot.signature.s).toEqual(honestSlot.signature.s); + expect(maliciousSlot.signature.v).toEqual(honestSlot.signature.v - 27); }); }); diff --git a/yarn-project/stdlib/src/block/proposal/attestations_and_signers.ts b/yarn-project/stdlib/src/block/proposal/attestations_and_signers.ts index 6b8a3c37b046..d5444b4f1611 100644 --- a/yarn-project/stdlib/src/block/proposal/attestations_and_signers.ts +++ b/yarn-project/stdlib/src/block/proposal/attestations_and_signers.ts @@ -159,19 +159,20 @@ export class MaliciousCommitteeAttestationsAndSigners extends CommitteeAttestati } /** - * Malicious extension of CommitteeAttestationsAndSigners that rewrites one non-empty signature slot's - * recovery byte to yParity form (v ∈ {0, 1}) in the packed output, after the honest `packAttestations` - * has already canonicalized it to v ∈ {27, 28}. Models a malicious selected proposer that hand-crafts - * `propose()` calldata L1 accepts but no honest node can byte-replay: the signature still recovers to the - * same member (r, s and the recovery parity are preserved), the bitmap bit stays set, and `getSigners()` - * stays consistent, so `propose()` does not revert `SignersSizeMismatch` -- yet the checkpoint can never - * be proven (`ECDSA.recover` rejects v ∉ {27, 28}). For testing only. + * Malicious extension of CommitteeAttestationsAndSigners that rewrites every non-proposer signature slot's + * recovery byte to yParity form (v ∈ {0, 1}) in the packed output, after the honest `packAttestations` has + * already canonicalized it to v ∈ {27, 28}. Models a malicious selected proposer that hand-crafts + * `propose()` calldata L1 accepts but no honest node can byte-replay: each rewritten signature still + * recovers to the same member (r, s and the recovery parity are preserved), the bitmap bits stay set, and + * `getSigners()` stays consistent, so `propose()` does not revert `SignersSizeMismatch` -- yet the + * checkpoint can never be proven (`ECDSA.recover` rejects v ∉ {27, 28}). The proposer's own slot is left + * canonical so L1 `verifyProposer` (which recovers that slot) still accepts the checkpoint. For testing only. */ -export class YParityCommitteeAttestationsAndSigners extends CommitteeAttestationsAndSigners { +export class MaliciousYParityCommitteeAttestationsAndSigners extends CommitteeAttestationsAndSigners { constructor( attestations: CommitteeAttestation[], - /** Committee index of the (non-empty, non-proposer) signature slot whose recovery byte to force to yParity. */ - private targetIndex: number, + /** Committee index of the proposer's own slot, left canonical so `propose()` passes `verifyProposer`. */ + private proposerIndex: number, signatureContext: CoordinationSignatureContext, ) { super(attestations, signatureContext); @@ -181,20 +182,16 @@ export class YParityCommitteeAttestationsAndSigners extends CommitteeAttestation const packed = super.getPackedAttestations(); const data = hexToBuffer(packed.signaturesOrAddresses); - // Walk the packed byte-vector to find the v-byte offset of the target slot. A signed slot occupies - // 65 bytes (v, r, s); an empty slot occupies 20 bytes (address only). + // Walk the packed byte-vector and rewrite every non-proposer signed slot's v-byte to yParity form. A + // signed slot occupies 65 bytes (v, r, s); an empty slot occupies 20 bytes (address only). let offset = 0; for (let i = 0; i < this.attestations.length; i++) { const isSigned = !this.attestations[i].signature.isEmpty(); - if (i === this.targetIndex) { - if (!isSigned) { - throw new Error(`Target slot ${i} is not a signature slot; cannot force a yParity recovery byte`); - } + if (isSigned && i !== this.proposerIndex) { // `packAttestations` canonicalized v to 27/28; rewrite back to the equivalent yParity byte (0/1), // preserving the recovery parity so the signature still recovers to the same member. const v = data[offset]; data[offset] = v >= 27 ? v - 27 : v; - break; } offset += isSigned ? 65 : 20; } diff --git a/yarn-project/stdlib/src/block/validate_block_result.test.ts b/yarn-project/stdlib/src/block/validate_block_result.test.ts index 5b12380f2b5c..b634264b0178 100644 --- a/yarn-project/stdlib/src/block/validate_block_result.test.ts +++ b/yarn-project/stdlib/src/block/validate_block_result.test.ts @@ -15,7 +15,7 @@ import { describe('ValidateCheckpointResult', () => { // A non-trivial packed tuple with a signature slot and an address-only slot, so the round-trip exercises // both packed segments rather than the empty (0x, 0x) tuple. - const packedAttestations = CommitteeAttestationsAndSigners.packAttestations([ + const verbatimAttestations = CommitteeAttestationsAndSigners.packAttestations([ new CommitteeAttestation(EthAddress.ZERO, new Signature(Buffer32.random(), Buffer32.random(), 27)), CommitteeAttestation.fromAddress(EthAddress.random()), ]); @@ -39,7 +39,7 @@ describe('ValidateCheckpointResult', () => { attestors: [EthAddress.random(), EthAddress.random()], invalidIndex: 4, attestations: [CommitteeAttestation.random(), CommitteeAttestation.random()], - packedAttestations, + verbatimAttestations, }; const serialized = serializeValidateCheckpointResult(result); const deserialized = deserializeValidateCheckpointResult(serialized); @@ -56,7 +56,7 @@ describe('ValidateCheckpointResult', () => { seed: 2n, attestors: [EthAddress.random(), EthAddress.random()], attestations: [CommitteeAttestation.random(), CommitteeAttestation.random()], - packedAttestations, + verbatimAttestations, }; const serialized = serializeValidateCheckpointResult(result); const deserialized = deserializeValidateCheckpointResult(serialized); @@ -74,11 +74,11 @@ describe('ValidateCheckpointResult', () => { attestors: [EthAddress.random()], invalidIndex: 0, attestations: [CommitteeAttestation.random()], - packedAttestations, + verbatimAttestations, }; const deserialized = deserializeValidateCheckpointResult(serializeValidateCheckpointResult(result)); expect(deserialized.valid).toBe(false); - expect((deserialized as typeof result).packedAttestations).toEqual(packedAttestations); + expect((deserialized as typeof result).verbatimAttestations).toEqual(verbatimAttestations); }); }); }); diff --git a/yarn-project/stdlib/src/block/validate_block_result.ts b/yarn-project/stdlib/src/block/validate_block_result.ts index 88aa22bb918f..0e809bb519f2 100644 --- a/yarn-project/stdlib/src/block/validate_block_result.ts +++ b/yarn-project/stdlib/src/block/validate_block_result.ts @@ -39,7 +39,7 @@ export type ValidateCheckpointNegativeResult = * and revert `invalidateBadAttestation`/`invalidateInsufficientAttestations`. Always populated on the * calldata validation path, the only production source of invalidation evidence. */ - packedAttestations: ViemCommitteeAttestations; + verbatimAttestations: ViemCommitteeAttestations; /** Reason for the checkpoint being invalid: not enough attestations were posted */ reason: 'insufficient-attestations'; } @@ -62,7 +62,7 @@ export type ValidateCheckpointNegativeResult = * invalidation evidence is byte-faithful to the stored `attestationsHash`. See the same field on the * insufficient-attestations variant for why a repack cannot be used. */ - packedAttestations: ViemCommitteeAttestations; + verbatimAttestations: ViemCommitteeAttestations; /** Reason for the checkpoint being invalid: an invalid attestation was posted */ reason: 'invalid-attestation'; /** Index in the attestations array of the invalid attestation posted */ @@ -88,7 +88,7 @@ export const ValidateCheckpointResultSchema: ZodFor = seed: schemas.BigInt, attestors: z.array(schemas.EthAddress), attestations: z.array(CommitteeAttestation.schema), - packedAttestations: ViemCommitteeAttestationsSchema, + verbatimAttestations: ViemCommitteeAttestationsSchema, reason: z.literal('insufficient-attestations'), }), z.object({ @@ -99,7 +99,7 @@ export const ValidateCheckpointResultSchema: ZodFor = seed: schemas.BigInt, attestors: z.array(schemas.EthAddress), attestations: z.array(CommitteeAttestation.schema), - packedAttestations: ViemCommitteeAttestationsSchema, + verbatimAttestations: ViemCommitteeAttestationsSchema, reason: z.literal('invalid-attestation'), invalidIndex: z.number(), }), @@ -111,8 +111,8 @@ export function serializeValidateCheckpointResult(result: ValidateCheckpointResu } const checkpointBuffer = serializeCheckpointInfo(result.checkpoint); - const signatureIndices = hexToBuffer(result.packedAttestations.signatureIndices); - const signaturesOrAddresses = hexToBuffer(result.packedAttestations.signaturesOrAddresses); + const signatureIndices = hexToBuffer(result.verbatimAttestations.signatureIndices); + const signaturesOrAddresses = hexToBuffer(result.verbatimAttestations.signaturesOrAddresses); return serializeToBuffer( result.valid, result.reason, @@ -148,13 +148,13 @@ export function deserializeValidateCheckpointResult(bufferOrReader: Buffer | Buf const attestors = reader.readVector(EthAddress, MAX_COMMITTEE_SIZE); const attestations = reader.readVector(CommitteeAttestation, MAX_COMMITTEE_SIZE); const invalidIndex = reader.readNumber(); - const packedAttestations: ViemCommitteeAttestations = { + const verbatimAttestations: ViemCommitteeAttestations = { signatureIndices: bufferToHex(reader.readBuffer()), signaturesOrAddresses: bufferToHex(reader.readBuffer()), }; if (reason === 'insufficient-attestations') { - return { valid, reason, checkpoint, committee, epoch, seed, attestors, attestations, packedAttestations }; + return { valid, reason, checkpoint, committee, epoch, seed, attestors, attestations, verbatimAttestations }; } else if (reason === 'invalid-attestation') { return { valid, @@ -166,7 +166,7 @@ export function deserializeValidateCheckpointResult(bufferOrReader: Buffer | Buf attestors, invalidIndex, attestations, - packedAttestations, + verbatimAttestations, }; } else { const _: never = reason; From 7b2715bb24b8bd1d3bf92734caae4367b3e9f8d2 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Mon, 6 Jul 2026 12:35:23 -0300 Subject: [PATCH 5/5] test(e2e): drop redundant yParity proving test and orphaned attester 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. --- ...arity_attestation_proving.parallel.test.ts | 103 ------------------ .../stdlib/src/interfaces/validator.ts | 7 -- yarn-project/validator-client/src/config.ts | 4 - .../validator-client/src/validator.ts | 21 +--- 4 files changed, 1 insertion(+), 134 deletions(-) delete mode 100644 yarn-project/end-to-end/src/multi-node/invalid-attestations/yparity_attestation_proving.parallel.test.ts diff --git a/yarn-project/end-to-end/src/multi-node/invalid-attestations/yparity_attestation_proving.parallel.test.ts b/yarn-project/end-to-end/src/multi-node/invalid-attestations/yparity_attestation_proving.parallel.test.ts deleted file mode 100644 index 1a0ec3e188e4..000000000000 --- a/yarn-project/end-to-end/src/multi-node/invalid-attestations/yparity_attestation_proving.parallel.test.ts +++ /dev/null @@ -1,103 +0,0 @@ -import type { AztecNodeService } from '@aztec/aztec-node'; -import type { Logger } from '@aztec/aztec.js/log'; -import type { Delayer } from '@aztec/ethereum/l1-tx-utils'; -import { asyncMap } from '@aztec/foundation/async-map'; -import { CheckpointNumber } from '@aztec/foundation/branded-types'; -import { retryUntil } from '@aztec/foundation/retry'; - -import { jest } from '@jest/globals'; - -import type { EndToEndContext } from '../../fixtures/utils.js'; -import { - MOCK_GOSSIP_MULTI_VALIDATOR_OPTS, - MULTI_VALIDATOR_BLOCK_PRODUCTION_TIMING, - MultiNodeTestContext, - type RegisteredValidator, - buildMockGossipValidators, -} from '../multi_node_test_context.js'; - -jest.setTimeout(1000 * 60 * 10); - -const NODE_COUNT = 3; - -// A committee member that emits its own attestations in yParity (v ∈ {0, 1}) form models the A-1351 -// attester-side DoS. Without normalization the non-canonical recovery byte reaches L1 in the honest -// proposer's bundle and epoch proving reverts (ValidatorSelectionLib.verifyAttestations → ECDSA.recover -// rejects v ∉ {27, 28}), so the on-chain proven tip stalls. With A-1351 applied the honest proposer -// canonicalizes the byte on pool ingress and again in orderAttestations before the L1 bundle, so the -// checkpoint lands canonical, the epoch proves, and the proven tip advances. -// -// This is a prover-backed regression: the base invalidation suite starts no prover node and A-1351 -// produces no on-chain event, so proven-tip advancement is the only observable signal. Modelled on -// block-production/proof_boundary.parallel.test.ts (createProverNode + Delayer). To reproduce the red -// state, revert the three A-1351 normalization layers (orderAttestations, the attestation pool ingress, -// CheckpointAttestation.withNormalizedSignature) and the packAttestations v-canonicalization. The proven -// tip then stays at 0 and the test times out: on a pre-A-1401 tree the proof tx is sent but reverts on L1 -// (ECDSA.recover), while on this branch A-1401's own detection invalidates the non-canonical checkpoint -// before it can be proven. Either way, without A-1351 the epoch never proves. -describe('multi-node/invalid-attestations/yparity_attestation_proving', () => { - let context: EndToEndContext; - let logger: Logger; - let test: MultiNodeTestContext; - let validators: RegisteredValidator[]; - let proverNode: AztecNodeService; - - afterEach(async () => { - jest.restoreAllMocks(); - await test?.teardown(); - }); - - it('proves an epoch when a committee member emits yParity attestations', async () => { - validators = buildMockGossipValidators(NODE_COUNT); - test = await MultiNodeTestContext.setup({ - ...MOCK_GOSSIP_MULTI_VALIDATOR_OPTS, - ...MULTI_VALIDATOR_BLOCK_PRODUCTION_TIMING, - initialValidators: validators, - aztecProofSubmissionEpochs: 1, - }); - ({ context, logger } = test); - - // node 0 is the malicious committee member (rewrites its own attestations to yParity form); the rest - // are honest. One malicious member out of three still leaves quorum, so checkpoints keep landing. - logger.warn(`Starting ${NODE_COUNT} validator nodes (node 0 emits yParity attestations)`); - await asyncMap(validators, ({ privateKey }, i) => - test.createValidatorNode([privateKey], { - minTxsPerBlock: 0, - maxTxsPerBlock: 1, - injectYParityOwnAttestation: i === 0, - }), - ); - - proverNode = await test.createProverNode({ cancelTxOnTimeout: false, maxSpeedUpAttempts: 0, dontStart: true }); - context.proverNode = proverNode; - await proverNode.getProverNode()!.start(); - const proverDelayer: Delayer = proverNode.getProverNode()!.getDelayer()!; - - // Let the chain build the first checkpoint, produced while the malicious member is gossiping. - await test.waitUntilCheckpointNumber(CheckpointNumber(1), test.L2_SLOT_DURATION_IN_S * 6); - - // Advance the L1 clock by a full epoch so epoch 0 becomes provable, keeping the interval miner running. - const block = await test.l1Client.getBlock({ includeTransactions: false }); - const warpTo = Number(block.timestamp) + test.epochDuration * test.L2_SLOT_DURATION_IN_S; - logger.warn(`Warping L1 to ${warpTo} to make epoch 0 provable`); - await test.context.cheatCodes.eth.warp(warpTo, { resetBlockInterval: true }); - - // The prover must submit a proof AND the on-chain proven tip must advance past genesis. In the red - // state (A-1351 reverted) the proof tx is still sent but reverts, so the proven tip stays at 0. - await retryUntil( - async () => { - await test.monitor.run(true); - const provenNumber = await test.rollup.getProvenCheckpointNumber(); - return proverDelayer.getSentTxHashes().length > 0 && Number(provenNumber) >= 1; - }, - 'prover advances L1 proven checkpoint despite yParity attestations', - test.L2_SLOT_DURATION_IN_S * 16, - 1, - ); - - expect(proverDelayer.getSentTxHashes().length).toBeGreaterThan(0); - expect(Number(await test.rollup.getProvenCheckpointNumber())).toBeGreaterThanOrEqual(1); - - logger.warn(`Test succeeded '${expect.getState().currentTestName}'`); - }); -}); diff --git a/yarn-project/stdlib/src/interfaces/validator.ts b/yarn-project/stdlib/src/interfaces/validator.ts index 62b2521d2a14..f17e96e2a475 100644 --- a/yarn-project/stdlib/src/interfaces/validator.ts +++ b/yarn-project/stdlib/src/interfaces/validator.ts @@ -66,12 +66,6 @@ export type ValidatorClientConfig = ValidatorHASignerConfig & /** Agree to attest to equivocated checkpoint proposals (for testing purposes only) */ attestToEquivocatedProposals?: boolean; - /** - * Rewrite this validator's own attestation signature to yParity (v ∈ {0, 1}) form after signing, - * modelling a malicious committee member (for testing purposes only). - */ - injectYParityOwnAttestation?: boolean; - /** Accept proposal validation regardless of slot timing (for testing only) */ skipProposalSlotValidation?: boolean; @@ -126,7 +120,6 @@ export const ValidatorClientConfigSchema = zodFor WatcherEmitter) return undefined; } - let attestations = await this.validationService.attestToCheckpointProposal(proposal, attestors, checkpointNumber); - - if (this.config.injectYParityOwnAttestation) { - this.log.warn(`Injecting yParity form into own attestations for slot ${proposal.slotNumber}`); - attestations = attestations.map(a => toYParityAttestation(a)); - } + const attestations = await this.validationService.attestToCheckpointProposal(proposal, attestors, checkpointNumber); // Track the proposal we attested to (to prevent equivocation) this.lastAttestedProposal = proposal; @@ -1133,17 +1128,3 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) return authResponse.toBuffer(); } } - -/** - * Rewrites an attestation's recovery byte to yParity (v ∈ {0, 1}) form, preserving (r, s) and the recovery - * parity so it still recovers to the same signer. Models a malicious committee member; for testing only. - */ -function toYParityAttestation(attestation: CheckpointAttestation): CheckpointAttestation { - const signature = attestation.signature; - const yParityV = signature.v >= 27 ? signature.v - 27 : signature.v; - return new CheckpointAttestation( - attestation.payload, - new Signature(signature.r, signature.s, yParityV), - attestation.proposerSignature, - ); -}