fix(ethereum): prevent unhandled L1 rejections from tx monitor loops during shutdown (v5 line, partial)#24551
Merged
spalladino merged 1 commit intoJul 6, 2026
Conversation
AztecBot
added
ci-draft
AztecBot
changed the title
…during shutdown Guards L1TxUtils.monitorTransaction's loop-top getL1Timestamp() call to break quietly when interrupted, and adds a .catch to sequencer-publisher's fire-and-forget backupDroppedInSim call. Scoped to the 2 files that apply cleanly to merge-train/spartan-v5; publisher_manager.ts and checkpoint_proposal_job.ts are covered separately in #24560 since this branch's code has diverged there.
AztecBot
force-pushed
the
cb/fix-flake-sentinel-status-slash-timestamp
branch
from
06aadfc to
219364d
Compare
spalladino
approved these changes
Jul 6, 2026
spalladino
pushed a commit
that referenced
this pull request
Jul 6, 2026
…ts during shutdown (v5 line) (#24560) Ports the remaining part of the `sentinel_status_slash` flake fix to the v5 line. The flake crashed the jest process after all tests passed: an in-flight `eth_getBlockByNumber` against the torn-down anvil surfaced as an unhandled `L1RpcError` (`ECONNREFUSED`). CI log: http://ci.aztec-labs.com/9bb09d2f904e290a This complements #24551, which (after retargeting to `merge-train/spartan-v5`) now carries only the two files that cherry-picked cleanly from `next` — `l1_tx_utils.ts` (guard the loop-top `getL1Timestamp()` in `monitorTransaction`) and `sequencer-publisher.ts` (`.catch` on the fire-and-forget `backupDroppedInSim`). The other two files had diverged on this branch and are addressed here; #24551's title/body should be updated to reflect that it covers 2 of the original 4 files, with this PR covering the rest. ### `yarn-project/ethereum/src/publisher_manager.ts` — fixed This branch's `stop()` (which, unlike `next`, also clears the `started` flag and supports restart via `start()`) interrupted all publishers and the funder but returned immediately, so an in-flight tx monitor loop could still be issuing L1 RPC calls after test teardown. Applied the equivalent of the `next` fix, fitted to this branch's structure: `stop()` now awaits `waitMonitoringStopped()` on every publisher and the funder before returning. `L1TxUtils.waitMonitoringStopped(timeoutSeconds = 10)` already exists on this branch, is self-bounded, and swallows its own timeout with a warning, so shutdown cannot hang. ### `yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts` — no change needed On `next`, the votes-only path assigned a bare `pendingL1Submission = publisher.sendRequestsAt(...)` with no rejection handler, and only the last job's submission was awaited at shutdown, so an orphaned job's rejection could crash the process. On this branch that field no longer exists: both fire-and-forget paths go through `this.pendingRequests.trackRequest(promise, () => this.interrupt())`, backed by the sequencer's shared `RequestsTracker` (`requests_tracker.ts`). `trackRequest` attaches a rejection handler synchronously at track time (`promise.then(delete, delete)`), so a tracked promise can never surface as an unhandled rejection regardless of whether shutdown awaits it. Additionally, `Sequencer.stop()` calls `pendingRequests.interruptRequests()` followed by `awaitRequests()`, so all tracked requests — not just the last one — are interrupted and drained at shutdown. The `next` bug does not exist here, so no change was forced. (Minor observability note, not a bug: a rejection on the votes-only path is swallowed silently by the tracker rather than logged.) Verified `publisher_manager.ts` parses cleanly; the full workspace type-check runs in CI (this container lacks the bootstrapped bb.js/noir/l1-artifacts toolchain). No tracked issue. --- *Created by [claudebox](https://claudebox.work/v2/sessions/0959caa9f03f59fa) · group: `slackbot`*
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
2 participants
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.
Flake
multi-node/slashing/sentinel_status_slash.parallel.test.ts("slashes an attestor that gets stopped after the network is running") flaked on PR #24507 (unrelated change). CI log: http://ci.aztec-labs.com/9bb09d2f904e290aWhat actually failed
The test itself passed. Immediately after, the node process crashed with an unhandled rejection (
ECONNREFUSEDon anvil after teardown) — something was still polling L1 after shutdown. Root cause:L1TxUtils.monitorTransaction's loop callsgetL1Timestamp()at the top of its loop, outside the loop's own try/catch, and nothing waited for in-flight monitor iterations to actually stop during shutdown.Scope of this PR (rebased onto
merge-train/spartan-v5)This PR was originally written against
nextand touched 4 files. After rebasing ontomerge-train/spartan-v5, 2 of those files cherry-picked cleanly and are included here:yarn-project/ethereum/src/l1_tx_utils/l1_tx_utils.ts— guards the loop-topgetL1Timestamp()call to break quietly when interrupted.yarn-project/sequencer-client/src/publisher/sequencer-publisher.ts— adds.catchto a bare fire-and-forgetbackupDroppedInSim(...)call.The other 2 files (
publisher_manager.ts,checkpoint_proposal_job.ts) have diverged in structure on this branch and needed their own investigation rather than a blind cherry-pick — that's covered separately in #24560, which foundpublisher_manager.tshad the same defect (fixed there) andcheckpoint_proposal_job.tsdid not (this branch already routes fire-and-forget L1 requests through aRequestsTrackerthat attaches its own rejection handler).No tracked issue exists for this flake; reference only.
Created by claudebox · group:
slackbot