fix(dkg): do not complete an active-round session without key material#857
Merged
Merged
Conversation
0xHansLee
requested review from
jdubpark,
lucas2brh and
stevemilk
as code owners
July 21, 2026 05:19
|
Every failure path calls |
0xHansLee
force-pushed
the
fix-resume-active-requires-key-material
branch
from
July 23, 2026 05:26
ed828fc to
07b3e2d
Compare
resumeFailedSession force-completed a failed active-round session even when its GlobalPubKey/PubKeyShare were empty (this node's finalization failed while the round advanced on-chain via other validators). The session was marked completed but processDecryptQueue skips it for lacking a global public key, so the node silently contributed zero partial decryptions. Now the active branch only completes sessions that hold key material. Sessions missing it re-run finalization locally via the kernel (recompute+seal the share from persisted deals) without submitting a stale on-chain finalize vote; on failure the session is left failed and logged rather than silently completed. Because ResumeDKGService re-enters this recovery every block while the session stays failed, the recovery is now bounded: a node-local attempt counter caps retries at maxActiveRecoveryAttempts, after which the session is left failed with an escalation alert. A divergent on-chain key (deterministic, never self-heals) escalates immediately. All of this is node-local JSON/atomic state under isDKGSvcEnabled, so it never touches consensus state, GasUsed, or the app hash. Re-dispatch and exhaustion are gated on whether an attempt is actually in flight (the per-round service lock dkgSvcRound), not on a wall-clock backoff. An earlier LastUpdate/time-based gate could be jammed shut by normal active-round threshold-decrypt traffic (AddDecryptRequest bumps the same LastUpdate timestamp), permanently blocking re-dispatch and escalation; it also risked a premature "exhausted" escalation when a single attempt outran the backoff window. dkgSvcRound is untouched by decrypt-request queuing, so decrypt traffic cannot jam the gate, and while an attempt is in flight the block simply returns without spawning a doomed goroutine. The retry counter is bumped inside the per-round lock as each real attempt starts, so per-block dispatches that lose the lock race no longer burn the budget into a false escalation. The session key material read/written on the recovery path is guarded by DKGSession.mu via HasKeyMaterial/SetKeyMaterial so a concurrent ABCI-thread read cannot tear a slice header. Recovery and completion now run under a SINGLE continuous hold of the per-round lock. recoverActiveSessionKeyMaterial does the cap check, attempt increment, kernel finalize, key-match guards, and completion inline, and a lock-free completeSessionLocked helper (extracted from handleDKGComplete) advances the session to Completed without releasing and re-acquiring the lock. This closes the finalize/complete gap: previously the session was left PhaseFinalized with the lock free between the two acquisitions, a window in which ResumeDKGService's stuck-check (isSessionStuckForStage(PhaseFinalized, DKGStageActive)) could MarkFailed a recovery that had just succeeded. The cap/escalation now live inside the locked recovery, so escalation can never fire prematurely while an attempt still holds the lock and fires exactly once after the final attempt concludes.
0xHansLee
force-pushed
the
fix-resume-active-requires-key-material
branch
from
July 23, 2026 07:03
07b3e2d to
c3034fa
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
resumeFailedSessionforce-completed any failed active-round session, even one with emptyGlobalPubKey/PubKeyShare. The node marked itself finished while contributing zero partial decryptions.Fix
Complete only sessions that hold key material. A keyless active-round session is recovered node-locally: recompute and seal the share via the kernel (
callTEEFinalizeDKG, no on-chain finalize vote for an already-active round). The recovery is guarded by:tryAcquireDKGSvc), preventing concurrent recovery goroutines from racing on the shared session; andGlobalPubKeymatches the on-chain consensus key before completing.On failure the session is left failed and logged. Node-local only — no consensus/
KVStorestate is touched.Tests
TestResumeFailedSession_ActiveStage_RecoversKeyMaterial,_MissingKeyMaterial,_DivergentKeyTestRecoverActiveSessionKeyMaterial_ConcurrentSingleFinalize(-race)issue: none