Skip to content

Commit 82bb4d8

Browse files
authored
feat: Validators invalidate invalid blocks (#16120)
We expect proposers to invalidate the previous block if it is invalid, but if they fail to do so, validators will eventually do it, prioritizing the committee members and then any validator whatsoever. This commit includes other fixes: - If a proposer cannot build a block due to not enough txs, it still tries to invalidate the previous one. - The archiver keeps track of the earliest (not latest) invalid block it has seen, so the sequencer can use this info to invalidate the earliest one. Builds on top of #16067
1 parent b18c823 commit 82bb4d8

File tree

15 files changed

+340
-59
lines changed

15 files changed

+340
-59
lines changed

yarn-project/archiver/src/archiver/archiver.test.ts

Lines changed: 44 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { makeBlockAttestationFromBlock } from '@aztec/stdlib/testing';
2323
import { getTelemetryClient } from '@aztec/telemetry-client';
2424

2525
import { jest } from '@jest/globals';
26+
import assert from 'assert';
2627
import { type MockProxy, mock } from 'jest-mock-extended';
2728
import { type FormattedBlock, type Log, type Transaction, encodeFunctionData, multicall3Abi, toHex } from 'viem';
2829

@@ -380,7 +381,7 @@ describe('Archiver', () => {
380381
});
381382
}, 10_000);
382383

383-
it('ignores block 2 because it had invalid attestations', async () => {
384+
it('ignores blocks because of invalid attestations', async () => {
384385
let latestBlockNum = await archiver.getBlockNumber();
385386
expect(latestBlockNum).toEqual(0);
386387

@@ -395,21 +396,27 @@ describe('Archiver', () => {
395396
const blobHashes = await Promise.all(blocks.map(makeVersionedBlobHashes));
396397
const blobsFromBlocks = await Promise.all(blocks.map(b => makeBlobsFromBlock(b)));
397398

398-
// And define a bad block 2 with attestations from random signers
399-
const badBlock2 = await makeBlock(2);
400-
badBlock2.archive.root = new Fr(0x1002);
401-
const badBlock2RollupTx = await makeRollupTx(badBlock2, times(3, Secp256k1Signer.random));
402-
const badBlock2BlobHashes = await makeVersionedBlobHashes(badBlock2);
403-
const badBlock2Blobs = await makeBlobsFromBlock(badBlock2);
399+
// And define bad blocks with attestations from random signers
400+
const makeBadBlock = async (blockNumber: number) => {
401+
const badBlock = await makeBlock(blockNumber);
402+
badBlock.archive.root = new Fr(0x1000 + blockNumber);
403+
const badBlockRollupTx = await makeRollupTx(badBlock, times(3, Secp256k1Signer.random));
404+
const badBlockBlobHashes = await makeVersionedBlobHashes(badBlock);
405+
const badBlockBlobs = await makeBlobsFromBlock(badBlock);
406+
return [badBlock, badBlockRollupTx, badBlockBlobHashes, badBlockBlobs] as const;
407+
};
408+
409+
const [badBlock2, badBlock2RollupTx, badBlock2BlobHashes, badBlock2Blobs] = await makeBadBlock(2);
410+
const [badBlock3, badBlock3RollupTx, badBlock3BlobHashes, badBlock3Blobs] = await makeBadBlock(3);
404411

405412
// Return the archive root for the bad block 2 when L1 is queried
406413
mockRollupRead.archiveAt.mockImplementation((args: readonly [bigint]) =>
407414
Promise.resolve((args[0] === 2n ? badBlock2 : blocks[Number(args[0] - 1n)]).archive.root.toString()),
408415
);
409416

410-
logger.warn(`Created 3 valid blocks`);
411-
blocks.forEach(block => logger.warn(`Block ${block.number} with root ${block.archive.root.toString()}`));
417+
blocks.forEach(b => logger.warn(`Created valid block ${b.number} with root ${b.archive.root.toString()}`));
412418
logger.warn(`Created invalid block 2 with root ${badBlock2.archive.root.toString()}`);
419+
logger.warn(`Created invalid block 3 with root ${badBlock3.archive.root.toString()}`);
413420

414421
// During the first archiver loop, we fetch block 1 and the block 2 with bad attestations
415422
publicClient.getBlockNumber.mockResolvedValue(85n);
@@ -432,11 +439,35 @@ describe('Archiver', () => {
432439
}),
433440
);
434441

435-
// Now we go for another loop, where a proper block 2 is proposed with correct attestations
442+
// Now another loop, where we propose a block 3 with bad attestations
443+
logger.warn(`Adding new block 3 with bad attestations`);
444+
publicClient.getBlockNumber.mockResolvedValue(90n);
445+
makeL2BlockProposedEvent(85n, 3n, badBlock3.archive.root.toString(), badBlock3BlobHashes);
446+
mockRollup.read.status.mockResolvedValue([
447+
0n,
448+
GENESIS_ROOT,
449+
3n,
450+
badBlock3.archive.root.toString(),
451+
blocks[0].archive.root.toString(),
452+
]);
453+
publicClient.getTransaction.mockResolvedValueOnce(badBlock3RollupTx);
454+
blobSinkClient.getBlobSidecar.mockResolvedValueOnce(badBlock3Blobs);
455+
456+
// We should still be at block 1, and the pending chain validation status should still be invalid and point to block 2
457+
// since we want the archiver to always return the earliest block with invalid attestations
458+
await archiver.syncImmediate();
459+
latestBlockNum = await archiver.getBlockNumber();
460+
expect(latestBlockNum).toEqual(1);
461+
const validationStatus = await archiver.getPendingChainValidationStatus();
462+
assert(!validationStatus.valid);
463+
expect(validationStatus.block.block.number).toEqual(2);
464+
expect(validationStatus.block.block.archive.root.toString()).toEqual(badBlock2.archive.root.toString());
465+
466+
// Now we go for another loop, where proper blocks 2 and 3 are proposed with correct attestations
436467
// IRL there would be an "Invalidated" event, but we are not currently relying on it
437-
logger.warn(`Adding new block 2 with correct attestations and a block 3`);
468+
logger.warn(`Adding new blocks 2 and 3 with correct attestations`);
438469
publicClient.getBlockNumber.mockResolvedValue(100n);
439-
makeL2BlockProposedEvent(90n, 2n, blocks[1].archive.root.toString(), blobHashes[1]);
470+
makeL2BlockProposedEvent(94n, 2n, blocks[1].archive.root.toString(), blobHashes[1]);
440471
makeL2BlockProposedEvent(95n, 3n, blocks[2].archive.root.toString(), blobHashes[2]);
441472
mockRollup.read.status.mockResolvedValue([
442473
0n,
@@ -452,6 +483,7 @@ describe('Archiver', () => {
452483
);
453484

454485
// Now we should move to block 3
486+
await archiver.syncImmediate();
455487
await waitUntilArchiverBlock(3);
456488
latestBlockNum = await archiver.getBlockNumber();
457489
expect(latestBlockNum).toEqual(3);

yarn-project/archiver/src/archiver/archiver.ts

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -354,15 +354,25 @@ export class Archiver extends (EventEmitter as new () => ArchiverEmitter) implem
354354
currentL1BlockNumber,
355355
currentL1Timestamp,
356356
);
357+
358+
// Update the pending chain validation status with the last block validation result.
359+
// Again, we only update if validation status changed, so in a sequence of invalid blocks
360+
// we keep track of the first invalid block so we can invalidate that one if needed.
361+
if (
362+
rollupStatus.validationResult &&
363+
rollupStatus.validationResult?.valid !== this.pendingChainValidationStatus.valid
364+
) {
365+
this.pendingChainValidationStatus = rollupStatus.validationResult;
366+
}
367+
357368
// And lastly we check if we are missing any L2 blocks behind us due to a possible L1 reorg.
358369
// We only do this if rollup cant prune on the next submission. Otherwise we will end up
359370
// re-syncing the blocks we have just unwound above. We also dont do this if the last block is invalid,
360371
// since the archiver will rightfully refuse to sync up to it.
361-
if (!rollupCanPrune && rollupStatus.lastBlockValidationResult.valid) {
372+
if (!rollupCanPrune && this.pendingChainValidationStatus.valid) {
362373
await this.checkForNewBlocksBeforeL1SyncPoint(rollupStatus, blocksSynchedTo, currentL1BlockNumber);
363374
}
364375

365-
this.pendingChainValidationStatus = rollupStatus.lastBlockValidationResult;
366376
this.instrumentation.updateL1BlockHeight(currentL1BlockNumber);
367377
}
368378

@@ -620,7 +630,7 @@ export class Archiver extends (EventEmitter as new () => ArchiverEmitter) implem
620630
provenArchive,
621631
pendingBlockNumber: Number(pendingBlockNumber),
622632
pendingArchive,
623-
lastBlockValidationResult: { valid: true } as ValidateBlockResult,
633+
validationResult: undefined as ValidateBlockResult | undefined,
624634
};
625635
this.log.trace(`Retrieved rollup status at current L1 block ${currentL1BlockNumber}.`, {
626636
localPendingBlockNumber,
@@ -795,17 +805,22 @@ export class Archiver extends (EventEmitter as new () => ArchiverEmitter) implem
795805
const validBlocks: PublishedL2Block[] = [];
796806

797807
for (const block of publishedBlocks) {
798-
const isProven = block.block.number <= provenBlockNumber;
799-
rollupStatus.lastBlockValidationResult = isProven
800-
? { valid: true }
801-
: await validateBlockAttestations(block, this.epochCache, this.l1constants, this.log);
808+
const validationResult = await validateBlockAttestations(block, this.epochCache, this.l1constants, this.log);
809+
810+
// Only update the validation result if it has changed, so we can keep track of the first invalid block
811+
// in case there is a sequence of more than one invalid block, as we need to invalidate the first one.
812+
if (rollupStatus.validationResult?.valid !== validationResult.valid) {
813+
rollupStatus.validationResult = validationResult;
814+
}
802815

803-
if (!rollupStatus.lastBlockValidationResult.valid) {
816+
if (!validationResult.valid) {
804817
this.log.warn(`Skipping block ${block.block.number} due to invalid attestations`, {
805818
blockHash: block.block.hash(),
806819
l1BlockNumber: block.l1.blockNumber,
807-
...pick(rollupStatus.lastBlockValidationResult, 'reason'),
820+
...pick(validationResult, 'reason'),
808821
});
822+
// We keep consuming blocks if we find an invalid one, since we do not listen for BlockInvalidated events
823+
// We just pretend the invalid ones are not there and keep consuming the next blocks
809824
continue;
810825
}
811826

yarn-project/archiver/src/archiver/validation.test.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ describe('validateBlockAttestations', () => {
4646
const result = await validateBlockAttestations(block, epochCache, constants, logger);
4747

4848
expect(result.valid).toBe(true);
49-
expect(result.block).toBe(block);
5049
expect(epochCache.getCommitteeForEpoch).toHaveBeenCalledWith(0n);
5150
});
5251

@@ -55,7 +54,6 @@ describe('validateBlockAttestations', () => {
5554
const result = await validateBlockAttestations(block, epochCache, constants, logger);
5655

5756
expect(result.valid).toBe(true);
58-
expect(result.block).toBe(block);
5957
expect(epochCache.getCommitteeForEpoch).toHaveBeenCalledWith(0n);
6058
});
6159
});
@@ -101,7 +99,6 @@ describe('validateBlockAttestations', () => {
10199
const block = await makeBlock(signers.slice(0, 4), committee);
102100
const result = await validateBlockAttestations(block, epochCache, constants, logger);
103101
expect(result.valid).toBe(true);
104-
expect(result.block).toBe(block);
105102
});
106103
});
107104
});

yarn-project/archiver/src/archiver/validation.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,8 @@ export async function validateBlockAttestations(
3737
});
3838

3939
if (!committee || committee.length === 0) {
40-
// Q: Should we accept blocks with no committee?
4140
logger?.warn(`No committee found for epoch ${epoch} at slot ${slot}. Accepting block without validation.`, logData);
42-
return { valid: true, block: publishedBlock };
41+
return { valid: true };
4342
}
4443

4544
const committeeSet = new Set(committee.map(member => member.toString()));
@@ -64,5 +63,5 @@ export async function validateBlockAttestations(
6463
}
6564

6665
logger?.debug(`Block attestations validated successfully for block ${block.number} at slot ${slot}`, logData);
67-
return { valid: true, block: publishedBlock };
66+
return { valid: true };
6867
}

yarn-project/end-to-end/src/e2e_epochs/epochs_invalidate_block.test.ts

Lines changed: 121 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ describe('e2e_epochs/epochs_invalidate_block', () => {
2828
let logger: Logger;
2929
let l1Client: ExtendedViemWalletClient;
3030
let rollupContract: RollupContract;
31+
let anvilPort = 8545;
3132

3233
let test: EpochsTestContext;
3334
let validators: (Operator & { privateKey: `0x${string}` })[];
@@ -50,6 +51,8 @@ describe('e2e_epochs/epochs_invalidate_block', () => {
5051
aztecProofSubmissionEpochs: 1024,
5152
startProverNode: false,
5253
aztecTargetCommitteeSize: VALIDATOR_COUNT,
54+
archiverPollingIntervalMS: 200,
55+
anvilPort: ++anvilPort,
5356
});
5457

5558
({ context, logger, l1Client } = test);
@@ -77,7 +80,7 @@ describe('e2e_epochs/epochs_invalidate_block', () => {
7780
await test.teardown();
7881
});
7982

80-
it('invalidates a block published without sufficient attestations', async () => {
83+
it('proposer invalidates previous block while posting its own', async () => {
8184
const sequencers = nodes.map(node => node.getSequencer()!);
8285
const initialBlockNumber = await nodes[0].getBlockNumber();
8386

@@ -126,10 +129,11 @@ describe('e2e_epochs/epochs_invalidate_block', () => {
126129
0.1,
127130
);
128131

129-
// Verify the BlockInvalidated event was emitted
132+
// Verify the BlockInvalidated event was emitted and that the block was removed
130133
const [event] = blockInvalidatedEvents;
131134
logger.warn(`BlockInvalidated event emitted`, { event });
132135
expect(event.args.blockNumber).toBeGreaterThan(initialBlockNumber);
136+
expect(test.rollup.address).toEqual(event.address);
133137

134138
// Wait for all nodes to sync the new block
135139
logger.warn('Waiting for all nodes to sync');
@@ -149,4 +153,119 @@ describe('e2e_epochs/epochs_invalidate_block', () => {
149153
expect(receipt.status).toBe('success');
150154
logger.warn(`Transaction included in block ${receipt.blockNumber}`);
151155
});
156+
157+
it('proposer invalidates previous block without publishing its own', async () => {
158+
const sequencers = nodes.map(node => node.getSequencer()!);
159+
const initialBlockNumber = await nodes[0].getBlockNumber();
160+
161+
// Configure all sequencers to skip collecting attestations before starting
162+
logger.warn('Configuring all sequencers to skip attestation collection and always publish blocks');
163+
sequencers.forEach(sequencer => {
164+
sequencer.updateSequencerConfig({ skipCollectingAttestations: true, minTxsPerBlock: 0 });
165+
});
166+
167+
// Disable skipCollectingAttestations after the first block is mined and prevent sequencers from publishing any more blocks
168+
test.monitor.once('l2-block', ({ l2BlockNumber }) => {
169+
logger.warn(`Disabling skipCollectingAttestations after L2 block ${l2BlockNumber} has been mined`);
170+
sequencers.forEach(sequencer => {
171+
sequencer.updateSequencerConfig({ skipCollectingAttestations: false, minTxsPerBlock: 100 });
172+
});
173+
});
174+
175+
// Start all sequencers
176+
await Promise.all(sequencers.map(s => s.start()));
177+
logger.warn(`Started all sequencers with skipCollectingAttestations=true`);
178+
179+
// Create a filter for BlockInvalidated events
180+
const blockInvalidatedFilter = await l1Client.createContractEventFilter({
181+
address: rollupContract.address,
182+
abi: RollupAbi,
183+
eventName: 'BlockInvalidated',
184+
fromBlock: 1n,
185+
toBlock: 'latest',
186+
});
187+
188+
// The next proposer should invalidate the previous block and publish a new one
189+
logger.warn('Waiting for next proposer to invalidate the previous block');
190+
191+
// Wait for the BlockInvalidated event
192+
const blockInvalidatedEvents = await retryUntil(
193+
async () => {
194+
const events = await l1Client.getFilterLogs({ filter: blockInvalidatedFilter });
195+
return events.length > 0 ? events : undefined;
196+
},
197+
'BlockInvalidated event',
198+
test.L2_SLOT_DURATION_IN_S * 5,
199+
0.1,
200+
);
201+
202+
// Verify the BlockInvalidated event was emitted and that the block was removed
203+
const [event] = blockInvalidatedEvents;
204+
logger.warn(`BlockInvalidated event emitted`, { event });
205+
expect(event.args.blockNumber).toBeGreaterThan(initialBlockNumber);
206+
expect(await test.rollup.getBlockNumber()).toEqual(BigInt(initialBlockNumber));
207+
});
208+
209+
it('committee member invalidates a block if proposer does not come through', async () => {
210+
const sequencers = nodes.map(node => node.getSequencer()!);
211+
const initialBlockNumber = await nodes[0].getBlockNumber();
212+
213+
// Configure all sequencers to skip collecting attestations before starting
214+
logger.warn('Configuring all sequencers to skip attestation collection and invalidation as proposer');
215+
const invalidationDelay = test.L1_BLOCK_TIME_IN_S * 4;
216+
sequencers.forEach(sequencer => {
217+
sequencer.updateSequencerConfig({
218+
skipCollectingAttestations: true,
219+
minTxsPerBlock: 0,
220+
skipInvalidateBlockAsProposer: true,
221+
secondsBeforeInvalidatingBlockAsCommitteeMember: invalidationDelay,
222+
});
223+
});
224+
225+
// Disable skipCollectingAttestations after the first block is mined
226+
let invalidBlockTimestamp: bigint | undefined;
227+
test.monitor.once('l2-block', ({ l2BlockNumber, timestamp }) => {
228+
logger.warn(`Disabling skipCollectingAttestations after L2 block ${l2BlockNumber} has been mined`);
229+
invalidBlockTimestamp = timestamp;
230+
sequencers.forEach(sequencer => {
231+
sequencer.updateSequencerConfig({ skipCollectingAttestations: false });
232+
});
233+
});
234+
235+
// Start all sequencers
236+
await Promise.all(sequencers.map(s => s.start()));
237+
logger.warn(`Started all sequencers with skipCollectingAttestations=true`);
238+
239+
// Create a filter for BlockInvalidated events
240+
const blockInvalidatedFilter = await l1Client.createContractEventFilter({
241+
address: rollupContract.address,
242+
abi: RollupAbi,
243+
eventName: 'BlockInvalidated',
244+
fromBlock: 1n,
245+
toBlock: 'latest',
246+
});
247+
248+
// Some committee member should invalidate the previous block
249+
logger.warn('Waiting for committee member to invalidate the previous block');
250+
251+
// Wait for the BlockInvalidated event
252+
const blockInvalidatedEvents = await retryUntil(
253+
async () => {
254+
const events = await l1Client.getFilterLogs({ filter: blockInvalidatedFilter });
255+
return events.length > 0 ? events : undefined;
256+
},
257+
'BlockInvalidated event',
258+
test.L2_SLOT_DURATION_IN_S * 5,
259+
0.1,
260+
);
261+
262+
// Verify the BlockInvalidated event was emitted
263+
const [event] = blockInvalidatedEvents;
264+
logger.warn(`BlockInvalidated event emitted`, { event });
265+
expect(event.args.blockNumber).toBeGreaterThan(initialBlockNumber);
266+
267+
// And check that the invalidation happened at least after the specified timeout
268+
const { timestamp: invalidationTimestamp } = await l1Client.getBlock({ blockNumber: event.blockNumber });
269+
expect(invalidationTimestamp).toBeGreaterThanOrEqual(invalidBlockTimestamp! + BigInt(invalidationDelay));
270+
});
152271
});

yarn-project/end-to-end/src/fixtures/utils.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,8 @@ export type SetupOptions = {
313313
automineL1Setup?: boolean;
314314
/** How many accounts to seed and unlock in anvil. */
315315
anvilAccounts?: number;
316+
/** Port to start anvil (defaults to 8545) */
317+
anvilPort?: number;
316318
} & Partial<AztecNodeConfig>;
317319

318320
/** Context for an end-to-end test as returned by the `setup` function */
@@ -404,7 +406,11 @@ export async function setup(
404406
);
405407
}
406408

407-
const res = await startAnvil({ l1BlockTime: opts.ethereumSlotDuration, accounts: opts.anvilAccounts });
409+
const res = await startAnvil({
410+
l1BlockTime: opts.ethereumSlotDuration,
411+
accounts: opts.anvilAccounts,
412+
port: opts.anvilPort,
413+
});
408414
anvil = res.anvil;
409415
config.l1RpcUrls = [res.rpcUrl];
410416
}

yarn-project/foundation/src/config/env_var.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,8 @@ export type EnvVar =
193193
| 'SEQ_ENFORCE_TIME_TABLE'
194194
| 'SEQ_MAX_L1_TX_INCLUSION_TIME_INTO_SLOT'
195195
| 'SEQ_ATTESTATION_PROPAGATION_TIME'
196+
| 'SEQ_SECONDS_BEFORE_INVALIDATING_BLOCK_AS_COMMITTEE_MEMBER'
197+
| 'SEQ_SECONDS_BEFORE_INVALIDATING_BLOCK_AS_NON_COMMITTEE_MEMBER'
196198
| 'SLASH_FACTORY_CONTRACT_ADDRESS'
197199
| 'SLASH_PRUNE_ENABLED'
198200
| 'SLASH_PRUNE_PENALTY'

0 commit comments

Comments
 (0)