From aaf372b14a9497c76b1e570cb3d1569b9bc618f0 Mon Sep 17 00:00:00 2001 From: Alex Gherghisan Date: Mon, 16 Oct 2023 10:56:53 +0000 Subject: [PATCH 1/2] refactor: remove unused parameter --- .../src/client/sequencer-client.ts | 1 - .../src/sequencer/sequencer.test.ts | 14 +------------- .../sequencer-client/src/sequencer/sequencer.ts | 3 +-- 3 files changed, 2 insertions(+), 16 deletions(-) diff --git a/yarn-project/sequencer-client/src/client/sequencer-client.ts b/yarn-project/sequencer-client/src/client/sequencer-client.ts index 577ae727639..3db4afbb563 100644 --- a/yarn-project/sequencer-client/src/client/sequencer-client.ts +++ b/yarn-project/sequencer-client/src/client/sequencer-client.ts @@ -55,7 +55,6 @@ export class SequencerClient { blockBuilder, l2BlockSource, l1ToL2MessageSource, - contractDataSource, publicProcessorFactory, config, ); diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts index f826b1112ef..28c1acb992d 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts @@ -6,16 +6,7 @@ import { makeEmptyProof, } from '@aztec/circuits.js'; import { P2P, P2PClientState } from '@aztec/p2p'; -import { - ContractDataSource, - L1ToL2MessageSource, - L2Block, - L2BlockSource, - MerkleTreeId, - Tx, - TxHash, - mockTx, -} from '@aztec/types'; +import { L1ToL2MessageSource, L2Block, L2BlockSource, MerkleTreeId, Tx, TxHash, mockTx } from '@aztec/types'; import { MerkleTreeOperations, WorldStateRunningState, WorldStateSynchronizer } from '@aztec/world-state'; import { MockProxy, mock } from 'jest-mock-extended'; @@ -82,8 +73,6 @@ describe('sequencer', () => { getBlockNumber: () => Promise.resolve(lastBlockNumber), }); - const contractDataSource = mock({}); - sequencer = new TestSubject( publisher, globalVariableBuilder, @@ -92,7 +81,6 @@ describe('sequencer', () => { blockBuilder, l2BlockSource, l1ToL2MessageSource, - contractDataSource, publicProcessorFactory, ); }); diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.ts index c0368c02434..930ffb53287 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.ts @@ -4,7 +4,7 @@ import { createDebugLogger } from '@aztec/foundation/log'; import { RunningPromise } from '@aztec/foundation/running-promise'; import { Timer, elapsed } from '@aztec/foundation/timer'; import { P2P } from '@aztec/p2p'; -import { ContractDataSource, L1ToL2MessageSource, L2Block, L2BlockSource, MerkleTreeId, Tx } from '@aztec/types'; +import { L1ToL2MessageSource, L2Block, L2BlockSource, MerkleTreeId, Tx } from '@aztec/types'; import { L2BlockBuiltStats } from '@aztec/types/stats'; import { WorldStateStatus, WorldStateSynchronizer } from '@aztec/world-state'; @@ -43,7 +43,6 @@ export class Sequencer { private blockBuilder: BlockBuilder, private l2BlockSource: L2BlockSource, private l1ToL2MessageSource: L1ToL2MessageSource, - private contractDataSource: ContractDataSource, private publicProcessorFactory: PublicProcessorFactory, config: SequencerConfig = {}, private log = createDebugLogger('aztec:sequencer'), From 21c3ed328ad7ee11481ac2c5185bfb80d63b5b16 Mon Sep 17 00:00:00 2001 From: Alex Gherghisan Date: Mon, 16 Oct 2023 18:31:02 +0000 Subject: [PATCH 2/2] fix: abort block building if block height changes --- .../src/sequencer/sequencer.test.ts | 32 +++++++++++++++++-- .../src/sequencer/sequencer.ts | 23 +++++++++++-- 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts index 28c1acb992d..e198f2645ca 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts @@ -9,7 +9,7 @@ import { P2P, P2PClientState } from '@aztec/p2p'; import { L1ToL2MessageSource, L2Block, L2BlockSource, MerkleTreeId, Tx, TxHash, mockTx } from '@aztec/types'; import { MerkleTreeOperations, WorldStateRunningState, WorldStateSynchronizer } from '@aztec/world-state'; -import { MockProxy, mock } from 'jest-mock-extended'; +import { MockProxy, mock, mockFn } from 'jest-mock-extended'; import times from 'lodash.times'; import { BlockBuilder } from '../block_builder/index.js'; @@ -65,7 +65,7 @@ describe('sequencer', () => { }); l2BlockSource = mock({ - getBlockNumber: () => Promise.resolve(lastBlockNumber), + getBlockNumber: mockFn().mockResolvedValue(lastBlockNumber), }); l1ToL2MessageSource = mock({ @@ -181,6 +181,34 @@ describe('sequencer', () => { expect(publisher.processL2Block).toHaveBeenCalledWith(block); expect(p2p.deleteTxs).toHaveBeenCalledWith([await invalidChainTx.getTxHash()]); }); + + fit('aborts building a block if the chain moves underneath it', async () => { + const tx = mockTx(); + tx.data.constants.txContext.chainId = chainId; + const block = L2Block.random(lastBlockNumber + 1); + const proof = makeEmptyProof(); + + p2p.getTxs.mockResolvedValueOnce([tx]); + blockBuilder.buildL2Block.mockResolvedValueOnce([block, proof]); + publisher.processL2Block.mockResolvedValueOnce(true); + globalVariableBuilder.buildGlobalVariables.mockResolvedValueOnce( + new GlobalVariables(chainId, version, new Fr(lastBlockNumber + 1), Fr.ZERO), + ); + + await sequencer.initialSync(); + + l2BlockSource.getBlockNumber + // let it work for a bit + .mockResolvedValueOnce(lastBlockNumber) + .mockResolvedValueOnce(lastBlockNumber) + .mockResolvedValueOnce(lastBlockNumber) + // then tell it to abort + .mockResolvedValue(lastBlockNumber + 1); + + await sequencer.work(); + + expect(publisher.processL2Block).not.toHaveBeenCalled(); + }); }); class TestSubject extends Sequencer { diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.ts index 930ffb53287..56d45ce6ead 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.ts @@ -130,6 +130,17 @@ export class Sequencer { this.log.info(`Retrieved ${pendingTxs.length} txs from P2P pool`); const blockNumber = (await this.l2BlockSource.getBlockNumber()) + 1; + + /** + * We'll call this function before running expensive operations to avoid wasted work. + */ + const assertBlockHeight = async () => { + const currentBlockNumber = await this.l2BlockSource.getBlockNumber(); + if (currentBlockNumber + 1 !== blockNumber) { + throw new Error('New block was emitted while building block'); + } + }; + const newGlobalVariables = await this.globalsBuilder.buildGlobalVariables(new Fr(blockNumber)); // Filter out invalid txs @@ -155,7 +166,7 @@ export class Sequencer { // Only accept processed transactions that are not double-spends, // public functions emitting nullifiers would pass earlier check but fail here. // Note that we're checking all nullifiers generated in the private execution twice, - // we could store the ones already checked and skip them here as an optimisation. + // we could store the ones already checked and skip them here as an optimization. const processedValidTxs = await this.takeValidTxs(processedTxs, newGlobalVariables); if (processedValidTxs.length === 0) { @@ -163,6 +174,8 @@ export class Sequencer { return; } + await assertBlockHeight(); + // Get l1 to l2 messages from the contract this.log('Requesting L1 to L2 messages from contract'); const l1ToL2Messages = await this.getPendingL1ToL2Messages(); @@ -171,6 +184,8 @@ export class Sequencer { // Build the new block by running the rollup circuits this.log(`Assembling block with txs ${processedValidTxs.map(tx => tx.hash).join(', ')}`); + await assertBlockHeight(); + const emptyTx = await processor.makeEmptyProcessedTx(); const [rollupCircuitsDuration, block] = await elapsed(() => this.buildBlock(processedValidTxs, l1ToL2Messages, emptyTx, newGlobalVariables), @@ -184,12 +199,16 @@ export class Sequencer { ...block.getStats(), } satisfies L2BlockBuiltStats); + await assertBlockHeight(); + await this.publishExtendedContractData(validTxs, block); + await assertBlockHeight(); + await this.publishL2Block(block); this.log.info(`Submitted rollup block ${block.number} with ${processedValidTxs.length} transactions`); } catch (err) { - this.log.error(`Rolling back world state DB due to error assembling block`, err); + this.log.error(`Rolling back world state DB due to error assembling block`, (err as any).stack); await this.worldState.getLatest().rollback(); } }