diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts index 9caa6a86f14..ef1685bc3bd 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts @@ -208,7 +208,7 @@ describe('sequencer', () => { globalVariableBuilder.buildGlobalVariables.mockResolvedValueOnce(mockedGlobalVariables); - await sequencer.work(); + await sequencer.doRealWork(); expect(blockBuilder.startNewBlock).toHaveBeenCalledWith( 2, @@ -222,33 +222,32 @@ describe('sequencer', () => { it.each([ { - previousState: SequencerState.PROPOSER_CHECK, delayedState: SequencerState.WAITING_FOR_TXS, }, // It would be nice to add the other states, but we would need to inject delays within the `work` loop - ])( - 'does not build a block if it does not have enough time left in the slot', - async ({ previousState, delayedState }) => { - // trick the sequencer into thinking that we are just too far into the slot - sequencer.setL1GenesisTime(Math.floor(Date.now() / 1000) - (sequencer.getTimeTable()[delayedState] + 1)); + ])('does not build a block if it does not have enough time left in the slot', async ({ delayedState }) => { + // trick the sequencer into thinking that we are just too far into the slot + sequencer.setL1GenesisTime(Math.floor(Date.now() / 1000) - (sequencer.getTimeTable()[delayedState] + 1)); - const tx = mockTxForRollup(); - tx.data.constants.txContext.chainId = chainId; + const tx = mockTxForRollup(); + tx.data.constants.txContext.chainId = chainId; - p2p.getTxs.mockReturnValueOnce([tx]); - blockBuilder.setBlockCompleted.mockResolvedValue(block); - publisher.proposeL2Block.mockResolvedValueOnce(true); + p2p.getTxs.mockReturnValueOnce([tx]); + blockBuilder.setBlockCompleted.mockResolvedValue(block); + publisher.proposeL2Block.mockResolvedValueOnce(true); - globalVariableBuilder.buildGlobalVariables.mockResolvedValueOnce(mockedGlobalVariables); + globalVariableBuilder.buildGlobalVariables.mockResolvedValueOnce(mockedGlobalVariables); - await expect(sequencer.work()).rejects.toThrow( - `Cannot set sequencer from ${previousState} to ${delayedState} as there is not enough time left in the slot.`, - ); + await expect(sequencer.doRealWork()).rejects.toThrow( + expect.objectContaining({ + name: 'SequencerTooSlowError', + message: expect.stringContaining(`Too far into slot to transition to ${delayedState}.`), + }), + ); - expect(blockBuilder.startNewBlock).not.toHaveBeenCalled(); - expect(publisher.proposeL2Block).not.toHaveBeenCalled(); - }, - ); + expect(blockBuilder.startNewBlock).not.toHaveBeenCalled(); + expect(publisher.proposeL2Block).not.toHaveBeenCalled(); + }); it('builds a block when it is their turn', async () => { const tx = mockTxForRollup(); @@ -265,7 +264,7 @@ describe('sequencer', () => { publisher.canProposeAtNextEthBlock.mockRejectedValue(new Error()); publisher.validateBlockForSubmission.mockRejectedValue(new Error()); - await sequencer.work(); + await sequencer.doRealWork(); expect(blockBuilder.startNewBlock).not.toHaveBeenCalled(); // Now we can propose, but lets assume that the content is still "bad" (missing sigs etc) @@ -274,14 +273,14 @@ describe('sequencer', () => { block.header.globalVariables.blockNumber.toBigInt(), ]); - await sequencer.work(); + await sequencer.doRealWork(); expect(blockBuilder.startNewBlock).not.toHaveBeenCalled(); // Now it is! publisher.validateBlockForSubmission.mockClear(); publisher.validateBlockForSubmission.mockResolvedValue(); - await sequencer.work(); + await sequencer.doRealWork(); expect(blockBuilder.startNewBlock).toHaveBeenCalledWith( 2, mockedGlobalVariables, @@ -314,7 +313,7 @@ describe('sequencer', () => { ); }); - await sequencer.work(); + await sequencer.doRealWork(); expect(blockBuilder.startNewBlock).toHaveBeenCalledWith( 2, @@ -343,7 +342,7 @@ describe('sequencer', () => { // We make the chain id on the invalid tx not equal to the configured chain id invalidChainTx.data.constants.txContext.chainId = new Fr(1n + chainId.value); - await sequencer.work(); + await sequencer.doRealWork(); expect(blockBuilder.startNewBlock).toHaveBeenCalledWith( 2, @@ -374,7 +373,7 @@ describe('sequencer', () => { (txs[invalidTransactionIndex].unencryptedLogs.functionLogs[0].logs[0] as Writeable).data = randomBytes(1024 * 1022); - await sequencer.work(); + await sequencer.doRealWork(); expect(blockBuilder.startNewBlock).toHaveBeenCalledWith( 2, @@ -402,20 +401,20 @@ describe('sequencer', () => { // block is not built with 0 txs p2p.getTxs.mockReturnValueOnce([]); //p2p.getTxs.mockReturnValueOnce(txs.slice(0, 4)); - await sequencer.work(); + await sequencer.doRealWork(); expect(blockBuilder.startNewBlock).toHaveBeenCalledTimes(0); // block is not built with 3 txs p2p.getTxs.mockReturnValueOnce(txs.slice(0, 3)); - await sequencer.work(); + await sequencer.doRealWork(); expect(blockBuilder.startNewBlock).toHaveBeenCalledTimes(0); // block is built with 4 txs p2p.getTxs.mockReturnValueOnce(txs.slice(0, 4)); const txHashes = txs.slice(0, 4).map(tx => tx.getTxHash()); - await sequencer.work(); + await sequencer.doRealWork(); expect(blockBuilder.startNewBlock).toHaveBeenCalledWith( 4, mockedGlobalVariables, @@ -442,12 +441,12 @@ describe('sequencer', () => { // block is not built with 0 txs p2p.getTxs.mockReturnValueOnce([]); - await sequencer.work(); + await sequencer.doRealWork(); expect(blockBuilder.startNewBlock).toHaveBeenCalledTimes(0); // block is not built with 3 txs p2p.getTxs.mockReturnValueOnce(txs.slice(0, 3)); - await sequencer.work(); + await sequencer.doRealWork(); expect(blockBuilder.startNewBlock).toHaveBeenCalledTimes(0); // flush the sequencer and it should build a block @@ -455,7 +454,7 @@ describe('sequencer', () => { // block is built with 0 txs p2p.getTxs.mockReturnValueOnce([]); - await sequencer.work(); + await sequencer.doRealWork(); expect(blockBuilder.startNewBlock).toHaveBeenCalledTimes(1); expect(blockBuilder.startNewBlock).toHaveBeenCalledWith( 2, @@ -483,12 +482,12 @@ describe('sequencer', () => { // block is not built with 0 txs p2p.getTxs.mockReturnValueOnce([]); - await sequencer.work(); + await sequencer.doRealWork(); expect(blockBuilder.startNewBlock).toHaveBeenCalledTimes(0); // block is not built with 3 txs p2p.getTxs.mockReturnValueOnce(txs.slice(0, 3)); - await sequencer.work(); + await sequencer.doRealWork(); expect(blockBuilder.startNewBlock).toHaveBeenCalledTimes(0); // flush the sequencer and it should build a block @@ -498,7 +497,7 @@ describe('sequencer', () => { const postFlushTxs = txs.slice(0, 3); p2p.getTxs.mockReturnValueOnce(postFlushTxs); const postFlushTxHashes = postFlushTxs.map(tx => tx.getTxHash()); - await sequencer.work(); + await sequencer.doRealWork(); expect(blockBuilder.startNewBlock).toHaveBeenCalledTimes(1); expect(blockBuilder.startNewBlock).toHaveBeenCalledWith( 3, @@ -537,7 +536,7 @@ describe('sequencer', () => { .mockResolvedValueOnce() .mockRejectedValueOnce(new Error()); - await sequencer.work(); + await sequencer.doRealWork(); expect(publisher.proposeL2Block).not.toHaveBeenCalled(); }); @@ -613,7 +612,7 @@ describe('sequencer', () => { // The previous epoch can be claimed publisher.getClaimableEpoch.mockImplementation(() => Promise.resolve(currentEpoch - 1n)); - await sequencer.work(); + await sequencer.doRealWork(); expect(publisher.proposeL2Block).toHaveBeenCalledWith(block, getSignatures(), [txHash], proofQuote); }); @@ -635,7 +634,7 @@ describe('sequencer', () => { publisher.getClaimableEpoch.mockImplementation(() => Promise.resolve(undefined)); - await sequencer.work(); + await sequencer.doRealWork(); expect(publisher.proposeL2Block).toHaveBeenCalledWith(block, getSignatures(), [txHash], undefined); }); @@ -659,7 +658,7 @@ describe('sequencer', () => { // The previous epoch can be claimed publisher.getClaimableEpoch.mockImplementation(() => Promise.resolve(currentEpoch - 1n)); - await sequencer.work(); + await sequencer.doRealWork(); expect(publisher.proposeL2Block).toHaveBeenCalledWith(block, getSignatures(), [txHash], undefined); }); @@ -681,7 +680,7 @@ describe('sequencer', () => { publisher.getClaimableEpoch.mockResolvedValue(undefined); - await sequencer.work(); + await sequencer.doRealWork(); expect(publisher.proposeL2Block).toHaveBeenCalledWith(block, getSignatures(), [txHash], undefined); }); @@ -706,7 +705,7 @@ describe('sequencer', () => { // The previous epoch can be claimed publisher.getClaimableEpoch.mockImplementation(() => Promise.resolve(currentEpoch - 1n)); - await sequencer.work(); + await sequencer.doRealWork(); expect(publisher.proposeL2Block).toHaveBeenCalledWith(block, getSignatures(), [txHash], undefined); }); @@ -761,7 +760,7 @@ describe('sequencer', () => { // The previous epoch can be claimed publisher.getClaimableEpoch.mockImplementation(() => Promise.resolve(currentEpoch - 1n)); - await sequencer.work(); + await sequencer.doRealWork(); expect(publisher.proposeL2Block).toHaveBeenCalledWith(block, getSignatures(), [txHash], validProofQuote); }); @@ -820,7 +819,7 @@ describe('sequencer', () => { // The previous epoch can be claimed publisher.getClaimableEpoch.mockImplementation(() => Promise.resolve(currentEpoch - 1n)); - await sequencer.work(); + await sequencer.doRealWork(); expect(publisher.proposeL2Block).toHaveBeenCalledWith(block, getSignatures(), [txHash], validQuotes[0]); }); }); @@ -834,8 +833,9 @@ class TestSubject extends Sequencer { public setL1GenesisTime(l1GenesisTime: number) { this.l1GenesisTime = l1GenesisTime; } - public override work() { + + public override doRealWork() { this.setState(SequencerState.IDLE, true /** force */); - return super.work(); + return super.doRealWork(); } } diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.ts index 66208d9302a..2e30f8cab68 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.ts @@ -39,7 +39,7 @@ import { prettyLogViemErrorMsg } from '../publisher/utils.js'; import { type TxValidatorFactory } from '../tx_validator/tx_validator_factory.js'; import { type SequencerConfig } from './config.js'; import { SequencerMetrics } from './metrics.js'; -import { SequencerState, orderAttestations } from './utils.js'; +import { SequencerState, getSecondsIntoSlot, orderAttestations } from './utils.js'; export type ShouldProposeArgs = { pendingTxsCount?: number; @@ -47,6 +47,20 @@ export type ShouldProposeArgs = { processedTxsCount?: number; }; +export class SequencerTooSlowError extends Error { + constructor( + public readonly currentState: SequencerState, + public readonly proposedState: SequencerState, + public readonly maxAllowedTime: number, + public readonly currentTime: number, + ) { + super( + `Too far into slot to transition to ${proposedState}. max allowed: ${maxAllowedTime}s, time into slot: ${currentTime}s`, + ); + this.name = 'SequencerTooSlowError'; + } +} + /** * Sequencer client * - Wins a period of time to become the sequencer (depending on finalized protocol). @@ -216,7 +230,7 @@ export class Sequencer { * - Submit block * - If our block for some reason is not included, revert the state */ - private async doRealWork() { + protected async doRealWork() { this.setState(SequencerState.SYNCHRONIZING); // Update state when the previous block has been synced const prevBlockSynced = await this.isBlockSynced(); @@ -314,6 +328,13 @@ export class Sequencer { protected async work() { try { await this.doRealWork(); + } catch (err) { + if (err instanceof SequencerTooSlowError) { + this.log.warn(err.message); + } else { + // Re-throw other errors + throw err; + } } finally { this.setState(SequencerState.IDLE); } @@ -350,7 +371,7 @@ export class Sequencer { } } - doIHaveEnoughTimeLeft(proposedState: SequencerState): boolean { + doIHaveEnoughTimeLeft(proposedState: SequencerState, secondsIntoSlot: number): boolean { if (!this.enforceTimeTable) { return true; } @@ -359,7 +380,6 @@ export class Sequencer { return true; } - const secondsIntoSlot = (Date.now() / 1000 - this.l1GenesisTime) % AZTEC_SLOT_DURATION; const bufferSeconds = this.timeTable[proposedState] - secondsIntoSlot; this.metrics.recordStateTransitionBufferMs(bufferSeconds * 1000, proposedState); @@ -382,10 +402,9 @@ export class Sequencer { ); return; } - if (!this.doIHaveEnoughTimeLeft(proposedState)) { - throw new Error( - `Cannot set sequencer from ${this.state} to ${proposedState} as there is not enough time left in the slot.`, - ); + const secondsIntoSlot = getSecondsIntoSlot(this.l1GenesisTime); + if (!this.doIHaveEnoughTimeLeft(proposedState, secondsIntoSlot)) { + throw new SequencerTooSlowError(this.state, proposedState, this.timeTable[proposedState], secondsIntoSlot); } this.state = proposedState; } diff --git a/yarn-project/sequencer-client/src/sequencer/utils.ts b/yarn-project/sequencer-client/src/sequencer/utils.ts index 8c878571d74..eee02811b3a 100644 --- a/yarn-project/sequencer-client/src/sequencer/utils.ts +++ b/yarn-project/sequencer-client/src/sequencer/utils.ts @@ -1,4 +1,5 @@ import type { BlockAttestation, EthAddress } from '@aztec/circuit-types'; +import { AZTEC_SLOT_DURATION } from '@aztec/circuits.js'; import { Signature } from '@aztec/foundation/eth-signature'; export enum SequencerState { @@ -72,3 +73,7 @@ export function orderAttestations(attestations: BlockAttestation[], orderAddress return orderedAttestations; } + +export function getSecondsIntoSlot(l1GenesisTime: number): number { + return (Date.now() / 1000 - l1GenesisTime) % AZTEC_SLOT_DURATION; +}