Skip to content

feat(frost/signing): enforce AttemptContextHash on receive (RFC-21 Phase-6 milestone)#3989

Merged
mswilkison merged 1 commit into
feat/frost-schnorr-migration-scaffoldfrom
feat/frost-roast-attempt-context-hash-required-2026-05-23
May 23, 2026
Merged

feat(frost/signing): enforce AttemptContextHash on receive (RFC-21 Phase-6 milestone)#3989
mswilkison merged 1 commit into
feat/frost-schnorr-migration-scaffoldfrom
feat/frost-roast-attempt-context-hash-required-2026-05-23

Conversation

@mswilkison
Copy link
Copy Markdown
Contributor

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

…ase-6 milestone)

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.

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.

* pkg/frost/signing/attempt_context_binding_validation_frost_native.go
  (new, gated frost_native)
  - attemptContextHashCarrier interface so the helper covers all
    three FROST/tbtc-signer message types via their existing
    GetAttemptContextHash methods.
  - verifyMessageAttemptContextHash: looks up the session's
    handle binding via currentAttemptHandleForCollect. No
    binding -> return nil (legacy / default build). Binding
    present + matching hash -> return nil. Binding present +
    missing hash -> ErrAttemptContextHashMissing. Binding
    present + mismatched hash -> ErrAttemptContextHashMismatch.

* pkg/frost/signing/native_frost_protocol_frost_native.go and
* pkg/frost/signing/native_ffi_primitive_transitional_frost_native.go
  Three receive callbacks updated. After the existing
  shouldAcceptNativeFROSTMessage gate, each callback now calls
  verifyMessageAttemptContextHash. Failure paths call
  evidence.RecordReject(senderID, "attempt_context_hash_mismatch")
  so the policy can permanently exclude peers that consistently
  send stale-attempt messages.

Tests:

* attempt_context_binding_validation_frost_native_test.go (gated
  frost_native && frost_roast_retry, 5 cases)
  - No binding -> any message passes
  - Binding + matching hash -> passes
  - Binding + missing hash -> ErrAttemptContextHashMissing
  - Binding + mismatched hash -> ErrAttemptContextHashMismatch
  - Integration with a real
    nativeFROSTRoundOneCommitmentMessage via SetAttemptContextHash;
    rebinding to a different context produces a mismatch

* attempt_context_binding_validation_default_build_test.go
  (gated frost_native && !frost_roast_retry, 1 case)
  - In the default build the helper always passes regardless of
    message contents, matching the rollback promise.

Verification:

* go build ./... + go build -tags 'frost_native frost_tbtc_signer
  frost_roast_retry' ./... -- both clean
* go test ./pkg/frost/... -- pass
* go test -tags 'frost_native frost_tbtc_signer' ./pkg/frost/... -- pass
* go test -tags 'frost_native frost_tbtc_signer frost_roast_retry'
  ./pkg/frost/... -- pass (5 packages)
* staticcheck -checks '-SA1019' ./pkg/frost/... -- silent
* gofmt -l ./pkg/frost/signing/ -- silent
* go vet ./pkg/frost/... -- clean

Migration path:

* Phase 1B (already shipped): AttemptContextHash is structurally
  validated when present, optional otherwise.
* This PR: the field is enforced *only* when the session has a
  ROAST-attempt binding. Sessions without a binding -- including
  every default-build session and every non-ROAST tagged-build
  session -- continue to ignore the field.
* Future PR: once production has rolled out a version that
  populates the field on every outbound message, enforcement can
  be made unconditional (binding-or-not).
Base automatically changed from feat/frost-roast-reject-conflict-evidence-2026-05-23 to feat/frost-schnorr-migration-scaffold May 23, 2026 04:10
@mswilkison mswilkison merged commit 1597e8f into feat/frost-schnorr-migration-scaffold May 23, 2026
14 checks passed
@mswilkison mswilkison deleted the feat/frost-roast-attempt-context-hash-required-2026-05-23 branch May 23, 2026 04:10
mswilkison added a commit that referenced this pull request May 23, 2026
…nfo (#3990)

## Summary

Process-wide cumulative counters for the three evidence categories
(overflow / reject / conflict), exposed through keep-core's
\`clientinfo\` registry so operators can observe per-category event
rates via the standard Prometheus scrape.

In default builds and unregistered-coordinator states, the
metrics-emitting recorder is bypassed entirely (the receive loops
use \`attempt.NoOpRecorder\`), so the counters stay at zero. Once
the ROAST-retry registry is populated and live signing flows
record evidence, the counters increment -- providing the
"do I have ROAST retry running?" smoke test from operator
dashboards.

Stacked on #3989 (AttemptContextHash enforcement).

## What lands

| File | Role |
|---|---|
| \`roast_retry_metrics.go\` (new, untagged) | Cumulative atomic
counters; \`RegisterRoastRetryMetrics(*clientinfo.Registry)\` registers
Source functions under the \`frost_roast_retry\` application prefix;
\`metricsEmittingRecorder\` wraps the bounded recorder and bumps the
counter on each Record* call. |
| \`roast_retry_recorder.go\` (modified) |
\`roastRetryRecorderForCollect\` now wraps the bounded recorder with
\`newMetricsEmittingRecorder\` when the registry is populated. |

## Metrics exposed

Via \`clientinfo.Registry.ObserveApplicationSource\`:

| Metric name | Description |
|---|---|
| \`frost_roast_retry_overflow_events_total\` | Cumulative count of
receive-channel overflow events |
| \`frost_roast_retry_reject_events_total\` | Cumulative count of
validation-gate rejections (incl. \`attempt_context_hash_mismatch\` from
#3989) |
| \`frost_roast_retry_conflict_events_total\` | Cumulative count of
first-write-wins equivocation events |

## Test coverage (6 cases)

- Counters increment on `Record*` (different per-category counts)
- Snapshot delegates to inner recorder
- Nil inner falls back to NoOp without panicking
- Unregistered coordinator → NoOp recorder → no counter bumps
- Concurrent counter increments are race-safe (16 workers × 100 calls)
- `RegisterRoastRetryMetrics(nil)` is a no-op (defensive guard)

## Operator wiring

The keep-core node's startup sequence should call:

\`\`\`go
signing.RegisterRoastRetryMetrics(clientinfoRegistry)
\`\`\`

alongside the existing registry observation calls. A follow-up to
\`docs/development/frost-roast-retry-rollout.adoc\` will document
this step.

## Verification

| Command | Result |
|---|---|
| \`go build ./...\` | clean |
| \`go test ./pkg/frost/...\` | pass (5 packages) |
| \`go test -race ./pkg/frost/signing/...\` | pass |
| \`go test -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 |

## Test plan

- [ ] CI green.
- [ ] Reviewer confirms the process-wide cumulative counter shape
(alternative: per-session gauges, more granular but harder to query at a
glance).
- [ ] Reviewer confirms the \`frost_roast_retry\` application prefix is
acceptable (alternative: more specific prefix like
\`frost_roast_retry_evidence\`).
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