diff --git a/pkg/frost/roast/multi_coordinator_soak_test.go b/pkg/frost/roast/multi_coordinator_soak_test.go index 18caa65e5c..333c68a75f 100644 --- a/pkg/frost/roast/multi_coordinator_soak_test.go +++ b/pkg/frost/roast/multi_coordinator_soak_test.go @@ -347,14 +347,16 @@ func TestSoak_ParkedMemberIsReinstatedNextAttempt(t *testing.T) { } } -func TestSoak_InfeasibilityWhenBelowThreshold(t *testing.T) { +func TestSoak_TransientSilenceBelowThresholdRecovers(t *testing.T) { members := []group.MemberIndex{1, 2, 3, 4, 5} nodes := newSoakHarness(t, members) prev := soakStartingContext(t, members) - // Threshold = 5 (all members required). Silence two members. - // Next attempt's IncludedSet would be 3 (= 5 - 2 silenced), below 5. - // NextAttempt must return ErrAttemptInfeasible. + // Threshold = 5 (all members required). Silence two members, dropping the + // next attempt's IncludedSet to 3 (= 5 - 2 silenced), below threshold. No + // member is permanently excluded, so NextAttempt must NOT fail: each honest + // node transiently parks the silenced members for reinstatement by a later + // attempt. silence := map[group.MemberIndex]bool{ 4: true, 5: true, @@ -393,15 +395,29 @@ func TestSoak_InfeasibilityWhenBelowThreshold(t *testing.T) { } bundle, _ := aggregator.node.coord.AggregateBundle(aggregator.handle) - // Verify each non-coordinator's NextAttempt returns infeasible. + // Each honest node's NextAttempt must succeed (not fail closed) and park the + // silenced members transiently rather than excluding them permanently. for _, b := range begins { - _, err := b.node.coord.NextAttempt(b.handle, bundle, 5, []byte{0x01}) - if !errors.Is(err, ErrAttemptInfeasible) { + next, err := b.node.coord.NextAttempt(b.handle, bundle, 5, []byte{0x01}) + if err != nil { t.Fatalf( - "node %d NextAttempt: expected ErrAttemptInfeasible; got %v", + "node %d NextAttempt: transient silence must not fail the session; got %v", b.node.self, err, ) } + if !memberSliceContains(next.TransientlyParked, 4) || + !memberSliceContains(next.TransientlyParked, 5) { + t.Fatalf( + "node %d: silenced members 4 and 5 must be transiently parked; got %v", + b.node.self, next.TransientlyParked, + ) + } + if len(next.ExcludedSet) != 0 { + t.Fatalf( + "node %d: silence must not permanently exclude; got %v", + b.node.self, next.ExcludedSet, + ) + } } } diff --git a/pkg/frost/roast/next_attempt.go b/pkg/frost/roast/next_attempt.go index 42d4aa63d1..1865be1d5f 100644 --- a/pkg/frost/roast/next_attempt.go +++ b/pkg/frost/roast/next_attempt.go @@ -59,13 +59,19 @@ func ExclusionAccuserQuorum(groupSize, threshold uint) uint { return groupSize - threshold + 1 } -// ErrAttemptInfeasible is returned by NextAttempt when the next -// attempt's IncludedSet would drop below the signing threshold t and -// the session can no longer make progress with the original signer -// set. Callers must surface this to the application layer: the -// session is permanently failed. +// ErrAttemptInfeasible is returned by NextAttempt when PERMANENT exclusions +// leave fewer than threshold non-excluded members, so no future attempt can +// ever reach the signing threshold t and the session can no longer make +// progress with the original signer set. Callers must surface this to the +// application layer: the session is permanently failed. +// +// It is NOT returned merely because the next attempt's IncludedSet falls below +// threshold due to TRANSIENT parking (silence/overflow): parked members are +// reinstated on the following attempt, so a sub-threshold included set burns +// one attempt rather than failing the session. The infeasibility test is on the +// non-excluded (feasible) set, not the post-parking included set. var ErrAttemptInfeasible = errors.New( - "coordinator: next attempt is infeasible -- included set below threshold", + "coordinator: next attempt is infeasible -- non-excluded members below threshold", ) // NextAttempt computes the deterministic next attempt context from a @@ -117,8 +123,12 @@ var ErrAttemptInfeasible = errors.New( // TransientlyParked set automatically rejoin the next attempt's // IncludedSet (unless they are now permanently excluded). // -// 6. Infeasibility: if the next attempt's IncludedSet would have -// fewer than threshold members, return ErrAttemptInfeasible. +// 6. Infeasibility: if PERMANENT exclusions leave fewer than threshold +// non-excluded members -- so no future attempt can reach the +// threshold -- return ErrAttemptInfeasible. A next IncludedSet that +// falls below threshold only because of TRANSIENT parking is NOT +// infeasible: the parked members rejoin on the following attempt +// (step 5), so it burns one attempt rather than failing the session. // // Verifiability roadmap: permanent exclusion on a *single* piece of // evidence becomes sound once the wire format carries @@ -242,23 +252,47 @@ func computeNextAttempt( // (5) Reinstate previously parked members by re-including them // (unless newly permanently excluded or re-parked). - included := original.sorted() - included = filterOut(included, exclSet) - included = filterOut(included, parkSet) - - // (6) Infeasibility check. - if threshold > 0 && uint(len(included)) < threshold { + // + // Feasibility (step 6) is judged against the PERMANENTLY-available set -- + // the original signer set minus permanent exclusions -- NOT the + // post-parking included set. Transiently-parked members (overflow and + // silence) are reinstated by a later attempt (steps 3-4), so counting them + // as gone here would let a single transient mass-silence event, or one + // byzantine elected coordinator that omits snapshots, permanently fail a + // session the original signer set can still complete -- exactly the + // grind-to-ErrAttemptInfeasible failure the accuser quorum exists to + // prevent, here via the ungated silence-parking path. Permanent exclusion + // is the only thing that can render a session infeasible. + feasible := filterOut(original.sorted(), exclSet) + + // (6) Infeasibility check: only when permanent exclusions leave fewer than + // threshold members can no future attempt ever reach the threshold. + if threshold > 0 && uint(len(feasible)) < threshold { return attempt.AttemptContext{}, fmt.Errorf( - "%w: %d eligible, threshold %d", + "%w: %d non-excluded, threshold %d", ErrAttemptInfeasible, - len(included), + len(feasible), threshold, ) } + // The next attempt's included set is the feasible set minus this attempt's + // transient parking. It may fall below threshold (the parked members are + // reinstated next attempt); that burns one attempt rather than failing the + // session. + included := filterOut(feasible, parkSet) + nextParked := parkSet.sorted() + if len(included) == 0 { + // Every non-excluded member is transiently parked this attempt. Parking + // is strictly transient and an AttemptContext requires a non-empty + // included set, so reinstate the parked members now (they retry this + // attempt) rather than producing an empty, unconstructable attempt. + included = feasible + nextParked = nil + } + // Convert ExcludedSet to its canonical (sorted, deduped) slice. nextExcluded := exclSet.sorted() - nextParked := parkSet.sorted() next, err := attempt.NewAttemptContextWithParking( prev.SessionID, diff --git a/pkg/frost/roast/next_attempt_test.go b/pkg/frost/roast/next_attempt_test.go index 216941747a..1723f6d3ed 100644 --- a/pkg/frost/roast/next_attempt_test.go +++ b/pkg/frost/roast/next_attempt_test.go @@ -387,18 +387,101 @@ func TestNextAttempt_PolicyIsDeterministic(t *testing.T) { } } -func TestNextAttempt_InfeasibilityWhenBelowThreshold(t *testing.T) { +func TestNextAttempt_InfeasibilityWhenPermanentExclusionsBelowThreshold(t *testing.T) { f := newNextAttemptFixture() - f.threshold = 5 // Require all 5 members. - // Silently lose 2 members -> only 3 remain in IncludedSet, below - // threshold of 5. - f.bundleSenders = []group.MemberIndex{1, 2, 3} + f.threshold = 5 // n-of-n: the accuser quorum is 1, so one reject establishes. + // Permanently exclude member 3 via an established reject accusation. Only 4 + // non-excluded members remain, below the threshold of 5, and a permanently + // excluded member is never reinstated -- so the session is genuinely + // infeasible. + f.rejects[1] = map[group.MemberIndex]uint{3: 1} _, err := computeNextAttempt(f.prev(t), f.bundle(t), f.threshold, f.dkgGroupPublicKey, fakeVerifier{}) if !errors.Is(err, ErrAttemptInfeasible) { t.Fatalf("expected ErrAttemptInfeasible, got %v", err) } } +func TestNextAttempt_TransientSilenceBelowThresholdDoesNotPermanentlyFail(t *testing.T) { + f := newNextAttemptFixture() + f.threshold = 5 // Require all 5 members. + // Silently lose 2 members -> the next attempt's included set drops to 3 + // (below threshold), but NO member is permanently excluded. The session + // must NOT be declared infeasible: the silenced members are transiently + // parked and a later attempt reinstates them. + f.bundleSenders = []group.MemberIndex{1, 2, 3} + next, err := computeNextAttempt(f.prev(t), f.bundle(t), f.threshold, f.dkgGroupPublicKey, fakeVerifier{}) + if err != nil { + t.Fatalf("transient silence must not permanently fail the session, got %v", err) + } + if !memberSliceContains(next.TransientlyParked, 4) || + !memberSliceContains(next.TransientlyParked, 5) { + t.Fatalf( + "silenced members 4 and 5 must be transiently parked; got parked %v", + next.TransientlyParked, + ) + } + if len(next.ExcludedSet) != 0 { + t.Fatalf( + "transient silence must not permanently exclude; got excluded %v", + next.ExcludedSet, + ) + } +} + +func TestNextAttempt_TransientSilenceRecoversAcrossTwoAttempts(t *testing.T) { + f := newNextAttemptFixture() + f.threshold = 5 + + // Attempt N -> N+1: members 4 and 5 are silent. They are parked (not + // excluded) and the next attempt's included set is {1,2,3} (sub-threshold), + // but the session is not failed. + f.bundleSenders = []group.MemberIndex{1, 2, 3} + attemptN1, err := computeNextAttempt( + f.prev(t), f.bundle(t), f.threshold, f.dkgGroupPublicKey, fakeVerifier{}, + ) + if err != nil { + t.Fatalf("attempt N+1: transient silence must not fail, got %v", err) + } + if !memberSliceContains(attemptN1.TransientlyParked, 4) || + !memberSliceContains(attemptN1.TransientlyParked, 5) { + t.Fatalf( + "attempt N+1: members 4 and 5 must be parked; got %v", + attemptN1.TransientlyParked, + ) + } + + // Attempt N+1 -> N+2: the previously-parked members are reinstated, so the + // included set returns to all five and the session recovers without + // intervention. The bundle defaults to N+1's included set {1,2,3}, which all + // respond. + g := newNextAttemptFixture() + g.threshold = 5 + g.included = attemptN1.IncludedSet + g.excluded = attemptN1.ExcludedSet + g.parked = attemptN1.TransientlyParked + g.attemptNumber = attemptN1.AttemptNumber + attemptN2, err := computeNextAttempt( + g.prev(t), g.bundle(t), g.threshold, g.dkgGroupPublicKey, fakeVerifier{}, + ) + if err != nil { + t.Fatalf("attempt N+2: %v", err) + } + for _, m := range []group.MemberIndex{1, 2, 3, 4, 5} { + if !memberSliceContains(attemptN2.IncludedSet, m) { + t.Fatalf( + "attempt N+2 must reinstate all members; missing %d, got included %v", + m, attemptN2.IncludedSet, + ) + } + } + if len(attemptN2.TransientlyParked) != 0 { + t.Fatalf( + "attempt N+2 should have no parked members; got %v", + attemptN2.TransientlyParked, + ) + } +} + func TestNextAttempt_ThresholdZeroDisablesInfeasibilityCheck(t *testing.T) { f := newNextAttemptFixture() f.threshold = 0