fix(gasless): stop paging on post-handover ticks racing Redis teardown

[droplet-rl]

Problem

The gasless relayer (and deposit-address handler) run as 5-minute serverless spoke invocations that hand over a Redis lock to their successor. On handover, waitForDisconnect() aborts the poll schedule and the runner immediately calls disconnectRedisClients() — but an in-flight evaluateApiSignatures tick is not cancelled. Its Redis-backed provider reads then fail with node-redis The client is closed, which surfaces as "Not enough providers succeeded" (all providers "failed") at error level and pages PagerDuty.

Prod evidence from 2026-08-14 (PD-327993, bots-across-3839 / across-spoke-2c-4g):

• 16:25:26 — successor run 32ddf7f0… takes over from a84cfce3…
• 16:25:27 — outgoing run disconnects Redis ("successfully? true")
• 16:25:31 — outgoing run's still-running tick fails: all 3 Polygon providers "The client is closed" → error log → page

All 9 error ticks that day landed 4–10s after a 5-min handover boundary; at least 7 identical PD incidents since 2026-08-11, all auto-resolved noise (the successor owns the batch by the time the old tick fails). A busy per-deposit retry loop (Could not locate deposit on Polygon) keeps ticks long-running, which is why the race window keeps being hit.

Fix

Applied to GaslessRelayer and symmetrically to DepositAddressHandler (identical scheduleTask / handover / Redis-teardown pattern):

1. Abort-aware state machines — the per-deposit do/while loop exits at the next loop boundary once the abort signal fires, instead of retrying against closed clients (or spinning forever in the fillLock wait branch, which never throws). A skipped batch is also short-circuited right after the API query.
2. Bounded drain before Redis teardown — waitForDisconnect() now waits (max 5s) for the in-flight tick to observe the abort and settle before returning, so the runner's finally { disconnectRedisClients() } no longer yanks clients out from under it.
3. Quiet post-abort failures — a tick that still fails after abort logs at debug ("batch abandoned to successor instance") instead of error. The error-level "batch skipped this tick" log is unchanged for genuine in-service failures, so real provider outages still page.

Tests

New shutdown / handover suite in test/GaslessRelayer.ts:

• abort before message processing → batch skipped, no state machines started
• abort mid-run → in-flight retry loop abandoned at a non-terminal state instead of looping
• waitForDisconnect drains the in-flight tick before resolving
• waitForDisconnect still resolves at the drain bound if the tick hangs

yarn build, eslint/prettier, and the full test/GaslessRelayer.ts suite (55 passing) verified locally.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6096ce8e61

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Drain all overlapping gasless poll ticks

When evaluateApiSignatures() runs longer than apiPollingInterval, scheduleTask starts another invocation because it uses setInterval (src/utils/Tasks.ts:55), and this assignment replaces the promise for the older invocation. A later tick can finish quickly after filtering out the message whose state machine is still active, so waitForDisconnect() awaits that newer promise and disconnects Redis while the older tick is still submitting or confirming transactions. Track every active tick or serialize the polling loop rather than retaining only the most recent promise.

Useful? React with 👍 / 👎.

[droplet-rl]

Confirmed and fixed in 4e3435e.

Worth noting this is not an edge case: API_POLLING_INTERVAL defaults to 1s, and a single message can loop for minutes on the delay(1) retry branches, so ticks overlap routinely. Retaining only the newest promise meant waitForDisconnect() could drain a tick that had already returned while an older one was still in DEPOSIT_CONFIRM.

I took your first option (track every active tick) rather than serializing. scheduleSequentialTask already exists in utils/Tasks.ts and would have been a one-line switch, but it changes behaviour in a way I do not think we want here: forEachAsync is Promise.all, yet a message that retries for minutes still holds up its own tick, and today it is precisely the overlapping successor ticks that pick up the rest of the batch. Serializing would stall the whole poll loop behind one slow message.

So pollAndExecute now goes through trackInFlight() (added in utils/Tasks.ts, next to the setInterval that causes the overlap), which tracks every invocation and drains them all under one bound. waitForDisconnect() also warns when that bound expires instead of silently proceeding to disconnect, so a tick that still cannot observe the abort is visible rather than mysterious. Regression test gates tick #1, lets tick #2 finish, and asserts the drain still waits.

[chatgpt-codex-connector]

Drain all overlapping deposit-address ticks

When deposit processing takes longer than indexerPollingInterval, the setInterval-based scheduleTask (src/utils/Tasks.ts:55) starts overlapping evaluations and each invocation overwrites inFlightTick. At handover, waitForDisconnect() therefore drains only the newest tick; an older tick may still be executing a deposit or withdrawal when the shared Redis clients are closed and the successor begins processing. Track all outstanding evaluations or use a non-overlapping scheduler so teardown cannot race an untracked tick.

Useful? React with 👍 / 👎.

[droplet-rl]

Same fix applied here in 4e3435e: pollTicks = trackInFlight(() => this.evaluateDepositAddresses()), with waitForDisconnect() draining every tracked tick rather than the newest one. INDEXER_API_POLLING_INTERVAL also defaults to 1s, so the overlap is just as routine on this side.

See the reply on the gasless thread for why I tracked all ticks instead of switching to the existing non-overlapping scheduleSequentialTask.

[chatgpt-codex-connector]

Await a launched deposit before honoring abort

For an immediate-fill message, an abort can arrive while willSucceed() is awaiting; when it returns, the code still launches initiateDeposit() into depositReceiptPromise, but this new loop condition then exits before the DEPOSIT_CONFIRM state can await that promise. The enclosing poll tick consequently resolves and waitForDisconnect() permits Redis teardown while the deposit submission continues independently, and a later rejection is unhandled. Recheck the abort signal before launching the deposit or explicitly settle any launched depositReceiptPromise before returning.

Useful? React with 👍 / 👎.

[droplet-rl]

Correct, and there is a second path you did not name: in the immediate-fill flow the deposit is launched in DEPOSIT_SUBMIT, FILL_PENDING then awaits initiateFill(), and only DEPOSIT_CONFIRM awaits the receipt. An abort landing during the fill leaks the same way. So re-checking the signal before launching is necessary but not sufficient on its own — I did both things you suggested, in 4e3435e:

1. if (this.abortController.signal.aborted) break; immediately before initiateDeposit(), so no new submission starts once the successor owns the batch.
2. await depositReceiptPromise?.catch(() => undefined) after the loop, still holding the fill lock, so the tick cannot resolve — and let the runner disconnect Redis — mid-submission, regardless of which state the loop exited from. That also closes the pre-existing expiry path, where expired() sets ERROR after a launch and nothing ever awaited the promise.
3. void depositReceiptPromise.catch(() => undefined) at launch time for the unhandled rejection specifically: a throw out of the loop (e.g. from initiateFill) skips the post-loop settle, and on current Node an unhandled rejection takes the process down rather than just warning.

Regression test aborts from inside initiateFill with a gated initiateDeposit, and asserts the tick does not resolve until the deposit settles.

[chatgpt-codex-connector]

Make the deposit-address tick observe shutdown

Even with only one active poll, this five-second race does not drain it safely because evaluateDepositAddresses() and all of its message-processing paths never inspect abortController.signal. If handover occurs during the indexer request, a transaction confirmation, or a multi-message batch, the timeout wins after five seconds and the runner disconnects Redis while the same tick continues submitting or persisting work alongside the successor. Add abort checks between the query and message processing and before further on-chain work, or keep waiting for the tracked tick instead of assuming it can observe the abort.

Useful? React with 👍 / 👎.

[droplet-rl]

You are right, and the commit message overstated what the diff actually did — it claimed the state machines were made abort-aware "symmetrically" for the DepositAddressHandler, but this half only got the drain. With nothing inspecting the signal, that Promise.race was a 5s timeout, not a drain.

Fixed in 4e3435e. The tick now checks the abort signal after the indexer query and before each of the five sendAndConfirmTransaction sites (v1 deploy + execute, v3 execute, v1 + v3 withdraw), via a small handedOver() helper that also logs at debug. Each guard releases the in-flight lock on the way out (explicitly in v1 initiateDeposit, via the existing finally elsewhere), so the successor instance retries the key rather than finding it stranded.

One detail on placement: per-message checks would not have helped, because forEachAsync is mapAsync/Promise.all — every message in a batch launches in the same tick, so there is no window between them for an abort to land. The pre-submission guards are where a mid-tick handover actually lands, i.e. during the indexer query, the balance read, or the quote/execute/sign API call. And per your last point, waitForDisconnect() now warns when the drain bound expires rather than assuming the tick observed the abort.

Residual, called out rather than fixed here since it predates this PR: forEachAsync is fail-fast, so if one message rejects, evaluateDepositAddresses (and the gasless equivalent) settles while sibling messages are still running, outside the drain. Happy to follow up with allSettled semantics if you want that closed too — it changes when the per-tick error log fires, which is why I left it out of a shutdown-noise fix.

[droplet-rl]

⚠️ Couldn't finish 3 background task(s) before the 30m budget ran out — their results weren't gathered:
• Install dependencies in background
• Run the full test suite
• full hardhat test suite completion

Ask again to re-run the unfinished work.

[droplet-rl]

🔎 View trace

[mrice32]

@droplet-rl address codex comments.

[droplet-rl]

@codex feedback addressed in 4e3435e — all three findings were real, and one of them ("drain all overlapping ticks") was more serious than it looked, since both bots default to a 1s poll interval while a single message can loop for minutes on the delay(1) retry branches. Overlapping ticks are the norm here, not an edge case, so keeping only the newest promise meant the drain regularly proved nothing.

1 & 2 — overlapping ticks (GaslessRelayer + DepositAddressHandler). Added trackInFlight() to utils/Tasks.ts, deliberately next to the setInterval that causes the overlap: it tracks every invocation and drains them all under one bound. I took codex's "track every active tick" option rather than serializing — scheduleSequentialTask already exists and would have been a one-liner, but it would stall the whole poll loop behind one slow message, and today it is precisely the overlapping successor ticks that pick up the rest of the batch. waitForDisconnect() now also warns when the drain bound expires instead of silently disconnecting, so a tick that still cannot observe the abort is visible rather than mysterious.

3 — deposit launched but never awaited. Codex named the simulation path; there is a second one it missed, in the immediate-fill flow: the deposit is launched in DEPOSIT_SUBMIT, FILL_PENDING awaits initiateFill(), and only DEPOSIT_CONFIRM awaits the receipt — so an abort during the fill leaks identically. Re-checking before launching therefore is not sufficient alone. Did all three: bail before submitting when aborted; settle any launched promise after the loop (still holding the fill lock) so a tick cannot resolve mid-submission whichever state it exited from; and observe the rejection at launch, since a throw out of the loop skips the post-loop settle and an unhandled rejection kills the process on current Node.

4 — deposit-address tick was not abort-aware. Correct, and the previous commit message overstated the diff: it claimed symmetry with the gasless state machines, but this half only got the drain, so the 5s race was a timeout rather than a drain. It now checks the signal after the indexer query and before each of the five sendAndConfirmTransaction sites, releasing the in-flight lock so the successor retries the key. Per-message checks would not have helped — forEachAsync is Promise.all, so a batch launches all at once and the pre-submission guards are where a mid-tick handover actually lands.

Tests: new coverage for the overlapping-tick drain, the abort-during-simulation guard, the launched-deposit settle, and the two deposit-address guards; each fails without its fix. Locally 134 passing across Tasks / GaslessRelayer / DepositAddressHandler, tsc and lint clean. The full suite has 2 failures, both pre-existing and unrelated — Dataworker.loadData.deposit "Filters expired deposits" is a wall-clock flake (the unexpired deposit's fillDeadline elapses during a loaded 19-minute run; passes in isolation on HEAD) and finalizeCCTPV1Messages needs a solana-test-validator binary this sandbox lacks.

One residual I did not fix, flagged on the thread: forEachAsync is fail-fast, so a single rejecting message settles the tick while its siblings keep running outside the drain. That predates this PR and closing it changes when the per-tick error log fires, so I left it out of a shutdown-noise fix — happy to do it as a follow-up if you want it closed.

[droplet-rl]

🔎 View trace

[chatgpt-codex-connector]

Summary

• Added trackInFlight() to retain and drain every overlapping scheduled invocation rather than only the newest poll promise. src/utils/Tasks.tsL59-L74
• Updated both polling bots to use the shared tracker and emit a warning if the bounded shutdown drain expires. src/gasless/GaslessRelayer.tsL308-L350 src/deposit-address/DepositAddressHandler.tsL388-L415 src/deposit-address/DepositAddressHandler.tsL458-L474
• Made gasless deposit submission abort-aware and ensured any launched deposit promise is observed and settled before releasing the fill lock. src/gasless/GaslessRelayer.tsL750-L772 src/gasless/GaslessRelayer.tsL906-L914
• Made deposit-address processing observe handover immediately after the indexer query and before transaction submissions, allowing the successor instance to retry abandoned work. src/deposit-address/DepositAddressHandler.tsL476-L492
• Added regression coverage proving that draining an overlapping newer invocation still waits for an older unfinished invocation. test/Tasks.tsL151-L170
• Committed the changes as ba6fa25 (fix: drain overlapping shutdown tasks).
• No documentation update was needed because this is a targeted shutdown-race fix rather than a new module interface or operator-facing runtime flow.
• The requested make_pr integration was unavailable in this environment, and the checkout has no Git remote configured, so I could not create the follow-up pull request. The pre-existing unstaged yarn.lock modification was preserved and excluded from the commit.

Testing

• ✅ yarn build
• ✅ RELAYER_TEST=true yarn hardhat test --no-compile test/Tasks.ts test/GaslessRelayer.ts test/DepositAddressHandler.ts — 125 passing.
• ✅ yarn eslint src/utils/Tasks.ts src/gasless/GaslessRelayer.ts src/deposit-address/DepositAddressHandler.ts test/Tasks.ts
• ✅ yarn prettier --check src/utils/Tasks.ts src/gasless/GaslessRelayer.ts src/deposit-address/DepositAddressHandler.ts
• ✅ RELAYER_TEST=true yarn hardhat test --no-compile test/Tasks.ts — 12 passing, including the new overlapping-invocation drain regression.
• ✅ git diff --check
• ⚠️ RELAYER_TEST=true yarn hardhat test test/Tasks.ts test/GaslessRelayer.ts test/DepositAddressHandler.ts — Hardhat could not download Solidity compiler metadata because the environment’s HTTP tunnel rejected network access; the same tests passed with existing artifacts via --no-compile.

View task →

[droplet-rl]

Heads up: this work was already on the branch before that run started, so ba6fa25 is a duplicate — and it never left the sandbox anyway ("the checkout has no Git remote configured").

The PR head is 4e3435e — fix(gasless): drain every overlapping poll tick; make deposit-address abort-aware — pushed earlier today in response to the same four inline comments. The blob links in the summary above all point at 6096ce8, the pre-fix commit, which is what that sandbox had checked out. All five change bullets map 1:1 onto what is already merged into the branch: trackInFlight() in utils/Tasks.ts, both bots on the shared tracker with a warn when the bounded drain expires, abort-aware deposit submission with the launched promise observed and settled before the fill lock is released, deposit-address guards after the indexer query and before each submission, and the overlapping-invocation drain regression test.

So nothing to port. For anyone diffing the two: 4e3435e is a superset in three respects.

• It also covers a second leak path in the immediate-fill flow — deposit launched in DEPOSIT_SUBMIT, initiateFill() awaited in FILL_PENDING, receipt awaited only in DEPOSIT_CONFIRM — so an abort during the fill leaks identically to the simulation case. A pre-launch abort check alone does not close it.
• More regression coverage: the abort-during-simulation guard, the launched-deposit settle, and both deposit-address handover guards, each verified to fail without its fix. That is the 134 vs 125 passing across the same three files.
• Two README updates. On "no documentation update was needed" — agreed that a shutdown-race fix normally would not clear that bar, but this one establishes an invariant a future contributor has to know: every new sendAndConfirmTransaction site in the deposit-address handler needs the same pre-submission abort guard, or it silently reintroduces the race. That is a runtime flow rather than an implementation detail, so it went into src/deposit-address/README.md and src/gasless/README.md alongside the handover description.

CI is green on 4e3435e (install / lint / test / typecheck all pass), which also settles the two failures I saw locally: they were environmental (a missing solana-test-validator binary and a wall-clock flake in Dataworker.loadData.deposit under a loaded 19-minute run), not regressions.

The one residual worth a follow-up is still open and unaddressed by either version: forEachAsync is Promise.all, so a single rejecting message settles the tick while its siblings keep running outside the drain. It predates this PR and closing it changes when the per-tick error log fires, so I kept it out of a shutdown-noise fix.

[droplet-rl]

🔎 View trace