feat(frost): conclude the signing done check on the t-subset (7.3 t-of-included PR3/3) (#4094)

## What

Completes RFC-21 Phase 7.3 t-of-included finalize. With PR2 (#4093) an
interactive signing attempt finalizes over the first `t` responsive
committers of an (optionally) oversized included set. The non-subset and
offline members never broadcast a signing done check, so the outer
`signingDoneCheck` — which required a confirmation from **every**
attempt member — would hang an otherwise-successful attempt to its
timeout and force a needless retry.

This concludes the done check on a **deterministic threshold quorum**,
keeping the legacy path byte-for-byte unchanged.

## Design (locked via a Codex + Gemini consult)

My first cut completed on "the first `t` arrivals," which is
**network-order-dependent**: the resulting `latestEndBlock` (a per-node
max over a network-order-dependent subset) differs across honest nodes,
and that value feeds batch scheduling (`signBatch`: `signingStartBlock =
prev endBlock + interlude`) → the batch desyncs. The consult locked this
design:

- **Non-oversized (`included == honestThreshold`; today's selector
output and the whole coarse path)**: the legacy all-members rule is
**UNCHANGED** — wait for every attempt member, require all signatures
equal, return `max(endBlock)`.
- **Oversized (`included > honestThreshold`)**: bucket done checks **by
signature**, conclude once a bucket holds `>= honestThreshold` distinct
senders (the minimum that proves a valid threshold signature).
**Minority buckets (divergent / adversarial signatures) are ignored,
never fatal** — one bad done message can't fracture the group. The end
block is the **deterministic `attemptTimeoutBlock`** (every honest node
computes it identically), not a max over done messages.

With honest majority (`t > groupSize/2`) at most one signature bucket
can reach `t` — even under coordinator equivocation (two disjoint
`t`-subsets can't coexist when `2t > n`) — so the quorum is unique. That
means **no body-hash / proto change** is needed (bucket by the existing
signature field), and **no `signBatch` change** (the oversized path
feeds `attemptTimeoutBlock` through the existing return). The
`>1`-quorum branch is unreachable and intentionally non-fatal (no noisy
post-success failure).

## Inert until oversizing

`included == honestThreshold` today, so the legacy branch is taken and
behavior is identical to before; the quorum path only activates once
selection oversizes the set (MacLane's policy knob).

## Tests (`signing_done_test.go`)

- Legacy `TestSigningDoneCheck{,_MissingConfirmation,_AnotherSignature}`
— unchanged, still green (they use an attempt-member set of size
`honestThreshold`).
- `TestSigningDoneCheck_ThresholdSubsetConcludes` — oversized; reporters
carry different end blocks but the result is the deterministic
`attemptTimeoutBlock`.
- `TestSigningDoneCheck_OversizedIgnoresMinorityDivergentSignature` —
`t` correct + 1 divergent → concludes on the quorum, ignores the
minority (no error).
- `TestSigningDoneCheck_OversizedSplitBelowQuorumTimesOut` — 2+2 split,
no bucket reaches `t` → times out.

Validated: build+vet across default / `frost_roast_retry` /
`frost_native frost_roast_retry` / CGO; the done-check + signing-loop +
roast-transition tests with `-race`; gofmt clean. The
`signingDoneCheckStrategy` interface + `mockSigningDoneCheck` are
untouched (only the constructor signature changed).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Author: mswilkison

pkg/tbtc/signing.go

@@ -373,6 +373,7 @@ func (se *signingExecutor) signWithTaprootMerkleRoot(
 
 			doneCheck := newSigningDoneCheck(
 				se.groupParameters.GroupSize,
+				se.groupParameters.HonestThreshold,
 				se.broadcastChannel,
 				se.membershipValidator,
 			)

pkg/tbtc/signing_done.go

@@ -47,23 +47,42 @@ func (sdm *signingDoneMessage) Type() string {
 // successful signature calculation across all signing group members.
 type signingDoneCheck struct {
 	groupSize           int
+	honestThreshold     int
 	broadcastChannel    net.BroadcastChannel
 	membershipValidator *group.MembershipValidator
 
-	receiveCtx           context.Context
-	cancelReceiveCtx     context.CancelFunc
-	expectedSignersCount int
-	doneSigners          map[group.MemberIndex]*signingDoneMessage
-	doneSignersMutex     sync.RWMutex
+	receiveCtx       context.Context
+	cancelReceiveCtx context.CancelFunc
+	// attemptMemberCount is len(attemptMembersIndexes) for the live attempt - the
+	// number of confirmations the legacy (non-oversized) rule waits for.
+	attemptMemberCount int
+	// oversized is true when the attempt's included set is larger than the honest
+	// threshold: the RFC-21 Phase 7.3 t-of-included case, where the attempt is
+	// signed by a t-subset so the non-subset / offline members never report done
+	// and the legacy all-members rule would hang. It selects the
+	// quorum-by-signature completion rule. When false (included == threshold, the
+	// pre-oversizing selector output and the whole coarse path) the legacy rule is
+	// used UNCHANGED.
+	oversized bool
+	// attemptTimeoutBlock is the deterministic block the attempt concludes by
+	// (announcementEndBlock + signingAttemptMaximumProtocolBlocks). On the oversized
+	// path it is returned as the result end block instead of a network-order-
+	// dependent max over done messages, so every honest node feeds signBatch the
+	// same next-signature start block (signingStartBlock = prev endBlock + interlude).
+	attemptTimeoutBlock uint64
+	doneSigners         map[group.MemberIndex]*signingDoneMessage
+	doneSignersMutex    sync.RWMutex
 }
 
 func newSigningDoneCheck(
 	groupSize int,
+	honestThreshold int,
 	broadcastChannel net.BroadcastChannel,
 	membershipValidator *group.MembershipValidator,
 ) *signingDoneCheck {
 	return &signingDoneCheck{
 		groupSize:           groupSize,
+		honestThreshold:     honestThreshold,
 		broadcastChannel:    broadcastChannel,
 		membershipValidator: membershipValidator,
 	}
@@ -91,7 +110,17 @@ func (sdc *signingDoneCheck) listen(
 	sdc.receiveCtx, sdc.cancelReceiveCtx = context.WithCancel(ctx)
 
 	sdc.doneSignersMutex.Lock()
-	sdc.expectedSignersCount = len(attemptMembersIndexes)
+	sdc.attemptMemberCount = len(attemptMembersIndexes)
+	// An included set larger than the honest threshold is the RFC-21 Phase 7.3
+	// t-of-included case: the attempt is signed by a t-subset and the non-subset /
+	// offline members never report done, so the all-members rule would hang. The
+	// oversized path concludes on a quorum of >= honestThreshold matching
+	// signatures instead, with a deterministic end block. included == threshold
+	// (today's selector output and the whole coarse path) keeps the legacy rule
+	// byte-for-byte, so behavior is unchanged until participant selection oversizes
+	// the set.
+	sdc.oversized = len(attemptMembersIndexes) > sdc.honestThreshold
+	sdc.attemptTimeoutBlock = attemptTimeoutBlock
 	sdc.doneSigners = make(map[group.MemberIndex]*signingDoneMessage)
 	sdc.doneSignersMutex.Unlock()
 
@@ -149,12 +178,12 @@ func (sdc *signingDoneCheck) signalDone(
 	}, net.BackoffRetransmissionStrategy)
 }
 
-// waitUntilAllDone blocks until it receives all the required done checks from
-// members or until the passed context is done. In the first case, it returns
-// the signature computed by the signing members and the block at which the
-// slowest signer completed the signature computation process. If the expected
-// done checks are not received on time, the function returns an error. If at
-// least one signature is different from others, the function returns an error.
+// waitUntilAllDone blocks until the attempt's completion rule is met or the
+// passed context is done. On success it returns the agreed signature and a
+// deterministic end block (the same value on every honest node): on the legacy
+// path the block at which the slowest attempt member completed, on the oversized
+// path the attempt timeout block. It returns errWaitDoneTimedOut if the rule is
+// not met on time, and a non-nil error on a fatal divergence (legacy path only).
 func (sdc *signingDoneCheck) waitUntilAllDone(ctx context.Context) (
 	*signing.Result,
 	uint64,
@@ -171,35 +200,123 @@ func (sdc *signingDoneCheck) waitUntilAllDone(ctx context.Context) (
 			return nil, 0, errWaitDoneTimedOut
 
 		case <-ticker.C:
-			expectedSignersCount, doneSigners := sdc.snapshotDoneSigners()
-			if expectedSignersCount == len(doneSigners) {
-				var signature *frost.Signature
-				var latestEndBlock uint64
-
-				for _, doneMessage := range doneSigners {
-					if signature == nil {
-						signature = doneMessage.signature
-					} else {
-						if !signature.Equals(doneMessage.signature) {
-							return nil, 0, fmt.Errorf(
-								"not matching signatures detected: [%v] and [%v]",
-								signature,
-								doneMessage.signature,
-							)
-						}
-					}
-
-					if doneMessage.endBlock > latestEndBlock {
-						latestEndBlock = doneMessage.endBlock
-					}
-				}
-
-				return &signing.Result{Signature: signature}, latestEndBlock, nil
+			result, endBlock, concluded, err := sdc.evaluateDone()
+			if err != nil {
+				return nil, 0, err
+			}
+			if concluded {
+				return result, endBlock, nil
 			}
 		}
 	}
 }
 
+// evaluateDone snapshots the done checks collected so far and applies the
+// attempt's completion rule. It returns (result, endBlock, true, nil) once the
+// attempt can conclude, (nil, 0, false, nil) while still waiting, or
+// (nil, 0, false, err) on a fatal divergence. The legacy (non-oversized) rule
+// and the oversized t-of-included rule are kept fully separate so the legacy /
+// coarse path is byte-for-byte unchanged.
+func (sdc *signingDoneCheck) evaluateDone() (*signing.Result, uint64, bool, error) {
+	sdc.doneSignersMutex.RLock()
+	oversized := sdc.oversized
+	attemptMemberCount := sdc.attemptMemberCount
+	attemptTimeoutBlock := sdc.attemptTimeoutBlock
+	honestThreshold := sdc.honestThreshold
+	doneSigners := make([]*signingDoneMessage, 0, len(sdc.doneSigners))
+	for _, doneMessage := range sdc.doneSigners {
+		doneSigners = append(doneSigners, doneMessage.clone())
+	}
+	sdc.doneSignersMutex.RUnlock()
+
+	if oversized {
+		return concludeOversizedDone(doneSigners, honestThreshold, attemptTimeoutBlock)
+	}
+	return concludeLegacyDone(doneSigners, attemptMemberCount)
+}
+
+// concludeLegacyDone is the pre-7.3 rule, UNCHANGED: conclude once every attempt
+// member confirmed, require all signatures equal, and return the max end block.
+// The attemptMemberCount > 0 guard rejects the pre-listen state (no attempt
+// configured) so an empty done set is never read as success.
+func concludeLegacyDone(
+	doneSigners []*signingDoneMessage,
+	attemptMemberCount int,
+) (*signing.Result, uint64, bool, error) {
+	if attemptMemberCount == 0 || len(doneSigners) != attemptMemberCount {
+		return nil, 0, false, nil
+	}
+
+	var signature *frost.Signature
+	var latestEndBlock uint64
+	for _, doneMessage := range doneSigners {
+		if signature == nil {
+			signature = doneMessage.signature
+		} else if !signature.Equals(doneMessage.signature) {
+			return nil, 0, false, fmt.Errorf(
+				"not matching signatures detected: [%v] and [%v]",
+				signature,
+				doneMessage.signature,
+			)
+		}
+
+		if doneMessage.endBlock > latestEndBlock {
+			latestEndBlock = doneMessage.endBlock
+		}
+	}
+
+	return &signing.Result{Signature: signature}, latestEndBlock, true, nil
+}
+
+// concludeOversizedDone is the RFC-21 Phase 7.3 t-of-included rule: bucket the
+// done checks by signature (one done message per sender, so each member is in
+// exactly one bucket) and conclude once a bucket holds >= honestThreshold
+// distinct senders - the minimum that proves a valid threshold signature.
+// Minority buckets (divergent or adversarial signatures) are IGNORED, never
+// fatal, so a single bad done message cannot fracture the group. The end block
+// is the deterministic attempt timeout block, not a network-order-dependent max,
+// so every honest node returns the same value for batch scheduling.
+func concludeOversizedDone(
+	doneSigners []*signingDoneMessage,
+	honestThreshold int,
+	attemptTimeoutBlock uint64,
+) (*signing.Result, uint64, bool, error) {
+	if honestThreshold <= 0 {
+		return nil, 0, false, nil
+	}
+
+	bucketSig := map[string]*frost.Signature{}
+	bucketCount := map[string]int{}
+	for _, doneMessage := range doneSigners {
+		serialized := doneMessage.signature.Serialize()
+		key := string(serialized[:])
+		if _, ok := bucketSig[key]; !ok {
+			bucketSig[key] = doneMessage.signature
+		}
+		bucketCount[key]++
+	}
+
+	var quorumSig *frost.Signature
+	quorums := 0
+	for key, count := range bucketCount {
+		if count >= honestThreshold {
+			quorums++
+			quorumSig = bucketSig[key]
+		}
+	}
+
+	// Exactly one >= t bucket is the only reachable outcome under honest majority
+	// (honestThreshold > groupSize/2 means two disjoint >= t buckets cannot
+	// coexist). It carries the one valid signature and concludes deterministically.
+	// quorums == 0 keeps waiting; quorums > 1 is unreachable and intentionally NOT
+	// concluded (the attempt fails via the ctx timeout rather than picking a bucket
+	// nondeterministically - a bare done-message split is not coordinator blame).
+	if quorums == 1 {
+		return &signing.Result{Signature: quorumSig}, attemptTimeoutBlock, true, nil
+	}
+	return nil, 0, false, nil
+}
+
 // isValidDoneMessage validates the given signingDoneMessage in the context
 // of the given signing attempt.
 func (sdc *signingDoneCheck) isValidDoneMessage(
@@ -250,21 +367,6 @@ func (sdc *signingDoneCheck) recordDoneMessage(
 	return true
 }
 
-func (sdc *signingDoneCheck) snapshotDoneSigners() (
-	int,
-	[]*signingDoneMessage,
-) {
-	sdc.doneSignersMutex.RLock()
-	defer sdc.doneSignersMutex.RUnlock()
-
-	result := make([]*signingDoneMessage, 0, len(sdc.doneSigners))
-	for _, doneMessage := range sdc.doneSigners {
-		result = append(result, doneMessage.clone())
-	}
-
-	return sdc.expectedSignersCount, result
-}
-
 func (sdm *signingDoneMessage) clone() *signingDoneMessage {
 	if sdm == nil {
 		return nil