Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: don't crash sequencer when it is too slow #9790

Merged
merged 1 commit into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 45 additions & 45 deletions yarn-project/sequencer-client/src/sequencer/sequencer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ describe('sequencer', () => {

globalVariableBuilder.buildGlobalVariables.mockResolvedValueOnce(mockedGlobalVariables);

await sequencer.work();
await sequencer.doRealWork();

expect(blockBuilder.startNewBlock).toHaveBeenCalledWith(
2,
Expand All @@ -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();
Expand All @@ -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)
Expand All @@ -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,
Expand Down Expand Up @@ -314,7 +313,7 @@ describe('sequencer', () => {
);
});

await sequencer.work();
await sequencer.doRealWork();

expect(blockBuilder.startNewBlock).toHaveBeenCalledWith(
2,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -374,7 +373,7 @@ describe('sequencer', () => {
(txs[invalidTransactionIndex].unencryptedLogs.functionLogs[0].logs[0] as Writeable<UnencryptedL2Log>).data =
randomBytes(1024 * 1022);

await sequencer.work();
await sequencer.doRealWork();

expect(blockBuilder.startNewBlock).toHaveBeenCalledWith(
2,
Expand Down Expand Up @@ -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,
Expand All @@ -442,20 +441,20 @@ 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
sequencer.flush();

// 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,
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -537,7 +536,7 @@ describe('sequencer', () => {
.mockResolvedValueOnce()
.mockRejectedValueOnce(new Error());

await sequencer.work();
await sequencer.doRealWork();

expect(publisher.proposeL2Block).not.toHaveBeenCalled();
});
Expand Down Expand Up @@ -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);
});

Expand All @@ -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);
});

Expand All @@ -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);
});

Expand All @@ -681,7 +680,7 @@ describe('sequencer', () => {

publisher.getClaimableEpoch.mockResolvedValue(undefined);

await sequencer.work();
await sequencer.doRealWork();
expect(publisher.proposeL2Block).toHaveBeenCalledWith(block, getSignatures(), [txHash], undefined);
});

Expand All @@ -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);
});

Expand Down Expand Up @@ -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);
});

Expand Down Expand Up @@ -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]);
});
});
Expand All @@ -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();
}
}
35 changes: 27 additions & 8 deletions yarn-project/sequencer-client/src/sequencer/sequencer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,28 @@ 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;
validTxsCount?: number;
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).
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -350,7 +371,7 @@ export class Sequencer {
}
}

doIHaveEnoughTimeLeft(proposedState: SequencerState): boolean {
doIHaveEnoughTimeLeft(proposedState: SequencerState, secondsIntoSlot: number): boolean {
if (!this.enforceTimeTable) {
return true;
}
Expand All @@ -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);

Expand All @@ -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;
}
Expand Down
5 changes: 5 additions & 0 deletions yarn-project/sequencer-client/src/sequencer/utils.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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;
}
Loading