From f4e7898704e4f1351ae021e84364165434d7dc87 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 30 Jul 2025 15:26:34 -0300 Subject: [PATCH] feat: Validators invalidate invalid blocks 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. --- .../archiver/src/archiver/archiver.test.ts | 56 ++++++-- .../archiver/src/archiver/archiver.ts | 33 +++-- .../archiver/src/archiver/validation.test.ts | 3 - .../archiver/src/archiver/validation.ts | 5 +- .../epochs_invalidate_block.test.ts | 123 +++++++++++++++++- yarn-project/end-to-end/src/fixtures/utils.ts | 8 +- yarn-project/foundation/src/config/env_var.ts | 2 + .../prover-node/src/prover-node-publisher.ts | 7 +- yarn-project/sequencer-client/src/config.ts | 21 ++- .../src/publisher/sequencer-publisher.ts | 25 +++- .../src/sequencer/sequencer.test.ts | 3 +- .../src/sequencer/sequencer.ts | 97 ++++++++++++-- .../stdlib/src/block/l2_block_source.ts | 6 +- .../src/interfaces/aztec-node-admin.test.ts | 2 + yarn-project/stdlib/src/interfaces/configs.ts | 8 ++ 15 files changed, 340 insertions(+), 59 deletions(-) diff --git a/yarn-project/archiver/src/archiver/archiver.test.ts b/yarn-project/archiver/src/archiver/archiver.test.ts index f94584f9ac68..12fa3cb21970 100644 --- a/yarn-project/archiver/src/archiver/archiver.test.ts +++ b/yarn-project/archiver/src/archiver/archiver.test.ts @@ -23,6 +23,7 @@ import { makeBlockAttestationFromBlock } from '@aztec/stdlib/testing'; import { getTelemetryClient } from '@aztec/telemetry-client'; import { jest } from '@jest/globals'; +import assert from 'assert'; import { type MockProxy, mock } from 'jest-mock-extended'; import { type FormattedBlock, type Log, type Transaction, encodeFunctionData, multicall3Abi, toHex } from 'viem'; @@ -380,7 +381,7 @@ describe('Archiver', () => { }); }, 10_000); - it('ignores block 2 because it had invalid attestations', async () => { + it('ignores blocks because of invalid attestations', async () => { let latestBlockNum = await archiver.getBlockNumber(); expect(latestBlockNum).toEqual(0); @@ -395,21 +396,27 @@ describe('Archiver', () => { const blobHashes = await Promise.all(blocks.map(makeVersionedBlobHashes)); const blobsFromBlocks = await Promise.all(blocks.map(b => makeBlobsFromBlock(b))); - // And define a bad block 2 with attestations from random signers - const badBlock2 = await makeBlock(2); - badBlock2.archive.root = new Fr(0x1002); - const badBlock2RollupTx = await makeRollupTx(badBlock2, times(3, Secp256k1Signer.random)); - const badBlock2BlobHashes = await makeVersionedBlobHashes(badBlock2); - const badBlock2Blobs = await makeBlobsFromBlock(badBlock2); + // And define bad blocks with attestations from random signers + const makeBadBlock = async (blockNumber: number) => { + const badBlock = await makeBlock(blockNumber); + badBlock.archive.root = new Fr(0x1000 + blockNumber); + const badBlockRollupTx = await makeRollupTx(badBlock, times(3, Secp256k1Signer.random)); + const badBlockBlobHashes = await makeVersionedBlobHashes(badBlock); + const badBlockBlobs = await makeBlobsFromBlock(badBlock); + return [badBlock, badBlockRollupTx, badBlockBlobHashes, badBlockBlobs] as const; + }; + + const [badBlock2, badBlock2RollupTx, badBlock2BlobHashes, badBlock2Blobs] = await makeBadBlock(2); + const [badBlock3, badBlock3RollupTx, badBlock3BlobHashes, badBlock3Blobs] = await makeBadBlock(3); // Return the archive root for the bad block 2 when L1 is queried mockRollupRead.archiveAt.mockImplementation((args: readonly [bigint]) => Promise.resolve((args[0] === 2n ? badBlock2 : blocks[Number(args[0] - 1n)]).archive.root.toString()), ); - logger.warn(`Created 3 valid blocks`); - blocks.forEach(block => logger.warn(`Block ${block.number} with root ${block.archive.root.toString()}`)); + blocks.forEach(b => logger.warn(`Created valid block ${b.number} with root ${b.archive.root.toString()}`)); logger.warn(`Created invalid block 2 with root ${badBlock2.archive.root.toString()}`); + logger.warn(`Created invalid block 3 with root ${badBlock3.archive.root.toString()}`); // During the first archiver loop, we fetch block 1 and the block 2 with bad attestations publicClient.getBlockNumber.mockResolvedValue(85n); @@ -432,11 +439,35 @@ describe('Archiver', () => { }), ); - // Now we go for another loop, where a proper block 2 is proposed with correct attestations + // Now another loop, where we propose a block 3 with bad attestations + logger.warn(`Adding new block 3 with bad attestations`); + publicClient.getBlockNumber.mockResolvedValue(90n); + makeL2BlockProposedEvent(85n, 3n, badBlock3.archive.root.toString(), badBlock3BlobHashes); + mockRollup.read.status.mockResolvedValue([ + 0n, + GENESIS_ROOT, + 3n, + badBlock3.archive.root.toString(), + blocks[0].archive.root.toString(), + ]); + publicClient.getTransaction.mockResolvedValueOnce(badBlock3RollupTx); + blobSinkClient.getBlobSidecar.mockResolvedValueOnce(badBlock3Blobs); + + // We should still be at block 1, and the pending chain validation status should still be invalid and point to block 2 + // since we want the archiver to always return the earliest block with invalid attestations + await archiver.syncImmediate(); + latestBlockNum = await archiver.getBlockNumber(); + expect(latestBlockNum).toEqual(1); + const validationStatus = await archiver.getPendingChainValidationStatus(); + assert(!validationStatus.valid); + expect(validationStatus.block.block.number).toEqual(2); + expect(validationStatus.block.block.archive.root.toString()).toEqual(badBlock2.archive.root.toString()); + + // Now we go for another loop, where proper blocks 2 and 3 are proposed with correct attestations // IRL there would be an "Invalidated" event, but we are not currently relying on it - logger.warn(`Adding new block 2 with correct attestations and a block 3`); + logger.warn(`Adding new blocks 2 and 3 with correct attestations`); publicClient.getBlockNumber.mockResolvedValue(100n); - makeL2BlockProposedEvent(90n, 2n, blocks[1].archive.root.toString(), blobHashes[1]); + makeL2BlockProposedEvent(94n, 2n, blocks[1].archive.root.toString(), blobHashes[1]); makeL2BlockProposedEvent(95n, 3n, blocks[2].archive.root.toString(), blobHashes[2]); mockRollup.read.status.mockResolvedValue([ 0n, @@ -452,6 +483,7 @@ describe('Archiver', () => { ); // Now we should move to block 3 + await archiver.syncImmediate(); await waitUntilArchiverBlock(3); latestBlockNum = await archiver.getBlockNumber(); expect(latestBlockNum).toEqual(3); diff --git a/yarn-project/archiver/src/archiver/archiver.ts b/yarn-project/archiver/src/archiver/archiver.ts index eeec0ba83894..e54d3da85450 100644 --- a/yarn-project/archiver/src/archiver/archiver.ts +++ b/yarn-project/archiver/src/archiver/archiver.ts @@ -354,15 +354,25 @@ export class Archiver extends (EventEmitter as new () => ArchiverEmitter) implem currentL1BlockNumber, currentL1Timestamp, ); + + // Update the pending chain validation status with the last block validation result. + // Again, we only update if validation status changed, so in a sequence of invalid blocks + // we keep track of the first invalid block so we can invalidate that one if needed. + if ( + rollupStatus.validationResult && + rollupStatus.validationResult?.valid !== this.pendingChainValidationStatus.valid + ) { + this.pendingChainValidationStatus = rollupStatus.validationResult; + } + // And lastly we check if we are missing any L2 blocks behind us due to a possible L1 reorg. // We only do this if rollup cant prune on the next submission. Otherwise we will end up // re-syncing the blocks we have just unwound above. We also dont do this if the last block is invalid, // since the archiver will rightfully refuse to sync up to it. - if (!rollupCanPrune && rollupStatus.lastBlockValidationResult.valid) { + if (!rollupCanPrune && this.pendingChainValidationStatus.valid) { await this.checkForNewBlocksBeforeL1SyncPoint(rollupStatus, blocksSynchedTo, currentL1BlockNumber); } - this.pendingChainValidationStatus = rollupStatus.lastBlockValidationResult; this.instrumentation.updateL1BlockHeight(currentL1BlockNumber); } @@ -620,7 +630,7 @@ export class Archiver extends (EventEmitter as new () => ArchiverEmitter) implem provenArchive, pendingBlockNumber: Number(pendingBlockNumber), pendingArchive, - lastBlockValidationResult: { valid: true } as ValidateBlockResult, + validationResult: undefined as ValidateBlockResult | undefined, }; this.log.trace(`Retrieved rollup status at current L1 block ${currentL1BlockNumber}.`, { localPendingBlockNumber, @@ -795,17 +805,22 @@ export class Archiver extends (EventEmitter as new () => ArchiverEmitter) implem const validBlocks: PublishedL2Block[] = []; for (const block of publishedBlocks) { - const isProven = block.block.number <= provenBlockNumber; - rollupStatus.lastBlockValidationResult = isProven - ? { valid: true } - : await validateBlockAttestations(block, this.epochCache, this.l1constants, this.log); + const validationResult = await validateBlockAttestations(block, this.epochCache, this.l1constants, this.log); + + // Only update the validation result if it has changed, so we can keep track of the first invalid block + // in case there is a sequence of more than one invalid block, as we need to invalidate the first one. + if (rollupStatus.validationResult?.valid !== validationResult.valid) { + rollupStatus.validationResult = validationResult; + } - if (!rollupStatus.lastBlockValidationResult.valid) { + if (!validationResult.valid) { this.log.warn(`Skipping block ${block.block.number} due to invalid attestations`, { blockHash: block.block.hash(), l1BlockNumber: block.l1.blockNumber, - ...pick(rollupStatus.lastBlockValidationResult, 'reason'), + ...pick(validationResult, 'reason'), }); + // We keep consuming blocks if we find an invalid one, since we do not listen for BlockInvalidated events + // We just pretend the invalid ones are not there and keep consuming the next blocks continue; } diff --git a/yarn-project/archiver/src/archiver/validation.test.ts b/yarn-project/archiver/src/archiver/validation.test.ts index 3bc51dcee5cc..49333cf1c435 100644 --- a/yarn-project/archiver/src/archiver/validation.test.ts +++ b/yarn-project/archiver/src/archiver/validation.test.ts @@ -46,7 +46,6 @@ describe('validateBlockAttestations', () => { const result = await validateBlockAttestations(block, epochCache, constants, logger); expect(result.valid).toBe(true); - expect(result.block).toBe(block); expect(epochCache.getCommitteeForEpoch).toHaveBeenCalledWith(0n); }); @@ -55,7 +54,6 @@ describe('validateBlockAttestations', () => { const result = await validateBlockAttestations(block, epochCache, constants, logger); expect(result.valid).toBe(true); - expect(result.block).toBe(block); expect(epochCache.getCommitteeForEpoch).toHaveBeenCalledWith(0n); }); }); @@ -101,7 +99,6 @@ describe('validateBlockAttestations', () => { const block = await makeBlock(signers.slice(0, 4), committee); const result = await validateBlockAttestations(block, epochCache, constants, logger); expect(result.valid).toBe(true); - expect(result.block).toBe(block); }); }); }); diff --git a/yarn-project/archiver/src/archiver/validation.ts b/yarn-project/archiver/src/archiver/validation.ts index f013e78be1db..51b26cb976cc 100644 --- a/yarn-project/archiver/src/archiver/validation.ts +++ b/yarn-project/archiver/src/archiver/validation.ts @@ -37,9 +37,8 @@ export async function validateBlockAttestations( }); if (!committee || committee.length === 0) { - // Q: Should we accept blocks with no committee? logger?.warn(`No committee found for epoch ${epoch} at slot ${slot}. Accepting block without validation.`, logData); - return { valid: true, block: publishedBlock }; + return { valid: true }; } const committeeSet = new Set(committee.map(member => member.toString())); @@ -64,5 +63,5 @@ export async function validateBlockAttestations( } logger?.debug(`Block attestations validated successfully for block ${block.number} at slot ${slot}`, logData); - return { valid: true, block: publishedBlock }; + return { valid: true }; } diff --git a/yarn-project/end-to-end/src/e2e_epochs/epochs_invalidate_block.test.ts b/yarn-project/end-to-end/src/e2e_epochs/epochs_invalidate_block.test.ts index 0855389de3b4..c8780cf85dab 100644 --- a/yarn-project/end-to-end/src/e2e_epochs/epochs_invalidate_block.test.ts +++ b/yarn-project/end-to-end/src/e2e_epochs/epochs_invalidate_block.test.ts @@ -28,6 +28,7 @@ describe('e2e_epochs/epochs_invalidate_block', () => { let logger: Logger; let l1Client: ExtendedViemWalletClient; let rollupContract: RollupContract; + let anvilPort = 8545; let test: EpochsTestContext; let validators: (Operator & { privateKey: `0x${string}` })[]; @@ -50,6 +51,8 @@ describe('e2e_epochs/epochs_invalidate_block', () => { aztecProofSubmissionEpochs: 1024, startProverNode: false, aztecTargetCommitteeSize: VALIDATOR_COUNT, + archiverPollingIntervalMS: 200, + anvilPort: ++anvilPort, }); ({ context, logger, l1Client } = test); @@ -77,7 +80,7 @@ describe('e2e_epochs/epochs_invalidate_block', () => { await test.teardown(); }); - it('invalidates a block published without sufficient attestations', async () => { + it('proposer invalidates previous block while posting its own', async () => { const sequencers = nodes.map(node => node.getSequencer()!); const initialBlockNumber = await nodes[0].getBlockNumber(); @@ -126,10 +129,11 @@ describe('e2e_epochs/epochs_invalidate_block', () => { 0.1, ); - // Verify the BlockInvalidated event was emitted + // Verify the BlockInvalidated event was emitted and that the block was removed const [event] = blockInvalidatedEvents; logger.warn(`BlockInvalidated event emitted`, { event }); expect(event.args.blockNumber).toBeGreaterThan(initialBlockNumber); + expect(test.rollup.address).toEqual(event.address); // Wait for all nodes to sync the new block logger.warn('Waiting for all nodes to sync'); @@ -149,4 +153,119 @@ describe('e2e_epochs/epochs_invalidate_block', () => { expect(receipt.status).toBe('success'); logger.warn(`Transaction included in block ${receipt.blockNumber}`); }); + + it('proposer invalidates previous block without publishing its own', async () => { + const sequencers = nodes.map(node => node.getSequencer()!); + const initialBlockNumber = await nodes[0].getBlockNumber(); + + // 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.updateSequencerConfig({ skipCollectingAttestations: true, minTxsPerBlock: 0 }); + }); + + // Disable skipCollectingAttestations after the first block is mined and prevent sequencers from publishing any more blocks + test.monitor.once('l2-block', ({ l2BlockNumber }) => { + logger.warn(`Disabling skipCollectingAttestations after L2 block ${l2BlockNumber} has been mined`); + sequencers.forEach(sequencer => { + sequencer.updateSequencerConfig({ 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 BlockInvalidated events + const blockInvalidatedFilter = await l1Client.createContractEventFilter({ + address: rollupContract.address, + abi: RollupAbi, + eventName: 'BlockInvalidated', + fromBlock: 1n, + toBlock: 'latest', + }); + + // The next proposer should invalidate the previous block and publish a new one + logger.warn('Waiting for next proposer to invalidate the previous block'); + + // Wait for the BlockInvalidated event + const blockInvalidatedEvents = await retryUntil( + async () => { + const events = await l1Client.getFilterLogs({ filter: blockInvalidatedFilter }); + return events.length > 0 ? events : undefined; + }, + 'BlockInvalidated event', + test.L2_SLOT_DURATION_IN_S * 5, + 0.1, + ); + + // Verify the BlockInvalidated event was emitted and that the block was removed + const [event] = blockInvalidatedEvents; + logger.warn(`BlockInvalidated event emitted`, { event }); + expect(event.args.blockNumber).toBeGreaterThan(initialBlockNumber); + expect(await test.rollup.getBlockNumber()).toEqual(BigInt(initialBlockNumber)); + }); + + it('committee member invalidates a block if proposer does not come through', async () => { + const sequencers = nodes.map(node => node.getSequencer()!); + const initialBlockNumber = await nodes[0].getBlockNumber(); + + // Configure all sequencers to skip collecting attestations before starting + logger.warn('Configuring all sequencers to skip attestation collection and invalidation as proposer'); + const invalidationDelay = test.L1_BLOCK_TIME_IN_S * 4; + sequencers.forEach(sequencer => { + sequencer.updateSequencerConfig({ + skipCollectingAttestations: true, + minTxsPerBlock: 0, + skipInvalidateBlockAsProposer: true, + secondsBeforeInvalidatingBlockAsCommitteeMember: invalidationDelay, + }); + }); + + // Disable skipCollectingAttestations after the first block is mined + let invalidBlockTimestamp: bigint | undefined; + test.monitor.once('l2-block', ({ l2BlockNumber, timestamp }) => { + logger.warn(`Disabling skipCollectingAttestations after L2 block ${l2BlockNumber} has been mined`); + invalidBlockTimestamp = timestamp; + sequencers.forEach(sequencer => { + sequencer.updateSequencerConfig({ skipCollectingAttestations: false }); + }); + }); + + // Start all sequencers + await Promise.all(sequencers.map(s => s.start())); + logger.warn(`Started all sequencers with skipCollectingAttestations=true`); + + // Create a filter for BlockInvalidated events + const blockInvalidatedFilter = await l1Client.createContractEventFilter({ + address: rollupContract.address, + abi: RollupAbi, + eventName: 'BlockInvalidated', + fromBlock: 1n, + toBlock: 'latest', + }); + + // Some committee member should invalidate the previous block + logger.warn('Waiting for committee member to invalidate the previous block'); + + // Wait for the BlockInvalidated event + const blockInvalidatedEvents = await retryUntil( + async () => { + const events = await l1Client.getFilterLogs({ filter: blockInvalidatedFilter }); + return events.length > 0 ? events : undefined; + }, + 'BlockInvalidated event', + test.L2_SLOT_DURATION_IN_S * 5, + 0.1, + ); + + // Verify the BlockInvalidated event was emitted + const [event] = blockInvalidatedEvents; + logger.warn(`BlockInvalidated event emitted`, { event }); + expect(event.args.blockNumber).toBeGreaterThan(initialBlockNumber); + + // And check that the invalidation happened at least after the specified timeout + const { timestamp: invalidationTimestamp } = await l1Client.getBlock({ blockNumber: event.blockNumber }); + expect(invalidationTimestamp).toBeGreaterThanOrEqual(invalidBlockTimestamp! + BigInt(invalidationDelay)); + }); }); diff --git a/yarn-project/end-to-end/src/fixtures/utils.ts b/yarn-project/end-to-end/src/fixtures/utils.ts index e9d7e8bfd3e6..8e0389163809 100644 --- a/yarn-project/end-to-end/src/fixtures/utils.ts +++ b/yarn-project/end-to-end/src/fixtures/utils.ts @@ -313,6 +313,8 @@ export type SetupOptions = { automineL1Setup?: boolean; /** How many accounts to seed and unlock in anvil. */ anvilAccounts?: number; + /** Port to start anvil (defaults to 8545) */ + anvilPort?: number; } & Partial; /** Context for an end-to-end test as returned by the `setup` function */ @@ -404,7 +406,11 @@ export async function setup( ); } - const res = await startAnvil({ l1BlockTime: opts.ethereumSlotDuration, accounts: opts.anvilAccounts }); + const res = await startAnvil({ + l1BlockTime: opts.ethereumSlotDuration, + accounts: opts.anvilAccounts, + port: opts.anvilPort, + }); anvil = res.anvil; config.l1RpcUrls = [res.rpcUrl]; } diff --git a/yarn-project/foundation/src/config/env_var.ts b/yarn-project/foundation/src/config/env_var.ts index 80ca48b8502a..1981901bea24 100644 --- a/yarn-project/foundation/src/config/env_var.ts +++ b/yarn-project/foundation/src/config/env_var.ts @@ -192,6 +192,8 @@ export type EnvVar = | 'SEQ_ENFORCE_TIME_TABLE' | 'SEQ_MAX_L1_TX_INCLUSION_TIME_INTO_SLOT' | 'SEQ_ATTESTATION_PROPAGATION_TIME' + | 'SEQ_SECONDS_BEFORE_INVALIDATING_BLOCK_AS_COMMITTEE_MEMBER' + | 'SEQ_SECONDS_BEFORE_INVALIDATING_BLOCK_AS_NON_COMMITTEE_MEMBER' | 'SLASH_FACTORY_CONTRACT_ADDRESS' | 'SLASH_PRUNE_ENABLED' | 'SLASH_PRUNE_PENALTY' diff --git a/yarn-project/prover-node/src/prover-node-publisher.ts b/yarn-project/prover-node/src/prover-node-publisher.ts index 2f1f5c209f09..c4f580afb288 100644 --- a/yarn-project/prover-node/src/prover-node-publisher.ts +++ b/yarn-project/prover-node/src/prover-node-publisher.ts @@ -26,9 +26,6 @@ import { type Hex, type TransactionReceipt, encodeFunctionData } from 'viem'; import { ProverNodePublisherMetrics } from './metrics.js'; -/** - * Stats for a sent transaction. - */ /** Arguments to the submitEpochProof method of the rollup contract */ export type L1SubmitEpochProofArgs = { epochSize: number; @@ -142,11 +139,11 @@ export class ProverNodePublisher { } this.metrics.recordFailedTx(); - this.log.error(`Rollup.submitEpochProof tx status failed: ${txReceipt.transactionHash}`, ctx); + this.log.error(`Rollup.submitEpochProof tx status failed ${txReceipt.transactionHash}`, undefined, ctx); await this.sleepOrInterrupted(); } - this.log.verbose('L2 block data syncing interrupted while processing blocks.', ctx); + this.log.verbose('L2 block data syncing interrupted', ctx); return false; } diff --git a/yarn-project/sequencer-client/src/config.ts b/yarn-project/sequencer-client/src/config.ts index 034cc3072b01..76ebd831df1d 100644 --- a/yarn-project/sequencer-client/src/config.ts +++ b/yarn-project/sequencer-client/src/config.ts @@ -120,8 +120,27 @@ export const sequencerConfigMappings: ConfigMappingsType = { fakeProcessingDelayPerTxMs: { description: 'Used for testing to introduce a fake delay after processing each tx', }, + secondsBeforeInvalidatingBlockAsCommitteeMember: { + env: 'SEQ_SECONDS_BEFORE_INVALIDATING_BLOCK_AS_COMMITTEE_MEMBER', + description: + 'How many seconds to wait before trying to invalidate a block from the pending chain as a committee member (zero to never invalidate).' + + ' The next proposer is expected to invalidate, so the committee acts as a fallback.', + ...numberConfigHelper(144), // 12 L1 blocks + }, + secondsBeforeInvalidatingBlockAsNonCommitteeMember: { + env: 'SEQ_SECONDS_BEFORE_INVALIDATING_BLOCK_AS_NON_COMMITTEE_MEMBER', + description: + 'How many seconds to wait before trying to invalidate a block from the pending chain as a non-committee member (zero to never invalidate).' + + ' The next proposer is expected to invalidate, then the committee, so other sequencers act as a fallback.', + ...numberConfigHelper(432), // 36 L1 blocks + }, skipCollectingAttestations: { - description: 'Whether to skip collecting attestations from validators and only use self-attestations.', + description: + 'Whether to skip collecting attestations from validators and only use self-attestations (for testing only)', + ...booleanConfigHelper(false), + }, + skipInvalidateBlockAsProposer: { + description: 'Do not invalidate the previous block if invalid when we are the proposer (for testing only)', ...booleanConfigHelper(false), }, ...pickConfigMappings(p2pConfigMappings, ['txPublicSetupAllowList']), diff --git a/yarn-project/sequencer-client/src/publisher/sequencer-publisher.ts b/yarn-project/sequencer-client/src/publisher/sequencer-publisher.ts index 13837190edd2..ed760faee62c 100644 --- a/yarn-project/sequencer-client/src/publisher/sequencer-publisher.ts +++ b/yarn-project/sequencer-client/src/publisher/sequencer-publisher.ts @@ -382,10 +382,20 @@ export class SequencerPublisher { return undefined; } - const request = this.buildInvalidateBlockRequest(validationResult); const { reason, block } = validationResult; const blockNumber = block.block.number; const logData = { ...block.block.toBlockInfo(), reason }; + + const currentBlockNumber = await this.rollupContract.getBlockNumber(); + if (currentBlockNumber < validationResult.block.block.number) { + this.log.verbose( + `Skipping block ${blockNumber} invalidation since it has already been removed from the pending chain`, + { currentBlockNumber, ...logData }, + ); + return undefined; + } + + const request = this.buildInvalidateBlockRequest(validationResult); this.log.debug(`Simulating invalidate block ${blockNumber}`, logData); try { @@ -689,7 +699,7 @@ export class SequencerPublisher { throw err; } - this.log.debug(`Enqueuing block propose transaction`, { ...block.toBlockInfo(), ...opts }); + this.log.verbose(`Enqueuing block propose transaction`, { ...block.toBlockInfo(), ...opts }); await this.addProposeTx(block, proposeTxArgs, opts, ts); return true; } @@ -699,14 +709,17 @@ export class SequencerPublisher { return; } - const logData = { ...pick(request, 'gasUsed', 'blockNumber'), opts }; - this.log.debug(`Enqueuing invalidate block`, logData); + // We issue the simulation against the rollup contract, so we need to account for the overhead of the multicall3 + const gasLimit = this.l1TxUtils.bumpGasLimit(BigInt(Math.ceil((Number(request.gasUsed) * 64) / 63))); + + const logData = { ...pick(request, 'gasUsed', 'blockNumber'), gasLimit, opts }; + this.log.verbose(`Enqueuing invalidate block request`, logData); this.addRequest({ action: `invalidate-by-${request.reason}`, request: request.request, - gasConfig: { gasLimit: request.gasUsed, txTimeoutAt: opts.txTimeoutAt }, + gasConfig: { gasLimit, txTimeoutAt: opts.txTimeoutAt }, lastValidL2Slot: this.getCurrentL2Slot() + 2n, - checkSuccess: (req, result) => { + checkSuccess: (_req, result) => { const success = result && result.receipt && diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts index 85e8007367a1..c136c116d7ca 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts @@ -19,6 +19,7 @@ import { type MerkleTreeReadOperations, type MerkleTreeWriteOperations, type PublicProcessorLimits, + type SequencerConfig, WorldStateRunningState, type WorldStateSyncStatus, type WorldStateSynchronizer, @@ -254,7 +255,7 @@ describe('sequencer', () => { dateProvider = new TestDateProvider(); - const config = { enforceTimeTable: true, maxTxsPerBlock: 4 }; + const config: SequencerConfig = { enforceTimeTable: true, maxTxsPerBlock: 4 }; sequencer = new TestSubject( publisher, // TODO(md): add the relevant methods to the validator client that will prevent it stalling when waiting for attestations diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.ts index f05fe3fd77be..5a9299d3a59a 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.ts @@ -339,9 +339,9 @@ export class Sequencer extends (EventEmitter as new () => TypedEventEmitter TypedEventEmitter addr.equals(proposerInNextSlot))) { this.log.debug(`Cannot propose block ${newBlockNumber} since we are not a proposer`, { us: validatorAddresses, proposer: proposerInNextSlot, ...syncLogData, }); + // If the pending chain is invalid, we may need to invalidate the block if no one else is doing it. + if (!syncedTo.pendingChainValidationStatus.valid) { + await this.considerInvalidatingBlock(syncedTo, slot, validatorAddresses); + } return; } - // Double check we are good for proposing at the next block before we start operations. - // We should never fail this check assuming the logic above is good. - const proposerAddress = proposerInNextSlot ?? EthAddress.ZERO; - - // Prepare invalidation request if the pending chain is invalid (undefined if no need) + // Prepare invalidation request if the pending chain is invalid (returns undefined if no need) const invalidateBlock = await this.publisher.simulateInvalidateBlock(syncedTo.pendingChainValidationStatus); // Check with the rollup if we can indeed propose at the next L2 slot. This check should not fail // if all the previous checks are good, but we do it just in case. + const proposerAddress = proposerInNextSlot ?? EthAddress.ZERO; const canProposeCheck = await this.publisher.canProposeAtNextEthBlock( chainTipArchive, proposerAddress, @@ -431,6 +431,10 @@ export class Sequencer extends (EventEmitter as new () => TypedEventEmitter this.validatorClient!.signWithAddress(proposerAddress, msg).then(s => s.toString()), ); + if (invalidateBlock && !this.config.skipInvalidateBlockAsProposer) { + this.publisher.enqueueInvalidateBlock(invalidateBlock); + } + this.setState(SequencerState.INITIALIZING_PROPOSAL, slot); this.log.verbose(`Preparing proposal for block ${newBlockNumber} at slot ${slot}`, { proposer: proposerInNextSlot?.toString(), @@ -781,10 +785,6 @@ export class Sequencer extends (EventEmitter as new () => TypedEventEmitter TypedEventEmitter>>, + currentSlot: bigint, + ourValidatorAddresses: EthAddress[], + ): Promise { + const { pendingChainValidationStatus, l1Timestamp } = syncedTo; + if (pendingChainValidationStatus.valid) { + return; + } + + const invalidL1Timestamp = pendingChainValidationStatus.block.l1.timestamp; + const timeSinceChainInvalid = this.dateProvider.nowInSeconds() - Number(invalidL1Timestamp); + const invalidBlockNumber = pendingChainValidationStatus.block.block.number; + + const { secondsBeforeInvalidatingBlockAsCommitteeMember, secondsBeforeInvalidatingBlockAsNonCommitteeMember } = + this.config; + + const logData = { + invalidL1Timestamp, + l1Timestamp, + invalidBlock: pendingChainValidationStatus.block.block.toBlockInfo(), + secondsBeforeInvalidatingBlockAsCommitteeMember, + secondsBeforeInvalidatingBlockAsNonCommitteeMember, + ourValidatorAddresses, + currentSlot, + }; + + const inCurrentCommittee = () => + this.publisher.epochCache + .getCommittee(currentSlot) + .then(c => c?.committee?.some(member => ourValidatorAddresses.some(addr => addr.equals(member)))); + + const invalidateAsCommitteeMember = + secondsBeforeInvalidatingBlockAsCommitteeMember !== undefined && + secondsBeforeInvalidatingBlockAsCommitteeMember > 0 && + timeSinceChainInvalid > secondsBeforeInvalidatingBlockAsCommitteeMember && + (await inCurrentCommittee()); + + const invalidateAsNonCommitteeMember = + secondsBeforeInvalidatingBlockAsNonCommitteeMember !== undefined && + secondsBeforeInvalidatingBlockAsNonCommitteeMember > 0 && + timeSinceChainInvalid > secondsBeforeInvalidatingBlockAsNonCommitteeMember; + + if (!invalidateAsCommitteeMember && !invalidateAsNonCommitteeMember) { + this.log.debug(`Not invalidating pending chain`, logData); + return; + } + + const invalidateBlock = await this.publisher.simulateInvalidateBlock(pendingChainValidationStatus); + if (!invalidateBlock) { + this.log.warn(`Failed to simulate invalidate block`, logData); + return; + } + + this.log.info( + invalidateAsCommitteeMember + ? `Invalidating block ${invalidBlockNumber} as committee member` + : `Invalidating block ${invalidBlockNumber} as non-committee member`, + logData, + ); + + this.publisher.enqueueInvalidateBlock(invalidateBlock); + await this.publisher.sendRequests(); + } + private getSlotStartBuildTimestamp(slotNumber: number | bigint): number { return ( Number(this.l1Constants.l1GenesisTime) + diff --git a/yarn-project/stdlib/src/block/l2_block_source.ts b/yarn-project/stdlib/src/block/l2_block_source.ts index 1a70b459545e..844217c4d063 100644 --- a/yarn-project/stdlib/src/block/l2_block_source.ts +++ b/yarn-project/stdlib/src/block/l2_block_source.ts @@ -130,8 +130,8 @@ export interface L2BlockSource { isPendingChainInvalid(): Promise; /** - * Returns the status of the pending chain validation. - * This includes whether the chain is valid, and if not, the reason for invalidation. + * Returns the status of the pending chain validation. If the chain is invalid, reports the earliest consecutive block + * that is invalid, along with the reason for being invalid, which can be used to trigger an invalidation. */ getPendingChainValidationStatus(): Promise; @@ -141,7 +141,7 @@ export interface L2BlockSource { /** Result type for validating a block attestations */ export type ValidateBlockResult = - | { valid: true; block?: PublishedL2Block } + | { valid: true } | { valid: false; block: PublishedL2Block; committee: EthAddress[]; reason: 'insufficient-attestations' } | { valid: false; diff --git a/yarn-project/stdlib/src/interfaces/aztec-node-admin.test.ts b/yarn-project/stdlib/src/interfaces/aztec-node-admin.test.ts index 757f4bbcf9ea..3c1aa44d800e 100644 --- a/yarn-project/stdlib/src/interfaces/aztec-node-admin.test.ts +++ b/yarn-project/stdlib/src/interfaces/aztec-node-admin.test.ts @@ -86,6 +86,8 @@ class MockAztecNodeAdmin implements AztecNodeAdmin { slashInactivityMaxPenalty: 1000n, slashProposerRoundPollingIntervalSeconds: 1000, slasherPrivateKey: new SecretValue(undefined), + secondsBeforeInvalidatingBlockAsCommitteeMember: 0, + secondsBeforeInvalidatingBlockAsNonCommitteeMember: 0, }); } startSnapshotUpload(_location: string): Promise { diff --git a/yarn-project/stdlib/src/interfaces/configs.ts b/yarn-project/stdlib/src/interfaces/configs.ts index 955ffe4b33cd..a9bdbc5510f0 100644 --- a/yarn-project/stdlib/src/interfaces/configs.ts +++ b/yarn-project/stdlib/src/interfaces/configs.ts @@ -44,8 +44,14 @@ export interface SequencerConfig { fakeProcessingDelayPerTxMs?: number; /** How many seconds it takes for proposals and attestations to travel across the p2p layer (one-way) */ attestationPropagationTime?: number; + /** How many seconds before invalidating a block as a committee member (zero to never invalidate) */ + secondsBeforeInvalidatingBlockAsCommitteeMember?: number; + /** How many seconds before invalidating a block as a non-committee member (zero to never invalidate) */ + secondsBeforeInvalidatingBlockAsNonCommitteeMember?: number; /** Skip collecting attestations (for testing only) */ skipCollectingAttestations?: boolean; + /** Do not invalidate the previous block if invalid when we are the proposer (for testing only) */ + skipInvalidateBlockAsProposer?: boolean; } export const SequencerConfigSchema = z.object({ @@ -67,4 +73,6 @@ export const SequencerConfigSchema = z.object({ fakeProcessingDelayPerTxMs: z.number().optional(), attestationPropagationTime: z.number().optional(), skipCollectingAttestations: z.boolean().optional(), + secondsBeforeInvalidatingBlockAsCommitteeMember: z.number(), + secondsBeforeInvalidatingBlockAsNonCommitteeMember: z.number(), }) satisfies ZodFor;