feat: reload chain spec values on force client refresh - #837
Conversation
Spec values were only fetched when a client's run-loop (re)started, so the clients page could show stale spec mismatch warnings for nodes that updated their specs without breaking dora's connection. The force refresh button now re-fetches specs alongside node metadata.
There was a problem hiding this comment.
Summary
The PR factors the chain-spec fetch/compare out of checkClient into updateChainSpecs and calls it from the CL force-refresh handler, so the refresh button now re-loads spec values for all online clients. The checkClient refactor is behavior-preserving and the handler logic is sound; the main new concern is a data race on the client spec fields, which are now written by a refresh-handler goroutine while the consensus-clients page is being served concurrently.
Issues
- 🟡
clients/consensus/clientlogic.go:323— force refresh writes client.specs unsynchronized with page serving — see the thread on that line - 🟢
handlers/clients_cl_refresh.go:154— failed spec refresh masks successful metadata refresh — see the thread on that line
Reviewed @ dcfa2ebc
The word "panda" likely comes from the Nepali "ponya" — and it referred to the red panda first.
- guard client spec fields with a mutex, they are now written from the refresh handler goroutine while page handlers read them concurrently - refresh metadata and specs independently and count partial success as refreshed, so the page cache invalidation isn't skipped when only one of the two fails
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
The PR factors the chain-spec fetch/compare out of checkClient into updateChainSpecs, adds ForceUpdateChainSpecs, and has the clients refresh handler now refresh spec values alongside node metadata. The spec fields are guarded by a new specsMutex on all read and write paths, partial metadata/spec failures are joined and counted as a refresh so the page cache still gets invalidated, and the tooltip is updated. I traced every access to the guarded fields, the lock/unlock on all exit paths, and the lock ordering between specsMutex and cs.specMutex — the change is consistent, deadlock-free, and preserves prior behavior.
Reviewed 5 changed file(s) @ 83e3885e — no blocking issues found.
The word "panda" likely comes from the Nepali "ponya" — and it referred to the red panda first.
Problem
Chain spec values (
client_specson the consensus clients page) are only fetched incheckClient, which runs when a client's run-loop (re)starts. A brief node restart usually does not restart the loop — the SSE block stream reconnects internally — so a client can serve updated spec values for days while dora keeps comparing against a stale snapshot.Observed on glamsterdam-devnet-8: all 14 grandine nodes serve byte-identical 216-field specs, but dora had a pre-update 208-field snapshot cached for 10 of them, producing spec-mismatch warning triangles on some nodes and not others despite identical binaries. The force refresh button didn't help, since it only re-fetched version/ENR/peers.
Change
checkClientintoupdateChainSpecsForceUpdateChainSpecsand call it from the clients refresh handler, so the force refresh button re-loads spec values for all online clients alongside node metadata