feat(frost/roast): close M4 -- reject + conflict evidence categories#3988
Merged
mswilkison merged 1 commit intoMay 23, 2026
Conversation
Closes the M4 gap from the original PR #3866 review by adding the two evidence categories the RFC-21 Phase-2 work left as future work: validation-rejection evidence and first-write-wins-conflict evidence. With this PR, the NextAttempt policy can permanently exclude misbehaving peers on all four ROAST blame channels -- transport-overflow, validation-reject, equivocation-conflict, and silence -- instead of just overflow + silence. Why this matters: a peer that only sends malformed messages (validation rejects, never overflows the channel) was previously indistinguishable from a silent peer. The transient silence- parking policy would bench-and-reinstate them indefinitely, never permanently excluding the malicious behaviour. Same for a peer equivocating mid-attempt: the existing first-write-wins assembly correctly dropped the conflicting retransmission but only logged the event -- the bundle carried no structured evidence the coordinator's policy could act on. * pkg/frost/roast/attempt/evidence_recorder.go - EvidenceRecorder interface gains RecordReject(sender, reason) and RecordConflict(sender). - RejectQuotaDefault = 8, ConflictQuotaDefault = 4 (matches categoryQuota in RFC-21 Layer A). - Evidence struct extended with Rejects (map[MemberIndex][]RejectEntry: per-(sender, reason)) and Conflicts (map[MemberIndex]uint). - boundedRecorder: per-reason quota counter keeps each reason bucket independent so a peer cannot saturate one reason to mask another. Conflicts counter saturates at the conflict quota. - noOpRecorder: every category discards. - NewBoundedRecorderWithQuotas(overflow, reject, conflict) constructor for tests; existing NewBoundedRecorderWithQuota preserved for backward compat (defaults reject + conflict quotas). * pkg/frost/roast/transition_message.go - RejectEntry (Sender + Reason + Count) and ConflictEntry (Sender + Count) wire types added. - LocalEvidenceSnapshot gains Rejects []RejectEntry and Conflicts []ConflictEntry, both omitempty. - NewLocalEvidenceSnapshot canonicalises into sorted slices: rejects ascending by Sender then by Reason; conflicts ascending by Sender. - Evidence() reconstructs the map form for downstream consumption. - Validate() enforces sorted-ascending invariants on both new slices. * pkg/frost/roast/next_attempt.go - RejectExclusionThreshold = 1; ConflictExclusionThreshold = 1 (per RFC-21 Layer B). - computeNextAttempt now consults rejectBlamedSenders and conflictBlamedSenders alongside the existing overflowBlamed set. All three feed into the permanent ExcludedSet. - blamedSenders helper factored to share the threshold-comparison + sort logic across the three category helpers. * pkg/frost/signing/native_frost_protocol_frost_native.go and * pkg/frost/signing/native_ffi_primitive_transitional_frost_native.go - Three reject sites: in each of the three receive loops, the shouldAcceptNativeFROSTMessage failure path now calls evidence.RecordReject(senderID, "validation_gate_rejected") before returning. (Previously the message was just dropped.) - Three conflict sites: the first-write-wins assembly loop's "dropping conflicting" branch now calls evidence.RecordConflict(senderID) immediately before the existing log line. (Previously only the log line.) Tests (15 new cases): * pkg/frost/roast/attempt/evidence_recorder_categories_test.go (7) - RecordReject accumulates by reason - RecordReject per-reason quota saturates - Per-reason quotas independent across reasons - RecordConflict accumulates and saturates - All three categories present in Snapshot after mixed input - NoOp recorder inert across all categories - RFC-quota constants match documented values * pkg/frost/roast/next_attempt_categories_test.go (5) - Single reject crosses threshold -> permanent exclusion - Single conflict crosses threshold -> permanent exclusion - Reject and conflict on different senders -> both excluded - Empty rejects+conflicts -> no exclusion (sanity) - Threshold constants match RFC-21 * Receive-loop wiring is covered by existing send/recv tests combined with the recorder unit tests; no new behaviour test added at the integration level because the NoOp default keeps pre-RFC-21 receive semantics observably unchanged. Verification: * go build ./... + go build -tags 'frost_native frost_tbtc_signer frost_roast_retry' ./... -- both clean * go test ./pkg/frost/... + go test -race ./pkg/frost/roast/... + go test -tags 'frost_native frost_tbtc_signer frost_roast_retry' ./pkg/frost/... -- all pass (5 packages) * staticcheck -checks '-SA1019' ./pkg/frost/... -- silent * go vet ./pkg/frost/... + gofmt -l ./pkg/frost/ -- clean This PR completes M4 from the original PR #3866 review. All four ROAST evidence categories (overflow, reject, conflict, silence) are now operational; the NextAttempt policy excludes on the first three and parks transiently on the fourth, matching RFC-21 Layer B exactly.
3 tasks
9bc93c1
into
feat/frost-schnorr-migration-scaffold
15 checks passed
mswilkison
added a commit
that referenced
this pull request
May 23, 2026
…ase-6 milestone) (#3989) ## Summary Closes the Phase-6 milestone the RFC named but the implementation skipped: receive callbacks now reject messages whose \`AttemptContextHash\` does not match the session's bound \`AttemptContext\`. Default builds and sessions without a ROAST-attempt binding skip enforcement entirely, so the change is observationally identical to pre-Phase-6 behaviour outside the ROAST path. Stacked on #3988 (M4 closure). ## Why this matters The Phase 1B \`AttemptContextHash\` field was structural-only (present, 32 bytes) until now. Senders could populate it but receivers ignored the value -- meaning a peer could send a message bound to attempt N to a receiver running attempt N+1 of the same session and the receiver would accept it as long as \`SessionID\` matched. This PR closes that gap. ## What lands | Surface | Behaviour | |---|---| | \`verifyMessageAttemptContextHash(msg, sessionID)\` | No binding → pass (legacy/default). Binding + matching hash → pass. Binding + missing hash → \`ErrAttemptContextHashMissing\`. Binding + mismatch → \`ErrAttemptContextHashMismatch\`. | | \`attemptContextHashCarrier\` interface | One implementation covers all three FROST/tbtc-signer message types via their existing \`GetAttemptContextHash\` methods. | | 3 receive callbacks updated | After \`shouldAcceptNativeFROSTMessage\`, call \`verifyMessageAttemptContextHash\`. Failure → \`evidence.RecordReject(senderID, "attempt_context_hash_mismatch")\` so the policy can permanently exclude peers that consistently send stale-attempt messages. | ## Test coverage | File | Build | Cases | |---|---|---| | \`attempt_context_binding_validation_frost_native_test.go\` | \`frost_native && frost_roast_retry\` | 5 (no-binding, matching, missing, mismatch, real-message integration with rebind) | | \`attempt_context_binding_validation_default_build_test.go\` | \`frost_native && !frost_roast_retry\` | 1 (default build always passes; rollback promise upheld) | ## Migration path 1. **Phase 1B (already shipped):** field structurally validated when present, optional otherwise. 2. **This PR:** enforced *only* when the session has a ROAST-attempt binding. Default builds and non-ROAST tagged sessions continue to ignore the field. 3. **Future PR:** once production has rolled out a version that populates the field on every outbound message, enforcement can be made unconditional. ## Verification | Command | Result | |---|---| | \`go build ./...\` + tagged | both clean | | \`go test ./pkg/frost/...\` | pass | | \`go test -tags 'frost_native frost_tbtc_signer'\` | pass | | \`go test -tags 'frost_native frost_tbtc_signer frost_roast_retry'\` | pass | | \`staticcheck -checks '-SA1019' ./pkg/frost/...\` | silent | | \`go vet ./pkg/frost/...\` | clean | | \`gofmt -l ./pkg/frost/signing/\` | silent | ## Test plan - [ ] CI green. - [ ] Reviewer confirms the "no binding = skip enforcement" gate is acceptable (alternative: always enforce when build tag set, regardless of binding -- riskier during transitions). - [ ] Reviewer confirms the failure-mode rejects record evidence rather than just dropping (so misbehaving peers accumulate exclusion-worthy counts).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the M4 gap from the original PR #3866 review by adding
the two evidence categories the RFC-21 Phase-2 work left as future
work: validation-rejection evidence and first-write-wins-conflict
evidence.
With this PR, the `NextAttempt` policy can permanently exclude
misbehaving peers on all four ROAST blame channels --
transport-overflow, validation-reject, equivocation-conflict, and
silence -- instead of just overflow + silence.
Why this matters
A peer that only sends malformed messages (validation rejects,
never overflows the channel) was previously indistinguishable from
a silent peer. The transient silence-parking policy would
bench-and-reinstate them indefinitely, never permanently excluding
the malicious behaviour. Same for a peer equivocating mid-attempt:
the existing first-write-wins assembly correctly dropped the
conflicting retransmission but only logged the event -- the bundle
carried no structured evidence the coordinator's policy could act
on.
What lands
Recorder API
Wire types
Both fields use `omitempty` so pre-PR snapshots round-trip without
the new fields. `Validate()` enforces sorted-ascending invariants.
NextAttempt policy
`computeNextAttempt` merges `overflowBlamed`, `rejectBlamed`,
`conflictBlamed` into the permanent ExcludedSet. The
`blamedSenders` helper is factored out so all three categories
share the deterministic sort + threshold-comparison logic.
Receive-loop wiring
Three reject sites and three conflict sites updated across the two
files that house the three FROST/tbtc-signer receive loops:
Test coverage (15 new cases)
Verification
RFC-21 status
With this PR, all four ROAST evidence categories are operational.
M4 from the original PR #3866 review is fully closed. The
keep-core code arc for RFC-21 is now feature-complete; remaining
work is operations-side (integration testnet, manifest flip).
Test plan