fix: Don't update state if we failed to execute sufficient transactions

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<CheckpointBuilder>();
       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<CheckpointBuilder>();
       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,

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,17 +565,22 @@ 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
             ? new Gas(this.config.maxDABlockGas ?? Infinity, this.config.maxL2BlockGas ?? Infinity)
             : 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,22 +592,24 @@ 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');
         return undefined;
       }
 
       // 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,22 +640,26 @@ 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<Tx>,
     blockNumber: BlockNumber,
     blockTimestamp: bigint,
-    blockBuilderOptions: PublicProcessorLimits,
+    blockBuilderOptions: PublicProcessorLimits & { minValidTxs?: number },
   ) {
     try {
       const workTimer = new Timer();
       const result = await checkpointBuilder.buildBlock(pendingTxs, blockNumber, blockTimestamp, blockBuilderOptions);
       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;
     }

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<string> = new Set();
@@ -74,7 +74,7 @@ export class MockCheckpointBuilder implements ICheckpointBlockBuilder {
     pendingTxs: Iterable<Tx> | AsyncIterable<Tx>,
     blockNumber: BlockNumber,
     timestamp: bigint,
-    opts: PublicProcessorLimits,
+    opts: PublicProcessorLimits & { minValidTxs?: number },
   ): Promise<BuildBlockInCheckpointResult> {
     this.buildBlockCalls.push({ blockNumber, timestamp, opts });
 

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<Tx> | AsyncIterable<Tx>,
     blockNumber: BlockNumber,
     timestamp: bigint,
-    opts: PublicProcessorLimits,
+    opts: PublicProcessorLimits & { minValidTxs?: number },
   ): Promise<BuildBlockInCheckpointResult>;
 }
 

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>();
+      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', () => {

yarn-project/validator-client/src/checkpoint_builder.ts

@@ -25,15 +25,16 @@ import {
   FullNodeBlockBuilderConfigKeys,
   type ICheckpointBlockBuilder,
   type ICheckpointsBuilder,
+  InsufficientValidTxsError,
   type MerkleTreeWriteOperations,
-  NoValidTxsError,
   type PublicProcessorLimits,
   type WorldStateSynchronizer,
 } from '@aztec/stdlib/interfaces/server';
 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<Tx> | AsyncIterable<Tx>,
     blockNumber: BlockNumber,
     timestamp: bigint,
-    opts: PublicProcessorLimits & { expectedEndState?: StateReference } = {},
+    opts: PublicProcessorLimits & { expectedEndState?: StateReference; minValidTxs?: number } = {},
   ): Promise<BuildBlockInCheckpointResult> {
     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. */