*: fix DKG exchanger deadlock#4590
Conversation
|
There was a problem hiding this comment.
Pull request overview
Fixes a self-deadlock in the DKG partial-signature exchanger by removing the shared size-1 result channel and adopting a per-call “await query” pattern similar to other workflow components, plus a regression test to ensure pushPsigs doesn’t block when no exchange call is draining results.
Changes:
- Replace the shared
sigDatasChanflow with per-exchangepending queries (exchangeQuery) and a resolver (resolveQueriesUnsafe). - Update
pushPsigsto store collected signature data and resolve any satisfied pendingexchangequeries. - Add a regression test that would have deadlocked under the prior shared-channel design.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| dkg/exchanger.go | Reworks exchanger result delivery from a shared channel to per-call buffered response channels with query resolution. |
| dkg/exchanger_internal_test.go | Adds a regression test ensuring repeated pushPsigs calls don’t block without an exchange receiver. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4590 +/- ##
==========================================
+ Coverage 57.40% 57.55% +0.15%
==========================================
Files 245 245
Lines 33560 33668 +108
==========================================
+ Hits 19265 19379 +114
+ Misses 11856 11845 -11
- Partials 2439 2444 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
pinebit
left a comment
There was a problem hiding this comment.
Reviewed manually + GPT, no issues observed.



exchangeandpushPsigsshared a single size-1 channel.exchangefirst callsStoreInternalto store this node's own partial signatures and only then enters the loop that receives collected signatures from the channel.pushPsigsis theparsigdbthreshold subscriber, so it both handles peer signatures (viaStoreExternal) and runs synchronously on theexchangegoroutine (viaStoreInternal) - and on every threshold it sends the collected map on that shared channel.Example scenario:
pushPsigssends A's map and fills the size-1 channel.StoreInternalis still running (theexchangegoroutine has not reached its receive loop yet, so nothing is draining the channel).StoreInternalcompletes the threshold for validator B, sopushPsigsis invoked inline on theexchangegoroutine.pushPsigstries to send B's map on the already-full channel and blocks.exchangegoroutine is now stuck insideStoreInternaland never reaches the receive loop that would drain the channel, so the send can never complete — a self-deadlock.I've tried to keep it as close as possible to established paradigms in the other parts of the code, as the DKG is vital part and I personally would like as small changes as possible. The fix reworks exchange to use the same await pattern as
dutydb/aggsigdb: each call registers a query with its own buffered response channel, and every store resolves the queries whose signatures are fully collected.Also added a test to mimic the faulty behaviour which now is passing.
category: bug
ticket: none