Skip to content

*: fix DKG exchanger deadlock#4590

Merged
KaloyanTanev merged 1 commit into
mainfrom
kalo/fix-psig-chan
Jul 8, 2026
Merged

*: fix DKG exchanger deadlock#4590
KaloyanTanev merged 1 commit into
mainfrom
kalo/fix-psig-chan

Conversation

@KaloyanTanev

Copy link
Copy Markdown
Collaborator

exchange and pushPsigs shared a single size-1 channel. exchange first calls StoreInternal to store this node's own partial signatures and only then enters the loop that receives collected signatures from the channel. pushPsigs is the parsigdb threshold subscriber, so it both handles peer signatures (via StoreExternal) and runs synchronously on the exchange goroutine (via StoreInternal) - and on every threshold it sends the collected map on that shared channel.

Example scenario:

  1. Peers' partial signatures for validator A arrive and complete its threshold, so pushPsigs sends A's map and fills the size-1 channel.
  2. Current node's StoreInternal is still running (the exchange goroutine has not reached its receive loop yet, so nothing is draining the channel).
  3. That same StoreInternal completes the threshold for validator B, so pushPsigs is invoked inline on the exchange goroutine.
  4. pushPsigs tries to send B's map on the already-full channel and blocks.
  5. The exchange goroutine is now stuck inside StoreInternal and never reaches the receive loop that would drain the channel, so the send can never complete — a self-deadlock.
  6. This node never broadcasts its aggregated result, so the other peers block waiting for it too, and the whole ceremony stalls.

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

@sonarqubecloud

sonarqubecloud Bot commented Jul 8, 2026

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 sigDatasChan flow with per-exchange pending queries (exchangeQuery) and a resolver (resolveQueriesUnsafe).
  • Update pushPsigs to store collected signature data and resolve any satisfied pending exchange queries.
  • 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.

Comment thread dkg/exchanger.go
@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.71429% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.55%. Comparing base (80ddd49) to head (d987b6f).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
dkg/exchanger.go 85.71% 5 Missing and 1 partial ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@pinebit pinebit left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed manually + GPT, no issues observed.

@KaloyanTanev KaloyanTanev merged commit 6c31a57 into main Jul 8, 2026
12 checks passed
@KaloyanTanev KaloyanTanev deleted the kalo/fix-psig-chan branch July 8, 2026 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants