diff --git a/pkg/frost/signing/roast_runner_engine_frost_native_test.go b/pkg/frost/signing/roast_runner_engine_frost_native_test.go index 7ecdfdf6bd..45b9eb9e94 100644 --- a/pkg/frost/signing/roast_runner_engine_frost_native_test.go +++ b/pkg/frost/signing/roast_runner_engine_frost_native_test.go @@ -33,15 +33,16 @@ type fakeInteractiveSigningEngine struct { commitmentsByMember map[uint16][]byte shareByMember map[uint16][]byte - mu sync.Mutex - openCalls int - round1Calls int - newPackageCalls int - round2Calls int - aggregateCalls int - abortCalls int - deriveCalls int - lastAggregateShares []nativeFROSTSignatureShare + mu sync.Mutex + openCalls int + round1Calls int + newPackageCalls int + round2Calls int + aggregateCalls int + abortCalls int + deriveCalls int + lastAggregateShares []nativeFROSTSignatureShare + lastNewPackageCommitments []nativeFROSTCommitment } func (f *fakeInteractiveSigningEngine) DeriveInteractiveAttemptContext( @@ -93,6 +94,20 @@ func (f *fakeInteractiveSigningEngine) abortCallCount() int { return f.abortCalls } +func (f *fakeInteractiveSigningEngine) round2CallCount() int { + f.mu.Lock() + defer f.mu.Unlock() + return f.round2Calls +} + +// newPackageCommitments returns a copy of the commitments the engine last built a +// signing package over - the chosen t-subset for the coordinator. +func (f *fakeInteractiveSigningEngine) newPackageCommitments() []nativeFROSTCommitment { + f.mu.Lock() + defer f.mu.Unlock() + return append([]nativeFROSTCommitment(nil), f.lastNewPackageCommitments...) +} + func newFakeInteractiveSigningEngine() *fakeInteractiveSigningEngine { return &fakeInteractiveSigningEngine{ attemptID: "attempt-1", @@ -153,6 +168,7 @@ func (f *fakeInteractiveSigningEngine) NewSigningPackage( ) ([]byte, error) { f.mu.Lock() f.newPackageCalls++ + f.lastNewPackageCommitments = append([]nativeFROSTCommitment(nil), commitments...) f.mu.Unlock() return f.signingPackage, nil } diff --git a/pkg/frost/signing/roast_runner_frost_native.go b/pkg/frost/signing/roast_runner_frost_native.go index a943d4f393..fd4d87d4ac 100644 --- a/pkg/frost/signing/roast_runner_frost_native.go +++ b/pkg/frost/signing/roast_runner_frost_native.go @@ -6,6 +6,7 @@ import ( "bytes" "context" "fmt" + "sort" "github.com/keep-network/keep-core/pkg/frost/roast" "github.com/keep-network/keep-core/pkg/frost/roast/attempt" @@ -30,9 +31,11 @@ import ( // only as sound as that authentication. // 2. Delivery must not let a slow or flooding peer block an honest broadcaster // indefinitely. The runner does not fully drain every stream - it bounds the -// equivocation drains, and the coordinator never reads its own package -// stream - so the transport must apply backpressure or drop, never block -// forever, on an undrained or oversubscribed stream. +// equivocation drains, the coordinator never reads its own package stream, +// and for t-of-included finalize the coordinator stops reading commitments +// once t have arrived while non-coordinators never read commitments at all - +// so the transport must apply backpressure or drop, never block forever, on +// an undrained or oversubscribed stream. // // Round-1 commitments are unsigned: with authenticated senders the worst case is // a member's own bad or equivocated commitment, which surfaces as a round-2 @@ -47,11 +50,17 @@ type interactiveSigningRunner struct { // message inconsistent with the attempt it is bound to. messageDigest []byte threshold uint16 - engine interactiveSigningEngine - collector *roast.Round2Collector - coordinator roast.Coordinator - signer roast.Signer - bus RunnerBus + // includedMembers is the attempt's included set as a lookup, cached at + // construction. It gates which shares the collector retains as evidence (any + // included member's, even a non-signer observer's divergent share), distinct + // from the per-attempt signer set that gates which shares count toward the + // aggregate. + includedMembers map[group.MemberIndex]struct{} + engine interactiveSigningEngine + collector *roast.Round2Collector + coordinator roast.Coordinator + signer roast.Signer + bus RunnerBus // sub is established at construction (before any Run broadcasts) so a node // never misses a peer message broadcast before it subscribed. sub *RunnerBusSubscriber @@ -89,21 +98,33 @@ func newInteractiveSigningRunner( "roast runner: member %d is not in the attempt's included set", member, ) } + // The coordinator's round-1 collection proceeds until t responsive committers, + // so t must not exceed the included set - otherwise the subset can never form + // and the coordinator would block to the ctx deadline on every attempt. A + // well-formed attempt always selects at least threshold members; fail fast at + // construction rather than silently degrade into timeout-driven retries. + if int(threshold) > len(attemptCtx.IncludedSet) { + return nil, fmt.Errorf( + "roast runner: threshold [%d] exceeds the attempt's included set size [%d]", + threshold, len(attemptCtx.IncludedSet), + ) + } // The signed message is the binding's MessageDigest, derived here rather than // accepted as a parameter: a caller-supplied message that diverged from the // digest the attempt (and its package/share envelopes) is bound to could mark // an attempt for digest A succeeded with a signature over digest B. return &interactiveSigningRunner{ - attempt: attempt, - member: member, - messageDigest: append([]byte(nil), attemptCtx.MessageDigest[:]...), - threshold: threshold, - engine: engine, - collector: collector, - coordinator: coordinator, - signer: signer, - bus: bus, - sub: bus.Subscribe(), + attempt: attempt, + member: member, + messageDigest: append([]byte(nil), attemptCtx.MessageDigest[:]...), + threshold: threshold, + includedMembers: setOf(attemptCtx.IncludedSet), + engine: engine, + collector: collector, + coordinator: coordinator, + signer: signer, + bus: bus, + sub: bus.Subscribe(), }, nil } @@ -169,21 +190,32 @@ func (r *interactiveSigningRunner) Run(ctx context.Context) ([]byte, error) { attemptID := open.AttemptID // Cleanup on conclusion, by outcome: - // - SUCCESS: prune this attempt's round-2 collector state per the + // - SUCCESS as a SIGNER: prune this attempt's round-2 collector state per the // prune-on-conclusion contract (nothing needs it), so a collector reused - // across attempts does not retain concluded attempts indefinitely. - // - FAILURE / early exit: abort the engine session so it drops this - // attempt's resident secret nonces, but do NOT prune the collector. A - // failure path (the root-divergence return below, or - once it lands - an - // aggregate share-verification failure) retains signed evidence that the - // blame/retry path must still read via CoordinatorPackageProofs / - // ClassifyCandidateCulprits; the caller prunes after snapshotting or - // propagating it. + // across attempts does not retain concluded attempts indefinitely. Round 2 + // already consumed our round-1 nonces, so no abort is needed. + // - SUCCESS as an OBSERVER: a member that committed in round 1 but was not in + // the chosen signing subset obtains the signature by aggregating the + // subset's broadcast shares WITHOUT signing, so its round-1 nonces are + // still resident in the engine. Prune the collector AND abort the session + // to drop those nonces - the success branch otherwise suppresses the abort. + // - FAILURE / early exit: abort the engine session so it drops this attempt's + // resident secret nonces, but do NOT prune the collector. A failure path + // (the root-divergence return below, or an aggregate share-verification + // failure) retains signed evidence that the blame/retry path must still + // read via CoordinatorPackageProofs / ClassifyCandidateCulprits; the caller + // prunes after snapshotting or propagating it. // Best-effort: neither may mask the run's real outcome. succeeded := false + // signedRound2 records whether this node ran round 2 and thereby consumed its + // round-1 nonces; an observer never does, so it must still abort on success. + signedRound2 := false defer func() { if succeeded { r.collector.PruneAttempt(contextHash[:]) + if !signedRound2 { + _, _ = r.engine.InteractiveSessionAbort(binding.SessionID(), &attemptID) + } return } _, _ = r.engine.InteractiveSessionAbort(binding.SessionID(), &attemptID) @@ -202,10 +234,16 @@ func (r *interactiveSigningRunner) Run(ctx context.Context) ([]byte, error) { } r.broadcast(RunnerMsgCommitments, contextHash, ownCommitments) - // 4. Collect every included member's commitments. + // 4. Only the elected coordinator collects commitments - it alone builds the + // signing package. It gathers the first t responsive committers (its own + // already seeded) and builds the package over exactly that subset, so a member + // past the t-th to commit (slow or offline) never stalls the attempt. Every + // other member broadcast its own commitment above and now awaits the package. commitments := map[group.MemberIndex][]byte{r.member: ownCommitments} - if err := r.collectCommitments(ctx, r.sub.Commitments(), contextHash, includedSet, commitments); err != nil { - return nil, fmt.Errorf("roast runner: collect commitments: %w", err) + if r.member == elected { + if err := r.collectCommitments(ctx, r.sub.Commitments(), contextHash, includedSet, commitments); err != nil { + return nil, fmt.Errorf("roast runner: collect commitments: %w", err) + } } // 5. The elected coordinator builds, signs, and broadcasts the signing @@ -240,31 +278,55 @@ func (r *interactiveSigningRunner) Run(ctx context.Context) ([]byte, error) { return nil, fmt.Errorf("roast runner: signing package taproot root diverges from the bound session root") } - // 6. Round 2: our signature share, recorded locally and broadcast. - ownShare, err := r.engine.InteractiveRound2(binding.SessionID(), attemptID, uint16(r.member), pkg.SigningPackageBytes) - if err != nil { - return nil, fmt.Errorf("roast runner: round 2: %w", err) - } - ownSubmission, ownSubmissionEnvelope, err := r.signShareSubmission(pkg, contextHash, elected, ownShare) - if err != nil { - return nil, err - } - if err := r.collector.RecordShareSubmission(ownSubmission); err != nil { - return nil, fmt.Errorf("roast runner: record own share submission: %w", err) + // 6. The package names the chosen signing subset (the first t responsive + // committers the coordinator built it over). A local member IN that subset is + // a signer; one NOT in it is an OBSERVER - it committed in round 1 but was not + // chosen, so it does not sign and only aggregates the subset's broadcast + // shares. An empty signer set is the full-included flow (back-compat / no + // oversizing): every included member signs. SignerIDs() is safe here - + // Unmarshal and AuthenticateSigningPackage (via RecordSigningPackage) both + // Validate, so each id is a real, ascending, distinct member index. + signerSet := pkg.SignerIDs() + if len(signerSet) == 0 { + signerSet = includedSet + } + + // 7. Round 2 (signers only): our signature share, recorded locally and + // broadcast. An observer skips round 2 entirely, leaving its round-1 nonces + // resident - the cleanup defer aborts them on success. + shares := map[group.MemberIndex][]byte{} + if memberInSet(r.member, signerSet) { + ownShare, err := r.engine.InteractiveRound2(binding.SessionID(), attemptID, uint16(r.member), pkg.SigningPackageBytes) + if err != nil { + return nil, fmt.Errorf("roast runner: round 2: %w", err) + } + // Round 2 consumed our round-1 nonces: a successful signer prunes without + // aborting; only a non-signing observer still needs the abort. + signedRound2 = true + ownSubmission, ownSubmissionEnvelope, err := r.signShareSubmission(pkg, contextHash, elected, ownShare) + if err != nil { + return nil, err + } + if err := r.collector.RecordShareSubmission(ownSubmission); err != nil { + return nil, fmt.Errorf("roast runner: record own share submission: %w", err) + } + r.broadcast(RunnerMsgShareSubmission, contextHash, ownSubmissionEnvelope) + shares[r.member] = ownShare } - r.broadcast(RunnerMsgShareSubmission, contextHash, ownSubmissionEnvelope) - // 7. Collect a share from every signer in the package (own already in), as - // inner FROST share bytes. The package was built from the whole responsive - // included set, so the aggregate needs a share from each of them (silent- - // member subsetting is the retry path's concern, not the happy flow). - shares := map[group.MemberIndex][]byte{r.member: ownShare} - if err := r.collectShares(ctx, r.sub.Shares(), contextHash, includedSet, shares); err != nil { + // 8. Collect a round-2 share from every member in the chosen signing subset (a + // signer already has its own; an observer collects all t), as inner FROST + // share bytes. A subset signer that never broadcasts a share stalls the + // attempt to the ctx deadline -> fail -> the existing ROAST retry path. + if err := r.collectShares(ctx, r.sub.Shares(), contextHash, signerSet, shares); err != nil { return nil, fmt.Errorf("roast runner: collect shares: %w", err) } - // 8. Aggregate. A share-verification failure surfaces the typed error with - // candidate culprits for the (separate) blame path. + // 9. Aggregate the subset's shares. Aggregation is a public operation over the + // package and the t broadcast shares, so signers and observers alike obtain + // the same signature; an observer aggregates against its own open session + // without having contributed a share. A share-verification failure surfaces + // the typed error with candidate culprits for the (separate) blame path. signature, err := r.engine.InteractiveAggregate( binding.SessionID(), attemptID, @@ -276,13 +338,13 @@ func (r *interactiveSigningRunner) Run(ctx context.Context) ([]byte, error) { return nil, fmt.Errorf("roast runner: aggregate: %w", err) } - // 9. Mark the attempt succeeded so the cleanup path produces no transition + // 10. Mark the attempt succeeded so the cleanup path produces no transition // bundle for a completed attempt. if err := r.coordinator.MarkSucceeded(binding.Handle()); err != nil { return nil, fmt.Errorf("roast runner: mark succeeded: %w", err) } - // Aggregation consumed the nonces and the attempt is finalized; suppress the - // deferred abort. + // The attempt is finalized; the cleanup defer prunes. A signer's nonces are + // spent so it does not abort; an observer aborts via the !signedRound2 branch. succeeded = true return signature, nil } @@ -338,7 +400,13 @@ func (r *interactiveSigningRunner) obtainSigningPackage( if err != nil { return nil, fmt.Errorf("roast runner: new signing package: %w", err) } - envelope, err := r.signSigningPackage(contextHash, elected, frostPackage) + // The chosen signing subset is exactly the members whose commitments the + // package was built over (the first t responsive committers). Carry it in + // signer_ids so non-coordinators learn which members to await shares from and + // which committed members are observers. The FROST SigningPackageBytes is the + // cryptographic source of truth, so this is a liveness hint, never blame. + signerIDs := sortedMemberIndices(commitments) + envelope, err := r.signSigningPackage(contextHash, elected, signerIDs, frostPackage) if err != nil { return nil, err } @@ -349,12 +417,14 @@ func (r *interactiveSigningRunner) obtainSigningPackage( func (r *interactiveSigningRunner) signSigningPackage( contextHash [attempt.MessageDigestLength]byte, elected group.MemberIndex, + signerIDs []group.MemberIndex, frostPackage []byte, ) ([]byte, error) { pkg := &roast.SigningPackage{ AttemptContextHash: append([]byte(nil), contextHash[:]...), CoordinatorIDValue: uint32(elected), SigningPackageBytes: frostPackage, + SignerIDsValue: memberIndicesToUint32(signerIDs), } if root := r.attempt.TaprootMerkleRoot(); root != nil { pkg.TaprootMerkleRoot = append([]byte(nil), root[:]...) @@ -408,8 +478,14 @@ func (r *interactiveSigningRunner) signShareSubmission( return sub, envelope, nil } -// collectCommitments fills `into` with every included member's commitments -// (own already seeded), taking the first body per sender. +// collectCommitments (coordinator only) fills `into` with the first t responsive +// committers' round-1 commitments - the coordinator's own already seeded, then +// the first t-1 included peers to arrive - and STOPS at t (r.threshold). The +// coordinator builds the FROST package over exactly this t-subset, so a member +// past the t-th to commit (slow or offline) never stalls the attempt: collection +// proceeds the instant t have committed. If ctx expires first (fewer than t ever +// commit), the run fails into the existing ROAST retry path. t <= len(included) +// always, so the loop terminates once t included members have committed. func (r *interactiveSigningRunner) collectCommitments( ctx context.Context, stream <-chan RunnerMessage, @@ -418,7 +494,7 @@ func (r *interactiveSigningRunner) collectCommitments( into map[group.MemberIndex][]byte, ) error { included := setOf(includedSet) - for len(into) < len(included) { + for len(into) < int(r.threshold) { select { case <-ctx.Done(): return ctx.Err() @@ -443,24 +519,28 @@ func (r *interactiveSigningRunner) collectCommitments( return nil } -// collectShares fills `into` with each included member's inner FROST share -// bytes (own already seeded), counting the first accepted body per sender, and -// retains EVERY well-formed share in the collector - including a sender's later, -// body-different ones - which is where member equivocation is detected. +// collectShares fills `into` with each share from the chosen signing subset +// (`signerSet`) as inner FROST share bytes - a signer's own already seeded, an +// observer's none - counting the first accepted body per sender, and retains +// EVERY well-formed share in the collector (including a sender's later, +// body-different ones) which is where member equivocation is detected. It +// collects over the signer set, NOT the full included set: a committed member +// the coordinator did not choose is an observer that contributes no share, so +// waiting for it would stall every attempt that omitted an offline member. func (r *interactiveSigningRunner) collectShares( ctx context.Context, stream <-chan RunnerMessage, contextHash [attempt.MessageDigestLength]byte, - includedSet []group.MemberIndex, + signerSet []group.MemberIndex, into map[group.MemberIndex][]byte, ) error { - included := setOf(includedSet) - for len(into) < len(included) { + signers := setOf(signerSet) + for len(into) < len(signers) { select { case <-ctx.Done(): return ctx.Err() case msg := <-stream: - r.recordShareMessage(msg, contextHash, included, into) + r.recordShareMessage(msg, contextHash, signers, into) } } // `into` is full, but the slot-filling share may have body-different @@ -472,7 +552,7 @@ func (r *interactiveSigningRunner) collectShares( for i, n := 0, len(stream); i < n; i++ { select { case msg := <-stream: - r.recordShareMessage(msg, contextHash, included, into) + r.recordShareMessage(msg, contextHash, signers, into) default: return nil } @@ -481,23 +561,34 @@ func (r *interactiveSigningRunner) collectShares( } // recordShareMessage validates a share-submission bus message, retains it in the -// collector, and counts it toward `into` when it is the sender's first accepted -// share. Recording BEFORE the already-collected check is what lets the collector -// observe member equivocation (a body-different second signed share -> +// collector, and counts it toward `into` only when the sender is in the chosen +// signing subset and it is the sender's first accepted share. Retention is gated +// by INCLUDED-set membership, not the signer set: the collector deliberately +// keeps an included non-signer's DIVERGENT share - a targeted coordinator- +// equivocation victim that signed a different package - as +// EquivocationKindDivergentShare evidence for the f+1 blame/transition path, so +// the runner must hand any included member's well-formed share to the collector +// even though only signer-set shares count toward the aggregate. Recording BEFORE +// the already-collected check is what lets the collector observe member +// equivocation (a body-different second signed share -> // EquivocationKindShareConflict / DivergentShare); a divergent / conflicting / -// unauthenticated share is retained where applicable but never counted. -// Retention is bounded (the collector keeps the first per submitter and only -// emits on the rest), and the bus delivers only body-different duplicates. +// unauthenticated share is retained where applicable but never counted. Retention +// is bounded (the collector keeps the first per submitter and only emits on the +// rest), and the bus delivers only body-different duplicates. func (r *interactiveSigningRunner) recordShareMessage( msg RunnerMessage, contextHash [attempt.MessageDigestLength]byte, - included map[group.MemberIndex]struct{}, + signers map[group.MemberIndex]struct{}, into map[group.MemberIndex][]byte, ) { if msg.Attempt != contextHash { return } - if _, want := included[msg.Sender]; !want { + // Retain shares from any INCLUDED member, not just the signing subset: an + // included non-signer's divergent share is targeted-equivocation evidence the + // collector keeps. An outsider (not in the included set) is dropped here rather + // than handed to the collector, which would reject it as not-included anyway. + if _, included := r.includedMembers[msg.Sender]; !included { return } var sub roast.ShareSubmission @@ -519,6 +610,12 @@ func (r *interactiveSigningRunner) recordShareMessage( return } recordErr := r.collector.RecordShareSubmission(&sub) + // Only the chosen signing subset's shares count toward this aggregate; a + // committed-but-unchosen observer's share is retained above as evidence but + // never counted. + if _, isSigner := signers[msg.Sender]; !isSigner { + return + } if _, have := into[msg.Sender]; have { return } @@ -684,3 +781,27 @@ func setOf(members []group.MemberIndex) map[group.MemberIndex]struct{} { } return out } + +// sortedMemberIndices returns the keys of a member-indexed map in ascending +// order - the chosen signing subset the coordinator carries in the package's +// signer_ids, which SigningPackage.Validate requires strictly ascending (the map +// keys are distinct, so sorting yields a strictly-ascending list). +func sortedMemberIndices(m map[group.MemberIndex][]byte) []group.MemberIndex { + out := make([]group.MemberIndex, 0, len(m)) + for member := range m { + out = append(out, member) + } + sort.Slice(out, func(i, j int) bool { return out[i] < out[j] }) + return out +} + +// memberIndicesToUint32 widens an ascending member-index slice to the wire +// (uint32) form SigningPackage.SignerIDsValue carries. Widening uint8 -> +// uint32 is lossless, and ascending/distinct order is preserved. +func memberIndicesToUint32(members []group.MemberIndex) []uint32 { + out := make([]uint32, 0, len(members)) + for _, member := range members { + out = append(out, uint32(member)) + } + return out +} diff --git a/pkg/frost/signing/roast_runner_frost_native_test.go b/pkg/frost/signing/roast_runner_frost_native_test.go index bedcf37ec6..7eea79b79c 100644 --- a/pkg/frost/signing/roast_runner_frost_native_test.go +++ b/pkg/frost/signing/roast_runner_frost_native_test.go @@ -127,10 +127,150 @@ func (h harness) runAndAssertAllSucceed(t *testing.T) { } } +// The happy path over an oversized group (group size 3, threshold 2): the +// coordinator finalizes over the first t responsive committers and the remaining +// committed member is an observer that aggregates the subset's broadcast shares. +// Which member observes depends on bus timing, but every node obtains the +// signature and reaches Succeeded. The deterministic subset/observer dynamics are +// pinned in TestInteractiveSigningRunner_OversizedAllOnline_FinalizesOverThreshold +// and the offline/observer tests below. +// runMembers runs only the given members' runners concurrently (the rest stay +// offline / non-responsive, never committing) and returns each member's +// signature and error keyed by member index. +func (h harness) runMembers(t *testing.T, members []group.MemberIndex) (map[group.MemberIndex][]byte, map[group.MemberIndex]error) { + t.Helper() + runCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + var mu sync.Mutex + sigs := map[group.MemberIndex][]byte{} + errs := map[group.MemberIndex]error{} + var wg sync.WaitGroup + for _, m := range members { + wg.Add(1) + go func(member group.MemberIndex) { + defer wg.Done() + sig, err := h.runners[member-1].Run(runCtx) + mu.Lock() + sigs[member] = sig + errs[member] = err + mu.Unlock() + }(m) + } + wg.Wait() + return sigs, errs +} + func TestInteractiveSigningRunner_HappyPath(t *testing.T) { buildInteractiveSigningHarness(t, 3, 2).runAndAssertAllSucceed(t) } +// A non-responsive (offline) included member must NOT stall the attempt: the +// coordinator finalizes over the first t responsive committers. Group size 3, +// threshold 2, one NON-COORDINATOR member never runs (never commits), so the two +// responsive members - the coordinator plus one peer - finalize over the t-subset +// and both succeed, and the coordinator's package omits the offline member. +func TestInteractiveSigningRunner_FinalizesOverResponsiveThresholdSubset(t *testing.T) { + h := buildInteractiveSigningHarness(t, 3, 2) + elected := h.runners[0].attempt.ElectedCoordinator() + + var offline group.MemberIndex + for _, m := range h.includedSet { + if m != elected { + offline = m + break + } + } + online := make([]group.MemberIndex, 0, 2) + for _, m := range h.includedSet { + if m != offline { + online = append(online, m) + } + } + + sigs, errs := h.runMembers(t, online) + for _, m := range online { + if errs[m] != nil { + t.Fatalf("online member %d failed: %v", m, errs[m]) + } + if string(sigs[m]) != "fake-bip340-signature" { + t.Fatalf("online member %d unexpected signature: %q", m, sigs[m]) + } + state, err := h.coords[m-1].State(h.handles[m-1]) + if err != nil { + t.Fatalf("member %d state: %v", m, err) + } + if state != roast.AttemptStateSucceeded { + t.Fatalf("member %d: expected Succeeded, got %v", m, state) + } + } + + // The coordinator built the package over exactly the t responsive committers; + // the offline member is absent from the signing subset. + coordCommitments := h.engines[elected-1].newPackageCommitments() + if len(coordCommitments) != 2 { + t.Fatalf("coordinator built package over %d commitments, want t=2", len(coordCommitments)) + } + gotIDs := map[string]struct{}{} + for _, c := range coordCommitments { + gotIDs[c.Identifier] = struct{}{} + } + if _, present := gotIDs[fmt.Sprintf("frost-id-%d", offline)]; present { + t.Fatalf("offline member %d was included in the signing subset: %v", offline, gotIDs) + } + for _, m := range online { + if _, present := gotIDs[fmt.Sprintf("frost-id-%d", m)]; !present { + t.Fatalf("online member %d missing from the signing subset: %v", m, gotIDs) + } + } +} + +// An oversized all-online group (size 3, threshold 2): every member commits, the +// coordinator finalizes over exactly t of them, and the remaining committed +// member observes. All three obtain the signature and reach Succeeded; exactly t +// members sign round 2 and the n-t observers abort their unconsumed round-1 +// nonces. The coordinator's broadcast package names exactly t ascending signers. +func TestInteractiveSigningRunner_OversizedAllOnline_FinalizesOverThreshold(t *testing.T) { + const n, threshold = 3, 2 + h := buildInteractiveSigningHarness(t, n, threshold) + + // A sniffer subscribed before any Run captures the coordinator's broadcast + // package so the test can inspect its signer_ids. + sniffer := h.bus.Subscribe() + elected := h.runners[0].attempt.ElectedCoordinator() + + h.runAndAssertAllSucceed(t) + + totalRound2, totalAbort := 0, 0 + for _, e := range h.engines { + totalRound2 += e.round2CallCount() + totalAbort += e.abortCallCount() + } + if totalRound2 != threshold { + t.Fatalf("expected exactly t=%d round-2 signers, got %d", threshold, totalRound2) + } + // The n-t observers each abort their unconsumed round-1 nonces; signers do not. + if totalAbort != n-threshold { + t.Fatalf("expected %d observer aborts, got %d", n-threshold, totalAbort) + } + + signerIDs := captureCoordinatorSignerIDs(t, sniffer, elected, h.contextHash) + if len(signerIDs) != threshold { + t.Fatalf("package signer_ids = %v, want exactly t=%d", signerIDs, threshold) + } + for i := 1; i < len(signerIDs); i++ { + if signerIDs[i] <= signerIDs[i-1] { + t.Fatalf("signer_ids not strictly ascending: %v", signerIDs) + } + } + includedSetMap := setOf(h.includedSet) + for _, id := range signerIDs { + if _, ok := includedSetMap[id]; !ok { + t.Fatalf("signer id %d not in included set %v", id, h.includedSet) + } + } +} + // A concluded attempt must leave no retained round-2 collector state, per the // collector's prune-on-conclusion contract (else a long-lived collector reused // across attempts accumulates every attempt's envelopes). The collector exposes @@ -267,8 +407,9 @@ func TestInteractiveSigningRunner_AbortsNativeAttemptOnEarlyExit(t *testing.T) { t.Fatalf("runner: %v", err) } - // No second node ever broadcasts, so Run blocks in collectCommitments until - // the short deadline fires - an early exit with the session already open. + // No second node ever broadcasts, so Run blocks waiting for round 1 to reach + // the threshold (as coordinator) or for the coordinator's package (otherwise) + // until the short deadline fires - an early exit with the session already open. runCtx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) defer cancel() if _, err := runner.Run(runCtx); err == nil { @@ -325,17 +466,19 @@ func TestInteractiveSigningRunner_RejectsEngineCoordinatorMismatch(t *testing.T) } } -// The happy path must key signing-package commitments and aggregate shares by the -// engine-derived FROST identifiers, not a Go-fabricated placeholder. +// The happy path must key aggregate shares by the engine-derived FROST +// identifiers, not a Go-fabricated placeholder. A full-included attempt +// (threshold == group size) has every member sign, so the aggregate carries +// every member's engine-derived identifier (no observer subsetting to vary it). func TestInteractiveSigningRunner_UsesEngineDerivedFrostIdentifiers(t *testing.T) { - h := buildInteractiveSigningHarness(t, 3, 2) + h := buildInteractiveSigningHarness(t, 2, 2) h.runAndAssertAllSucceed(t) got := map[string]struct{}{} for _, share := range h.engines[0].lastAggregateShares { got[share.Identifier] = struct{}{} } - for _, want := range []string{"frost-id-1", "frost-id-2", "frost-id-3"} { + for _, want := range []string{"frost-id-1", "frost-id-2"} { if _, ok := got[want]; !ok { t.Fatalf("aggregate missing engine-derived identifier %q; got %v", want, got) } @@ -837,6 +980,10 @@ func TestNewInteractiveSigningRunner_RejectsInvalidConstruction(t *testing.T) { "member not included": func() (*interactiveSigningRunner, error) { return newInteractiveSigningRunner(ara, 99, 2, engine, collector, coord, signer, bus) }, + "threshold exceeds included set": func() (*interactiveSigningRunner, error) { + // included set has 3 members; a threshold of 4 can never form a subset. + return newInteractiveSigningRunner(ara, 1, 4, engine, collector, coord, signer, bus) + }, } for name, build := range tests { t.Run(name, func(t *testing.T) { @@ -846,3 +993,227 @@ func TestNewInteractiveSigningRunner_RejectsInvalidConstruction(t *testing.T) { }) } } + +// An OBSERVER - a member that committed in round 1 but is NOT in the package's +// signer set - must aggregate the subset's broadcast shares to obtain the +// signature, mark its attempt Succeeded, and return the signature WITHOUT running +// round 2; and even on success it must abort the engine session to drop its +// unconsumed round-1 nonces (success otherwise suppresses the abort, leaking +// them). Driven deterministically: the coordinator's signed package (signer_ids +// excluding this member) and the signers' shares are pre-injected on the bus. +func TestInteractiveSigningRunner_ObserverAggregatesAndAbortsWithoutSigning(t *testing.T) { + included := []group.MemberIndex{1, 2, 3} + dkgKey := []byte{0x01, 0x02} + ctx, err := attempt.NewAttemptContext( + "session-1", "key-group-1", dkgKey, + [attempt.MessageDigestLength]byte{0x42}, 0, included, nil, + ) + if err != nil { + t.Fatalf("attempt context: %v", err) + } + signer := fixedTestSigner{} + verifier := roast.NoOpSignatureVerifier() + bus := NewInProcessRunnerBus(16) + contextHash := ctx.Hash() + + // The election is deterministic for the attempt; probe it so the observer is a + // NON-coordinator (a coordinator is always a signer) and the signer subset is + // the other two members. + probe := roast.NewInMemoryCoordinatorWithSigning(1, signer, verifier) + probeHandle, err := probe.BeginAttempt(ctx) + if err != nil { + t.Fatalf("probe begin: %v", err) + } + probeAttempt, err := NewActiveRoastAttempt(probe, probeHandle, ctx, "session-1", nil, dkgKey) + if err != nil { + t.Fatalf("probe active attempt: %v", err) + } + elected := probeAttempt.ElectedCoordinator() + + var observer group.MemberIndex + for _, m := range included { + if m != elected { + observer = m + break + } + } + // included is ascending, so iterating it (skipping the observer) yields an + // ascending signer subset - what signer_ids requires. + signers := make([]group.MemberIndex, 0, 2) + for _, m := range included { + if m != observer { + signers = append(signers, m) + } + } + + coord := roast.NewInMemoryCoordinatorWithSigning(observer, signer, verifier) + handle, err := coord.BeginAttempt(ctx) + if err != nil { + t.Fatalf("begin attempt: %v", err) + } + ara, err := NewActiveRoastAttempt(coord, handle, ctx, "session-1", nil, dkgKey) + if err != nil { + t.Fatalf("active attempt: %v", err) + } + engine := newFakeInteractiveSigningEngine() + engine.coordinatorIdentifier = uint16(ara.ElectedCoordinator()) + runner, err := newInteractiveSigningRunner( + ara, observer, 2, engine, roast.NewRound2Collector(verifier), coord, signer, bus, + ) + if err != nil { + t.Fatalf("runner: %v", err) + } + + // Pre-inject the coordinator's signed package (signer_ids = the two signers, + // excluding the observer) plus both signers' shares, so the observer's await + // and share collection complete from the bus. + pkgEnvelope, pkgBodyHash := craftSigningPackageWithSigners( + t, contextHash, elected, []byte("fake-signing-package"), signers, signer, + ) + bus.Broadcast(RunnerMessage{Type: RunnerMsgSigningPackage, Sender: elected, Attempt: contextHash, Payload: pkgEnvelope}) + for _, s := range signers { + shareEnvelope := craftShareSubmission( + t, contextHash, s, elected, pkgBodyHash, []byte(fmt.Sprintf("share-%d", s)), signer, + ) + bus.Broadcast(RunnerMessage{Type: RunnerMsgShareSubmission, Sender: s, Attempt: contextHash, Payload: shareEnvelope}) + } + + runCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + sig, err := runner.Run(runCtx) + if err != nil { + t.Fatalf("observer run failed: %v", err) + } + if string(sig) != "fake-bip340-signature" { + t.Fatalf("observer unexpected signature: %q", sig) + } + if engine.round2CallCount() != 0 { + t.Fatalf("observer ran round 2 %d times, want 0 (it is not a signer)", engine.round2CallCount()) + } + if engine.abortCallCount() != 1 { + t.Fatalf("observer aborted %d times, want exactly 1 (drop unconsumed round-1 nonces on success)", engine.abortCallCount()) + } + state, err := coord.State(handle) + if err != nil { + t.Fatalf("observer state: %v", err) + } + if state != roast.AttemptStateSucceeded { + t.Fatalf("observer expected Succeeded, got %v", state) + } +} + +// An included NON-SIGNER (observer) that broadcasts a DIVERGENT share - a +// targeted coordinator-equivocation victim that signed a different package - must +// still be RETAINED by the collector as EquivocationKindDivergentShare evidence +// for the f+1 blame/transition path, even though its share does not count toward +// the aggregate (it is not in the signing subset). Retention is gated by +// included-set membership, not the signer set. +func TestInteractiveSigningRunner_RetainsObserverDivergentShareAsEvidence(t *testing.T) { + included := []group.MemberIndex{1, 2, 3} + runner, collector, contextHash, elected := buildEquivocationRunner(t, included) + signer := fixedTestSigner{} + if err := collector.BeginAttempt(contextHash[:], elected, included); err != nil { + t.Fatalf("collector begin: %v", err) + } + // Authoritative package over the signing subset {1,2}; its body hash binds + // accepted shares. + authEnvelope, _ := craftSigningPackage(t, contextHash, elected, []byte("authoritative-package"), signer) + authPkg := &roast.SigningPackage{} + if err := authPkg.Unmarshal(authEnvelope); err != nil { + t.Fatalf("unmarshal authoritative: %v", err) + } + if err := collector.RecordSigningPackage(authPkg); err != nil { + t.Fatalf("record authoritative: %v", err) + } + + // Member 3 is an included observer (not in the {1,2} signing subset) but was + // handed a DIFFERENT package, so it broadcasts a share bound to that other + // package's body hash - divergent vs the authoritative package. + _, otherBodyHash := craftSigningPackage(t, contextHash, elected, []byte("equivocating-package-for-3"), signer) + divergentShare := craftShareSubmission(t, contextHash, 3, elected, otherBodyHash, []byte("share-3-divergent"), signer) + msg := RunnerMessage{Type: RunnerMsgShareSubmission, Sender: 3, Attempt: contextHash, Payload: divergentShare} + + evidence := captureEquivocationEvidence(t) + into := map[group.MemberIndex][]byte{} + // Signer set excludes member 3 (the observer); counting must skip it while + // retention must not. + runner.recordShareMessage(msg, contextHash, setOf([]group.MemberIndex{1, 2}), into) + + if _, counted := into[3]; counted { + t.Fatal("observer (non-signer) share was counted toward the aggregate") + } + got := evidence() + if len(got) != 1 || got[0].Kind != roast.EquivocationKindDivergentShare || got[0].Sender != 3 { + t.Fatalf("expected one divergent-share evidence retained from member 3, got %+v", got) + } +} + +// captureCoordinatorSignerIDs drains a sniffer subscriber's signing-package +// stream for the elected coordinator's package for the attempt and returns its +// signer_ids. It fails the test if no such package was observed. +func captureCoordinatorSignerIDs( + t *testing.T, + sniffer *RunnerBusSubscriber, + elected group.MemberIndex, + contextHash [attempt.MessageDigestLength]byte, +) []group.MemberIndex { + t.Helper() + for { + select { + case msg := <-sniffer.SigningPackages(): + if msg.Sender != elected || msg.Attempt != contextHash { + continue + } + pkg := &roast.SigningPackage{} + if err := pkg.Unmarshal(msg.Payload); err != nil { + t.Fatalf("unmarshal coordinator package: %v", err) + } + return pkg.SignerIDs() + default: + t.Fatal("coordinator package not observed on the bus") + return nil + } + } +} + +// craftSigningPackageWithSigners builds a coordinator-signed package carrying an +// explicit signer subset (signer_ids, which must be ascending/distinct), and +// returns its envelope and body hash. +func craftSigningPackageWithSigners( + t *testing.T, + contextHash [attempt.MessageDigestLength]byte, + elected group.MemberIndex, + body []byte, + signers []group.MemberIndex, + signer roast.Signer, +) ([]byte, [32]byte) { + t.Helper() + signerIDs := make([]uint32, 0, len(signers)) + for _, m := range signers { + signerIDs = append(signerIDs, uint32(m)) + } + pkg := &roast.SigningPackage{ + AttemptContextHash: append([]byte(nil), contextHash[:]...), + CoordinatorIDValue: uint32(elected), + SigningPackageBytes: append([]byte(nil), body...), + SignerIDsValue: signerIDs, + } + payload, err := pkg.SignableBytes() + if err != nil { + t.Fatalf("signing package signable bytes: %v", err) + } + sig, err := signer.Sign(payload) + if err != nil { + t.Fatalf("sign signing package: %v", err) + } + pkg.CoordinatorSignature = sig + envelope, err := pkg.Marshal() + if err != nil { + t.Fatalf("marshal signing package: %v", err) + } + bodyHash, err := pkg.BodyHash() + if err != nil { + t.Fatalf("signing package body hash: %v", err) + } + return envelope, bodyHash +}