Skip to content

Commit

Permalink
fix: handle sequencer building block mid-synch (#11735)
Browse files Browse the repository at this point in the history
This PR fixes a race condition in the sequencer that would happen in e2e
tests (or sandbox, basically anywhere we warp time because of auto mine)
where the sequencer would not handle a block being synchronized in the
middle of block production.

This PR merges getting the chain tip with checking if its components are
synchronized. This way it at least will fail at `canProposeAtTime` since
the provided archive might be old.

Thanks @spalladino and @LHerskind for help debugging this one :)
  • Loading branch information
alexghr authored Feb 5, 2025
1 parent 6c15604 commit 435a0af
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 12 deletions.
44 changes: 44 additions & 0 deletions yarn-project/sequencer-client/src/sequencer/sequencer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,45 @@ describe('sequencer', () => {
expectPublisherProposeL2Block(postFlushTxHashes);
});

it('settles on the chain tip before it starts building a block', async () => {
// this test simulates a synch happening right after the sequencer starts building a bloxk
// simulate every component being synched
const firstBlock = await L2Block.random(1);
const currentTip = firstBlock;
const syncedToL2Block = { number: currentTip.number, hash: (await currentTip.hash()).toString() };
worldState.status.mockImplementation(() =>
Promise.resolve({ state: WorldStateRunningState.IDLE, syncedToL2Block }),
);
p2p.getStatus.mockImplementation(() => Promise.resolve({ state: P2PClientState.IDLE, syncedToL2Block }));
l2BlockSource.getL2Tips.mockImplementation(() =>
Promise.resolve({
latest: syncedToL2Block,
proven: { number: 0, hash: undefined },
finalized: { number: 0, hash: undefined },
}),
);
l1ToL2MessageSource.getBlockNumber.mockImplementation(() => Promise.resolve(currentTip.number));

// simulate a synch happening right after
l2BlockSource.getBlockNumber.mockResolvedValueOnce(currentTip.number);
l2BlockSource.getBlockNumber.mockResolvedValueOnce(currentTip.number + 1);
// now the new tip is actually block 2
l2BlockSource.getBlock.mockImplementation(n =>
n === -1
? L2Block.random(currentTip.number + 1)
: n === currentTip.number
? Promise.resolve(currentTip)
: Promise.resolve(undefined),
);

publisher.canProposeAtNextEthBlock.mockResolvedValueOnce(undefined);
await sequencer.doRealWork();
expect(publisher.enqueueProposeL2Block).not.toHaveBeenCalled();
// even though the chain tip moved, the sequencer should still have tried to build a block against the old archive
// this should get caught by the rollup
expect(publisher.canProposeAtNextEthBlock).toHaveBeenCalledWith(currentTip.archive.root.toBuffer());
});

it('aborts building a block if the chain moves underneath it', async () => {
const tx = await makeTx();
mockPendingTxs([tx]);
Expand Down Expand Up @@ -560,6 +599,11 @@ describe('sequencer', () => {

l2BlockSource.getBlock.mockResolvedValue(L2Block.random(blockNumber - 1));
l2BlockSource.getBlockNumber.mockResolvedValue(blockNumber - 1);
l2BlockSource.getL2Tips.mockResolvedValue({
latest: { number: blockNumber - 1, hash },
proven: { number: 0, hash: undefined },
finalized: { number: 0, hash: undefined },
});

l1ToL2MessageSource.getBlockNumber.mockResolvedValue(blockNumber - 1);

Expand Down
40 changes: 28 additions & 12 deletions yarn-project/sequencer-client/src/sequencer/sequencer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
GENESIS_ARCHIVE_ROOT,
Gas,
type GlobalVariables,
INITIAL_L2_BLOCK_NUM,
StateReference,
} from '@aztec/circuits.js';
import { AztecAddress } from '@aztec/foundation/aztec-address';
Expand Down Expand Up @@ -232,20 +233,18 @@ export class Sequencer {
protected async doRealWork() {
this.setState(SequencerState.SYNCHRONIZING, 0n);
// Update state when the previous block has been synced
const prevBlockSynced = await this.isBlockSynced();
const chainTip = await this.getChainTip();
// Do not go forward with new block if the previous one has not been mined and processed
if (!prevBlockSynced) {
if (!chainTip) {
return;
}

this.setState(SequencerState.PROPOSER_CHECK, 0n);

const chainTip = await this.l2BlockSource.getBlock(-1);

const newBlockNumber = (chainTip?.header.globalVariables.blockNumber.toNumber() ?? 0) + 1;
const newBlockNumber = chainTip.blockNumber + 1;

// If we cannot find a tip archive, assume genesis.
const chainTipArchive = chainTip?.archive.root ?? new Fr(GENESIS_ARCHIVE_ROOT);
const chainTipArchive = chainTip.archive;

const slot = await this.slotForProposal(chainTipArchive.toBuffer(), BigInt(newBlockNumber));
if (!slot) {
Expand Down Expand Up @@ -763,14 +762,16 @@ export class Sequencer {
* We don't check against the previous block submitted since it may have been reorg'd out.
* @returns Boolean indicating if our dependencies are synced to the latest block.
*/
protected async isBlockSynced() {
protected async getChainTip(): Promise<{ blockNumber: number; archive: Fr } | undefined> {
const syncedBlocks = await Promise.all([
this.worldState.status().then((s: WorldStateSynchronizerStatus) => s.syncedToL2Block),
this.l2BlockSource.getL2Tips().then(t => t.latest),
this.p2pClient.getStatus().then(s => s.syncedToL2Block.number),
this.p2pClient.getStatus().then(p2p => p2p.syncedToL2Block),
this.l1ToL2MessageSource.getBlockNumber(),
] as const);

const [worldState, l2BlockSource, p2p, l1ToL2MessageSource] = syncedBlocks;

const result =
// check that world state has caught up with archiver
// note that the archiver reports undefined hash for the genesis block
Expand All @@ -780,18 +781,33 @@ export class Sequencer {
// this should change to hashes once p2p client handles reorgs
// and once we stop pretending that the l1tol2message source is not
// just the archiver under a different name
p2p >= l2BlockSource.number &&
l1ToL2MessageSource >= l2BlockSource.number;
(!l2BlockSource.hash || p2p.hash === l2BlockSource.hash) &&
l1ToL2MessageSource === l2BlockSource.number;

this.log.debug(`Sequencer sync check ${result ? 'succeeded' : 'failed'}`, {
worldStateNumber: worldState.number,
worldStateHash: worldState.hash,
l2BlockSourceNumber: l2BlockSource.number,
l2BlockSourceHash: l2BlockSource.hash,
p2pNumber: p2p,
p2pNumber: p2p.number,
p2pHash: p2p.hash,
l1ToL2MessageSourceNumber: l1ToL2MessageSource,
});
return result;

if (!result) {
return undefined;
}
if (worldState.number >= INITIAL_L2_BLOCK_NUM) {
const block = await this.l2BlockSource.getBlock(worldState.number);
if (!block) {
// this shouldn't really happen because a moment ago we checked that all components were in synch
return undefined;
}

return { blockNumber: block.number, archive: block.archive.root };
} else {
return { blockNumber: INITIAL_L2_BLOCK_NUM - 1, archive: new Fr(GENESIS_ARCHIVE_ROOT) };
}
}

private getSlotStartTimestamp(slotNumber: number | bigint): number {
Expand Down

0 comments on commit 435a0af

Please sign in to comment.