Update proposal preparations when a consensus client becomes active - #431
Update proposal preparations when a consensus client becomes active#431ThomasDalla wants to merge 1 commit into
Conversation
2891fe6 to
de30bd7
Compare
|
Confirmed working against mainnet. Three configured beacon nodes, one of them restarted deliberately; timings are relative to the moment its connection dropped:
Duties were unaffected throughout: submissions to the restarted node failed with Two things worth flagging from the run:
The node here took ~2m45s to become active again, well past the 30s connection-state poll, so this run does not exercise the limitation noted in the description — a restart fast enough to go unobserved between polls still gets no update. Node identifiers and validator details omitted; happy to provide anything more specific that would help review. |
Proposal preparations are held in memory by beacon nodes, so a node that restarts loses them until vouch's next scheduled update, which runs once per epoch. A node that is inactive when that update runs is skipped, and not retried until the following epoch. A proposal in that window can be made with the beacon node's own default fee recipient rather than the configured one. Provide hooks to the consensus clients so that proposal preparations are updated as soon as a client becomes active, in addition to the existing per-epoch update. The updates use the service context rather than the one supplied to the hook. A client checks its connection state on the way in to a request that finds it inactive, so the hook can be called with that request's context, which is cancelled as soon as the request completes; using it cancels the preparations mid-flight.
de30bd7 to
ccdd500
Compare
Fixes #430.
What this changes
Beacon nodes hold proposal preparations in memory, so a node that restarts has none until
Vouch's next scheduled update. That update runs once per epoch, and a node that is inactive
when it runs is skipped via
ErrNotActiveand not retried until the following epoch — so anode can be back up and synced for minutes with no fee-recipient preferences. A proposal in
that window, if the block is built locally, uses the beacon node's own default fee recipient.
This updates proposal preparations as soon as a consensus client becomes active, in addition
to the existing per-epoch update.
Why this shape
go-eth2-clientalready detects the transition.http.Hooks.OnActiveis fired fromCheckConnectionStatewhen a client goes from inactive to active, on the existing 30-secondconnection-state poll. Vouch simply never passed hooks when constructing its clients, so the
change is to provide them — no new polling, timers or connection tracking.
Callbacks are registered in
clients.gobecausefetchClientis where every client isconstructed and cached, and the hooks have to be supplied at construction time. The hook
reads the callback list when it fires rather than capturing it, so it does not matter whether
a client is constructed before or after a callback is registered.
UpdatePreparationsis called rather than targeting the client that just became active.It already submits to every configured submitter in one call, which is cheap, and it means a
reconnect that coincides with another node also being unavailable still ends up updating both
once they are back. Targeting a single node would need address-matching against the submitter
list for no practical gain.
The
nilcheck onproposalPrepareris deliberate.initProposalPreparerreturns a nilservice (with a nil error) when the chain is not yet bellatrix-capable; without the check the
first activation would panic inside a client-owned goroutine.
Callbacks are called synchronously.
OnActiveis already invoked in its own goroutine bythe client, so there is nothing to gain from another one, and this keeps a panic attributable.
The callback list is copied under the read lock before dispatch, so a callback that registers
another callback cannot deadlock — covered by a test.
What this does not cover
Connection state is polled every 30 seconds, so a beacon node that restarts and starts
answering again within a single poll interval is never observed as inactive and no update is
triggered. This makes the exposure window small in the common case rather than removing it
entirely. Tying preparation updates to known proposal duties would close it properly, but
that is a larger change and I did not want to bundle it here.
Validator registrations (
blockrelay) have the same once-per-epoch shape and could reusethe same callback, but they were not the cause of the issue, so I have left them alone to
keep this focused. Happy to add that here or separately if you would like it.
Testing
go build ./...,go vet ./...andgo test ./...all pass.and registration from within a callback; they pass under
-race.behaves after a real beacon node restart.
Happy to adjust naming, comments, or add a CHANGELOG entry if you would like this in a
different form.