From 8c6aef2e9a85fd64acfc428e57c3192f0d852316 Mon Sep 17 00:00:00 2001 From: maclane Date: Mon, 15 Jun 2026 11:56:03 -0400 Subject: [PATCH 1/2] feat(frost/roast): Phase 7.2b-4a candidate-culprit blame classifier ClassifyCandidateCulprits turns the engine's candidate culprits (the members whose FROST shares failed verification, #4062) into this observer's RejectEntry accusations, adjudicated against the Round2Collector's retained operator-signed bytes - where the engine's crypto verdict becomes envelope-bound, attributable member blame (frozen Q1 boundary; the engine never inspects envelopes). Per attempt, for each candidate against the authoritative package: - accepted retained share that re-verifies INVALID -> RejectEntry (the observer holds the member's operator-signed share: self-incriminating, independently checkable; feeds NextAttempt's existing f+1 establishment gate). - accepted share that re-verifies VALID (yet flagged) -> nothing: not self-incriminating under this observer's package; coordinator-directed faults are a SEPARATE path (7.2b-4b), never inferred here. - divergent share only -> nothing (NEUTRAL: possible targeted coordinator equivocation, must not alone exclude). - no retained share / indeterminate re-verification -> nothing (fail closed). FROST share re-verification is engine-backed (Go has no native FROST share crypto - the collector's SignatureVerifier checks operator sigs, not the share equation): the new Round2ShareVerifier interface is that seam, injected so this lands and reviews now with a fake verifier; the engine-backed impl wires in with the interactive orchestration. Reuses the existing reject category + ExclusionAccuserQuorum/NextAttempt f+1 machinery; ConflictEntry stays reserved for self-conflicting bytes. Deterministic output (deduped, ascending by member, Count 1). Retained bytes are snapshotted under the collector lock; re-verification runs lock-free. Design converged via Codex+Gemini consultation. Co-Authored-By: Claude Opus 4.8 --- pkg/frost/roast/round2_classifier.go | 186 +++++++++++++++++ pkg/frost/roast/round2_classifier_test.go | 234 ++++++++++++++++++++++ 2 files changed, 420 insertions(+) create mode 100644 pkg/frost/roast/round2_classifier.go create mode 100644 pkg/frost/roast/round2_classifier_test.go diff --git a/pkg/frost/roast/round2_classifier.go b/pkg/frost/roast/round2_classifier.go new file mode 100644 index 0000000000..732c2940ec --- /dev/null +++ b/pkg/frost/roast/round2_classifier.go @@ -0,0 +1,186 @@ +package roast + +import ( + "fmt" + "sort" + + "github.com/keep-network/keep-core/pkg/protocol/group" +) + +// candidateCulpritRejectReason is the canonical Reason recorded on a RejectEntry +// minted from an engine candidate culprit whose retained share re-verifies +// invalid. It is the Go-side reason string; the engine never supplies it. +const candidateCulpritRejectReason = "invalid_signature_share" + +// Round2ShareVerifier re-verifies a retained round-2 signature share against an +// attempt's authoritative signing package using FROST share verification. +// +// The Go host has no native FROST share crypto - the collector's +// SignatureVerifier checks OPERATOR signatures over envelopes, not the FROST +// share equation - so a faithful re-verification is engine-backed. This +// interface is that seam: implementations perform PURE crypto verification only +// (no envelope or operator-signature inspection, no blame), staying within the +// engine's crypto-only boundary (frozen Q1). +// +// VerifyRetainedShare reports whether submitter's retained, operator-signed +// share envelope is a valid FROST signature share for the authoritative signing +// package envelope the collector accepted for the attempt: +// +// - (true, nil): the share is a VALID FROST share under the package. +// - (false, nil): the share is provably INVALID under the package - the +// self-incriminating condition that justifies a reject accusation. +// - (_, err): verification was INDETERMINATE (envelope decode failure, +// missing verifying material, ambiguous taproot/key context). The caller +// MUST NOT blame the member on an indeterminate result. +// +// Implementations must be safe for concurrent calls from multiple goroutines. +type Round2ShareVerifier interface { + VerifyRetainedShare( + signingPackageEnvelope []byte, + shareEnvelope []byte, + submitter group.MemberIndex, + ) (valid bool, err error) +} + +// classifierCandidate pairs a candidate culprit with the retained bytes the +// classifier needs to adjudicate it, snapshotted under the collector lock so the +// (engine-backed) re-verification runs lock-free. shareEnvelope is nil when the +// member has no ACCEPTED retained share for the attempt (divergent-only or +// absent), which the caller classifies as non-blamable. +type classifierCandidate struct { + member group.MemberIndex + shareEnvelope []byte +} + +// ClassifyCandidateCulprits turns the engine's candidate culprits for an attempt +// into this observer's reject accusations, applying the frozen Q1 boundary: the +// crypto-only engine reports who failed FROST verification against the package +// the coordinator aggregated, but only the Go host - against its OWN retained, +// operator-signed bytes - decides attributable member blame. +// +// For each candidate, against the attempt's authoritative package: +// +// - an ACCEPTED retained share that re-verifies INVALID -> a RejectEntry. The +// observer holds the member's operator-signed share and shows it invalid +// against the package it accepted: self-incriminating, independently +// checkable evidence (RFC-21 Layer B's "no bare counters" rule). +// - an ACCEPTED retained share that re-verifies VALID (yet the engine flagged +// it) -> nothing. The candidate is not self-incriminating under THIS +// observer's retained package; the cause (a substituted package, share, or +// root, or other coordinator input) is not provable from these bytes, so the +// member must not be blamed. Coordinator-directed faults are a SEPARATE +// adjudication path (Phase 7.2b-4b: package / divergent-share f+1 +// comparison), never inferred here. +// - a DIVERGENT share only (validly signed but binding a different package / +// coordinator) -> nothing. Kept NEUTRAL: a divergent share can be targeted +// coordinator equivocation, so it must not alone permanently exclude its +// member. +// - no retained share at this observer -> nothing (nothing to corroborate). +// - an INDETERMINATE re-verification -> nothing (fail closed against blame). +// +// The emitted accusations feed this observer's LocalEvidenceSnapshot.Rejects and +// hence NextAttempt's f+1 establishment gate; classification here never excludes +// anyone by itself. The result is deterministic - deduplicated and ascending by +// member, Count 1 each - so honest observers over identical retained bytes agree +// byte-for-byte. Returns ErrRound2UnknownAttempt if the attempt was never begun, +// ErrRound2NoSigningPackage if no authoritative package was recorded, and an +// error for a nil verifier. +func (c *Round2Collector) ClassifyCandidateCulprits( + attemptContextHash []byte, + candidates []group.MemberIndex, + verifier Round2ShareVerifier, +) ([]RejectEntry, error) { + if verifier == nil { + return nil, fmt.Errorf( + "roast: ClassifyCandidateCulprits requires a non-nil Round2ShareVerifier", + ) + } + + // Snapshot the retained bytes the candidates need under the lock, then + // release it before the (engine-backed, potentially slow) re-verification - + // mirroring the collector's authenticate-outside-the-lock discipline. The + // copies are collector-owned, so a concurrent PruneAttempt or record + // mutation cannot race the lock-free re-verification below. + signingPackageEnvelope, snapshot, err := c.snapshotCandidatesForClassification( + attemptContextHash, + candidates, + ) + if err != nil { + return nil, err + } + + rejects := make([]RejectEntry, 0, len(snapshot)) + for _, candidate := range snapshot { + if candidate.shareEnvelope == nil { + // No ACCEPTED share retained here: a divergent-only share (NEUTRAL), a + // member that never submitted to this observer, or an absent + // submission. Nothing self-incriminating to accuse with. + continue + } + valid, verr := verifier.VerifyRetainedShare( + signingPackageEnvelope, + candidate.shareEnvelope, + candidate.member, + ) + if verr != nil { + // Indeterminate (undecodable / ambiguous): fail closed against blame. + continue + } + if valid { + // Valid under this observer's authoritative package: not + // self-incriminating. Coordinator-directed faults are Phase 7.2b-4b. + continue + } + rejects = append(rejects, RejectEntry{ + Sender: candidate.member, + Reason: candidateCulpritRejectReason, + Count: 1, + }) + } + return rejects, nil +} + +// snapshotCandidatesForClassification copies, under the collector lock, the +// authoritative package envelope and each deduplicated, ascending candidate's +// ACCEPTED retained share envelope, so re-verification can run lock-free. +// Divergent-only and absent candidates are carried with a nil share envelope. +func (c *Round2Collector) snapshotCandidatesForClassification( + attemptContextHash []byte, + candidates []group.MemberIndex, +) ([]byte, []classifierCandidate, error) { + c.mu.Lock() + defer c.mu.Unlock() + + record, ok := c.attempts[round2AttemptKey(attemptContextHash)] + if !ok { + return nil, nil, ErrRound2UnknownAttempt + } + if record.signingPackageEnvelope == nil { + return nil, nil, ErrRound2NoSigningPackage + } + + signingPackageEnvelope := append([]byte(nil), record.signingPackageEnvelope...) + + // Deduplicate + sort so each member is adjudicated once, in a deterministic + // order, regardless of how the engine ordered or repeated the candidates. + seen := make(map[group.MemberIndex]struct{}, len(candidates)) + unique := make([]group.MemberIndex, 0, len(candidates)) + for _, member := range candidates { + if _, dup := seen[member]; dup { + continue + } + seen[member] = struct{}{} + unique = append(unique, member) + } + sort.Slice(unique, func(i, j int) bool { return unique[i] < unique[j] }) + + snapshot := make([]classifierCandidate, 0, len(unique)) + for _, member := range unique { + entry := classifierCandidate{member: member} + if share, ok := record.shares[member]; ok && share != nil { + entry.shareEnvelope = append([]byte(nil), share.envelope...) + } + snapshot = append(snapshot, entry) + } + return signingPackageEnvelope, snapshot, nil +} diff --git a/pkg/frost/roast/round2_classifier_test.go b/pkg/frost/roast/round2_classifier_test.go new file mode 100644 index 0000000000..fe954a5444 --- /dev/null +++ b/pkg/frost/roast/round2_classifier_test.go @@ -0,0 +1,234 @@ +package roast + +import ( + "bytes" + "errors" + "fmt" + "reflect" + "testing" + + "github.com/keep-network/keep-core/pkg/protocol/group" +) + +// shareVerdict is one fakeShareVerifier outcome: (valid, err). err models an +// INDETERMINATE re-verification; valid is consulted only when err is nil. +type shareVerdict struct { + valid bool + err error +} + +// fakeShareVerifier is a configurable Round2ShareVerifier for the classifier +// tests. Members without a configured verdict default to VALID (no blame); the +// calls slice records every member actually re-verified, so tests can assert +// that divergent-only and absent candidates are never handed to the verifier. +type fakeShareVerifier struct { + verdicts map[group.MemberIndex]shareVerdict + calls *[]group.MemberIndex +} + +func (f fakeShareVerifier) VerifyRetainedShare( + _ []byte, + _ []byte, + submitter group.MemberIndex, +) (bool, error) { + if f.calls != nil { + *f.calls = append(*f.calls, submitter) + } + v, ok := f.verdicts[submitter] + if !ok { + return true, nil + } + return v.valid, v.err +} + +// recordAcceptedShare records an authoritative-package-bound (accepted) share for +// submitter on an attempt already set up by recordTestPackage. +func recordAcceptedShare(t *testing.T, c *Round2Collector, submitter group.MemberIndex, pkgHash []byte) { + t.Helper() + if err := c.RecordShareSubmission(signedTestShareSubmission(t, submitter, pkgHash)); err != nil { + t.Fatalf("record accepted share for %d: %v", submitter, err) + } +} + +func TestClassifyCandidateCulprits_InvalidAcceptedShareEmitsReject(t *testing.T) { + c := NewRound2Collector(fakeVerifier{}) + elected := group.MemberIndex(testShareCoordinatorID) + pkgHash := recordTestPackage(t, c, elected) + recordAcceptedShare(t, c, 3, pkgHash) + + verifier := fakeShareVerifier{verdicts: map[group.MemberIndex]shareVerdict{3: {valid: false}}} + rejects, err := c.ClassifyCandidateCulprits(pinnedContextHash[:], []group.MemberIndex{3}, verifier) + if err != nil { + t.Fatalf("classify: %v", err) + } + want := []RejectEntry{{Sender: 3, Reason: candidateCulpritRejectReason, Count: 1}} + if !reflect.DeepEqual(rejects, want) { + t.Fatalf("want %+v, got %+v", want, rejects) + } +} + +func TestClassifyCandidateCulprits_ValidAcceptedShareEmitsNothing(t *testing.T) { + c := NewRound2Collector(fakeVerifier{}) + elected := group.MemberIndex(testShareCoordinatorID) + pkgHash := recordTestPackage(t, c, elected) + recordAcceptedShare(t, c, 3, pkgHash) + + // The engine flagged 3, but THIS observer's retained share re-verifies valid + // against the package it accepted: not self-incriminating -> no accusation. + verifier := fakeShareVerifier{verdicts: map[group.MemberIndex]shareVerdict{3: {valid: true}}} + rejects, err := c.ClassifyCandidateCulprits(pinnedContextHash[:], []group.MemberIndex{3}, verifier) + if err != nil { + t.Fatalf("classify: %v", err) + } + if len(rejects) != 0 { + t.Fatalf("a valid-but-flagged candidate must not be blamed, got %+v", rejects) + } +} + +func TestClassifyCandidateCulprits_IndeterminateEmitsNothing(t *testing.T) { + c := NewRound2Collector(fakeVerifier{}) + elected := group.MemberIndex(testShareCoordinatorID) + pkgHash := recordTestPackage(t, c, elected) + recordAcceptedShare(t, c, 3, pkgHash) + + // An indeterminate re-verification (undecodable / ambiguous) must fail closed + // against blame. + verifier := fakeShareVerifier{verdicts: map[group.MemberIndex]shareVerdict{ + 3: {valid: false, err: fmt.Errorf("ambiguous taproot context")}, + }} + rejects, err := c.ClassifyCandidateCulprits(pinnedContextHash[:], []group.MemberIndex{3}, verifier) + if err != nil { + t.Fatalf("classify: %v", err) + } + if len(rejects) != 0 { + t.Fatalf("indeterminate verification must not blame, got %+v", rejects) + } +} + +func TestClassifyCandidateCulprits_DivergentShareIsNeutral(t *testing.T) { + _ = captureEquivocationEvidence(t) + c := NewRound2Collector(fakeVerifier{}) + elected := group.MemberIndex(testShareCoordinatorID) + _ = recordTestPackage(t, c, elected) + + // Member 3 has ONLY a divergent share (binds a non-authoritative package). + wrong := bytes.Repeat([]byte{0x11}, SigningPackageHashLength) + if err := c.RecordShareSubmission(signedTestShareSubmission(t, 3, wrong)); !errors.Is(err, ErrShareRetainedNotAccepted) { + t.Fatalf("setup divergent share: want ErrShareRetainedNotAccepted, got %v", err) + } + + calls := []group.MemberIndex{} + verifier := fakeShareVerifier{ + // Would blame 3 if consulted - but a divergent-only candidate must be + // skipped BEFORE re-verification. + verdicts: map[group.MemberIndex]shareVerdict{3: {valid: false}}, + calls: &calls, + } + rejects, err := c.ClassifyCandidateCulprits(pinnedContextHash[:], []group.MemberIndex{3}, verifier) + if err != nil { + t.Fatalf("classify: %v", err) + } + if len(rejects) != 0 { + t.Fatalf("a divergent-only candidate must stay neutral, got %+v", rejects) + } + if len(calls) != 0 { + t.Fatalf("the verifier must not be consulted for a divergent-only candidate, got %v", calls) + } +} + +func TestClassifyCandidateCulprits_AbsentCandidateIsNothing(t *testing.T) { + c := NewRound2Collector(fakeVerifier{}) + elected := group.MemberIndex(testShareCoordinatorID) + pkgHash := recordTestPackage(t, c, elected) + recordAcceptedShare(t, c, 3, pkgHash) + + calls := []group.MemberIndex{} + verifier := fakeShareVerifier{verdicts: map[group.MemberIndex]shareVerdict{5: {valid: false}}, calls: &calls} + // 5 is in the included set but never submitted a share to this observer. + rejects, err := c.ClassifyCandidateCulprits(pinnedContextHash[:], []group.MemberIndex{5}, verifier) + if err != nil { + t.Fatalf("classify: %v", err) + } + if len(rejects) != 0 { + t.Fatalf("a candidate with no retained share must not be blamed, got %+v", rejects) + } + if len(calls) != 0 { + t.Fatalf("the verifier must not be consulted for an absent candidate, got %v", calls) + } +} + +func TestClassifyCandidateCulprits_MultipleSortedAndDeduplicated(t *testing.T) { + c := NewRound2Collector(fakeVerifier{}) + elected := group.MemberIndex(testShareCoordinatorID) + pkgHash := recordTestPackage(t, c, elected) + // All three included members submitted accepted shares. + recordAcceptedShare(t, c, 3, pkgHash) + recordAcceptedShare(t, c, 5, pkgHash) + recordAcceptedShare(t, c, 7, pkgHash) + + // 3 and 7 re-verify invalid; 5 re-verifies valid (not blamed). + verifier := fakeShareVerifier{verdicts: map[group.MemberIndex]shareVerdict{ + 3: {valid: false}, + 5: {valid: true}, + 7: {valid: false}, + }} + // Candidates arrive unsorted and duplicated. + rejects, err := c.ClassifyCandidateCulprits( + pinnedContextHash[:], + []group.MemberIndex{7, 5, 3, 7, 3}, + verifier, + ) + if err != nil { + t.Fatalf("classify: %v", err) + } + want := []RejectEntry{ + {Sender: 3, Reason: candidateCulpritRejectReason, Count: 1}, + {Sender: 7, Reason: candidateCulpritRejectReason, Count: 1}, + } + if !reflect.DeepEqual(rejects, want) { + t.Fatalf("want deduplicated, ascending rejects %+v, got %+v", want, rejects) + } +} + +func TestClassifyCandidateCulprits_Errors(t *testing.T) { + elected := group.MemberIndex(testShareCoordinatorID) + + t.Run("nil verifier", func(t *testing.T) { + c := NewRound2Collector(fakeVerifier{}) + _ = recordTestPackage(t, c, elected) + if _, err := c.ClassifyCandidateCulprits(pinnedContextHash[:], []group.MemberIndex{3}, nil); err == nil { + t.Fatal("a nil verifier must be rejected, not panic") + } + }) + + t.Run("unknown attempt", func(t *testing.T) { + c := NewRound2Collector(fakeVerifier{}) + _, err := c.ClassifyCandidateCulprits(pinnedContextHash[:], []group.MemberIndex{3}, fakeShareVerifier{}) + if !errors.Is(err, ErrRound2UnknownAttempt) { + t.Fatalf("want ErrRound2UnknownAttempt, got %v", err) + } + }) + + t.Run("no signing package recorded", func(t *testing.T) { + c := NewRound2Collector(fakeVerifier{}) + if err := c.BeginAttempt(pinnedContextHash[:], elected, testIncludedSet()); err != nil { + t.Fatalf("begin: %v", err) + } + _, err := c.ClassifyCandidateCulprits(pinnedContextHash[:], []group.MemberIndex{3}, fakeShareVerifier{}) + if !errors.Is(err, ErrRound2NoSigningPackage) { + t.Fatalf("want ErrRound2NoSigningPackage, got %v", err) + } + }) + + t.Run("no candidates yields no rejects", func(t *testing.T) { + c := NewRound2Collector(fakeVerifier{}) + _ = recordTestPackage(t, c, elected) + rejects, err := c.ClassifyCandidateCulprits(pinnedContextHash[:], nil, fakeShareVerifier{}) + if err != nil { + t.Fatalf("classify: %v", err) + } + if len(rejects) != 0 { + t.Fatalf("no candidates must yield no rejects, got %+v", rejects) + } + }) +} From 8f8e5bf1a45d92d9ffd0f37b17de5a6d084e0820 Mon Sep 17 00:00:00 2001 From: maclane Date: Mon, 15 Jun 2026 12:29:04 -0400 Subject: [PATCH 2/2] fixup(frost/roast): tri-state Round2ShareVerifier (fold Gemini P2 on #4063) The (valid bool, error) verifier seam conflated a member's OWN undecodable / garbage FROST share bytes (operator-signed -> self-incriminating -> blame) with a not-the-member's-fault execution failure. A Go FFI bridge would naturally map a Rust/FROST deserialization error to a Go error, which the classifier read as "indeterminate -> don't blame", letting that cheater dodge a RejectEntry. Replace it with an explicit ShareVerificationResult tri-state (ShareValid / ShareInvalid / ShareIndeterminate): ShareInvalid covers both a mathematically invalid share AND undecodable member bytes; ShareIndeterminate is reserved for failures that are NOT the member's fault. The classifier blames ONLY ShareInvalid - everything else, including any future verdict, fails closed. This forces the engine-backed implementer to categorize the boundary deliberately rather than leak member-fault into an error channel. Co-Authored-By: Claude Opus 4.8 --- pkg/frost/roast/round2_classifier.go | 56 +++++++++++++++-------- pkg/frost/roast/round2_classifier_test.go | 49 ++++++++------------ 2 files changed, 58 insertions(+), 47 deletions(-) diff --git a/pkg/frost/roast/round2_classifier.go b/pkg/frost/roast/round2_classifier.go index 732c2940ec..b22b89edd7 100644 --- a/pkg/frost/roast/round2_classifier.go +++ b/pkg/frost/roast/round2_classifier.go @@ -12,6 +12,32 @@ import ( // invalid. It is the Go-side reason string; the engine never supplies it. const candidateCulpritRejectReason = "invalid_signature_share" +// ShareVerificationResult is the verdict of a FROST share re-verification. +// +// It is deliberately a three-way enum rather than a (bool, error): the boundary +// between a MEMBER-attributable failure (blame) and a not-the-member's-fault +// failure (don't blame) is security-critical, and a (bool, error) shape silently +// routes a member's own undecodable/garbage share bytes into the error channel - +// where the classifier would read them as "indeterminate" and let the cheater +// dodge blame. The enum forces the (engine-backed) implementer to categorize the +// failure boundary deliberately. +type ShareVerificationResult int + +const ( + // ShareValid: the retained share is a valid FROST signature share for the + // authoritative package. Not blamable. + ShareValid ShareVerificationResult = iota + // ShareInvalid: the share is MEMBER-attributable garbage - mathematically + // invalid against the package, OR undecodable/malformed share bytes the + // member operator-signed. Self-incriminating, hence blamable. + ShareInvalid + // ShareIndeterminate: verification could not be completed for a reason that + // is NOT the member's fault - missing verifying material, ambiguous + // key/taproot context, an undecodable AUTHORITATIVE PACKAGE, or an engine / + // FFI execution failure. Fail closed against blame. + ShareIndeterminate +) + // Round2ShareVerifier re-verifies a retained round-2 signature share against an // attempt's authoritative signing package using FROST share verification. // @@ -24,14 +50,11 @@ const candidateCulpritRejectReason = "invalid_signature_share" // // VerifyRetainedShare reports whether submitter's retained, operator-signed // share envelope is a valid FROST signature share for the authoritative signing -// package envelope the collector accepted for the attempt: -// -// - (true, nil): the share is a VALID FROST share under the package. -// - (false, nil): the share is provably INVALID under the package - the -// self-incriminating condition that justifies a reject accusation. -// - (_, err): verification was INDETERMINATE (envelope decode failure, -// missing verifying material, ambiguous taproot/key context). The caller -// MUST NOT blame the member on an indeterminate result. +// package envelope the collector accepted. Implementations MUST map a member's +// OWN invalid or undecodable share bytes to ShareInvalid (it is self-incriminating +// member fault), and reserve ShareIndeterminate for failures that are not the +// member's fault (see the constants). Misclassifying undecodable member bytes as +// ShareIndeterminate would let a cheater escape blame. // // Implementations must be safe for concurrent calls from multiple goroutines. type Round2ShareVerifier interface { @@ -39,7 +62,7 @@ type Round2ShareVerifier interface { signingPackageEnvelope []byte, shareEnvelope []byte, submitter group.MemberIndex, - ) (valid bool, err error) + ) ShareVerificationResult } // classifierCandidate pairs a candidate culprit with the retained bytes the @@ -117,18 +140,15 @@ func (c *Round2Collector) ClassifyCandidateCulprits( // submission. Nothing self-incriminating to accuse with. continue } - valid, verr := verifier.VerifyRetainedShare( + // Blame ONLY a member-attributable invalid share. ShareValid (not + // self-incriminating under this observer's package - coordinator-directed + // faults are Phase 7.2b-4b), ShareIndeterminate (not the member's fault), + // and any future verdict fail closed against blame. + if verifier.VerifyRetainedShare( signingPackageEnvelope, candidate.shareEnvelope, candidate.member, - ) - if verr != nil { - // Indeterminate (undecodable / ambiguous): fail closed against blame. - continue - } - if valid { - // Valid under this observer's authoritative package: not - // self-incriminating. Coordinator-directed faults are Phase 7.2b-4b. + ) != ShareInvalid { continue } rejects = append(rejects, RejectEntry{ diff --git a/pkg/frost/roast/round2_classifier_test.go b/pkg/frost/roast/round2_classifier_test.go index fe954a5444..2e7138235f 100644 --- a/pkg/frost/roast/round2_classifier_test.go +++ b/pkg/frost/roast/round2_classifier_test.go @@ -3,26 +3,18 @@ package roast import ( "bytes" "errors" - "fmt" "reflect" "testing" "github.com/keep-network/keep-core/pkg/protocol/group" ) -// shareVerdict is one fakeShareVerifier outcome: (valid, err). err models an -// INDETERMINATE re-verification; valid is consulted only when err is nil. -type shareVerdict struct { - valid bool - err error -} - // fakeShareVerifier is a configurable Round2ShareVerifier for the classifier -// tests. Members without a configured verdict default to VALID (no blame); the -// calls slice records every member actually re-verified, so tests can assert +// tests. Members without a configured verdict default to ShareValid (no blame); +// the calls slice records every member actually re-verified, so tests can assert // that divergent-only and absent candidates are never handed to the verifier. type fakeShareVerifier struct { - verdicts map[group.MemberIndex]shareVerdict + verdicts map[group.MemberIndex]ShareVerificationResult calls *[]group.MemberIndex } @@ -30,15 +22,14 @@ func (f fakeShareVerifier) VerifyRetainedShare( _ []byte, _ []byte, submitter group.MemberIndex, -) (bool, error) { +) ShareVerificationResult { if f.calls != nil { *f.calls = append(*f.calls, submitter) } - v, ok := f.verdicts[submitter] - if !ok { - return true, nil + if r, ok := f.verdicts[submitter]; ok { + return r } - return v.valid, v.err + return ShareValid } // recordAcceptedShare records an authoritative-package-bound (accepted) share for @@ -56,7 +47,9 @@ func TestClassifyCandidateCulprits_InvalidAcceptedShareEmitsReject(t *testing.T) pkgHash := recordTestPackage(t, c, elected) recordAcceptedShare(t, c, 3, pkgHash) - verifier := fakeShareVerifier{verdicts: map[group.MemberIndex]shareVerdict{3: {valid: false}}} + // ShareInvalid covers both a mathematically invalid share and undecodable + // share bytes the member operator-signed: either is self-incriminating. + verifier := fakeShareVerifier{verdicts: map[group.MemberIndex]ShareVerificationResult{3: ShareInvalid}} rejects, err := c.ClassifyCandidateCulprits(pinnedContextHash[:], []group.MemberIndex{3}, verifier) if err != nil { t.Fatalf("classify: %v", err) @@ -75,7 +68,7 @@ func TestClassifyCandidateCulprits_ValidAcceptedShareEmitsNothing(t *testing.T) // The engine flagged 3, but THIS observer's retained share re-verifies valid // against the package it accepted: not self-incriminating -> no accusation. - verifier := fakeShareVerifier{verdicts: map[group.MemberIndex]shareVerdict{3: {valid: true}}} + verifier := fakeShareVerifier{verdicts: map[group.MemberIndex]ShareVerificationResult{3: ShareValid}} rejects, err := c.ClassifyCandidateCulprits(pinnedContextHash[:], []group.MemberIndex{3}, verifier) if err != nil { t.Fatalf("classify: %v", err) @@ -91,11 +84,9 @@ func TestClassifyCandidateCulprits_IndeterminateEmitsNothing(t *testing.T) { pkgHash := recordTestPackage(t, c, elected) recordAcceptedShare(t, c, 3, pkgHash) - // An indeterminate re-verification (undecodable / ambiguous) must fail closed - // against blame. - verifier := fakeShareVerifier{verdicts: map[group.MemberIndex]shareVerdict{ - 3: {valid: false, err: fmt.Errorf("ambiguous taproot context")}, - }} + // An indeterminate re-verification (not the member's fault) must fail closed + // against blame - distinct from ShareValid, but likewise emits nothing. + verifier := fakeShareVerifier{verdicts: map[group.MemberIndex]ShareVerificationResult{3: ShareIndeterminate}} rejects, err := c.ClassifyCandidateCulprits(pinnedContextHash[:], []group.MemberIndex{3}, verifier) if err != nil { t.Fatalf("classify: %v", err) @@ -121,7 +112,7 @@ func TestClassifyCandidateCulprits_DivergentShareIsNeutral(t *testing.T) { verifier := fakeShareVerifier{ // Would blame 3 if consulted - but a divergent-only candidate must be // skipped BEFORE re-verification. - verdicts: map[group.MemberIndex]shareVerdict{3: {valid: false}}, + verdicts: map[group.MemberIndex]ShareVerificationResult{3: ShareInvalid}, calls: &calls, } rejects, err := c.ClassifyCandidateCulprits(pinnedContextHash[:], []group.MemberIndex{3}, verifier) @@ -143,7 +134,7 @@ func TestClassifyCandidateCulprits_AbsentCandidateIsNothing(t *testing.T) { recordAcceptedShare(t, c, 3, pkgHash) calls := []group.MemberIndex{} - verifier := fakeShareVerifier{verdicts: map[group.MemberIndex]shareVerdict{5: {valid: false}}, calls: &calls} + verifier := fakeShareVerifier{verdicts: map[group.MemberIndex]ShareVerificationResult{5: ShareInvalid}, calls: &calls} // 5 is in the included set but never submitted a share to this observer. rejects, err := c.ClassifyCandidateCulprits(pinnedContextHash[:], []group.MemberIndex{5}, verifier) if err != nil { @@ -167,10 +158,10 @@ func TestClassifyCandidateCulprits_MultipleSortedAndDeduplicated(t *testing.T) { recordAcceptedShare(t, c, 7, pkgHash) // 3 and 7 re-verify invalid; 5 re-verifies valid (not blamed). - verifier := fakeShareVerifier{verdicts: map[group.MemberIndex]shareVerdict{ - 3: {valid: false}, - 5: {valid: true}, - 7: {valid: false}, + verifier := fakeShareVerifier{verdicts: map[group.MemberIndex]ShareVerificationResult{ + 3: ShareInvalid, + 5: ShareValid, + 7: ShareInvalid, }} // Candidates arrive unsorted and duplicated. rejects, err := c.ClassifyCandidateCulprits(