Skip to content

Commit 3d2ff18

Browse files
mswilkisonmswilkison
authored
feat(frost/signing): RFC-21 Phase 6.3 -- wire orchestration at executor adapter (#3983)
## Summary Third Phase-6 PR. Wires \`BeginOrchestrationForSession\` into the \`nativeExecutionFFIExecutorAdapter.Execute\` method with the strict error-handling discipline from RFC-21 (#3980). Stacked on #3982 (Phase 6.2). ## Error taxonomy implemented | Source | Class | Action | |---|---|---| | \`BuildAttemptContextFromRequest\` failure (any reason) | Static -- deterministic per request | Log INFO, fall back (no orchestration) | | \`ErrRoastRetryReadinessOptOut\` | Static -- deterministic per env var | Log INFO, fall back | | \`ErrNoRoastRetryCoordinatorRegistered\` | Static -- deterministic per registration | Log INFO, fall back | | Any other \`BeginOrchestrationForSession\` error | **Runtime** -- non-deterministic across nodes | **HARD FAIL** | The hard-fail discipline is load-bearing for safety. A fall-back-on-runtime-error policy lets node A run the legacy shuffle while node B proceeds with the ROAST state machine, splitting the signing group on \`NextAttempt\`. Gemini's Phase-6 review flagged this as a critical risk; this PR is the implementation of the resolution. ## What lands | File | Build tag | Role | |---|---|---| | \`roast_retry_executor_entry_default_build.go\` | \`!frost_native\` | Permanent stub returning \`(nil, nil)\`. Executor adapter compiles + runs with zero orchestration overhead in default builds. | | \`roast_retry_executor_entry_frost_native.go\` | \`frost_native\` | Real implementation walking (build context, begin, return cleanup) with error classification. Defensive nil-logger handling. | | \`roast_retry_orchestration.go\` (extended) | untagged | Adds \`ErrNoRoastRetryCoordinatorRegistered\` sentinel; \`BeginOrchestrationForSession\` wraps it via \`fmt.Errorf %w\` | | \`native_ffi_executor_adapter.go\` (modified) | untagged | \`Execute\` calls the entry helper after building the FFI request, defers cleanup, then proceeds to \`primitive.Sign\` | ## Why \`BuildAttemptContextFromRequest\` failures are STATIC Even though they look like "runtime" errors (nil fields, zero attempt numbers, etc.), they are **per-input deterministic**: the same \`NativeExecutionFFISigningRequest\` produces the same construction outcome on every honest node. Two honest nodes can only disagree on construction success if they receive different requests, which is an upstream-orchestrator bug rather than a ROAST concern. Falling back to legacy in this case preserves liveness without splitting the signing group. Coordinator state-machine errors (BeginAttempt OOM, internal invariant violations) are genuinely non-deterministic per-node and therefore must hard-fail. ## Test coverage | File | Build | Cases | |---|---|---| | \`roast_retry_executor_entry_test.go\` | default | 1 (stub returns (nil, nil) for any input) | | \`roast_retry_executor_entry_frost_native_test.go\` | \`frost_native\` | 4 (no coordinator registered; FrostUniFFIV1; nil signer material; zero attempt number -- all static fallbacks) | | \`roast_retry_executor_entry_frost_roast_retry_test.go\` | \`frost_native && frost_roast_retry\` | 4 (env var unset; registry empty; happy path; **HARD FAIL on runtime BeginAttempt error**) | ## Verification | Command | Result | |---|---| | \`go build ./...\` | clean | | \`go test ./pkg/frost/...\` | pass (default build) | | \`go test -tags 'frost_native frost_tbtc_signer frost_roast_retry' ./pkg/frost/...\` | pass | | \`go test -race -tags 'frost_native frost_tbtc_signer frost_roast_retry' ./pkg/frost/...\` | pass | | \`staticcheck -checks '-SA1019' ./pkg/frost/...\` | silent | | \`go vet ./pkg/frost/...\` | clean | | \`gofmt -l ./pkg/frost/signing/\` | silent | ## Phase 6 status | PR | Scope | State | |---|---|---| | 6.1 (#3981) | DKG group-public-key extraction | open | | 6.2 (#3982) | BuildAttemptContextFromRequest | open | | **6.3 (this)** | **Wire orchestration at executor adapter** | **open** | | 6.4 | Migrate three signing call sites onto EvaluateRoastRetryForSigning | next | ## Test plan - [ ] CI green. - [ ] Reviewer confirms the static-vs-runtime classification matches the RFC-21 Phase-6 discipline. - [ ] Reviewer confirms the defensive nil-logger fallback is acceptable (alternative: require all callers to pass a real logger).
2 parents 6aab2fc + fcadd9b commit 3d2ff18

7 files changed

Lines changed: 517 additions & 16 deletions

‎pkg/frost/signing/native_ffi_executor_adapter.go‎

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -85,21 +85,39 @@ func (nefea *nativeExecutionFFIExecutorAdapter) Execute(
8585
return nil, fmt.Errorf("%w: [%v]", ErrNativeCryptographyUnavailable, err)
8686
}
8787

88-
signature, err := nefea.primitive.Sign(
89-
ctx,
90-
logger,
91-
&NativeExecutionFFISigningRequest{
92-
Message: request.Message,
93-
SessionID: request.SessionID,
94-
MemberIndex: request.MemberIndex,
95-
GroupSize: request.GroupSize,
96-
DishonestThreshold: request.DishonestThreshold,
97-
Channel: request.Channel,
98-
MembershipValidator: request.MembershipValidator,
99-
SignerMaterial: signerMaterial,
100-
Attempt: cloneAttempt(request.Attempt),
101-
},
102-
)
88+
ffiRequest := &NativeExecutionFFISigningRequest{
89+
Message: request.Message,
90+
SessionID: request.SessionID,
91+
MemberIndex: request.MemberIndex,
92+
GroupSize: request.GroupSize,
93+
DishonestThreshold: request.DishonestThreshold,
94+
Channel: request.Channel,
95+
MembershipValidator: request.MembershipValidator,
96+
SignerMaterial: signerMaterial,
97+
Attempt: cloneAttempt(request.Attempt),
98+
}
99+
100+
// RFC-21 Phase 6.3: ROAST orchestration entry. The helper
101+
// returns (cleanup, error):
102+
// - cleanup non-nil, error nil -> orchestration active;
103+
// defer cleanup so success and failure return paths converge.
104+
// - cleanup nil, error nil -> static-configuration fallback
105+
// (env var unset, no coordinator registered, or material
106+
// format not extractable). Proceed without orchestration; the
107+
// receive loops use NoOp recorder semantics (Phase 5 behaviour).
108+
// - cleanup nil, error non-nil -> RUNTIME orchestration failure.
109+
// HARD FAIL to prevent group fracture across honest signers.
110+
// In the default build (no frost_native tag) the helper is a
111+
// permanent no-op returning (nil, nil).
112+
orchCleanup, orchErr := attemptRoastRetryOrchestrationFromRequest(ffiRequest, logger)
113+
if orchErr != nil {
114+
return nil, orchErr
115+
}
116+
if orchCleanup != nil {
117+
defer orchCleanup()
118+
}
119+
120+
signature, err := nefea.primitive.Sign(ctx, logger, ffiRequest)
103121
if err != nil {
104122
return nil, err
105123
}

‎pkg/frost/signing/roast_retry_executor_entry_default_build.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
//go:build !frost_native
2+
3+
package signing
4+
5+
import "github.com/ipfs/go-log/v2"
6+
7+
// attemptRoastRetryOrchestrationFromRequest is the executor-adapter
8+
// entry point for RFC-21 Phase-6 ROAST orchestration. In the
9+
// default build (no frost_native tag) it is a permanent no-op
10+
// stub: orchestration cannot run without the frost_native code
11+
// path, so the executor adapter behaves exactly as in Phase 5.
12+
//
13+
// The function returns (cleanup, error). cleanup is non-nil when
14+
// orchestration started successfully; the executor adapter defers
15+
// it. error is non-nil only for RUNTIME failures the executor
16+
// must propagate to its caller (static-configuration errors are
17+
// logged and the cleanup is returned nil to signal "no
18+
// orchestration; fall back to legacy receive-loop semantics").
19+
//
20+
// The default-build stub returns (nil, nil) unconditionally.
21+
func attemptRoastRetryOrchestrationFromRequest(
22+
_ *NativeExecutionFFISigningRequest,
23+
_ log.StandardLogger,
24+
) (func(), error) {
25+
return nil, nil
26+
}

‎pkg/frost/signing/roast_retry_executor_entry_frost_native.go‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
//go:build frost_native
2+
3+
package signing
4+
5+
import (
6+
"errors"
7+
"fmt"
8+
9+
"github.com/ipfs/go-log/v2"
10+
)
11+
12+
// attemptRoastRetryOrchestrationFromRequest is the executor-adapter
13+
// entry point for RFC-21 Phase-6 ROAST orchestration. It:
14+
//
15+
// 1. Builds an attempt.AttemptContext from the FFI signing
16+
// request (BuildAttemptContextFromRequest, gated frost_native).
17+
//
18+
// 2. If construction fails with ErrUnsupportedSignerMaterialFormat
19+
// -- e.g. the deployment still uses FrostUniFFIV1 material --
20+
// the failure is a STATIC configuration condition: every
21+
// honest signer with the same deployment material observes the
22+
// same error deterministically. Log at INFO and return
23+
// (nil, nil) so the executor proceeds without orchestration.
24+
//
25+
// 3. Any other AttemptContext construction error is a RUNTIME
26+
// failure (nil fields, malformed material payload, etc.). Per
27+
// the RFC-21 Phase-6 orchestration error taxonomy, runtime
28+
// errors must HARD FAIL to prevent group fracture: node A
29+
// falling back to legacy while node B proceeds with ROAST
30+
// would split the participant set on NextAttempt.
31+
//
32+
// 4. Calls BeginOrchestrationForSession with the context.
33+
// ErrRoastRetryReadinessOptOut and
34+
// ErrNoRoastRetryCoordinatorRegistered are static-configuration
35+
// errors -- log at INFO and return (nil, nil). Any other error
36+
// is treated as RUNTIME and propagated unchanged.
37+
//
38+
// 5. On success returns the cleanup function the executor adapter
39+
// must defer.
40+
//
41+
// The function returns (cleanup, error):
42+
// - cleanup non-nil + error nil -> orchestration active; defer cleanup.
43+
// - cleanup nil + error nil -> static fallback; proceed legacy.
44+
// - cleanup nil + error non-nil -> runtime failure; propagate.
45+
func attemptRoastRetryOrchestrationFromRequest(
46+
request *NativeExecutionFFISigningRequest,
47+
logger log.StandardLogger,
48+
) (func(), error) {
49+
if logger == nil {
50+
// Defensive: existing executor-adapter tests pass nil here.
51+
// The helper logs static-fallback diagnostics, so a nil
52+
// logger must not panic the executor.
53+
logger = log.Logger("keep-frost-roast-orchestration")
54+
}
55+
ctx, err := BuildAttemptContextFromRequest(request)
56+
if err != nil {
57+
// All BuildAttemptContextFromRequest errors are treated as
58+
// STATIC fallbacks because they are deterministic per-input:
59+
// the same NativeExecutionFFISigningRequest produces the
60+
// same construction outcome on every honest node, so
61+
// every node would make the same fall-back decision. The
62+
// RFC-21 Phase-6 hard-fail discipline applies only to
63+
// non-deterministic RUNTIME errors that originate inside
64+
// the Coordinator state machine (next branch).
65+
logger.Infof(
66+
"ROAST orchestration unavailable for session %q: %v",
67+
request.SessionID,
68+
err,
69+
)
70+
return nil, nil
71+
}
72+
73+
handle, cleanup, err := BeginOrchestrationForSession(request.SessionID, ctx)
74+
if err != nil {
75+
switch {
76+
case errors.Is(err, ErrRoastRetryReadinessOptOut),
77+
errors.Is(err, ErrNoRoastRetryCoordinatorRegistered):
78+
// Static-configuration errors -> safe to fall back.
79+
logger.Infof(
80+
"ROAST retry disabled for session %q: %v",
81+
request.SessionID,
82+
err,
83+
)
84+
return nil, nil
85+
default:
86+
// Runtime failure: HARD FAIL.
87+
return nil, fmt.Errorf(
88+
"ROAST orchestration: begin session %q: %w",
89+
request.SessionID,
90+
err,
91+
)
92+
}
93+
}
94+
_ = handle // Phase 6.4+ uses this for retry adapter invocation.
95+
return cleanup, nil
96+
}

‎pkg/frost/signing/roast_retry_executor_entry_frost_native_test.go‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
//go:build frost_native
2+
3+
package signing
4+
5+
import (
6+
"encoding/json"
7+
"math/big"
8+
"testing"
9+
10+
"github.com/ipfs/go-log/v2"
11+
"github.com/keep-network/keep-core/pkg/protocol/group"
12+
)
13+
14+
func newEntryTestRequest(t *testing.T) *NativeExecutionFFISigningRequest {
15+
t.Helper()
16+
const hexKey = "0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f20"
17+
payload, _ := json.Marshal(&nativeFROSTUniFFIV2SignerMaterial{
18+
KeyPackage: &NativeFROSTKeyPackage{
19+
Identifier: "id",
20+
Data: []byte{0x01},
21+
},
22+
PublicKeyPackage: &NativeFROSTPublicKeyPackage{
23+
VerifyingKey: hexKey,
24+
},
25+
})
26+
return &NativeExecutionFFISigningRequest{
27+
Message: new(big.Int).SetBytes([]byte{0xab, 0xcd}),
28+
SessionID: "executor-entry-test",
29+
MemberIndex: 1,
30+
SignerMaterial: &NativeSignerMaterial{
31+
Format: NativeSignerMaterialFormatFrostUniFFIV2,
32+
Payload: payload,
33+
},
34+
Attempt: &Attempt{
35+
Number: 1,
36+
CoordinatorMemberIndex: 1,
37+
IncludedMembersIndexes: []group.MemberIndex{1, 2, 3, 4, 5},
38+
},
39+
}
40+
}
41+
42+
func TestEntry_StaticFallback_NoCoordinatorRegistered_TaggedBuild(t *testing.T) {
43+
// Without the frost_roast_retry build tag this is exercised by
44+
// the default-build test (which always falls through). Under the
45+
// frost_native build alone, the helper still treats the absence
46+
// of a registered coordinator as a static fallback because
47+
// BeginOrchestrationForSession returns
48+
// ErrNoRoastRetryCoordinatorRegistered (in the default build it
49+
// is the stub no-op-return-true).
50+
//
51+
// The helper must return (nil, nil) regardless: the executor
52+
// adapter proceeds without orchestration, matching Phase 5
53+
// receive semantics.
54+
logger := log.Logger("entry-static-test")
55+
cleanup, err := attemptRoastRetryOrchestrationFromRequest(
56+
newEntryTestRequest(t), logger,
57+
)
58+
if err != nil {
59+
t.Fatalf("static fallback must not surface an error: %v", err)
60+
}
61+
if cleanup != nil {
62+
t.Fatal("static fallback must not return a cleanup function")
63+
}
64+
}
65+
66+
func TestEntry_StaticFallback_UnsupportedSignerFormat(t *testing.T) {
67+
// FrostUniFFIV1 material -> ExtractDkgGroupPublicKeyFromMaterial
68+
// returns ErrUnsupportedSignerMaterialFormat. The helper must
69+
// treat this as STATIC (deterministic across deployments) and
70+
// fall back without surfacing an error.
71+
req := newEntryTestRequest(t)
72+
req.SignerMaterial = &NativeSignerMaterial{
73+
Format: NativeSignerMaterialFormatFrostUniFFIV1,
74+
Payload: []byte("{}"),
75+
}
76+
cleanup, err := attemptRoastRetryOrchestrationFromRequest(
77+
req, log.Logger("entry-v1-test"),
78+
)
79+
if err != nil {
80+
t.Fatalf("V1 material must be a static fallback: %v", err)
81+
}
82+
if cleanup != nil {
83+
t.Fatal("static fallback must not return a cleanup function")
84+
}
85+
}
86+
87+
func TestEntry_StaticFallback_OnNilSignerMaterial(t *testing.T) {
88+
// Nil signer material is a deterministic, per-input
89+
// construction-precondition failure: every honest node with
90+
// the same request would observe it identically. Treated as a
91+
// STATIC fallback so the executor adapter proceeds without
92+
// orchestration. The HARD-FAIL discipline is reserved for
93+
// non-deterministic Coordinator state-machine errors.
94+
req := newEntryTestRequest(t)
95+
req.SignerMaterial = nil
96+
cleanup, err := attemptRoastRetryOrchestrationFromRequest(
97+
req, log.Logger("entry-nil-mat-test"),
98+
)
99+
if err != nil {
100+
t.Fatalf("nil signer material must be a STATIC fallback; got %v", err)
101+
}
102+
if cleanup != nil {
103+
t.Fatal("static fallback must not return cleanup")
104+
}
105+
}
106+
107+
func TestEntry_StaticFallback_OnZeroAttemptNumber(t *testing.T) {
108+
// Zero attempt number is also a deterministic precondition
109+
// failure; treated as STATIC fallback.
110+
req := newEntryTestRequest(t)
111+
req.Attempt.Number = 0
112+
cleanup, err := attemptRoastRetryOrchestrationFromRequest(
113+
req, log.Logger("entry-zero-attempt-test"),
114+
)
115+
if err != nil {
116+
t.Fatalf("zero attempt number must be a STATIC fallback; got %v", err)
117+
}
118+
if cleanup != nil {
119+
t.Fatal("static fallback must not return cleanup")
120+
}
121+
}

0 commit comments

Comments
 (0)