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

chore: Fix flake in e2e-block-build #11002

Merged
merged 3 commits into from
Jan 2, 2025
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
18 changes: 11 additions & 7 deletions yarn-project/end-to-end/src/e2e_block_building.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ describe('e2e_block_building', () => {
logger.info(`Updating aztec node config`);
await aztecNode.setConfig({ minTxsPerBlock: 1, maxTxsPerBlock: TX_COUNT, enforceTimeTable: true });

// We tweak the sequencer so it uses a fake simulator that adds a 200ms delay to every public tx.
// We tweak the sequencer so it uses a fake simulator that adds a delay to every public tx.
const archiver = (aztecNode as AztecNodeService).getContractDataSource();
sequencer.sequencer.publicProcessorFactory = new TestPublicProcessorFactory(
archiver,
Expand All @@ -202,11 +202,13 @@ describe('e2e_block_building', () => {
);

// We also cheat the sequencer's timetable so it allocates little time to processing.
// This will leave the sequencer with just 2s to build the block, so it shouldn't be
// able to squeeze in more than 10 txs in each. This is sensitive to the time it takes
// to pick up and validate the txs, so we may need to bump it to work on CI.
sequencer.sequencer.timeTable[SequencerState.WAITING_FOR_TXS] = 2;
sequencer.sequencer.timeTable[SequencerState.CREATING_BLOCK] = 2;
// This will leave the sequencer with just a few seconds to build the block, so it shouldn't
// be able to squeeze in more than ~12 txs in each. This is sensitive to the time it takes
// to pick up and validate the txs, so we may need to bump it to work on CI. Note that we need
// at least 3s here so the archiver has time to loop once and sync, and the sequencer has at
// least 1s to loop.
sequencer.sequencer.timeTable[SequencerState.WAITING_FOR_TXS] = 4;
sequencer.sequencer.timeTable[SequencerState.CREATING_BLOCK] = 4;
sequencer.sequencer.processTxTime = 1;

// Flood the mempool with TX_COUNT simultaneous txs
Expand Down Expand Up @@ -615,9 +617,11 @@ type TestSequencer = Omit<Sequencer, 'publicProcessorFactory' | 'timeTable'> & {
};
type TestSequencerClient = Omit<SequencerClient, 'sequencer'> & { sequencer: TestSequencer };

const TEST_PUBLIC_TX_SIMULATION_DELAY_MS = 300;

class TestPublicTxSimulator extends PublicTxSimulator {
public override async simulate(tx: Tx): Promise<PublicTxResult> {
await sleep(200);
await sleep(TEST_PUBLIC_TX_SIMULATION_DELAY_MS);
return super.simulate(tx);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ describe('sequencer', () => {
await expect(sequencer.doRealWork()).rejects.toThrow(
expect.objectContaining({
name: 'SequencerTooSlowError',
message: expect.stringContaining(`Too far into slot to transition to ${delayedState}.`),
message: expect.stringContaining(`Too far into slot to transition to ${delayedState}`),
}),
);

Expand Down
8 changes: 4 additions & 4 deletions yarn-project/sequencer-client/src/sequencer/sequencer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export class SequencerTooSlowError extends Error {
public readonly currentTime: number,
) {
super(
`Too far into slot to transition to ${proposedState}. max allowed: ${maxAllowedTime}s, time into slot: ${currentTime}s`,
`Too far into slot to transition to ${proposedState} (max allowed: ${maxAllowedTime}s, time into slot: ${currentTime}s)`,
);
this.name = 'SequencerTooSlowError';
}
Expand Down Expand Up @@ -172,10 +172,10 @@ export class Sequencer {

private setTimeTable() {
// How late into the slot can we be to start working
const initialTime = 1;
const initialTime = 2;

// How long it takes to validate the txs collected and get ready to start building
const blockPrepareTime = 2;
const blockPrepareTime = 1;

// How long it takes to for attestations to travel across the p2p layer.
const attestationPropagationTime = 2;
Expand Down Expand Up @@ -430,7 +430,7 @@ export class Sequencer {
const bufferSeconds = maxAllowedTime - secondsIntoSlot;

if (bufferSeconds < 0) {
this.log.warn(`Too far into slot to transition to ${proposedState}`, { maxAllowedTime, secondsIntoSlot });
this.log.debug(`Too far into slot to transition to ${proposedState}`, { maxAllowedTime, secondsIntoSlot });
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was getting logged twice: once here, once where we logged the thrown SequencerTooSlowError.

return false;
}

Expand Down
Loading