Skip to content

fix(service): preserve invalidated slots in the initial-round DKG node list#91

Merged
0xHansLee merged 1 commit into
mainfrom
fix-initial-node-list-preserve-invalidated-slots
Jul 23, 2026
Merged

fix(service): preserve invalidated slots in the initial-round DKG node list#91
0xHansLee merged 1 commit into
mainfrom
fix-initial-node-list-preserve-invalidated-slots

Conversation

@0xHansLee

Copy link
Copy Markdown
Collaborator

Problem

The initial-round kyber node list was built from VERIFIED-only registrations. extractSortedPubKeys assigns each kyber position as a dense index sorted by reg.Index, so dropping a member invalidated mid-dealing shifts every later node's index. A node that first builds its DistKeyGenerator after an invalidation gets a shorter, shifted list that diverges from peers who built earlier — and fails the len(regs) == network.Total check, since network.Total is fixed at BeginDealing and never decremented.

Fix

Build (and validate) the node list from all registrations (any status) via GetAllRegisteredDKGRegistrations, mirroring the resharing path. Invalidated members keep their slot, so the list has a stable length equal to network.Total regardless of when a node builds. PID matching (matchPIDFromRegistrations/CachePID) is status-insensitive — it keys off each registration's own DkgPubKey and on-chain Index — so the retained slots are harmless.

Tests

  • TestFetchRoundContext_PreservesInvalidatedSlot

issue: none

@jinn-agent

jinn-agent Bot commented Jul 21, 2026

Copy link
Copy Markdown

The core fix is sound: swapping GetAllParticipantDKGRegistrations (VERIFIED-only) for GetAllRegisteredDKGRegistrations (all statuses) in fetchRoundContext mirrors the already-working resharing path and keeps the kyber position list stable regardless of when a node builds after a mid-dealing invalidation. The accompanying test covers the regression scenario and the negative (shrunk-list) case correctly. One assumption implicit in the fix is worth calling out, and the stub fallback has a subtle edge case noted below.


Review iteration 2 · Commit 527faac · 2026-07-22T08:56:59Z

@yingyangxu2026

Copy link
Copy Markdown
Contributor

The commit message says "Registrations stays VERIFIED-only for status-sensitive consumers," but in the diff rc.Registrations now carries all statuses — the same registrations variable feeds both SortedPubKeys and rc.Registrations, and the test asserts Len(Registrations, 3) after an invalidation. The message is stale / contradicts the code; worth updating so it doesn't mislead reviewers.

@0xHansLee 0xHansLee self-assigned this Jul 22, 2026
…e list

Build the initial-round kyber node list (SortedPubKeys) from the round's
ALL-status registrations (GetAllRegisteredDKGRegistrations) instead of the
VERIFIED/FINALIZED-only set. extractSortedPubKeys assigns kyber positions as a
dense index after sorting by reg.Index, so dropping a member invalidated
mid-dealing shifts every later node's index. A node first building its
DistKeyGenerator after the invalidation would then get a shorter, shifted list
diverging from peers who built earlier. This mirrors the resharing path
(fetchLatestPubKeysAndCoeffs).

network.Total is fixed at BeginDealing and never decremented, so the all-status
set keeps a stable length equal to network.Total (validateRegistrations still
enforces the count). The same registrations feed both SortedPubKeys and
rc.Registrations; PID matching is status-insensitive (it keys off each
registration's own DkgPubKey and on-chain Index), so the retained invalidated
slots are harmless.
@0xHansLee
0xHansLee force-pushed the fix-initial-node-list-preserve-invalidated-slots branch from fb1505d to 527faac Compare July 22, 2026 08:52
@0xHansLee
0xHansLee merged commit c210b74 into main Jul 23, 2026
13 checks passed
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.

2 participants