Skip to content

Commit

Permalink
fix: abort block building if block height changes
Browse files Browse the repository at this point in the history
  • Loading branch information
alexghr committed Oct 16, 2023
1 parent bbb2cf9 commit 14e5b7b
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 4 deletions.
32 changes: 30 additions & 2 deletions yarn-project/sequencer-client/src/sequencer/sequencer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -65,7 +65,7 @@ describe('sequencer', () => {
});

l2BlockSource = mock<L2BlockSource>({
getBlockNumber: () => Promise.resolve(lastBlockNumber),
getBlockNumber: mockFn().mockResolvedValue(lastBlockNumber),
});

l1ToL2MessageSource = mock<L1ToL2MessageSource>({
Expand Down Expand Up @@ -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 {
Expand Down
19 changes: 17 additions & 2 deletions yarn-project/sequencer-client/src/sequencer/sequencer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -155,14 +166,16 @@ 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) {
this.log('No txs processed correctly to build block. Exiting');
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();
Expand All @@ -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),
Expand All @@ -189,7 +204,7 @@ export class Sequencer {
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();
}
}
Expand Down

0 comments on commit 14e5b7b

Please sign in to comment.