Skip to content

Commit 6472a40

Browse files
authored
feat(tbtc): RFC-21 Phase 6.4 -- signing-loop participant-selection dispatcher (#3984)
## Summary **Closes Phase 6 of RFC-21.** Abstracts the participant-selection call site in \`pkg/tbtc/signing_loop.go\` behind a small dispatcher interface. The legacy implementation is installed as the default; Phase 7 will install the ROAST-driven implementation alongside AggregateBundle production. **The migration here is the *abstraction*, not a behavioural change.** Both default and \`frost_roast_retry\` builds today execute the same legacy retry shuffle. The dispatcher exists so Phase 7 can replace it without touching the call shape. Stacked on #3983 (Phase 6.3). ## Why this scope During implementation, the participant-migration target turned out to require two pieces that aren't fully wired yet: 1. AggregateBundle production at attempt-completion time (on the elected coordinator's node). 2. A per-session bundle registry so \`signing_loop\` can find the most recent \`TransitionMessage\` for a given message. Both are Phase 7 work. PR 6.4 ships the **dispatcher abstraction** that lets Phase 7 slot the ROAST implementation in without touching \`signing_loop.go\` itself, plus the legacy implementation as the operational fallback that the readiness env var disables. ## What lands | File | Role | |---|---| | \`pkg/tbtc/signing_loop_roast_dispatcher.go\` (new) | \`signingParticipantSelector\` interface (\`Select(members, seed, retryCount, honestThreshold, sessionID) → addresses\`). \`defaultSigningParticipantSelector()\` returns the legacy impl. | | \`pkg/tbtc/signing_loop_legacy_selector.go\` (new) | \`legacySigningParticipantSelector\` -- byte-identical call to \`retry.EvaluateRetryParticipantsForSigning\`. Documented as the rollback path through Phase 6. | | \`pkg/tbtc/signing_loop.go\` (modified) | \`signingRetryLoop\` gains \`participantSelector\` field; \`qualifiedOperatorsSet\` routes through it. \`pkg/frost/retry\` import removed (only the legacy selector uses it now). | ## Test coverage (5 cases) - \`defaultSigningParticipantSelector\` returns the legacy impl - legacy selector delegates to retry shuffle - legacy selector propagates retry-shuffle errors - \`signingRetryLoop.qualifiedOperatorsSet\` routes through the dispatcher (recording selector verifies) - selector errors propagate through \`signingRetryLoop\` with \`errors.Is\` preserving the sentinel ## What Phase 7 will add - AggregateBundle production at the executor-adapter end (the elected coordinator's node generates a \`TransitionMessage\` at attempt completion) - Per-session bundle registry so \`signing_loop\` can look up the most recent bundle for the message - ROAST-driven \`signingParticipantSelector\` that consumes the bundle via \`EvaluateRoastRetryForSigning\` and falls back to the legacy selector when no bundle is available - Readiness manifest flip once integration tests pass on a real testnet ## Pre-existing test failure note \`TestNode_RunCoordinationLayer\` fails under the \`frost_native frost_tbtc_signer frost_roast_retry\` tag combination on the integration tip *without* the Phase 6.4 changes (verified by checking out integration-tip's tbtc package and re-running). **Not introduced by this PR.** Tracked separately. Default-build \`go test ./pkg/tbtc/... ./pkg/frost/...\` (149s) passes cleanly. ## Verification | Command | Result | |---|---| | \`go build ./...\` | clean | | \`go test ./pkg/tbtc/... -count=1\` | pass (149s default build) | | \`go test ./pkg/frost/... -count=1\` | pass | | \`staticcheck -checks '-SA1019' ./pkg/tbtc/...\` | silent | | \`go vet ./pkg/tbtc/...\` | clean | | \`gofmt -l ./pkg/tbtc/\` | silent | ## Phase 6 complete | PR | Scope | State | |---|---|---| | 6.1 (#3981) | DKG group-public-key extraction | open | | 6.2 (#3982) | BuildAttemptContextFromRequest | open | | 6.3 (#3983) | Wire orchestration at executor adapter | open | | **6.4 (this)** | **signing-loop participant-selection dispatcher** | **open** | Phase 7: AggregateBundle wiring + ROAST selector + manifest flip. ## Test plan - [ ] CI green (default build). - [ ] Reviewer confirms the dispatcher abstraction is the right granularity (alternative: function-pointer indirection without an interface). - [ ] Reviewer confirms deferring participant migration to Phase 7 is acceptable. The trade-off: smaller Phase 6 PRs but Phase 7 also covers the AggregateBundle wiring + ROAST selector + manifest flip.
2 parents 3d2ff18 + 9c38f76 commit 6472a40

4 files changed

Lines changed: 233 additions & 2 deletions

File tree

pkg/tbtc/signing_loop.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313

1414
"github.com/ipfs/go-log/v2"
1515
"github.com/keep-network/keep-core/pkg/chain"
16-
"github.com/keep-network/keep-core/pkg/frost/retry"
1716
"github.com/keep-network/keep-core/pkg/frost/signing"
1817
"github.com/keep-network/keep-core/pkg/protocol/group"
1918
"golang.org/x/exp/slices"
@@ -107,6 +106,12 @@ type signingRetryLoop struct {
107106
attemptSeed int64
108107

109108
doneCheck signingDoneCheckStrategy
109+
110+
// participantSelector dispatches qualified-operator selection.
111+
// Default: legacy retry shuffle. Phase 7 may install a
112+
// ROAST-driven implementation behind the frost_roast_retry
113+
// build tag once AggregateBundle production is wired upstream.
114+
participantSelector signingParticipantSelector
110115
}
111116

112117
func newSigningRetryLoop(
@@ -130,6 +135,7 @@ func newSigningRetryLoop(
130135
attemptStartBlock: initialStartBlock,
131136
attemptSeed: signingAttemptSeed(message),
132137
doneCheck: doneCheck,
138+
participantSelector: defaultSigningParticipantSelector(),
133139
}
134140
}
135141

@@ -492,11 +498,16 @@ func (srl *signingRetryLoop) qualifiedOperatorsSet(
492498
)
493499
}
494500

495-
qualifiedOperators, err := retry.EvaluateRetryParticipantsForSigning(
501+
// RFC-21 Phase 6.4: dispatch through participantSelector so a
502+
// future ROAST-driven implementation can be installed behind
503+
// the frost_roast_retry build tag without touching this call
504+
// site. Default and current behaviour: legacy retry shuffle.
505+
qualifiedOperators, err := srl.participantSelector.Select(
496506
readySigningGroupOperators,
497507
srl.attemptSeed,
498508
retryCount,
499509
uint(srl.groupParameters.HonestThreshold),
510+
fmt.Sprintf("%v", srl.message),
500511
)
501512
if err != nil {
502513
return nil, fmt.Errorf(
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
package tbtc
2+
3+
import (
4+
"fmt"
5+
6+
"github.com/keep-network/keep-core/pkg/chain"
7+
"github.com/keep-network/keep-core/pkg/frost/retry"
8+
)
9+
10+
// legacySigningParticipantSelector is the pre-RFC-21 implementation:
11+
// it calls the pseudo-random retry shuffle in pkg/frost/retry.
12+
// Kept as the canonical fallback through Phase 6; Phase 7 may
13+
// remove it once the ROAST-driven retry path is fully wired and
14+
// the readiness manifest flips.
15+
//
16+
// The legacy code is *intentionally retained* through Phase 6 to
17+
// preserve the operational rollback path: if a deployment toggles
18+
// the readiness env var off, this implementation is what the
19+
// dispatcher falls back to.
20+
type legacySigningParticipantSelector struct{}
21+
22+
func (legacySigningParticipantSelector) Select(
23+
members []chain.Address,
24+
seed int64,
25+
retryCount uint,
26+
honestThreshold uint,
27+
_ string,
28+
) ([]chain.Address, error) {
29+
qualifiedOperators, err := retry.EvaluateRetryParticipantsForSigning(
30+
members,
31+
seed,
32+
retryCount,
33+
honestThreshold,
34+
)
35+
if err != nil {
36+
return nil, fmt.Errorf(
37+
"legacy participant selector: random operator selection failed: %w",
38+
err,
39+
)
40+
}
41+
return qualifiedOperators, nil
42+
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package tbtc
2+
3+
import (
4+
"github.com/keep-network/keep-core/pkg/chain"
5+
)
6+
7+
// signingParticipantSelector picks the set of operators qualified for
8+
// a signing attempt. The legacy implementation is the pseudo-random
9+
// retry shuffle in pkg/frost/retry; the RFC-21 Phase-6 migration
10+
// introduces this interface so an alternate ROAST-driven
11+
// implementation can be installed behind the frost_roast_retry build
12+
// tag without touching the call site.
13+
//
14+
// PR 6.4 ships the dispatcher with only the legacy implementation
15+
// installed; Phase 7 wires the ROAST-driven implementation along
16+
// with the supporting AggregateBundle production at the executor-
17+
// adapter layer. Until Phase 7, behaviour is byte-identical to
18+
// pre-RFC-21 retry semantics.
19+
type signingParticipantSelector interface {
20+
// Select returns the set of operators qualified to participate
21+
// in the given signing attempt. members is the set of operators
22+
// whose ready signal was received for this attempt. seed is the
23+
// per-message retry seed; retryCount is 0-based (i.e. 0 for the
24+
// first retry). honestThreshold is the group's signing
25+
// threshold.
26+
Select(
27+
members []chain.Address,
28+
seed int64,
29+
retryCount uint,
30+
honestThreshold uint,
31+
sessionID string,
32+
) ([]chain.Address, error)
33+
}
34+
35+
// defaultSigningParticipantSelector returns the legacy implementation
36+
// installed by every Phase-6 build (default + frost_roast_retry).
37+
// Phase 7 will install a ROAST-driven implementation in a follow-up
38+
// PR that also wires AggregateBundle production.
39+
func defaultSigningParticipantSelector() signingParticipantSelector {
40+
return legacySigningParticipantSelector{}
41+
}
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
package tbtc
2+
3+
import (
4+
"errors"
5+
"testing"
6+
7+
"github.com/keep-network/keep-core/pkg/chain"
8+
"github.com/keep-network/keep-core/pkg/protocol/group"
9+
)
10+
11+
// recordingSelector counts how often Select was called and returns
12+
// a fixed result. Tests use it to assert the dispatcher routes
13+
// participant selection through the configured selector rather
14+
// than the legacy path.
15+
type recordingSelector struct {
16+
calls int
17+
result []chain.Address
18+
err error
19+
}
20+
21+
func (r *recordingSelector) Select(
22+
members []chain.Address,
23+
_ int64,
24+
_ uint,
25+
_ uint,
26+
_ string,
27+
) ([]chain.Address, error) {
28+
r.calls++
29+
if r.err != nil {
30+
return nil, r.err
31+
}
32+
if r.result != nil {
33+
return r.result, nil
34+
}
35+
return members, nil
36+
}
37+
38+
func TestDefaultSigningParticipantSelector_IsLegacy(t *testing.T) {
39+
sel := defaultSigningParticipantSelector()
40+
if _, ok := sel.(legacySigningParticipantSelector); !ok {
41+
t.Fatalf(
42+
"defaultSigningParticipantSelector must return legacy implementation; got %T",
43+
sel,
44+
)
45+
}
46+
}
47+
48+
func TestLegacySigningParticipantSelector_DelegatesToRetryShuffle(t *testing.T) {
49+
members := []chain.Address{
50+
chain.Address("op-1"),
51+
chain.Address("op-2"),
52+
chain.Address("op-3"),
53+
chain.Address("op-4"),
54+
chain.Address("op-5"),
55+
}
56+
sel := legacySigningParticipantSelector{}
57+
got, err := sel.Select(members, 42, 0, 3, "session-x")
58+
if err != nil {
59+
t.Fatalf("unexpected error: %v", err)
60+
}
61+
if len(got) < 3 {
62+
t.Fatalf("expected at least 3 qualified operators, got %d", len(got))
63+
}
64+
}
65+
66+
func TestLegacySigningParticipantSelector_PropagatesErrors(t *testing.T) {
67+
sel := legacySigningParticipantSelector{}
68+
_, err := sel.Select(
69+
[]chain.Address{chain.Address("op-1")},
70+
0, 0,
71+
99, // honest threshold higher than member count
72+
"session-x",
73+
)
74+
if err == nil {
75+
t.Fatal("expected error from retry shuffle")
76+
}
77+
}
78+
79+
func TestSigningRetryLoopUsesDispatcher(t *testing.T) {
80+
sentinel := []chain.Address{
81+
chain.Address("op-1"),
82+
chain.Address("op-2"),
83+
chain.Address("op-3"),
84+
}
85+
recorder := &recordingSelector{result: sentinel}
86+
87+
srl := &signingRetryLoop{
88+
signingGroupOperators: chain.Addresses{
89+
chain.Address("op-1"),
90+
chain.Address("op-2"),
91+
chain.Address("op-3"),
92+
chain.Address("op-4"),
93+
chain.Address("op-5"),
94+
},
95+
groupParameters: &GroupParameters{
96+
HonestThreshold: 3,
97+
},
98+
attemptCounter: 1,
99+
attemptSeed: 42,
100+
participantSelector: recorder,
101+
}
102+
103+
set, err := srl.qualifiedOperatorsSet([]group.MemberIndex{1, 2, 3, 4, 5})
104+
if err != nil {
105+
t.Fatalf("unexpected error: %v", err)
106+
}
107+
if recorder.calls != 1 {
108+
t.Fatalf("expected dispatcher to be called once; got %d", recorder.calls)
109+
}
110+
if len(set) != len(sentinel) {
111+
t.Fatalf(
112+
"expected %d qualified operators (the sentinel), got %d",
113+
len(sentinel), len(set),
114+
)
115+
}
116+
}
117+
118+
func TestSigningRetryLoopPropagatesSelectorError(t *testing.T) {
119+
wantErr := errors.New("synthetic selector failure")
120+
srl := &signingRetryLoop{
121+
signingGroupOperators: chain.Addresses{
122+
chain.Address("op-1"),
123+
chain.Address("op-2"),
124+
},
125+
groupParameters: &GroupParameters{HonestThreshold: 2},
126+
attemptCounter: 1,
127+
attemptSeed: 0,
128+
participantSelector: &recordingSelector{err: wantErr},
129+
}
130+
_, err := srl.qualifiedOperatorsSet([]group.MemberIndex{1, 2})
131+
if err == nil {
132+
t.Fatal("expected selector error to propagate")
133+
}
134+
if !errors.Is(err, wantErr) {
135+
t.Fatalf("expected wrapped sentinel; got %v", err)
136+
}
137+
}

0 commit comments

Comments
 (0)