fix: Do not start block root rollup proof before block is built

yarn-project/prover-client/src/orchestrator/block-proving-state.ts

@@ -41,6 +41,7 @@ export class BlockProvingState {
   private rootParityInputs: Array<RootParityInput<typeof RECURSIVE_PROOF_LENGTH> | undefined> = [];
   private finalRootParityInputs: RootParityInput<typeof NESTED_RECURSIVE_PROOF_LENGTH> | undefined;
   public blockRootRollupPublicInputs: BlockRootOrBlockMergePublicInputs | undefined;
+  public blockRootRollupStarted: boolean = false;
   public finalProof: Proof | undefined;
   public block: L2Block | undefined;
   private txs: TxProvingState[] = [];
@@ -180,6 +181,7 @@ export class BlockProvingState {
   // Returns true if we have sufficient inputs to execute the block root rollup
   public isReadyForBlockRootRollup() {
     return !(
+      this.block === undefined ||
       this.mergeRollupInputs[0] === undefined ||
       this.finalRootParityInput === undefined ||
       this.mergeRollupInputs[0].inputs.findIndex(p => !p) !== -1

yarn-project/prover-client/src/orchestrator/orchestrator.ts

@@ -344,6 +344,8 @@ export class ProvingOrchestrator implements EpochProver {
     logger.verbose(`Block ${provingState.globalVariables.blockNumber} completed. Assembling header.`);
     await this.buildBlock(provingState);
 
+    // If the proofs were faster than the block building, then we need to try the block root rollup again here
+    this.checkAndEnqueueBlockRootRollup(provingState);
     return provingState.block!;
   }
 
@@ -871,11 +873,17 @@ export class ProvingOrchestrator implements EpochProver {
   }
 
   // Executes the block root rollup circuit
-  private enqueueBlockRootRollup(provingState: BlockProvingState | undefined) {
-    if (!provingState?.verifyState()) {
+  private enqueueBlockRootRollup(provingState: BlockProvingState) {
+    if (!provingState.block) {
+      throw new Error(`Invalid proving state for block root rollup, block not available`);
+    }
+
+    if (!provingState.verifyState()) {
       logger.debug('Not running block root rollup, state no longer valid');
       return;
     }
+
+    provingState.blockRootRollupStarted = true;
     const mergeInputData = provingState.getMergeInputs(0);
     const rootParityInput = provingState.finalRootParityInput!;
 
@@ -1063,11 +1071,15 @@ export class ProvingOrchestrator implements EpochProver {
     );
   }
 
-  private checkAndEnqueueBlockRootRollup(provingState: BlockProvingState | undefined) {
+  private checkAndEnqueueBlockRootRollup(provingState: BlockProvingState) {
     if (!provingState?.isReadyForBlockRootRollup()) {
       logger.debug('Not ready for root rollup');
       return;
     }
+    if (provingState.blockRootRollupStarted) {
+      logger.debug('Block root rollup already started');
+      return;
+    }
     this.enqueueBlockRootRollup(provingState);
   }
 

yarn-project/prover-client/src/orchestrator/orchestrator_workflow.test.ts

@@ -1,71 +1,103 @@
 import { type ServerCircuitProver } from '@aztec/circuit-types';
 import {
   Fr,
+  type GlobalVariables,
   NESTED_RECURSIVE_PROOF_LENGTH,
   NUM_BASE_PARITY_PER_ROOT_PARITY,
   RECURSIVE_PROOF_LENGTH,
   type RootParityInput,
 } from '@aztec/circuits.js';
-import { makeGlobalVariables, makeRootParityInput } from '@aztec/circuits.js/testing';
+import { makeRootParityInput } from '@aztec/circuits.js/testing';
+import { createDebugLogger } from '@aztec/foundation/log';
 import { promiseWithResolvers } from '@aztec/foundation/promise';
 import { sleep } from '@aztec/foundation/sleep';
-import { openTmpStore } from '@aztec/kv-store/utils';
-import { NoopTelemetryClient } from '@aztec/telemetry-client/noop';
-import { type MerkleTreeOperations, MerkleTrees } from '@aztec/world-state';
+import { type MerkleTreeOperations } from '@aztec/world-state';
 
 import { type MockProxy, mock } from 'jest-mock-extended';
 
-import { ProvingOrchestrator } from './orchestrator.js';
+import { makeBloatedProcessedTx } from '../mocks/fixtures.js';
+import { TestContext } from '../mocks/test_context.js';
+import { type ProvingOrchestrator } from './orchestrator.js';
+
+const logger = createDebugLogger('aztec:orchestrator-workflow');
 
 describe('prover/orchestrator', () => {
   describe('workflow', () => {
     let orchestrator: ProvingOrchestrator;
-    let mockProver: MockProxy<ServerCircuitProver>;
     let actualDb: MerkleTreeOperations;
-    beforeEach(async () => {
-      const telemetryClient = new NoopTelemetryClient();
-      actualDb = await MerkleTrees.new(openTmpStore(), telemetryClient).then(t => t.asLatest());
-      mockProver = mock<ServerCircuitProver>();
-      orchestrator = new ProvingOrchestrator(actualDb, mockProver, telemetryClient);
-    });
+    let globalVariables: GlobalVariables;
+    let context: TestContext;
 
-    it('calls root parity circuit only when ready', async () => {
-      // create a custom L2 to L1 message
-      const message = Fr.random();
+    describe('with mock prover', () => {
+      let mockProver: MockProxy<ServerCircuitProver>;
 
-      // and delay its proof
-      const pendingBaseParityResult = promiseWithResolvers<RootParityInput<typeof RECURSIVE_PROOF_LENGTH>>();
-      const expectedBaseParityResult = makeRootParityInput(RECURSIVE_PROOF_LENGTH, 0xff);
+      beforeEach(async () => {
+        mockProver = mock<ServerCircuitProver>();
+        context = await TestContext.new(logger, 'legacy', 4, () => Promise.resolve(mockProver));
+        ({ actualDb, orchestrator, globalVariables } = context);
+      });
 
-      mockProver.getRootParityProof.mockResolvedValue(makeRootParityInput(NESTED_RECURSIVE_PROOF_LENGTH));
+      it('calls root parity circuit only when ready', async () => {
+        // create a custom L2 to L1 message
+        const message = Fr.random();
 
-      mockProver.getBaseParityProof.mockImplementation(inputs => {
-        if (inputs.msgs[0].equals(message)) {
-          return pendingBaseParityResult.promise;
-        } else {
-          return Promise.resolve(makeRootParityInput(RECURSIVE_PROOF_LENGTH));
-        }
-      });
+        // and delay its proof
+        const pendingBaseParityResult = promiseWithResolvers<RootParityInput<typeof RECURSIVE_PROOF_LENGTH>>();
+        const expectedBaseParityResult = makeRootParityInput(RECURSIVE_PROOF_LENGTH, 0xff);
 
-      orchestrator.startNewEpoch(1, 1);
-      await orchestrator.startNewBlock(2, makeGlobalVariables(1), [message]);
+        mockProver.getRootParityProof.mockResolvedValue(makeRootParityInput(NESTED_RECURSIVE_PROOF_LENGTH));
 
-      await sleep(10);
-      expect(mockProver.getBaseParityProof).toHaveBeenCalledTimes(NUM_BASE_PARITY_PER_ROOT_PARITY);
-      expect(mockProver.getRootParityProof).not.toHaveBeenCalled();
+        mockProver.getBaseParityProof.mockImplementation(inputs => {
+          if (inputs.msgs[0].equals(message)) {
+            return pendingBaseParityResult.promise;
+          } else {
+            return Promise.resolve(makeRootParityInput(RECURSIVE_PROOF_LENGTH));
+          }
+        });
 
-      await sleep(10);
-      // even now the root parity should not have been called
-      expect(mockProver.getRootParityProof).not.toHaveBeenCalled();
+        orchestrator.startNewEpoch(1, 1);
+        await orchestrator.startNewBlock(2, globalVariables, [message]);
 
-      // only after the base parity proof is resolved, the root parity should be called
-      pendingBaseParityResult.resolve(expectedBaseParityResult);
+        await sleep(10);
+        expect(mockProver.getBaseParityProof).toHaveBeenCalledTimes(NUM_BASE_PARITY_PER_ROOT_PARITY);
+        expect(mockProver.getRootParityProof).not.toHaveBeenCalled();
 
-      // give the orchestrator a chance to calls its callbacks
-      await sleep(10);
-      expect(mockProver.getRootParityProof).toHaveBeenCalledTimes(1);
+        await sleep(10);
+        // even now the root parity should not have been called
+        expect(mockProver.getRootParityProof).not.toHaveBeenCalled();
 
-      orchestrator.cancel();
+        // only after the base parity proof is resolved, the root parity should be called
+        pendingBaseParityResult.resolve(expectedBaseParityResult);
+
+        // give the orchestrator a chance to calls its callbacks
+        await sleep(10);
+        expect(mockProver.getRootParityProof).toHaveBeenCalledTimes(1);
+
+        orchestrator.cancel();
+      });
+    });
+
+    describe('with simulated prover', () => {
+      beforeEach(async () => {
+        context = await TestContext.new(logger);
+        ({ actualDb, orchestrator, globalVariables } = context);
+      });
+
+      it('waits for block to be completed before enqueueing block root proof', async () => {
+        orchestrator.startNewEpoch(1, 1);
+        await orchestrator.startNewBlock(2, globalVariables, []);
+        await orchestrator.addNewTx(makeBloatedProcessedTx(actualDb, 1));
+        await orchestrator.addNewTx(makeBloatedProcessedTx(actualDb, 2));
+
+        // wait for the block root proof to try to be enqueued
+        await sleep(1000);
+
+        // now finish the block
+        await orchestrator.setBlockCompleted();
+
+        const result = await orchestrator.finaliseEpoch();
+        expect(result.proof).toBeDefined();
+      });
     });
   });
 });