test(e2e_epochs): set l1PublishingTime to match redistribution test timing model#24073
Draft
AztecBot wants to merge 1 commit into
Draft
test(e2e_epochs): set l1PublishingTime to match redistribution test timing model#24073AztecBot wants to merge 1 commit into
AztecBot wants to merge 1 commit into
Conversation
a8d079c to
3dc5349
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why the merge train dequeued
PR #24053 (
merge-train/spartan-v5→v5-next) was removed from the merge queue bygithub-merge-queue[bot](first at 18:17:50, again at 19:37:22) because thecicheck on the merge-queue commit failed on a single hard test:It failed its initial run and its single CI retry, so it was a hard failure, not a tolerated flake.
This is a timing flake, not a merge regression
c38271:v5-nextis already fully contained inmerge-train/spartan-v5, so the merge introduces no new code.c38271passed a fresh full no-cache CI run (ci-full-no-test-cache) ~20 min before the first dequeue — including this test.So the merge resolution (#24071/#24072) is correct; the dequeues were caused by this timing-sensitive e2e test.
Root cause and fix
epochs_mbps_redistributionbuilds a 3-block checkpoint and dumps the late txs right before the last sub-slot, expecting redistributed budget to fit them all in the last block (Set(lateBlockNumbers).size === 1). The test's own timing comment budgets 2s for L1 publish, but it never setl1PublishingTime, so the sequencer used the config default of 12s (sequencer-client/src/config.ts). That inflates the per-block timetable relative to the 36s slot / 8s sub-slot the test is tuned for, so the proposer cuts blocks earlier than the test assumes and the late txs spill across two blocks.nextalready carries this fix (its copy setsl1PublishingTime: 2in the same setup block). This ports that one value to the v5 line.nextalso addsenforceTimeTable: trueanddisableAnvilTestWatcher: truein the same block, but those are intentionally not ported:enforceTimeTable— on the v5 line the sequencer already defaults this totrue(cli-referencedefault: true); it is not referenced by any v5 test, so setting it is redundant.disableAnvilTestWatcher— does not exist as a config option anywhere on the v5 line; adding it would break compilation.Verification
l1PublishingTime?: numberis part ofPartial<AztecNodeConfig>viaSetupOptions(stdlib/src/interfaces/configs.ts:63), so the change is type-safe../bootstrap.sh ci/ multi-node e2e cannot run in this environment (no docker; CI dispatches to EC2), so this PR's own CI is the verification. CI re-triggered and being watched.Once green, this needs to land in
merge-train/spartan-v5so #24053 can re-enter the queue cleanly.cc @PhilWindle
Re-verified and CI re-triggered by claudebox (session
3297a5e4).