fix: wind down the signing domain refresh on shutdown - #679
Merged
Conversation
The periodic refresh of the RCA EIP-712 signing domain, which lets a running dipper follow an in-place contract upgrade, ran as a detached task that ignored shutdown. It now joins the service task tree with a stop handle, so it winds down with everything else.
The refresh was only signalled to stop and left running while shutdown moved on, so it could still be mid-refresh at exit. It now waits for the loop to drop its receiver, the same way every sibling service stops. Also drops a stray em-dash from a nearby comment.
MoonBoi9001
changed the title
The loop that periodically re-reads the signing domain from the contract lived in the chain client module root, away from the function it drives and from the other supervised background loops. It now sits beside them in network/service, with its types imported.
…lings Shutting the refresh down meant sending on a raw channel and then waiting on it by hand, inline in main, while every neighbouring service is stopped through a small handle type. The refresh now offers the same handle, so the shutdown sequence reads the same throughout.
…e refresh Startup already refuses to run unless the chain client is configured and enabled, yet the domain refresh re-tested both and carried the resulting optional value through three call sites whose fallback branches could never run. It now reads the validated config directly.
Waiting for an in-flight domain refresh to finish had no upper limit, so a provider that stopped responding could hold up the whole shutdown sequence behind a single best-effort RPC call. The wait now gives up after 15 seconds and warns instead of blocking.
Nothing checked that a tick reaches the refresh call, so the loop could stop following contract upgrades without a single test noticing. The new case runs it on a virtual clock and waits for three refreshes; tokio's test-util is enabled for the fake clock.
The loop logs a failed refresh and carries on, which is what keeps a temporary RPC outage from silently ending the service. That behaviour had no test, so the new case fails every attempt and checks the loop is still running and still trying.
The dipper re-reads the RecurringCollector signing domain on a timer, but a stop could not interrupt that read, so a provider going quiet mid-request held the process open for minutes. The read now loses to the stop signal and is dropped, costing one read-only round trip.
MoonBoi9001
changed the title
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
1 participant
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.
This PR makes the background refresh of the agreement signing domain wind down as part of graceful shutdown, and drops a refresh still in flight when the stop signal arrives. That refresh keeps a running dipper in step with an in-place upgrade of the contract it signs against: it re-reads the contract's EIP-712 domain on a timer and keeps the current domain if a read fails.
Until now the refresh ran as a detached background task that ignored the shutdown signal entirely, so the process could tear down around a task still doing work. It now lives in its own module with the same stop handle every other long-running service has. Because the read goes out to an RPC provider, one that accepts the request and then goes quiet would otherwise hold the process open until it gave up, which can outlast a deployment's grace period. The stop signal wins over the read instead, at the cost of one read-only round trip.