Skip to content

feat(frost/signing): RFC-21 Phase 6.3 -- wire orchestration at executor adapter#3983

Merged
mswilkison merged 1 commit into
feat/frost-schnorr-migration-scaffoldfrom
feat/frost-roast-executor-orchestration-2026-05-23
May 23, 2026
Merged

feat(frost/signing): RFC-21 Phase 6.3 -- wire orchestration at executor adapter#3983
mswilkison merged 1 commit into
feat/frost-schnorr-migration-scaffoldfrom
feat/frost-roast-executor-orchestration-2026-05-23

Conversation

@mswilkison
Copy link
Copy Markdown
Contributor

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).

…or adapter

Adds the entry-point helper that calls
BeginOrchestrationForSession from the
nativeExecutionFFIExecutorAdapter.Execute method, gated by the
frost_native build tag with a permanent default-build no-op stub.

Per the RFC-21 Phase-6 Resolved Decision on orchestration error
taxonomy (#3980):

  - BuildAttemptContextFromRequest failures are treated as STATIC
    fallbacks. They are per-input deterministic: the same
    NativeExecutionFFISigningRequest produces the same construction
    outcome on every honest node. Log at INFO and continue without
    orchestration.
  - BeginOrchestrationForSession failures matching
    ErrRoastRetryReadinessOptOut or
    ErrNoRoastRetryCoordinatorRegistered are STATIC fallbacks for
    the same reason (deterministic per deployment configuration).
  - Any other BeginOrchestrationForSession failure is a RUNTIME
    Coordinator state-machine error. HARD FAIL: return error from
    the executor adapter. The signing group must NOT have node A
    on legacy shuffle while node B is on ROAST state machine,
    which would fracture NextAttempt agreement.

New files:

* pkg/frost/signing/roast_retry_executor_entry_default_build.go
  (//go:build !frost_native)
  - attemptRoastRetryOrchestrationFromRequest permanent stub
    returning (nil, nil). The executor adapter compiles and runs
    in the default build with zero orchestration overhead.

* pkg/frost/signing/roast_retry_executor_entry_frost_native.go
  (//go:build frost_native)
  - Real implementation. Walks the (build context, begin, return
    cleanup) sequence with the error-classification discipline.
  - Defensive nil-logger handling so the existing executor-
    adapter tests (which pass nil) do not panic.

* pkg/frost/signing/roast_retry_orchestration.go (extended)
  - ErrNoRoastRetryCoordinatorRegistered sentinel.
  - BeginOrchestrationForSession wraps the sentinel via fmt.Errorf
    %w so callers can errors.Is it.

* pkg/frost/signing/native_ffi_executor_adapter.go (modified)
  - Execute now calls attemptRoastRetryOrchestrationFromRequest
    after building the FFI request, defers the cleanup if
    orchestration started, then proceeds to primitive.Sign as
    before.

Tests:

* roast_retry_executor_entry_test.go (default-build, 1 case)
  - Stub returns (nil, nil) for any input.

* roast_retry_executor_entry_frost_native_test.go (frost_native, 4
  cases)
  - Static fallback when no coordinator registered (default-build
    stub of RegisteredRoastRetryCoordinator returns false).
  - Static fallback for FrostUniFFIV1 (unsupported format).
  - Static fallback for nil signer material (deterministic
    precondition).
  - Static fallback for zero attempt number.

* roast_retry_executor_entry_frost_roast_retry_test.go
  (frost_native && frost_roast_retry, 4 cases)
  - Static fallback when readiness env var unset.
  - Static fallback when registry empty.
  - Happy path activates orchestration; binding exists; cleanup
    clears it.
  - HARD FAIL on synthetic runtime BeginAttempt error.

All pass under: go test ./pkg/frost/..., go test -tags
'frost_native frost_tbtc_signer frost_roast_retry' ./pkg/frost/...,
go test -race -tags 'frost_native frost_tbtc_signer
frost_roast_retry' ./pkg/frost/..., staticcheck -checks '-SA1019'
./pkg/frost/..., gofmt -l ./pkg/frost/signing/, go vet
./pkg/frost/....

Stacked on Phase 6.2 (#3982). Phase 6.4 will migrate the actual
participant-selection call sites to consume the
ROAST-coordinator-derived AttemptContext for retry decisions.
Base automatically changed from feat/frost-roast-attempt-context-from-request-2026-05-23 to feat/frost-schnorr-migration-scaffold May 23, 2026 03:39
@mswilkison mswilkison merged commit 3d2ff18 into feat/frost-schnorr-migration-scaffold May 23, 2026
15 checks passed
@mswilkison mswilkison deleted the feat/frost-roast-executor-orchestration-2026-05-23 branch May 23, 2026 03:40
mswilkison added a commit that referenced this pull request May 23, 2026
…spatcher (#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant