feat(frost/roast): Phase 7.2b-2 signed signing-package envelope (wire foundation)#4056
Merged
mswilkison merged 8 commits intoJun 14, 2026
Conversation
The wire foundation for the coordinator-distributed signing package (frozen spec section 6): SigningPackageBody {attempt_context_hash, coordinator_id, signing_package, taproot_merkle_root} + SignedSigningPackage {body, coordinator_signature} protos, and a SigningPackage Go type following the established sign-what-you-transmit / verify-what-you-received discipline (wire.go): SignableBytes caches the body marshaled once; Unmarshal retains the received body + envelope verbatim; Validate does structural checks.
Byte-preservation (the point of the envelope) is pinned by tests: received bytes survive re-broadcast verbatim, a non-canonical encoding is preserved, key-path and script-path roots round-trip, and Validate rejects malformed packages. proto generated with protoc-gen-go v1.36.3 into the existing gen/pb dir (no new Dockerfile gen-allowlist entry).
Coordinator signing/distribution and member-side authentication (elected-coordinator check, signature verification, root binding) + retention + the context-bound member-authenticated share submission follow on this branch; the engine never sees this envelope (blame adjudication is Go-side).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Self-review of #4056: coordinator_id is a wire uint32 but a member index is uint8 (group.MemberIndex, max 255), so Validate's zero-only check let an out-of-range coordinator_id pass while CoordinatorID() silently truncated it. Validate now also rejects coordinator_id > group.MaxMemberIndex, so the later member-side elected-coordinator check compares a faithful value. Test added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review of #4056: SigningPackageBody is wire-compatible with TransitionMessageBody (matching tags/types for attempt_context_hash, coordinator_id, and a length-delimited field 3), and the elected coordinator's operator key signs both - so a coordinator signature over a signing package could be replayed as a valid transition-message signature (cross-protocol signature confusion). SignableBytes now prepends a fixed domain tag so the coordinator signs the domain-tagged body. The bare body still travels on the wire verbatim (new bodyBytes()); the verifier reconstructs domain||body. The signed byte stream is unambiguously a signing package and can never be a valid transition-message body (it does not start with the attempt_context_hash tag), regardless of protobuf field layout. Test added pinning the body-level collision and that the domain-tagged signed bytes do not present a valid transition body. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review of #4056: Unmarshal materialized the whole signing_package field and copied it into the struct + cached the body before Validate applied the MaxSigningPackageBytes cap, so an oversized peer envelope forced large allocations before rejection. Unmarshal now rejects len(data) > MaxSignedSigningPackageBytes before proto.Unmarshal, and rejects an over-cap signing_package field before the field/body copies. Test added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e signing package The producer/consumer logic for the signed signing-package envelope: SignSigningPackage (the elected coordinator signs SignableBytes() - the domain-tagged body - with its operator key); AuthenticateSigningPackage (a member verifies the envelope is genuine evidence from the attempt's elected coordinator: it names the elected coordinator per RFC-21 Annex A, resolved by the caller via SelectCoordinator; the signature verifies under that coordinator's operator key; and attempt_context_hash matches the live attempt). Passing = attributable, so the member retains the exact received bytes before the sign/no-sign decision; failing = forgeable noise, rejected WITHOUT retention. Mirrors verifyBundleSignature. MatchesRoot is the root-binding sign/no-sign check, kept separate from authentication so a root-divergent but genuine-coordinator envelope is retained as equivocation evidence and then refused (retain-on-reject). Tests: round-trip authenticate; rejections (missing/tampered signature, non-elected coordinator, wrong attempt, a non-elected operator signing a body carrying the elected id); key/script-path root match. Still on this branch: member retention storage + retain-on-reject wiring, the context-bound Round2 share submission, and network distribution. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…n (P2)
The signed signing-package wire spec documented bare-body signatures, but
SignableBytes prefixes a fixed domain tag ("roast/signed-signing-package/v1"
+ 0x00) before the body. A Rust or other verifier built from this proto would
sign/verify the bare body and fail to interoperate with
AuthenticateSigningPackage.
Document the signed payload as `domain_tag || SigningPackageBody` in a
file-level cross-language contract note (with the exact tag bytes) and correct
the two message comments. Regenerated signing_package.pb.go is comment-only;
the descriptor path and symbols are unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…che (P1/P2) P1: the domain tag began with ASCII "ro" (0x72 0x6f), which is valid protobuf wire data (field 14, length 111), so a parser skips it and resumes - letting a crafted signing_package embed a TransitionMessageBody at the resume point and re-enabling cross-protocol signature replay. Begin the tag with byte 0x00 (an illegal protobuf tag, field 0) so the signed payload is undecodable as any protobuf message: a signing-package signature is rejected when another envelope decodes the forged body, and another coordinator-signed message's signature (over a body starting >= 0x08) can never verify against domain||body. The test now asserts undecodability, including a body that embeds a full valid transition body. P2: Unmarshal reset bodyCache and wireEnvelope but not signaturePayloadCache, so a reused receiver could authenticate a freshly decoded package against stale signable bytes. Clear the cache on Unmarshal; test added. Proto SIGNED PAYLOAD note updated to the new tag bytes (file-level comment; generated .pb.go is unchanged). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
mswilkison
added a commit
that referenced
this pull request
Jun 14, 2026
…es (review) Codex (PR #4057) flagged that the cross-language contract still documented snapshot and transition signatures as covering the bare body, while this PR makes Go sign/verify domain || body - so a non-Go implementation built from evidence.proto or RFC-21 would sign/verify different bytes and reject Go's evidence messages. Document the signed payload as `domain_tag || body` in evidence.proto (a file-level SIGNED PAYLOAD note with both exact tags + the four message comments) and in the RFC-21 wire-format decision. Regenerated evidence.pb.go is comment-only (descriptor and symbols unchanged). Same fix class as the signing-package proto contract on #4056. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…verify race) Mirror of the #4057 fix. SigningPackage.Unmarshal reset signaturePayloadCache to nil, so the first SignableBytes call on a PARSED package was a racing write - concurrent signature verification (AuthenticateSigningPackage) of one received package would race on it. Prime the cache in Unmarshal (clear it, then call SignableBytes once), restoring a pure-read SignableBytes on the verify path; it still discards a stale cache on a reused value. Adds a -race regression guard that verifies a parsed package concurrently. Full pkg/frost/roast suite passes under -race. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
mswilkison
added a commit
that referenced
this pull request
Jun 14, 2026
…sage signatures (#4057) ## What Gives the two remaining operator-key-signed ROAST bodies — `LocalEvidenceSnapshot` and `TransitionMessage` — their own **distinct domain tags**, completing the cross-protocol signature-confusion sweep started for the signing-package envelope on #4056. The node's operator key signs all of these bodies (snapshots as a member; transition messages + the signing package as the elected coordinator), and their protobuf bodies are wire-compatible (each begins with a field-1 attempt-context binding). Without domain separation, a signature over one body could be presented as a signature over another. #4056 added a tag to the signing package; a reviewer correctly noted the existing snapshot/transition bodies still signed bare bodies, so this PR closes that gap. ## How - **Distinct domain tags**, each beginning with byte `0x00` — an **illegal protobuf tag** (field number 0) — so the signed payload is undecodable as any protobuf message: - `\x00roast/signed-evidence-snapshot/v1\x00` - `\x00roast/signed-transition-message/v1\x00` This separates the domains in **both directions without relying on field layout**: a signature over one body can't be accepted on another envelope (its decoder `proto.Unmarshal`s and rejects the `0x00`-leading body), and a genuine serialized protobuf body always starts `>= 0x08`, so its signature can never verify against `domain || body`. Same construction as the signing-package fix on #4056. - **`bodyBytes()` / `SignableBytes()` split**: `bodyBytes()` is the bare body that travels on the wire (embedded by `wireEnvelopeBytes` / `Marshal`); `SignableBytes()` is `domain || body`, the only thing signed/verified. Both `Unmarshal` paths reset the signable-bytes cache so a reused receiver never verifies against stale bytes. - Renamed the bare-body cache `signedBody` → `bodyCache` and added `signaturePayloadCache`, so all three signed-body types share one structure. ## Compatibility Sign and verify both flow through `SignableBytes()`, so the change is **transparent to the sign sites** and the **wire envelope structure is unchanged**. It **does** change the bytes signed, so signatures are not compatible with the pre-change protocol — **all nodes must run the new code together**. Acceptable pre-mainnet; the external-audit gate is unchanged. ## Testing `gofmt` / `go vet` / `go build ./pkg/... ./cmd/...` clean; full `pkg/frost/roast` suite green. New `domain_separation_test.go` pins, for both bodies: domain tagging, undecodability as protobuf, the bare (untagged) wire body, prefix-free distinctness of the tags, and the reused-receiver cache reset. Existing verbatim/round-trip/verify tests continue to pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
mswilkison
merged commit f9eebb6
into
feat/frost-schnorr-migration-scaffold
15 checks passed
mswilkison
deleted the
feat/phase-7-2b-2-signed-package-envelope-2026-06-14
branch
mswilkison
added a commit
that referenced
this pull request
Jun 14, 2026
…ation) (#4058) ## What The wire foundation for the **context-bound, member-authenticated Round2 share submission** — the remaining 7.2b-2 piece. After a member authenticates the elected coordinator's `SignedSigningPackage` (#4056) and accepts its taproot root, it returns its FROST round-2 signature share **bound to that exact package**. That binding is the hard prerequisite for blame adjudication (Phase 7.2b-4): a member's share is provably tied to the specific package bytes it received, so coordinator equivocation across members is detectable and a member's submission is non-repudiable. This PR is the **wire type only** (mirrors how #4056 landed the signing-package envelope first). Member sign/retain + network distribution come next. ## How - **`share_submission.proto`**: `ShareSubmissionBody{attempt_context_hash, submitter_id, signing_package_hash, signature_share}` + `SignedShareSubmission{body, submitter_signature}`; generated `pb.go`. - **`ShareSubmission` Go wire type**, structurally identical to the signing-package envelope: - `bodyBytes()` (the bare body that travels) + `SignableBytes()` (`domain ‖ body`) with the leading-`0x00` illegal-tag domain `roast/signed-share-submission/v1` — consistent with the domain-separation sweep (#4057), so the share signature is non-confusable with the signing-package / snapshot / transition signatures. - `Marshal`/`Unmarshal`/`Validate`. `Unmarshal` bounds the envelope before `proto.Unmarshal`, rejects an over-cap `signature_share` before copying, and **primes the signable-bytes cache** so concurrent verification is race-free (incorporating the fix from #4056/#4057 up front). `submitter_id` is bounded to `group.MemberIndex`. ## Testing `gofmt`/`vet`/`build` clean; full `pkg/frost/roast` suite passes under `go test -race`. New tests: verbatim round-trip, non-canonical-encoding survival, domain-separated + undecodable-as-protobuf, **prefix-free distinctness vs the signing-package / snapshot / transition domains**, validate-rejects-malformed (incl. submitter out of member-index range, short hashes, over-cap share), marshal-requires-signature, unmarshal-rejects-oversize-before-copy, and a `-race` concurrency guard. ## Next Member-side `AuthenticateShareSubmission` (verify under the submitter's operator key + bind to the live attempt + the retained signing-package hash) and the network distribution path — then the equivocation compare + f+1 quorum blame (7.2b-4). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
mswilkison
added a commit
that referenced
this pull request
Jun 15, 2026
) ## What On a failed `InteractiveAggregate`, the engine now names **every** member whose signature share did not verify, as **CANDIDATE culprits**, instead of returning an opaque `validation_error`. This is the engine-side input to the Go host's envelope-bound blame adjudication (frozen Phase 7.2b spec, §6). Builds on 7.2b-2 (the signed signing-package envelopes, #4056–#4061): the deferral comment in `interactive_aggregate` ("Attributable blame waits for the signed-package envelopes (Phase 7.2b)") is now resolved. ## How - **AllCheaters, not FirstCheater.** `frost-secp256k1-tr`'s `aggregate`/`aggregate_with_tweak` delegate to `frost_core::aggregate(.., FirstCheater)`, so a failure names only one member. This switches the call to `frost_core::aggregate_custom(.., &verification_key_package, CheaterDetection::AllCheaters)`. - `verification_key_package` is exactly the (taproot-tweaked, when a root is set) `PublicKeyPackage` the `_with_tweak` wrapper derives internally, so the call is **equivalent on the success path** (verified against the `frost-secp256k1-tr` source by review). - Cheater detection only runs **after** the aggregate signature itself fails to verify, so there is **no happy-path cost** vs FirstCheater. - `frost-core = "=3.0.0"` added as a direct, version-matched dependency (already transitive via `frost-secp256k1-tr`; no new code enters the build). - **New error variant.** `EngineError::AggregateShareVerificationFailed { session_id, attempt_id, candidate_culprits: Vec<u16> }` — code `aggregate_share_verification_failed`, recovery class `recoverable` (a fresh attempt excluding the culprits can succeed). Fail-closed: **no signature**. - **u16 Go member identifiers.** Culprits are reported as `Vec<u16>`, the same identifier space as `excluded_member_identifiers` / `included_participants` / the allowlists that the Go host already keys on (the FROST go-string form is reserved for raw frost-protocol artifacts fed back into frost). `frost_identifier_to_u16` inverts `participant_identifier_to_frost_identifier`; identifiers that don't fit a u16 are dropped (they can't be real group members). - **FFI.** `ErrorResponse` gains an additive `candidate_culprits: Vec<u16>` field, `#[serde(skip_serializing_if = "Vec::is_empty")]` — identical idiom to `excluded_member_identifiers`, omitted for every other error, so existing Go clients are unaffected. ## Architecture boundary (Q1, frozen) The list is **CANDIDATE only**. The engine verifies pure FROST shares against the group's own verifying material and **never inspects operator-signed envelopes**. A coordinator that aggregated honest shares against a *substituted* signing package or taproot root would make those honest shares fail and appear here — so the engine deliberately does not adjudicate fault. Authoritative, envelope-bound blame is the Go host's job at an **f+1 accuser quorum** (§6), consuming this candidate list (7.2b-4). ## Tests - `interactive_aggregate_rejects_invalid_share_fail_closed` (real-crypto e2e): an invalid share through the actual `aggregate_custom(AllCheaters)` tweak path yields `AggregateShareVerificationFailed` naming **exactly** the cheating member, not the honest one. - `interactive_aggregate_names_all_invalid_share_culprits` (real-crypto e2e): both members of a threshold-2 subset cheat → **both** are named, proving AllCheaters end-to-end (not FirstCheater, not truncated). - `frost_identifier_to_u16_inverts_participant_mapping`: round-trips across the low/high byte boundary (255 → 256). - `aggregate_share_verification_failed_code_message_and_culprits`: code / message / recovery class / accessor; non-aggregate errors expose no culprits. - `interactive_aggregate_produces_and_self_verifies_bip340`: success path unchanged. Full suite + `cargo fmt --check` + `cargo clippy` green. ## Review folding (Codex + Gemini) - **Codex [P2]** — culprits now use `u16` Go member ids (was FROST go-string), matching `excluded_member_identifiers`. (Codex cited a spec that does not exist in-repo, but the u16 convention is dominant and correct — `AttemptExclusionEvidence` is the precedent.) - **Gemini [P3]** — added the multi-culprit real-crypto e2e above. - Gemini independently verified the crypto equivalence against the `frost-secp256k1-tr` source, the Q1-boundary preservation, the `frost-core` dep alignment, and the FFI/serde back-compat. ## Scope / follow-ups - The coarse (non-interactive) `Aggregate` keeps its generic error — it has no attempt context; 7.2b is the interactive ROAST path. - The existing belt-and-suspenders self-verify after aggregation is retained (now redundant with `aggregate_custom`'s internal verify, but cheap defense-in-depth). - **7.2b-4 (Go):** consume `candidate_culprits` in the f+1-quorum, envelope-bound blame adjudication over the `Round2Collector` evidence. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 7.2b-2: signed signing-package envelope — wire foundation
First increment of Phase 7.2b-2 (design note §2/§6/§7, merged in #4054). Adds the coordinator-distributed signed signing-package envelope's wire foundation; the coordinator/member logic follows on this branch (see below).
What this adds
pkg/frost/roast/gen/pb/signing_package.proto):SigningPackageBody {attempt_context_hash, coordinator_id, signing_package, taproot_merkle_root}andSignedSigningPackage {body, coordinator_signature}— generated into the existinggen/pbdir (protoc-gen-go v1.36.3; same dir asevidence.proto, so no new Dockerfile gen-allowlist entry).SigningPackageGo type (signing_package.go): the same signed-body, sign-what-you-transmit / verify-what-you-received discipline as the evidence envelopes (wire.go).SignableBytesmarshals the body once and caches it;Unmarshalretains the received body + envelope verbatim (the coordinator signature is checked over exactly those bytes);Validatedoes structural checks (32-byte attempt hash, non-zero coordinator, non-empty bounded signing package, 0-or-32-byte root, signature cap).Why byte-preservation
The point of the envelope is that signature validity never depends on a serializer's canonical form (across protobuf versions or the Go↔Rust boundary). Pinned by tests: received bytes survive re-broadcast verbatim, a hand-crafted non-canonical encoding is preserved, key-path and script-path roots round-trip,
Validaterejects malformed packages, andMarshalrefuses an unsigned package.Not in this increment (follows on this branch)
SigningPackageBodywith the operator key + distribute.coordinator_idper RFC-21 Annex A + signature under that key + attempt-hash match +taproot_merkle_root== live root) and retain (incl. retain-on-reject for divergent envelopes).Verification
gofmt,go build,go vet, and the fullpkg/frost/roast/...test suite are green.🤖 Generated with Claude Code