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 a07d4b7d6ba9..cc62248bb632 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 @@ -22,9 +22,8 @@ import { Checkpoint, type CheckpointData, L1PublishedData } from '@aztec/stdlib/ import type { L1RollupConstants } from '@aztec/stdlib/epoch-helpers'; import { GasFees } from '@aztec/stdlib/gas'; import { - type BuildBlockInCheckpointResult, + InsufficientValidTxsError, type MerkleTreeWriteOperations, - NoValidTxsError, type ResolvedSequencerConfig, type WorldStateSynchronizer, } from '@aztec/stdlib/interfaces/server'; @@ -774,7 +773,7 @@ describe('CheckpointProposalJob', () => { const checkpointBuilder = mock(); const failedTxs: FailedTx[] = txs.slice(1).map(tx => ({ tx, error: new Error('Invalid tx') })); - checkpointBuilder.buildBlock.mockResolvedValue({ failedTxs, numTxs: 1 } as BuildBlockInCheckpointResult); + checkpointBuilder.buildBlock.mockRejectedValue(new InsufficientValidTxsError(1, 2, failedTxs)); const checkpoint = await job.buildSingleBlock(checkpointBuilder, { blockNumber: newBlockNumber, @@ -795,7 +794,7 @@ describe('CheckpointProposalJob', () => { const checkpointBuilder = mock(); const failedTxs: FailedTx[] = txs.slice(1).map(tx => ({ tx, error: new Error('Invalid tx') })); - checkpointBuilder.buildBlock.mockRejectedValue(new NoValidTxsError(failedTxs)); + checkpointBuilder.buildBlock.mockRejectedValue(new InsufficientValidTxsError(0, 3, failedTxs)); const checkpoint = await job.buildSingleBlock(checkpointBuilder, { blockNumber: newBlockNumber, 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 e3a50a720d1b..a627ed5efe9e 100644 --- a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts +++ b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts @@ -34,7 +34,7 @@ import { type Checkpoint, validateCheckpoint } from '@aztec/stdlib/checkpoint'; import { getSlotStartBuildTimestamp } from '@aztec/stdlib/epoch-helpers'; import { Gas } from '@aztec/stdlib/gas'; import { - NoValidTxsError, + InsufficientValidTxsError, type PublicProcessorLimits, type ResolvedSequencerConfig, type WorldStateSynchronizer, @@ -565,7 +565,9 @@ export class CheckpointProposalJob implements Traceable { // Per-block limits derived at startup by computeBlockLimits(), further capped // by remaining checkpoint-level budgets inside CheckpointBuilder before each block is built. - const blockBuilderOptions: PublicProcessorLimits = { + // minValidTxs is passed into the builder so it can reject the block *before* updating state. + const minValidTxs = forceCreate ? 0 : (this.config.minValidTxsPerBlock ?? minTxs); + const blockBuilderOptions: PublicProcessorLimits & { minValidTxs?: number } = { maxTransactions: this.config.maxTxsPerBlock, maxBlockGas: this.config.maxL2BlockGas !== undefined || this.config.maxDABlockGas !== undefined @@ -573,9 +575,12 @@ export class CheckpointProposalJob implements Traceable { : undefined, deadline: buildDeadline, isBuildingProposal: true, + minValidTxs, }; - // Actually build the block by executing txs + // Actually build the block by executing txs. The builder throws InsufficientValidTxsError + // if the number of successfully processed txs is below minValidTxs, ensuring state is not + // updated for blocks that will be discarded. const buildResult = await this.buildSingleBlockWithCheckpointBuilder( checkpointBuilder, pendingTxs, @@ -587,14 +592,16 @@ export class CheckpointProposalJob implements Traceable { // If any txs failed during execution, drop them from the mempool so we don't pick them up again await this.dropFailedTxsFromP2P(buildResult.failedTxs); - // Check if we have created a block with enough txs. If there were invalid txs in the pool, or if execution took - // too long, then we may not get to minTxsPerBlock after executing public functions. - const minValidTxs = this.config.minValidTxsPerBlock ?? minTxs; - const numTxs = buildResult.status === 'no-valid-txs' ? 0 : buildResult.numTxs; - if (buildResult.status === 'no-valid-txs' || (!forceCreate && numTxs < minValidTxs)) { + if (buildResult.status === 'insufficient-valid-txs') { this.log.warn( `Block ${blockNumber} at index ${indexWithinCheckpoint} on slot ${this.slot} has too few valid txs to be proposed`, - { slot: this.slot, blockNumber, numTxs, indexWithinCheckpoint, minValidTxs, buildResult: buildResult.status }, + { + slot: this.slot, + blockNumber, + numTxs: buildResult.processedCount, + indexWithinCheckpoint, + minValidTxs, + }, ); this.eventEmitter.emit('block-build-failed', { reason: `Insufficient valid txs`, slot: this.slot }); this.metrics.recordBlockProposalFailed('insufficient_valid_txs'); @@ -602,7 +609,7 @@ export class CheckpointProposalJob implements Traceable { } // Block creation succeeded, emit stats and metrics - const { block, publicProcessorDuration, usedTxs, blockBuildDuration } = buildResult; + const { block, publicProcessorDuration, usedTxs, blockBuildDuration, numTxs } = buildResult; const blockStats = { eventName: 'l2-block-built', @@ -633,13 +640,13 @@ export class CheckpointProposalJob implements Traceable { } } - /** Uses the checkpoint builder to build a block, catching specific txs */ + /** Uses the checkpoint builder to build a block, catching InsufficientValidTxsError. */ private async buildSingleBlockWithCheckpointBuilder( checkpointBuilder: CheckpointBuilder, pendingTxs: AsyncIterable, blockNumber: BlockNumber, blockTimestamp: bigint, - blockBuilderOptions: PublicProcessorLimits, + blockBuilderOptions: PublicProcessorLimits & { minValidTxs?: number }, ) { try { const workTimer = new Timer(); @@ -647,8 +654,12 @@ export class CheckpointProposalJob implements Traceable { const blockBuildDuration = workTimer.ms(); return { ...result, blockBuildDuration, status: 'success' as const }; } catch (err: unknown) { - if (isErrorClass(err, NoValidTxsError)) { - return { failedTxs: err.failedTxs, status: 'no-valid-txs' as const }; + if (isErrorClass(err, InsufficientValidTxsError)) { + return { + failedTxs: err.failedTxs, + processedCount: err.processedCount, + status: 'insufficient-valid-txs' as const, + }; } throw err; } diff --git a/yarn-project/sequencer-client/src/test/mock_checkpoint_builder.ts b/yarn-project/sequencer-client/src/test/mock_checkpoint_builder.ts index 3063bfd0f90c..27b6bf911b07 100644 --- a/yarn-project/sequencer-client/src/test/mock_checkpoint_builder.ts +++ b/yarn-project/sequencer-client/src/test/mock_checkpoint_builder.ts @@ -32,7 +32,7 @@ export class MockCheckpointBuilder implements ICheckpointBlockBuilder { public buildBlockCalls: Array<{ blockNumber: BlockNumber; timestamp: bigint; - opts: PublicProcessorLimits; + opts: PublicProcessorLimits & { minValidTxs?: number }; }> = []; /** Track all consumed transaction hashes across buildBlock calls */ public consumedTxHashes: Set = new Set(); @@ -74,7 +74,7 @@ export class MockCheckpointBuilder implements ICheckpointBlockBuilder { pendingTxs: Iterable | AsyncIterable, blockNumber: BlockNumber, timestamp: bigint, - opts: PublicProcessorLimits, + opts: PublicProcessorLimits & { minValidTxs?: number }, ): Promise { this.buildBlockCalls.push({ blockNumber, timestamp, opts }); diff --git a/yarn-project/stdlib/src/interfaces/block-builder.ts b/yarn-project/stdlib/src/interfaces/block-builder.ts index 8cd272141823..aa2857072c92 100644 --- a/yarn-project/stdlib/src/interfaces/block-builder.ts +++ b/yarn-project/stdlib/src/interfaces/block-builder.ts @@ -87,11 +87,15 @@ export const FullNodeBlockBuilderConfigKeys: (keyof FullNodeBlockBuilderConfig)[ 'maxBlocksPerCheckpoint', ] as const; -/** Thrown when no valid transactions are available to include in a block after processing, and this is not the first block in a checkpoint. */ -export class NoValidTxsError extends Error { - constructor(public readonly failedTxs: FailedTx[]) { - super('No valid transactions to include in block'); - this.name = 'NoValidTxsError'; +/** Thrown when the number of successfully processed transactions is below the required minimum. */ +export class InsufficientValidTxsError extends Error { + constructor( + public readonly processedCount: number, + public readonly minRequired: number, + public readonly failedTxs: FailedTx[], + ) { + super(`Insufficient valid txs: got ${processedCount} but need ${minRequired}`); + this.name = 'InsufficientValidTxsError'; } } @@ -106,11 +110,12 @@ export type BuildBlockInCheckpointResult = { /** Interface for building blocks within a checkpoint context. */ export interface ICheckpointBlockBuilder { + /** Builds a single block within this checkpoint. Throws InsufficientValidTxsError if fewer than minValidTxs succeed. */ buildBlock( pendingTxs: Iterable | AsyncIterable, blockNumber: BlockNumber, timestamp: bigint, - opts: PublicProcessorLimits, + opts: PublicProcessorLimits & { minValidTxs?: number }, ): Promise; } diff --git a/yarn-project/validator-client/src/checkpoint_builder.test.ts b/yarn-project/validator-client/src/checkpoint_builder.test.ts index a57d9ca519e0..9dab4a4778c9 100644 --- a/yarn-project/validator-client/src/checkpoint_builder.test.ts +++ b/yarn-project/validator-client/src/checkpoint_builder.test.ts @@ -17,11 +17,12 @@ import type { ContractDataSource } from '@aztec/stdlib/contract'; import { Gas, GasFees } from '@aztec/stdlib/gas'; import { type FullNodeBlockBuilderConfig, + InsufficientValidTxsError, type MerkleTreeWriteOperations, - NoValidTxsError, type PublicProcessorLimits, type PublicProcessorValidator, } from '@aztec/stdlib/interfaces/server'; +import { TxHash } from '@aztec/stdlib/tx'; import type { CheckpointGlobalVariables, GlobalVariables, ProcessedTx, Tx } from '@aztec/stdlib/tx'; import type { TelemetryClient } from '@aztec/telemetry-client'; @@ -139,9 +140,7 @@ describe('CheckpointBuilder', () => { expect(lightweightCheckpointBuilder.addBlock).toHaveBeenCalled(); }); - it('allows building an empty first block in a checkpoint', async () => { - lightweightCheckpointBuilder.getBlockCount.mockReturnValue(0); - + it('allows building an empty block when minValidTxs is 0', async () => { const expectedBlock = await L2Block.random(blockNumber, { txsPerBlock: 0 }); lightweightCheckpointBuilder.addBlock.mockResolvedValue({ block: expectedBlock, timings: {} }); @@ -154,16 +153,14 @@ describe('CheckpointBuilder', () => { [], // debugLogs ]); - const result = await checkpointBuilder.buildBlock([], blockNumber, 1000n); + const result = await checkpointBuilder.buildBlock([], blockNumber, 1000n, { minValidTxs: 0 }); expect(result.block).toBe(expectedBlock); expect(result.numTxs).toBe(0); expect(lightweightCheckpointBuilder.addBlock).toHaveBeenCalled(); }); - it('throws NoValidTxsError when no valid transactions and not first block in checkpoint', async () => { - lightweightCheckpointBuilder.getBlockCount.mockReturnValue(1); - + it('throws InsufficientValidTxsError when fewer txs than minValidTxs', async () => { const failedTx = { tx: { txHash: Fr.random() } as unknown as Tx, error: new Error('tx failed') }; processor.process.mockResolvedValue([ [], // processedTxs - empty @@ -173,10 +170,46 @@ describe('CheckpointBuilder', () => { [], // debugLogs ]); - await expect(checkpointBuilder.buildBlock([], blockNumber, 1000n)).rejects.toThrow(NoValidTxsError); + await expect(checkpointBuilder.buildBlock([], blockNumber, 1000n, { minValidTxs: 1 })).rejects.toThrow( + InsufficientValidTxsError, + ); expect(lightweightCheckpointBuilder.addBlock).not.toHaveBeenCalled(); }); + + it('does not update state when some txs succeed but below minValidTxs', async () => { + const processedTx = mock(); + processedTx.hash = TxHash.random(); + const failedTx = { tx: { txHash: Fr.random() } as unknown as Tx, error: new Error('tx failed') }; + processor.process.mockResolvedValue([ + [processedTx], // processedTxs - 1 succeeded + [failedTx], // failedTxs - 1 failed + [], // usedTxs + [], // returnValues + [], // debugLogs + ]); + + const err = await checkpointBuilder + .buildBlock([], blockNumber, 1000n, { minValidTxs: 2 }) + .catch((e: unknown) => e); + + expect(err).toBeInstanceOf(InsufficientValidTxsError); + expect((err as InsufficientValidTxsError).processedCount).toBe(1); + expect((err as InsufficientValidTxsError).minRequired).toBe(2); + expect(lightweightCheckpointBuilder.addBlock).not.toHaveBeenCalled(); + }); + + it('defaults to minValidTxs=0 when not specified, allowing empty blocks', async () => { + const expectedBlock = await L2Block.random(blockNumber, { txsPerBlock: 0 }); + lightweightCheckpointBuilder.addBlock.mockResolvedValue({ block: expectedBlock, timings: {} }); + + processor.process.mockResolvedValue([[], [], [], [], []]); + + const result = await checkpointBuilder.buildBlock([], blockNumber, 1000n); + + expect(result.numTxs).toBe(0); + expect(lightweightCheckpointBuilder.addBlock).toHaveBeenCalled(); + }); }); describe('capLimitsByCheckpointBudgets', () => { diff --git a/yarn-project/validator-client/src/checkpoint_builder.ts b/yarn-project/validator-client/src/checkpoint_builder.ts index c484fd6dd764..906bcfe7da98 100644 --- a/yarn-project/validator-client/src/checkpoint_builder.ts +++ b/yarn-project/validator-client/src/checkpoint_builder.ts @@ -25,8 +25,8 @@ import { FullNodeBlockBuilderConfigKeys, type ICheckpointBlockBuilder, type ICheckpointsBuilder, + InsufficientValidTxsError, type MerkleTreeWriteOperations, - NoValidTxsError, type PublicProcessorLimits, type WorldStateSynchronizer, } from '@aztec/stdlib/interfaces/server'; @@ -34,6 +34,7 @@ import { type DebugLogStore, NullDebugLogStore } from '@aztec/stdlib/logs'; import { MerkleTreeId } from '@aztec/stdlib/trees'; import { type CheckpointGlobalVariables, GlobalVariables, StateReference, Tx } from '@aztec/stdlib/tx'; import { type TelemetryClient, getTelemetryClient } from '@aztec/telemetry-client'; +import { ForkCheckpoint } from '@aztec/world-state'; // Re-export for backward compatibility export type { BuildBlockInCheckpointResult } from '@aztec/stdlib/interfaces/server'; @@ -73,7 +74,7 @@ export class CheckpointBuilder implements ICheckpointBlockBuilder { pendingTxs: Iterable | AsyncIterable, blockNumber: BlockNumber, timestamp: bigint, - opts: PublicProcessorLimits & { expectedEndState?: StateReference } = {}, + opts: PublicProcessorLimits & { expectedEndState?: StateReference; minValidTxs?: number } = {}, ): Promise { const slot = this.checkpointBuilder.constants.slotNumber; @@ -103,34 +104,47 @@ export class CheckpointBuilder implements ICheckpointBlockBuilder { ...this.capLimitsByCheckpointBudgets(opts), }; - const [publicProcessorDuration, [processedTxs, failedTxs, usedTxs]] = await elapsed(() => - processor.process(pendingTxs, cappedOpts, validator), - ); + // We execute all merkle tree operations on a world state fork checkpoint + // This enables us to discard all modifications in the event that we fail to successfully process sufficient transactions + const forkCheckpoint = await ForkCheckpoint.new(this.fork); - // Throw if we didn't collect a single valid tx and we're not allowed to build empty blocks - // (only the first block in a checkpoint can be empty) - if (processedTxs.length === 0 && this.checkpointBuilder.getBlockCount() > 0) { - throw new NoValidTxsError(failedTxs); + try { + const [publicProcessorDuration, [processedTxs, failedTxs, usedTxs]] = await elapsed(() => + processor.process(pendingTxs, cappedOpts, validator), + ); + // Throw before updating state if we don't have enough valid txs + const minValidTxs = opts.minValidTxs ?? 0; + if (processedTxs.length < minValidTxs) { + throw new InsufficientValidTxsError(processedTxs.length, minValidTxs, failedTxs); + } + + // Commit the fork checkpoint + await forkCheckpoint.commit(); + + // Add block to checkpoint + const { block } = await this.checkpointBuilder.addBlock(globalVariables, processedTxs, { + expectedEndState: opts.expectedEndState, + }); + + this.log.debug('Built block within checkpoint', { + header: block.header.toInspect(), + processedTxs: processedTxs.map(tx => tx.hash.toString()), + failedTxs: failedTxs.map(tx => tx.tx.txHash.toString()), + }); + + return { + block, + publicProcessorDuration, + numTxs: processedTxs.length, + failedTxs, + usedTxs, + }; + } catch (err) { + // If we reached the point of committing the checkpoint, this does nothing + // Otherwise it reverts any changes made to the fork for this failed block + await forkCheckpoint.revert(); + throw err; } - - // Add block to checkpoint - const { block } = await this.checkpointBuilder.addBlock(globalVariables, processedTxs, { - expectedEndState: opts.expectedEndState, - }); - - this.log.debug('Built block within checkpoint', { - header: block.header.toInspect(), - processedTxs: processedTxs.map(tx => tx.hash.toString()), - failedTxs: failedTxs.map(tx => tx.tx.txHash.toString()), - }); - - return { - block, - publicProcessorDuration, - numTxs: processedTxs.length, - failedTxs, - usedTxs, - }; } /** Completes the checkpoint and returns it. */