From d58ee3548283c99d065d696734343c475906275d Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Fri, 27 Feb 2026 16:21:19 -0300 Subject: [PATCH] feat(sequencer): e2e tests for invalid signature recovery in checkpoint attestations Adds end-to-end tests covering A-71: ensuring nodes correctly detect and invalidate checkpoints with malleable (high-s value) or unrecoverable attestation signatures. Also refactors the invalidate block test suite to reduce code duplication via shared helpers. - Add `injectHighSValueAttestation` and `injectUnrecoverableSignatureAttestation` sequencer config options for testing - Update `Signature.random()` to produce valid ECDSA signatures with low s-values - Add `generateRecoverableSignature` and `generateUnrecoverableSignature` utilities - Refactor e2e invalidate block tests with `runDoubleInvalidationTest` and `runSingleInvalidationTest` helpers - Add unit tests for high-s validation, signature utilities, and Signature.random() Co-Authored-By: Claude Opus 4.6 --- .../archiver/src/modules/validation.test.ts | 23 +- .../epochs_invalidate_block.parallel.test.ts | 360 +++++++----------- .../secp256k1-signer/malleability.test.ts | 20 + .../src/crypto/secp256k1-signer/utils.ts | 32 ++ .../src/eth-signature/eth_signature.test.ts | 24 +- .../src/eth-signature/eth_signature.ts | 8 +- yarn-project/sequencer-client/src/config.ts | 10 + .../src/sequencer/checkpoint_proposal_job.ts | 34 +- yarn-project/stdlib/src/interfaces/configs.ts | 6 + 9 files changed, 293 insertions(+), 224 deletions(-) diff --git a/yarn-project/archiver/src/modules/validation.test.ts b/yarn-project/archiver/src/modules/validation.test.ts index aa11589bb5d5..0bfeb50f1566 100644 --- a/yarn-project/archiver/src/modules/validation.test.ts +++ b/yarn-project/archiver/src/modules/validation.test.ts @@ -2,7 +2,7 @@ import type { EpochCache } from '@aztec/epoch-cache'; import { CheckpointNumber, EpochNumber, SlotNumber } from '@aztec/foundation/branded-types'; import { Buffer32 } from '@aztec/foundation/buffer'; import { times } from '@aztec/foundation/collection'; -import { Secp256k1Signer } from '@aztec/foundation/crypto/secp256k1-signer'; +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'; @@ -153,6 +153,27 @@ describe('validateCheckpointAttestations', () => { expect(result.invalidIndex).toBe(0); }); + it('fails if an attestation signature has a high-s value (malleable signature)', async () => { + const checkpoint = await makeCheckpoint(signers.slice(0, 4), committee); + + // Flip the signature at index 2 to give it a high-s value + const original = checkpoint.attestations[2]; + const flipped = flipSignature(original.signature); + checkpoint.attestations[2] = new CommitteeAttestation(original.address, flipped); + + // Verify the flipped signature is detected as invalid + const attestations = getAttestationInfoFromPublishedCheckpoint(checkpoint); + expect(attestations[2].status).toBe('invalid-signature'); + + const result = await validateCheckpointAttestations(checkpoint, epochCache, constants, logger); + assert(!result.valid); + assert(result.reason === 'invalid-attestation'); + expect(result.checkpoint.checkpointNumber).toEqual(checkpoint.checkpoint.number); + expect(result.checkpoint.archive.toString()).toEqual(checkpoint.checkpoint.archive.root.toString()); + expect(result.committee).toEqual(committee); + 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/end-to-end/src/e2e_epochs/epochs_invalidate_block.parallel.test.ts b/yarn-project/end-to-end/src/e2e_epochs/epochs_invalidate_block.parallel.test.ts index 80c2fec76ff0..e4740dde721d 100644 --- a/yarn-project/end-to-end/src/e2e_epochs/epochs_invalidate_block.parallel.test.ts +++ b/yarn-project/end-to-end/src/e2e_epochs/epochs_invalidate_block.parallel.test.ts @@ -25,7 +25,6 @@ import { privateKeyToAccount } from 'viem/accounts'; import { getAnvilPort } from '../fixtures/fixtures.js'; import { type EndToEndContext, getPrivateKeyFromIndex } from '../fixtures/utils.js'; -import { proveInteraction } from '../test-wallet/utils.js'; import { EpochsTestContext } from './epochs_test.js'; jest.setTimeout(1000 * 60 * 10); @@ -109,6 +108,120 @@ describe('e2e_epochs/epochs_invalidate_block', () => { await test.teardown(); }); + /** + * Configures all sequencers with an attack config, enables the attack for a single checkpoint, + * disables it after the first checkpoint is mined (also stopping block production), and waits + * for the checkpoint to be invalidated. Verifies the chain rolled back to the initial state. + */ + async function runInvalidationTest(opts: { + attackConfig: Record; + disableConfig: Record; + }) { + const sequencers = nodes.map(node => node.getSequencer()!); + const initialCheckpointNumber = (await nodes[0].getL2Tips()).checkpointed.checkpoint.number; + + sequencers.forEach(sequencer => { + sequencer.updateConfig({ ...opts.attackConfig, minTxsPerBlock: 0 }); + }); + + // Disable the attack after the first checkpoint is mined and prevent further block production + test.monitor.once('checkpoint', ({ checkpointNumber }) => { + logger.warn(`Disabling attack after checkpoint ${checkpointNumber} has been mined`); + sequencers.forEach(sequencer => { + sequencer.updateConfig({ ...opts.disableConfig, minTxsPerBlock: 100 }); + }); + }); + + await Promise.all(sequencers.map(s => s.start())); + + // Wait for the CheckpointInvalidated event + const checkpointInvalidatedFilter = await l1Client.createContractEventFilter({ + address: rollupContract.address, + abi: RollupAbi, + eventName: 'CheckpointInvalidated', + fromBlock: 1n, + toBlock: 'latest', + }); + + const checkpointInvalidatedEvents = await retryUntil( + async () => { + const events = await l1Client.getFilterLogs({ filter: checkpointInvalidatedFilter }); + return events.length > 0 ? events : undefined; + }, + 'CheckpointInvalidated event', + test.L2_SLOT_DURATION_IN_S * 5, + 0.1, + ); + + // Verify the checkpoint was invalidated and the chain rolled back + const [event] = checkpointInvalidatedEvents; + logger.warn(`CheckpointInvalidated event emitted`, { event }); + expect(event.args.checkpointNumber).toBeGreaterThan(initialCheckpointNumber); + expect(await test.rollup.getCheckpointNumber()).toEqual(initialCheckpointNumber); + + logger.warn(`Test succeeded '${expect.getState().currentTestName}'`); + } + + /** + * Configures all sequencers with an attack config, starts them, waits for two consecutive + * invalidations of the same checkpoint (confirming the invalid-then-re-invalidated pattern), + * disables the attack, and verifies the chain progresses and all nodes sync. + */ + async function runDoubleInvalidationTest(opts: { + attackConfig: Record; + disableConfig: Record; + }) { + const sequencers = nodes.map(node => node.getSequencer()!); + sequencers.forEach(sequencer => { + sequencer.updateConfig({ ...opts.attackConfig, minTxsPerBlock: 0 }); + }); + + await Promise.all(sequencers.map(s => s.start())); + + // Wait until we see two invalidations, both should be for the same checkpoint + let lastInvalidatedCheckpointNumber: CheckpointNumber | undefined; + const invalidatePromise = promiseWithResolvers(); + const unsubscribe = rollupContract.listenToCheckpointInvalidated(data => { + logger.warn(`Checkpoint ${data.checkpointNumber} has been invalidated`, data); + if (lastInvalidatedCheckpointNumber === undefined) { + lastInvalidatedCheckpointNumber = data.checkpointNumber; + } else { + expect(data.checkpointNumber).toEqual(lastInvalidatedCheckpointNumber); + invalidatePromise.resolve(); + unsubscribe(); + } + }); + await Promise.race([ + timeoutPromise(1000 * test.L2_SLOT_DURATION_IN_S * 8, 'Waiting for two checkpoint invalidations'), + invalidatePromise.promise, + ]); + + sequencers.forEach(sequencer => { + sequencer.updateConfig(opts.disableConfig); + }); + + // Ensure chain progresses + const targetCheckpointNumber = CheckpointNumber(lastInvalidatedCheckpointNumber! + 2); + logger.warn(`Waiting until checkpoint ${targetCheckpointNumber} has been mined`); + await test.monitor.waitUntilCheckpoint(targetCheckpointNumber); + + // Wait for all nodes to sync + const targetBlock = targetCheckpointNumber; + logger.warn(`Waiting for all nodes to sync to block ${targetBlock}`); + await retryUntil( + async () => { + const blockNumbers = await Promise.all(nodes.map(node => node.getBlockNumber())); + logger.info(`Node synced block numbers: ${blockNumbers.join(', ')}`); + return blockNumbers.every(bn => bn > targetBlock); + }, + 'Node sync check', + test.L2_SLOT_DURATION_IN_S * 5, + 0.5, + ); + + logger.warn(`Test succeeded '${expect.getState().currentTestName}'`); + } + it('proposer invalidates previous checkpoint with multiple blocks while posting its own', async () => { const sequencers = nodes.map(node => node.getSequencer()!); const [initialCheckpointNumber, initialBlockNumber] = await nodes[0] @@ -213,123 +326,46 @@ describe('e2e_epochs/epochs_invalidate_block', () => { // Slot S+1: Checkpoint N is invalidated, and checkpoint N' (same number) is proposed instead, but also has invalid attestations // Slot S+2: Proposer tries to invalidate checkpoint N, when they should invalidate checkpoint N' instead, and fails it('chain progresses if a checkpoint with insufficient attestations is invalidated with an invalid one', async () => { - // Configure all sequencers to skip collecting attestations before starting and always build blocks - logger.warn('Configuring all sequencers to skip attestation collection'); - const sequencers = nodes.map(node => node.getSequencer()!); - sequencers.forEach(sequencer => { - sequencer.updateConfig({ skipCollectingAttestations: true, minTxsPerBlock: 0 }); + await runDoubleInvalidationTest({ + attackConfig: { skipCollectingAttestations: true }, + disableConfig: { skipCollectingAttestations: false }, }); - - // Start all sequencers - await Promise.all(sequencers.map(s => s.start())); - logger.warn(`Started all sequencers with skipCollectingAttestations=true`); - - // Wait until we see two invalidations, both should be for the same checkpoint - let lastInvalidatedCheckpointNumber: CheckpointNumber | undefined; - const invalidatePromise = promiseWithResolvers(); - const unsubscribe = rollupContract.listenToCheckpointInvalidated(data => { - logger.warn(`Checkpoint ${data.checkpointNumber} has been invalidated`, data); - if (lastInvalidatedCheckpointNumber === undefined) { - lastInvalidatedCheckpointNumber = data.checkpointNumber; - } else { - expect(data.checkpointNumber).toEqual(lastInvalidatedCheckpointNumber); - invalidatePromise.resolve(); - unsubscribe(); - } - }); - await Promise.race([timeoutPromise(1000 * test.L2_SLOT_DURATION_IN_S * 8), invalidatePromise.promise]); - - // Disable skipCollectingAttestations and send txs so MBPS can produce multi-block checkpoints - sequencers.forEach(sequencer => { - sequencer.updateConfig({ skipCollectingAttestations: false }); - }); - logger.warn('Sending transactions to enable multi-block checkpoints'); - const from = context.accounts[0]; - for (let i = 0; i < 4; i++) { - const tx = await proveInteraction(context.wallet, testContract.methods.emit_nullifier(new Fr(100 + i)), { from }); - await tx.send({ wait: NO_WAIT }); - } - - // Ensure chain progresses - const targetCheckpointNumber = CheckpointNumber(lastInvalidatedCheckpointNumber! + 2); - logger.warn(`Waiting until checkpoint ${targetCheckpointNumber} has been mined`); - await test.monitor.waitUntilCheckpoint(targetCheckpointNumber); - - // Wait for all nodes to sync the new block - const targetBlock = targetCheckpointNumber; - logger.warn(`Waiting for all nodes to sync to block ${targetBlock}`); - await retryUntil( - async () => { - const blockNumbers = await Promise.all(nodes.map(node => node.getBlockNumber())); - logger.info(`Node synced block numbers: ${blockNumbers.join(', ')}`); - return blockNumbers.every(bn => bn > targetBlock); - }, - 'Node sync check', - test.L2_SLOT_DURATION_IN_S * 5, - 0.5, - ); - - await test.assertMultipleBlocksPerSlot(2); - - logger.warn(`Test succeeded '${expect.getState().currentTestName}'`); }); // Regression for Joe's Q42025 London attack. Same as above but with an invalid signature instead of insufficient ones. it('chain progresses if a checkpoint with an invalid attestation is invalidated with an invalid one', async () => { - // Configure all sequencers to skip collecting attestations before starting and always build blocks - logger.warn('Configuring all sequencers to inject one invalid attestation'); - const sequencers = nodes.map(node => node.getSequencer()!); - sequencers.forEach(sequencer => { - sequencer.updateConfig({ injectFakeAttestation: true, minTxsPerBlock: 0 }); + await runDoubleInvalidationTest({ + attackConfig: { injectFakeAttestation: true }, + disableConfig: { injectFakeAttestation: false }, }); + }); - // Start all sequencers - await Promise.all(sequencers.map(s => s.start())); - logger.warn(`Started all sequencers with injectFakeAttestation=true`); - - // Wait until we see two invalidations, both should be for the same checkpoint - let lastInvalidatedCheckpointNumber: CheckpointNumber | undefined; - const invalidatePromise = promiseWithResolvers(); - const unsubscribe = rollupContract.listenToCheckpointInvalidated(data => { - logger.warn(`Checkpoint ${data.checkpointNumber} has been invalidated`, data); - if (lastInvalidatedCheckpointNumber === undefined) { - lastInvalidatedCheckpointNumber = data.checkpointNumber; - } else { - expect(data.checkpointNumber).toEqual(lastInvalidatedCheckpointNumber); - invalidatePromise.resolve(); - unsubscribe(); - } + // Regression for A-71: Ensure the node correctly invalidates checkpoints where an attestation has a malleable + // signature (high-s value). The Rollup contract uses OpenZeppelin's ECDSA recover which rejects high-s values + // per EIP-2, so these signatures recover to address(0) on L1 but may succeed offchain. + it('proposer invalidates checkpoint with high-s value attestation', async () => { + await runInvalidationTest({ + attackConfig: { injectHighSValueAttestation: true }, + disableConfig: { injectHighSValueAttestation: false }, }); - await Promise.race([ - timeoutPromise(1000 * test.L2_SLOT_DURATION_IN_S * 8, 'Invalidating checkpoints'), - invalidatePromise.promise, - ]); + }); - // Disable injectFakeAttestations - sequencers.forEach(sequencer => { - sequencer.updateConfig({ injectFakeAttestation: false }); + // Regression for A-71: Ensure the node correctly invalidates checkpoints where an attestation's signature + // cannot be recovered (e.g. r=0). On L1, ecrecover returns address(0) for such signatures. + it('proposer invalidates checkpoint with unrecoverable signature attestation', async () => { + await runInvalidationTest({ + attackConfig: { injectUnrecoverableSignatureAttestation: true }, + disableConfig: { injectUnrecoverableSignatureAttestation: false }, }); + }); - // Ensure chain progresses - const targetCheckpointNumber = CheckpointNumber(lastInvalidatedCheckpointNumber! + 2); - logger.warn(`Waiting until checkpoint ${targetCheckpointNumber} has been mined`); - await test.monitor.waitUntilCheckpoint(targetCheckpointNumber); - - // Wait for all nodes to sync the new block - const targetBlock = targetCheckpointNumber; - logger.warn(`Waiting for all nodes to sync to block ${targetBlock}`); - await retryUntil( - async () => { - const blockNumbers = await Promise.all(nodes.map(node => node.getBlockNumber())); - logger.info(`Node synced block numbers: ${blockNumbers.join(', ')}`); - return blockNumbers.every(bn => bn > targetBlock); - }, - 'Node sync check', - test.L2_SLOT_DURATION_IN_S * 5, - 0.5, - ); - - logger.warn(`Test succeeded '${expect.getState().currentTestName}'`); + // Regression for the node accepting attestations that did not conform to the committee order, + // but L1 requires the same ordering. See #18219. + it('proposer invalidates previous block with shuffled attestations', async () => { + await runInvalidationTest({ + attackConfig: { shuffleAttestationOrdering: true }, + disableConfig: { shuffleAttestationOrdering: false }, + }); }); // Here we disable invalidation checks from two of the proposers. Our goal is to get two invalid checkpoints @@ -441,116 +477,6 @@ describe('e2e_epochs/epochs_invalidate_block', () => { logger.warn(`Test succeeded '${expect.getState().currentTestName}'`); }); - it('proposer invalidates previous checkpoint without publishing its own', async () => { - const sequencers = nodes.map(node => node.getSequencer()!); - const initialCheckpointNumber = (await nodes[0].getL2Tips()).checkpointed.checkpoint.number; - - // Configure all sequencers to skip collecting attestations before starting - logger.warn('Configuring all sequencers to skip attestation collection and always publish blocks'); - sequencers.forEach(sequencer => { - sequencer.updateConfig({ skipCollectingAttestations: true, minTxsPerBlock: 0 }); - }); - - // Disable skipCollectingAttestations after the first block is mined and prevent sequencers from publishing any more blocks - test.monitor.once('checkpoint', ({ checkpointNumber }) => { - logger.warn(`Disabling skipCollectingAttestations after L2 block ${checkpointNumber} has been mined`); - sequencers.forEach(sequencer => { - sequencer.updateConfig({ skipCollectingAttestations: false, minTxsPerBlock: 100 }); - }); - }); - - // Start all sequencers - await Promise.all(sequencers.map(s => s.start())); - logger.warn(`Started all sequencers with skipCollectingAttestations=true`); - - // Create a filter for CheckpointInvalidated events - const checkpointInvalidatedFilter = await l1Client.createContractEventFilter({ - address: rollupContract.address, - abi: RollupAbi, - eventName: 'CheckpointInvalidated', - fromBlock: 1n, - toBlock: 'latest', - }); - - // The next proposer should invalidate the previous checkpoint - logger.warn('Waiting for next proposer to invalidate the previous checkpoint'); - - // Wait for the CheckpointInvalidated event - const checkpointInvalidatedEvents = await retryUntil( - async () => { - const events = await l1Client.getFilterLogs({ filter: checkpointInvalidatedFilter }); - return events.length > 0 ? events : undefined; - }, - 'CheckpointInvalidated event', - test.L2_SLOT_DURATION_IN_S * 5, - 0.1, - ); - - // Verify the CheckpointInvalidated event was emitted and that the block was removed - const [event] = checkpointInvalidatedEvents; - logger.warn(`CheckpointInvalidated event emitted`, { event }); - expect(event.args.checkpointNumber).toBeGreaterThan(initialCheckpointNumber); - expect(await test.rollup.getCheckpointNumber()).toEqual(initialCheckpointNumber); - - logger.warn(`Test succeeded '${expect.getState().currentTestName}'`); - }); - - // Same as test above but with shuffled attestations instead of missing attestations - // REFACTOR: Remove code duplication with above test (and others?) - it('proposer invalidates previous block with shuffled attestations', async () => { - const sequencers = nodes.map(node => node.getSequencer()!); - const initialCheckpointNumber = (await nodes[0].getL2Tips()).checkpointed.checkpoint.number; - - // Configure all sequencers to shuffle attestations before starting - logger.warn('Configuring all sequencers to shuffle attestations and always publish blocks'); - sequencers.forEach(sequencer => { - sequencer.updateConfig({ shuffleAttestationOrdering: true, minTxsPerBlock: 0 }); - }); - - // Disable shuffleAttestationOrdering after the first block is mined and prevent sequencers from publishing any more blocks - test.monitor.once('checkpoint', ({ checkpointNumber }) => { - logger.warn(`Disabling shuffleAttestationOrdering after L2 block ${checkpointNumber} has been mined`); - sequencers.forEach(sequencer => { - sequencer.updateConfig({ shuffleAttestationOrdering: false, minTxsPerBlock: 100 }); - }); - }); - - // Start all sequencers - await Promise.all(sequencers.map(s => s.start())); - logger.warn(`Started all sequencers with shuffleAttestationOrdering=true`); - - // Create a filter for CheckpointInvalidated events - const checkpointInvalidatedFilter = await l1Client.createContractEventFilter({ - address: rollupContract.address, - abi: RollupAbi, - eventName: 'CheckpointInvalidated', - fromBlock: 1n, - toBlock: 'latest', - }); - - // The next proposer should invalidate the previous checkpoint - logger.warn('Waiting for next proposer to invalidate the previous checkpoint'); - - // Wait for the CheckpointInvalidated event - const checkpointInvalidatedEvents = await retryUntil( - async () => { - const events = await l1Client.getFilterLogs({ filter: checkpointInvalidatedFilter }); - return events.length > 0 ? events : undefined; - }, - 'CheckpointInvalidated event', - test.L2_SLOT_DURATION_IN_S * 5, - 0.1, - ); - - // Verify the CheckpointInvalidated event was emitted and that the block was removed - const [event] = checkpointInvalidatedEvents; - logger.warn(`CheckpointInvalidated event emitted`, { event }); - expect(event.args.checkpointNumber).toBeGreaterThan(initialCheckpointNumber); - expect(await test.rollup.getCheckpointNumber()).toEqual(initialCheckpointNumber); - - logger.warn(`Test succeeded '${expect.getState().currentTestName}'`); - }); - it('committee member invalidates a block if proposer does not come through', async () => { const sequencers = nodes.map(node => node.getSequencer()!); const initialCheckpointNumber = await nodes[0].getL2Tips().then(t => t.checkpointed.checkpoint.number); diff --git a/yarn-project/foundation/src/crypto/secp256k1-signer/malleability.test.ts b/yarn-project/foundation/src/crypto/secp256k1-signer/malleability.test.ts index 4874b7ceabfe..d787f6908145 100644 --- a/yarn-project/foundation/src/crypto/secp256k1-signer/malleability.test.ts +++ b/yarn-project/foundation/src/crypto/secp256k1-signer/malleability.test.ts @@ -8,6 +8,8 @@ import { Secp256k1Signer } from './secp256k1_signer.js'; import { Secp256k1Error, flipSignature, + generateRecoverableSignature, + generateUnrecoverableSignature, makeEthSignDigest, normalizeSignature, recoverAddress, @@ -139,3 +141,21 @@ describe('ecdsa malleability', () => { expect(recoveredAddress.toString()).toEqual(expectedAddress.toString()); }); }); + +describe('generateRecoverableSignature', () => { + it('produces a signature from which an address can be recovered', () => { + const sig = generateRecoverableSignature(); + const hash = Buffer32.random(); + const recovered = tryRecoverAddress(hash, sig); + expect(recovered).toBeDefined(); + }); +}); + +describe('generateUnrecoverableSignature', () => { + it('produces a signature from which no address can be recovered', () => { + const sig = generateUnrecoverableSignature(); + const hash = Buffer32.random(); + const recovered = tryRecoverAddress(hash, sig); + expect(recovered).toBeUndefined(); + }); +}); diff --git a/yarn-project/foundation/src/crypto/secp256k1-signer/utils.ts b/yarn-project/foundation/src/crypto/secp256k1-signer/utils.ts index a8a459d01b4e..2226ae635550 100644 --- a/yarn-project/foundation/src/crypto/secp256k1-signer/utils.ts +++ b/yarn-project/foundation/src/crypto/secp256k1-signer/utils.ts @@ -210,3 +210,35 @@ export function recoverPublicKey(hash: Buffer32, signature: Signature, opts: Rec const publicKey = sig.recoverPublicKey(hash.buffer).toHex(false); return Buffer.from(publicKey, 'hex'); } + +/** Arbitrary hash used for testing signature recoverability. */ +const PROBE_HASH = Buffer32.fromBuffer(keccak256(Buffer.from('signature-recoverability-probe'))); + +/** + * Generates a random valid ECDSA signature that is recoverable to some address. + * Since Signature.random() produces real signatures via secp256k1 signing, the result is always + * recoverable, but we verify defensively by checking tryRecoverAddress. + */ +export function generateRecoverableSignature(): Signature { + for (let i = 0; i < 100; i++) { + const sig = Signature.random(); + if (tryRecoverAddress(PROBE_HASH, sig) !== undefined) { + return sig; + } + } + throw new Secp256k1Error('Failed to generate a recoverable signature after 100 attempts'); +} + +/** + * Generates a random signature where ECDSA address recovery fails. + * Uses random r/s values (not from real signing) so that r is unlikely to be a valid secp256k1 x-coordinate. + */ +export function generateUnrecoverableSignature(): Signature { + for (let i = 0; i < 100; i++) { + const sig = new Signature(Buffer32.random(), Buffer32.random(), 27); + if (tryRecoverAddress(PROBE_HASH, sig) === undefined) { + return sig; + } + } + throw new Secp256k1Error('Failed to generate an unrecoverable signature after 100 attempts'); +} diff --git a/yarn-project/foundation/src/eth-signature/eth_signature.test.ts b/yarn-project/foundation/src/eth-signature/eth_signature.test.ts index 2c642c2c32a8..dd19ae14520b 100644 --- a/yarn-project/foundation/src/eth-signature/eth_signature.test.ts +++ b/yarn-project/foundation/src/eth-signature/eth_signature.test.ts @@ -1,7 +1,9 @@ import { Buffer32 } from '@aztec/foundation/buffer'; -import { Secp256k1Signer, recoverAddress } from '@aztec/foundation/crypto/secp256k1-signer'; +import { Secp256k1Signer, recoverAddress, tryRecoverAddress } from '@aztec/foundation/crypto/secp256k1-signer'; import { Fr } from '@aztec/foundation/curves/bn254'; +import { secp256k1 } from '@noble/curves/secp256k1'; + import { Signature } from './eth_signature.js'; const randomSigner = () => { @@ -62,4 +64,24 @@ describe('eth signature', () => { const deserialized = Signature.fromString(serialized); checkEquivalence(signature, deserialized); }); + + it('random() produces a valid recoverable signature with low s-value', () => { + const sig = Signature.random(); + + // v should be 27 or 28 + expect([27, 28]).toContain(sig.v); + + // Signature should not be empty + expect(sig.isEmpty()).toBe(false); + + // s should be in the low half of the curve (low s-value) + const sBigInt = sig.s.toBigInt(); + const halfN = secp256k1.CURVE.n / 2n; + expect(sBigInt).toBeLessThanOrEqual(halfN); + + // Signature should be recoverable (tryRecoverAddress should return an address for any hash) + const hash = Buffer32.random(); + const recovered = tryRecoverAddress(hash, sig); + expect(recovered).toBeDefined(); + }); }); diff --git a/yarn-project/foundation/src/eth-signature/eth_signature.ts b/yarn-project/foundation/src/eth-signature/eth_signature.ts index c76343f37540..62545d3cc4e9 100644 --- a/yarn-project/foundation/src/eth-signature/eth_signature.ts +++ b/yarn-project/foundation/src/eth-signature/eth_signature.ts @@ -1,8 +1,10 @@ import { Buffer32 } from '@aztec/foundation/buffer'; import { BufferReader, serializeToBuffer } from '@aztec/foundation/serialize'; +import { secp256k1 } from '@noble/curves/secp256k1'; import { z } from 'zod'; +import { randomBytes } from '../crypto/random/index.js'; import { hasHexPrefix, hexToBuffer } from '../string/index.js'; /** @@ -77,8 +79,12 @@ export class Signature { return new Signature(Buffer32.fromBuffer(hexToBuffer(sig.r)), Buffer32.fromBuffer(hexToBuffer(sig.s)), sig.yParity); } + /** Generates a random valid ECDSA signature with a low s-value by signing a random message with a random key. */ static random(): Signature { - return new Signature(Buffer32.random(), Buffer32.random(), 1); + const privateKey = randomBytes(32); + const message = randomBytes(32); + const { r, s, recovery } = secp256k1.sign(message, privateKey); + return new Signature(Buffer32.fromBigInt(r), Buffer32.fromBigInt(s), recovery ? 28 : 27); } static empty(): Signature { diff --git a/yarn-project/sequencer-client/src/config.ts b/yarn-project/sequencer-client/src/config.ts index 469651fba387..a9932a201d8d 100644 --- a/yarn-project/sequencer-client/src/config.ts +++ b/yarn-project/sequencer-client/src/config.ts @@ -52,6 +52,8 @@ export const DefaultSequencerConfig: ResolvedSequencerConfig = { skipInvalidateBlockAsProposer: false, broadcastInvalidBlockProposal: false, injectFakeAttestation: false, + injectHighSValueAttestation: false, + injectUnrecoverableSignatureAttestation: false, fishermanMode: false, shuffleAttestationOrdering: false, skipPushProposedBlocksToArchiver: false, @@ -186,6 +188,14 @@ export const sequencerConfigMappings: ConfigMappingsType = { description: 'Inject a fake attestation (for testing only)', ...booleanConfigHelper(DefaultSequencerConfig.injectFakeAttestation), }, + injectHighSValueAttestation: { + description: 'Inject a malleable attestation with a high-s value (for testing only)', + ...booleanConfigHelper(DefaultSequencerConfig.injectHighSValueAttestation), + }, + injectUnrecoverableSignatureAttestation: { + description: 'Inject an attestation with an unrecoverable signature (for testing only)', + ...booleanConfigHelper(DefaultSequencerConfig.injectUnrecoverableSignatureAttestation), + }, fishermanMode: { env: 'FISHERMAN_MODE', description: 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 184e83a76506..b9e960fb7c1f 100644 --- a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts +++ b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts @@ -9,6 +9,11 @@ import { SlotNumber, } from '@aztec/foundation/branded-types'; import { randomInt } from '@aztec/foundation/crypto/random'; +import { + flipSignature, + generateRecoverableSignature, + generateUnrecoverableSignature, +} from '@aztec/foundation/crypto/secp256k1-signer'; import { Fr } from '@aztec/foundation/curves/bn254'; import { EthAddress } from '@aztec/foundation/eth-address'; import { Signature } from '@aztec/foundation/eth-signature'; @@ -759,7 +764,12 @@ export class CheckpointProposalJob implements Traceable { const sorted = orderAttestations(trimmed, committee); // Manipulate the attestations if we've been configured to do so - if (this.config.injectFakeAttestation || this.config.shuffleAttestationOrdering) { + if ( + this.config.injectFakeAttestation || + this.config.injectHighSValueAttestation || + this.config.injectUnrecoverableSignatureAttestation || + this.config.shuffleAttestationOrdering + ) { return this.manipulateAttestations(proposal.slotNumber, epoch, seed, committee, sorted); } @@ -788,7 +798,11 @@ export class CheckpointProposalJob implements Traceable { this.epochCache.computeProposerIndex(slotNumber, epoch, seed, BigInt(committee.length)), ); - if (this.config.injectFakeAttestation) { + if ( + this.config.injectFakeAttestation || + this.config.injectHighSValueAttestation || + this.config.injectUnrecoverableSignatureAttestation + ) { // Find non-empty attestations that are not from the proposer const nonProposerIndices: number[] = []; for (let i = 0; i < attestations.length; i++) { @@ -798,8 +812,20 @@ export class CheckpointProposalJob implements Traceable { } if (nonProposerIndices.length > 0) { const targetIndex = nonProposerIndices[randomInt(nonProposerIndices.length)]; - this.log.warn(`Injecting fake attestation in checkpoint for slot ${slotNumber} at index ${targetIndex}`); - unfreeze(attestations[targetIndex]).signature = Signature.random(); + if (this.config.injectHighSValueAttestation) { + this.log.warn( + `Injecting high-s value attestation in checkpoint for slot ${slotNumber} at index ${targetIndex}`, + ); + unfreeze(attestations[targetIndex]).signature = flipSignature(attestations[targetIndex].signature); + } else if (this.config.injectUnrecoverableSignatureAttestation) { + this.log.warn( + `Injecting unrecoverable signature attestation in checkpoint for slot ${slotNumber} at index ${targetIndex}`, + ); + unfreeze(attestations[targetIndex]).signature = generateUnrecoverableSignature(); + } else { + this.log.warn(`Injecting fake attestation in checkpoint for slot ${slotNumber} at index ${targetIndex}`); + unfreeze(attestations[targetIndex]).signature = generateRecoverableSignature(); + } } return new CommitteeAttestationsAndSigners(attestations); } diff --git a/yarn-project/stdlib/src/interfaces/configs.ts b/yarn-project/stdlib/src/interfaces/configs.ts index 3cd2912c078f..88b1366a6889 100644 --- a/yarn-project/stdlib/src/interfaces/configs.ts +++ b/yarn-project/stdlib/src/interfaces/configs.ts @@ -59,6 +59,10 @@ export interface SequencerConfig { broadcastInvalidBlockProposal?: boolean; /** Inject a fake attestation (for testing only) */ injectFakeAttestation?: boolean; + /** Inject a malleable attestation with a high-s value (for testing only) */ + injectHighSValueAttestation?: boolean; + /** Inject an attestation with an unrecoverable signature (for testing only) */ + injectUnrecoverableSignatureAttestation?: 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) */ @@ -104,6 +108,8 @@ export const SequencerConfigSchema = zodFor()( secondsBeforeInvalidatingBlockAsNonCommitteeMember: z.number(), broadcastInvalidBlockProposal: z.boolean().optional(), injectFakeAttestation: z.boolean().optional(), + injectHighSValueAttestation: z.boolean().optional(), + injectUnrecoverableSignatureAttestation: z.boolean().optional(), fishermanMode: z.boolean().optional(), shuffleAttestationOrdering: z.boolean().optional(), blockDurationMs: z.number().positive().optional(),