Skip to content

feat(frost/signing): RFC-21 Phase 1B -- optional AttemptContextHash on protocol messages#3964

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

feat(frost/signing): RFC-21 Phase 1B -- optional AttemptContextHash on protocol messages#3964
mswilkison merged 1 commit into
feat/frost-schnorr-migration-scaffoldfrom
feat/frost-roast-attempt-context-binding-2026-05-22

Conversation

@mswilkison

Copy link
Copy Markdown
Contributor

Summary

RFC-21 Phase 1B: extends the three FROST/tbtc-signer protocol message
types with an optional 32-byte AttemptContextHash field that binds
each message to the deterministic AttemptContext introduced in
Phase 1A (#3963).

No protocol behaviour change yet. Receivers do not match the hash
against a locally-computed context in this phase; that lands later
behind a build tag. Phase 1B is purely the wire-format extension.

Stacked on #3963.

Affected messages

  • nativeFROSTRoundOneCommitmentMessage
  • nativeFROSTRoundTwoSignatureShareMessage
  • buildTaggedTBTCSignerRoundContributionMessage

Migration contract

Property Behaviour
Field tag json:\"attemptContextHash,omitempty\" -- absent on the wire when the sender has not bound the message.
Old peer compatibility Pre-Phase-1B JSON unmarshals cleanly; field reports as absent via GetAttemptContextHash.
New peer compatibility New JSON with a 32-byte hash field unmarshals; GetAttemptContextHash returns (hash, true).
Validation Unmarshal rejects wrong-length hash fields; len must be exactly 32 or absent.
Equal-or-reject The existing first-write-wins helper (buildTaggedTBTCSignerRoundContributionMessagesEqual from #3959) now compares AttemptContextHash bytewise, so a peer mutating the binding mid-stream is reported as a conflict.

Why split from Phase 1A

Phase 1A (#3963) added the AttemptContext type with no consumers.
Phase 1B (this PR) adds the wire-format extension. Splitting keeps the
type definition reviewable independently of the wire change, since
the wire change touches existing message structs in production code
paths.

Test coverage

17 new tests under pkg/frost/signing/attempt_context_binding_test.go:

  • TestValidateAttemptContextHashField_AcceptsAbsentOrCorrectLength (3 sub-tests)
  • TestValidateAttemptContextHashField_RejectsWrongLength (3 sub-tests)
  • TestAttemptContextHashField_ArrayRoundTrip
  • TestAttemptContextHashField_ArrayToArrayAbsent
  • TestAttemptContextHashField_FromArrayDoesNotAliasCaller
  • TestRoundOneCommitmentMessage_OptionalFieldRoundTrip (2 sub-tests)
  • TestRoundOneCommitmentMessage_BackwardCompatWithOldJSON
  • TestRoundOneCommitmentMessage_RejectsWrongLengthHashField
  • TestRoundTwoSignatureShareMessage_OptionalFieldRoundTrip
  • TestRoundTwoSignatureShareMessage_BackwardCompatWithOldJSON
  • TestRoundTwoSignatureShareMessage_RejectsWrongLengthHashField
  • TestBuildTaggedTBTCSignerRoundContributionMessage_OptionalFieldRoundTrip
  • TestBuildTaggedTBTCSignerRoundContributionMessage_BackwardCompatWithOldJSON
  • TestBuildTaggedTBTCSignerRoundContributionMessage_RejectsWrongLengthHashField
  • TestBuildTaggedTBTCSignerRoundContributionMessagesEqual_HashFieldDifferentiates
  • TestRoundOneCommitmentMessage_JSONEncoderOmitsAbsentField

All pass under:

  • go test -tags 'frost_native frost_tbtc_signer' ./pkg/frost/...
  • pkg/tbtc regression subset:
    go test -tags 'frost_native frost_tbtc_signer' -run 'TestConfigureFrostSigningBackend|TestNewNode_ConfiguresFrostSigningBackend|TestSigningExecutor_Sign|TestRegisterSignerMaterialResolverForBuild' ./pkg/tbtc/

Test plan

  • CI green.
  • Reviewer confirms the omitempty/optional contract is what we
    want as the Phase 1 default (i.e., we are intentionally not
    rejecting messages that lack the field).
  • Reviewer confirms it's OK for the equal-or-reject helper to
    now treat "contribution with hash" and "contribution without
    hash" as unequal during the migration window (sketched in
    Migration contract table above).

…ield

Extends the three FROST/tbtc-signer protocol message types with an
optional 32-byte AttemptContextHash field that binds the message to a
specific RFC-21 AttemptContext (introduced in Phase 1A).

* nativeFROSTRoundOneCommitmentMessage
* nativeFROSTRoundTwoSignatureShareMessage
* buildTaggedTBTCSignerRoundContributionMessage

Migration contract (Phase 1B intentionally limited):
* Field uses omitempty -- absent on the wire when the sender has not
  bound the message to a context. Old peers continue to interop.
* Receiver-side Unmarshal validates length-when-present (must be
  exactly AttemptContextHashFieldLength = 32) but does not yet match
  against the locally-computed context. Higher-level acceptance lands
  in a later RFC-21 phase behind a build tag.
* Shared helpers in attempt_context_binding.go convert between the
  on-wire []byte form and the canonical [32]byte hash form. Senders
  use SetAttemptContextHash; receivers use GetAttemptContextHash to
  get the hash + presence flag.

Equal-or-reject is extended to compare AttemptContextHash bytewise,
so a peer that retransmits the same contribution but mutates the
binding mid-stream triggers the existing first-write-wins reject
path (introduced in PR #3959).

17 new tests cover: length validation; array<->slice round-trip
without caller aliasing; per-message marshal/unmarshal round-trip
with field absent and present; backward compatibility with
pre-Phase-1B JSON; wrong-length rejection; equal-or-reject
sensitivity to the new field. All pass under
`go test -tags 'frost_native frost_tbtc_signer' ./pkg/frost/...`
plus the pkg/tbtc regression subset.

Refs RFC-21 (docs/rfc/rfc-21-*); stacked on Phase 1A (#3963).
@mswilkison mswilkison force-pushed the feat/frost-roast-attempt-context-binding-2026-05-22 branch from b8dd9a2 to 65f3963 Compare May 22, 2026 23:21
Base automatically changed from feat/frost-roast-attempt-context-2026-05-22 to feat/frost-schnorr-migration-scaffold May 22, 2026 23:23
@mswilkison mswilkison merged commit a43a16c into feat/frost-schnorr-migration-scaffold May 22, 2026
13 of 14 checks passed
@mswilkison mswilkison deleted the feat/frost-roast-attempt-context-binding-2026-05-22 branch May 22, 2026 23:23
mswilkison added a commit that referenced this pull request May 22, 2026
…ild tag (#3965)

## Summary

Forward-fix for #3866 CI: the Phase 1B binding file and test
referenced message types defined in \`//go:build frost_native\`
files but were themselves untagged. Untagged staticcheck on
the integration branch (#3866) then reported
\`undefined: nativeFROSTRoundOneCommitmentMessage\` and the
client-lint job failed.

Adds \`//go:build frost_native\` to:

- \`pkg/frost/signing/attempt_context_binding.go\`
- \`pkg/frost/signing/attempt_context_binding_test.go\`

The helpers and tests are only exercised by gated code paths
(the three message-type methods all live behind \`frost_native\`),
so the build tag is the right locus.

## Why now

PRs #3963 (Phase 1A) and #3964 (Phase 1B) were merged into the
\`feat/frost-schnorr-migration-scaffold\` branch before #3866's
integration CI ran. Once the merges landed, #3866's
\`client-lint\` job rebuilt under the untagged staticcheck pass
and exposed the missing tag. This PR is the smallest possible
fix.

## Verification

Locally with module-pinned staticcheck 2025.1.1:

\`\`\`
go build ./...
go build -tags 'frost_native frost_tbtc_signer' ./pkg/frost/...
go test  -tags 'frost_native frost_tbtc_signer' ./pkg/frost/signing/
staticcheck -checks \"-SA1019\" ./... # whole repo, silent
staticcheck -checks \"-SA1019\" ./pkg/frost/signing  # silent
\`\`\`

## Test plan

- [ ] CI green: client-lint, client-vet, client-scan,
  client-build-test-publish all pass.
- [ ] #3866 lint job recovers once this merges into
  \`feat/frost-schnorr-migration-scaffold\`.
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