Skip to content

Commit 266272c

Browse files
authored
feat(frost/roast): RFC-21 Phase 2 -- receiver overflow tracking (no-op default) (#3966)
## Summary RFC-21 Phase 2: introduces the **`EvidenceRecorder`** interface and a bounded reference implementation in `pkg/frost/roast/attempt`, then plumbs it through the three FROST/tbtc-signer receive loops so the previously silent `select { default }` drops become bounded transition-evidence recording sites. **No protocol behaviour change.** All three call sites pass `attempt.NoOpRecorder()` as the recorder, so overflow-path observability is identical to before. A coordinator-aware caller in a later RFC-21 phase will inject a real recorder. ## What lands ### New package surface (`pkg/frost/roast/attempt`) | Surface | Role | |---|---| | `EvidenceRecorder` interface | `RecordOverflow(sender)` + `Snapshot() Evidence`. Phase 2 covers overflow only; later phases extend with reject / conflict / silence. | | `Evidence` snapshot | `Overflows map[group.MemberIndex]uint`. Empty map when no overflow. | | `NewBoundedRecorder()` | Thread-safe recorder with default per-sender quota of 8 (matches `categoryQuota.Overflow` in RFC-21 Layer A). | | `NewBoundedRecorderWithQuota(q)` | Test seam for non-default quotas. | | `NoOpRecorder()` | Discards events; returns an empty snapshot. The Phase 2 default at every call site. | ### New signing-package helper `pkg/frost/signing/evidence_overflow.go` adds `enqueueOrRecordOverflow[T senderIndexedMessage](payload, target, recorder) bool`. This is the shared select-or-record body that replaces the inline `select { default }` drops. Pulling it out lets the recorder integration be unit-tested in isolation without spinning up a network channel. ### Plumbing - `collectNativeFROSTRoundOneMessages` - `collectNativeFROSTRoundTwoMessages` - `collectBuildTaggedTBTCSignerRoundContributionMessages` Each now accepts an `attempt.EvidenceRecorder` parameter and calls `enqueueOrRecordOverflow` in place of the inline select. All three callers pass `attempt.NoOpRecorder()`. ## Why the helper refactor The original inline `select { default }` drops cannot be tested without a real channel / network. Extracting the body into `enqueueOrRecordOverflow[T]` makes the integration testable directly, which is what gives us confidence that overflow drops actually reach the recorder. ## Test coverage ### Recorder unit tests (7) under `pkg/frost/roast/attempt/evidence_recorder_test.go` - `TestNoOpRecorder_IsObservablyInert` - `TestBoundedRecorder_CountsOverflowsBySender` - `TestBoundedRecorder_SaturatesAtQuota` - `TestBoundedRecorder_DefaultQuotaIs8` (asserts the constant matches the RFC literal) - `TestBoundedRecorder_SnapshotIsDeepCopy` - `TestBoundedRecorder_ConcurrentRecordersAreRaceSafe` (8 producers × 16 senders × 200 records) - `TestNoOpRecorder_DistinctInstancesShareSemantics` ### Helper unit tests (8) under `pkg/frost/signing/evidence_overflow_test.go` - `TestEnqueueOrRecordOverflow_EnqueuesWhenChannelHasRoom` - `TestEnqueueOrRecordOverflow_RecordsOverflowWhenChannelIsFull` - `TestEnqueueOrRecordOverflow_NoOpRecorderHasNoObservableEffect` - `TestEnqueueOrRecordOverflow_WorksForRoundTwoMessages` - `TestEnqueueOrRecordOverflow_WorksForTBTCSignerContributionMessages` - `TestEnqueueOrRecordOverflow_RepeatedOverflowsSaturateAtQuota` - `TestEnqueueOrRecordOverflow_ConcurrentCallersAreRaceSafe` ### Verification | Command | Result | |---|---| | `go build ./...` | clean | | `go build -tags 'frost_native frost_tbtc_signer' ./pkg/frost/...` | clean | | `go test -tags 'frost_native frost_tbtc_signer' ./pkg/frost/...` | pass (5 packages) | | `go test -race -run TestEnqueueOrRecordOverflow_ConcurrentCallersAreRaceSafe` | pass | | `staticcheck -checks '-SA1019' ./pkg/frost/...` | silent | | `pkg/tbtc` regression subset | pass (91s) | ## Test plan - [ ] CI green: build/test/lint/scan/vet. - [ ] Reviewer confirms the NoOp-default approach is acceptable for Phase 2 (no behaviour change is the explicit goal). - [ ] Reviewer confirms the bounded recorder's default quota of 8 matches the RFC-21 specification.
2 parents ea16280 + 84cffcd commit 266272c

6 files changed

Lines changed: 472 additions & 12 deletions
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
package attempt
2+
3+
import (
4+
"sync"
5+
6+
"github.com/keep-network/keep-core/pkg/protocol/group"
7+
)
8+
9+
// OverflowQuotaDefault is the default per-sender overflow event quota
10+
// enforced by NewBoundedRecorder. It matches the categoryQuota.Overflow
11+
// value documented in RFC-21 Layer A.
12+
//
13+
// A peer that overflows the inbound message channel more than the
14+
// quota allows in a single attempt is recorded only up to the quota:
15+
// further overflows are silently dropped by the recorder. This bounds
16+
// the per-attempt evidence size to O(|IncludedSet| * quota) regardless
17+
// of how aggressively a peer (or its network link) misbehaves.
18+
const OverflowQuotaDefault uint = 8
19+
20+
// EvidenceRecorder collects bounded, per-attempt evidence of receive-
21+
// path anomalies that the ROAST coordinator's exclusion policy may
22+
// later consume.
23+
//
24+
// Phase 2 introduces only the overflow channel; future phases extend
25+
// the interface with separate methods for reject events, first-write-
26+
// wins conflicts, and silent peers.
27+
//
28+
// Implementations must be safe for concurrent calls from multiple
29+
// goroutines, since the receive-callback closure in pkg/frost/signing
30+
// is driven by network goroutines.
31+
type EvidenceRecorder interface {
32+
// RecordOverflow notes that the inbound message channel was full
33+
// when a payload from the named sender arrived, causing the
34+
// payload to be dropped at the receive callback. The recorder
35+
// applies its own quota; callers do not need to suppress at the
36+
// call site.
37+
RecordOverflow(sender group.MemberIndex)
38+
// Snapshot returns a copy of the recorded evidence so far. The
39+
// returned value does not alias internal state; the recorder may
40+
// continue receiving events after Snapshot is called.
41+
Snapshot() Evidence
42+
}
43+
44+
// Evidence is the per-attempt snapshot of receive-path anomalies
45+
// captured by an EvidenceRecorder. It is the value the ROAST
46+
// coordinator's NextAttempt policy consumes (in a later RFC-21
47+
// phase) to derive the next attempt's ExcludedSet.
48+
type Evidence struct {
49+
// Overflows maps each sender to the number of overflow events
50+
// observed for that sender during the attempt, saturated at the
51+
// recorder's overflow quota. A missing key means the sender did
52+
// not overflow at all during the attempt.
53+
Overflows map[group.MemberIndex]uint
54+
}
55+
56+
// NewBoundedRecorder returns an EvidenceRecorder with default
57+
// per-sender quotas. The recorder is safe for concurrent use.
58+
//
59+
// Phase 2 wiring uses NoOpRecorder by default at every call site;
60+
// real use of the bounded recorder lands in a later phase behind a
61+
// build tag, when the coordinator state machine arrives.
62+
func NewBoundedRecorder() EvidenceRecorder {
63+
return NewBoundedRecorderWithQuota(OverflowQuotaDefault)
64+
}
65+
66+
// NewBoundedRecorderWithQuota returns a recorder with a custom
67+
// overflow quota. Intended for tests; production callers should use
68+
// NewBoundedRecorder so the per-attempt evidence size is uniform
69+
// across the network.
70+
func NewBoundedRecorderWithQuota(overflowQuota uint) EvidenceRecorder {
71+
return &boundedRecorder{
72+
overflowQuota: overflowQuota,
73+
overflows: map[group.MemberIndex]uint{},
74+
}
75+
}
76+
77+
// NoOpRecorder returns a recorder that discards every event and
78+
// reports an empty Evidence on Snapshot. It is the default at every
79+
// Phase 2 call site so the receive loops' observable behaviour stays
80+
// identical to pre-Phase-2 until a later phase wires real recorders.
81+
func NoOpRecorder() EvidenceRecorder {
82+
return noOpRecorder{}
83+
}
84+
85+
type boundedRecorder struct {
86+
mu sync.Mutex
87+
overflowQuota uint
88+
overflows map[group.MemberIndex]uint
89+
}
90+
91+
func (r *boundedRecorder) RecordOverflow(sender group.MemberIndex) {
92+
r.mu.Lock()
93+
defer r.mu.Unlock()
94+
if r.overflows[sender] < r.overflowQuota {
95+
r.overflows[sender]++
96+
}
97+
}
98+
99+
func (r *boundedRecorder) Snapshot() Evidence {
100+
r.mu.Lock()
101+
defer r.mu.Unlock()
102+
out := make(map[group.MemberIndex]uint, len(r.overflows))
103+
for sender, count := range r.overflows {
104+
out[sender] = count
105+
}
106+
return Evidence{Overflows: out}
107+
}
108+
109+
type noOpRecorder struct{}
110+
111+
func (noOpRecorder) RecordOverflow(group.MemberIndex) {}
112+
113+
func (noOpRecorder) Snapshot() Evidence {
114+
return Evidence{Overflows: map[group.MemberIndex]uint{}}
115+
}
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
package attempt
2+
3+
import (
4+
"sync"
5+
"testing"
6+
7+
"github.com/keep-network/keep-core/pkg/protocol/group"
8+
)
9+
10+
func TestNoOpRecorder_IsObservablyInert(t *testing.T) {
11+
rec := NoOpRecorder()
12+
for i := 0; i < 1000; i++ {
13+
rec.RecordOverflow(group.MemberIndex(i%5 + 1))
14+
}
15+
snap := rec.Snapshot()
16+
if len(snap.Overflows) != 0 {
17+
t.Fatalf(
18+
"NoOp recorder must report zero overflows; got %d entries",
19+
len(snap.Overflows),
20+
)
21+
}
22+
}
23+
24+
func TestBoundedRecorder_CountsOverflowsBySender(t *testing.T) {
25+
rec := NewBoundedRecorder()
26+
rec.RecordOverflow(1)
27+
rec.RecordOverflow(2)
28+
rec.RecordOverflow(1)
29+
30+
snap := rec.Snapshot()
31+
if got := snap.Overflows[1]; got != 2 {
32+
t.Fatalf("sender 1 overflow count: got %d want 2", got)
33+
}
34+
if got := snap.Overflows[2]; got != 1 {
35+
t.Fatalf("sender 2 overflow count: got %d want 1", got)
36+
}
37+
if _, ok := snap.Overflows[3]; ok {
38+
t.Fatal("sender 3 should have no entry")
39+
}
40+
}
41+
42+
func TestBoundedRecorder_SaturatesAtQuota(t *testing.T) {
43+
const quota uint = 4
44+
rec := NewBoundedRecorderWithQuota(quota)
45+
46+
for i := uint(0); i < quota+10; i++ {
47+
rec.RecordOverflow(1)
48+
}
49+
snap := rec.Snapshot()
50+
if got := snap.Overflows[1]; got != quota {
51+
t.Fatalf(
52+
"overflow count must saturate at quota %d; got %d",
53+
quota, got,
54+
)
55+
}
56+
}
57+
58+
func TestBoundedRecorder_DefaultQuotaIs8(t *testing.T) {
59+
rec := NewBoundedRecorder()
60+
for i := 0; i < 100; i++ {
61+
rec.RecordOverflow(1)
62+
}
63+
if got := rec.Snapshot().Overflows[1]; got != OverflowQuotaDefault {
64+
t.Fatalf(
65+
"default quota mismatch; got %d want %d",
66+
got, OverflowQuotaDefault,
67+
)
68+
}
69+
if OverflowQuotaDefault != 8 {
70+
t.Fatalf(
71+
"RFC-21 Layer A specifies overflow quota = 8; constant is %d",
72+
OverflowQuotaDefault,
73+
)
74+
}
75+
}
76+
77+
func TestBoundedRecorder_SnapshotIsDeepCopy(t *testing.T) {
78+
rec := NewBoundedRecorder()
79+
rec.RecordOverflow(1)
80+
rec.RecordOverflow(1)
81+
82+
snap := rec.Snapshot()
83+
snap.Overflows[1] = 999
84+
snap.Overflows[42] = 7
85+
86+
freshSnap := rec.Snapshot()
87+
if got := freshSnap.Overflows[1]; got != 2 {
88+
t.Fatalf(
89+
"snapshot mutation leaked into recorder state: got %d want 2",
90+
got,
91+
)
92+
}
93+
if _, ok := freshSnap.Overflows[42]; ok {
94+
t.Fatal("snapshot mutation leaked a new key into recorder state")
95+
}
96+
}
97+
98+
func TestBoundedRecorder_ConcurrentRecordersAreRaceSafe(t *testing.T) {
99+
const (
100+
recordersPerSender = 8
101+
sendersN = 16
102+
recordsPerRecorder = 200
103+
)
104+
rec := NewBoundedRecorderWithQuota(uint(recordersPerSender * recordsPerRecorder * 10))
105+
106+
var wg sync.WaitGroup
107+
for senderIdx := 1; senderIdx <= sendersN; senderIdx++ {
108+
sender := group.MemberIndex(senderIdx)
109+
for w := 0; w < recordersPerSender; w++ {
110+
wg.Add(1)
111+
go func() {
112+
defer wg.Done()
113+
for n := 0; n < recordsPerRecorder; n++ {
114+
rec.RecordOverflow(sender)
115+
}
116+
}()
117+
}
118+
}
119+
wg.Wait()
120+
121+
snap := rec.Snapshot()
122+
for senderIdx := 1; senderIdx <= sendersN; senderIdx++ {
123+
want := uint(recordersPerSender * recordsPerRecorder)
124+
if got := snap.Overflows[group.MemberIndex(senderIdx)]; got != want {
125+
t.Fatalf(
126+
"sender %d concurrent count: got %d want %d",
127+
senderIdx, got, want,
128+
)
129+
}
130+
}
131+
}
132+
133+
func TestNoOpRecorder_DistinctInstancesShareSemantics(t *testing.T) {
134+
a := NoOpRecorder()
135+
b := NoOpRecorder()
136+
a.RecordOverflow(1)
137+
b.RecordOverflow(2)
138+
if len(a.Snapshot().Overflows) != 0 || len(b.Snapshot().Overflows) != 0 {
139+
t.Fatal("NoOp instances must not retain state")
140+
}
141+
}
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
//go:build frost_native
2+
3+
package signing
4+
5+
import (
6+
"github.com/keep-network/keep-core/pkg/frost/roast/attempt"
7+
"github.com/keep-network/keep-core/pkg/protocol/group"
8+
)
9+
10+
// senderIndexedMessage is the minimal contract a protocol message must
11+
// satisfy for enqueueOrRecordOverflow to handle it: the message must
12+
// expose its sender so the recorder can attribute overflow events to a
13+
// specific member.
14+
type senderIndexedMessage interface {
15+
SenderID() group.MemberIndex
16+
}
17+
18+
// enqueueOrRecordOverflow attempts to enqueue payload onto target. If
19+
// the channel is full, the overflow is recorded against the payload's
20+
// sender on the supplied recorder instead. Returns true if the payload
21+
// was enqueued, false if the overflow was recorded.
22+
//
23+
// This is the shared select-or-record body that replaces the three
24+
// inline select { default } drop sites in the FROST/tbtc-signer
25+
// receive loops. Pulling it out lets the recorder integration be unit-
26+
// tested directly without spinning up a network channel.
27+
//
28+
// Phase 2 callers pass attempt.NoOpRecorder(), so behaviour is
29+
// observably unchanged from before RFC-21 wiring. A coordinator-aware
30+
// caller in a later phase injects a real recorder.
31+
func enqueueOrRecordOverflow[T senderIndexedMessage](
32+
payload T,
33+
target chan<- T,
34+
recorder attempt.EvidenceRecorder,
35+
) bool {
36+
select {
37+
case target <- payload:
38+
return true
39+
default:
40+
recorder.RecordOverflow(payload.SenderID())
41+
return false
42+
}
43+
}

0 commit comments

Comments
 (0)