feat(frost/roast): protobuf signed-body envelopes for transition evidence#4040
Merged
mswilkison merged 3 commits intoJun 12, 2026
Conversation
…ence Post-merge follow-up #4 from the June 2026 review stack: replace json.Marshal as the canonical signed-bytes encoding for evidence snapshots and transition bundles before Phase 7 wiring ossifies the format. Design - sign what you transmit, verify what you received: - new pkg/frost/roast/gen/pb/evidence.proto: a snapshot is SignedLocalEvidenceSnapshot{body, operator_signature} where body is the serialized LocalEvidenceSnapshotBody; a transition message is SignedTransitionMessage{body, coordinator_signature} whose body embeds every member's signed snapshot envelope VERBATIM (repeated bytes signed_snapshots) - producers marshal a body exactly once at signing time and cache it; parsed messages retain the received body/envelope bytes verbatim; verification always runs over exact received bytes and nothing in the evidence chain is ever re-encoded - signature validity never depends on any serializer's canonical form, across protobuf library versions or across languages (the Phase 7 Rust signer verifies and parses these same bytes) - CanonicalSnapshotBytes/CanonicalBundleBytes are replaced by SignableBytes() accessors; the coordinator's first-write-wins conflict comparison now compares exact signed bytes - Marshal of a received message returns the received envelope verbatim, so evidence bytes survive re-broadcast; wire-legal but non-canonical encodings also survive (pinned by test) - RFC-21 "Evidence message format" decision rewritten accordingly, with the rationale for retiring canonical JSON Tests: existing suite migrated off JSON fixtures (encode helpers bypass production signing to exercise structural rejection); new wire_test.go pins byte-preservation through re-broadcast, verbatim snapshot-envelope embedding in bundles, non-canonical-encoding survival, and tampered-body verification failure. go build ./..., go vet, gofmt clean; frost + tbtc package tests green. Co-Authored-By: Claude Fable 5 <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 |
The image strips committed **/gen/**/*.go (.dockerignore) and regenerates protobufs in-image via make generate, which only sees the gen directories explicitly COPY'd before it runs. Without this line the new evidence.proto is absent at generation time and the later full COPY restores only the .proto (not the stripped .pb.go), so go mod tidy fails on the pkg/frost/roast/gen/pb import. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…o coupling Review follow-ups on #4040 (own findings; Codex and Gemini passes were clean): - SignableBytes/Marshal docs now state the returned slice is the internal cache and must not be mutated - in-tree callers are all read-only, this pins the contract for future ones - Makefile gen_proto comment points new proto packages at the Dockerfile gen-directory COPY allowlist, so the next proto package learns about the in-image regeneration coupling from the Makefile instead of from a CI failure Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mswilkison
merged commit Jun 12, 2026
b9dfffc
into
feat/frost-schnorr-migration-scaffold
16 checks passed
mswilkison
added a commit
that referenced
this pull request
Jun 12, 2026
…al cache Two P2 findings from Codex's re-review of the equivocation evidence path: - observer panics could escape RecordEvidence/verifyOwnObservationsPresent and abort the protocol path - contradicting emitEquivocationEvidence's own "never fails" contract. The observer call is now wrapped in recover-and-log. - snapshotEnvelopeForEvidence handed the observer the slice returned by Marshal, which is the snapshot's internal wire-envelope cache (the same must-not-mutate contract this stack pinned in the #4040 envelope work). An observer that retained and mutated it would corrupt the cached signed bytes used by later bundle aggregation. It now returns a defensive copy. Regression tests: a panicking observer still yields ErrSnapshotConflict from the protocol path; mutating the evidence envelope bytes leaves the snapshot's cached Marshal output intact. Race detector clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mswilkison
added a commit
that referenced
this pull request
Jun 12, 2026
…oints (#4044) Implements the binding retention condition from today's decision log (PR #4043): proof-carrying blame (follow-up item 7) is deferred until production, **provided** telemetry/logging retain enough signed bytes to diagnose whether targeted equivocation is occurring — otherwise the revisit condition lacks data. ## What this adds `EquivocationEvidence` events carrying the **exact signed snapshot envelopes** (wire bytes verbatim — the #4040 format makes these available at every detection point) for the three detections that exist today: - `snapshot_conflict` — a sender re-submits a *different* signed snapshot for the same attempt to the coordinator. Both envelopes are retained; two operator-signed bodies from the same sender for the same attempt are self-incriminating, which is exactly the substrate item 7's wire format will formalize. - `own_snapshot_mutated_in_bundle` — a bundle carries this member's snapshot with a signature that differs from what it submitted (both envelopes retained). - `own_snapshot_missing_from_bundle` — censorship detection (self envelope retained). Each event is logged in full (these are rare, and the bytes are the diagnosis) and forwarded to a process-wide observer hook following the repo's existing single-observer telemetry pattern, so hosts can persist evidence into their telemetry stack. Emission is purely additive on the existing error paths — encode failures degrade to nil fields with a log line, never perturbing the protocol path. ## Deliberately out of scope Cross-member comparison (a receiver checking a bundle's snapshot for sender X against X's direct broadcast) — that's item 7 proper. These are the detection points that exist today, instrumented so the production deferral is honest. Tests pin byte-exact envelope retention for all three kinds, and that an idempotent identical re-submission emits nothing. `go build ./...`, vet, gofmt clean; full frost suite green. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
mswilkison
added a commit
that referenced
this pull request
Jun 12, 2026
## What Phase 7.0: the spec-freeze candidate for the hardened interactive two-round FROST signing session — the production signing path — assembling the already-settled decisions (t-of-included-native per decisions 5/6, sidecar-shaped API per decision 2, OS-randomness-only production signing) into a precise contract, plus the deletion plan for the frozen transitional deterministic-nonce path. ## The load-bearing design change: nonce custody moves inside the engine Verified current state: the stateless primitives (`frost_ops.rs`) return serialized `SigningNonces` to the Go host and accept them back at `sign_share` — secret nonces cross the FFI twice, live in host memory between rounds, and single-use is enforced by caller discipline only. Calling `sign_share` twice with one nonce pair is the canonical FROST key-extraction failure and nothing prevents it today. The session layer (spec §4): nonces are generated and held in session-scoped engine memory behind an opaque handle, consumed atomically (durable consumption marker before the share leaves the engine), zeroized on use, and **never persisted and never exported**. Restart loses in-flight nonces by construction (attempt fails safe); the cloned-state nonce-reuse class becomes structurally impossible; and after Phase 7 no secret signing material transits the FFI in either direction — which is also the audit story for that boundary. ## Also specified - **Session API** (§5): `InteractiveSessionOpen/Round1/Round2/Aggregate/Abort`, idempotent-or-fail-closed, strict-mode attempt contexts only, consumed-registry semantics carried from the coarse path, transport-agnostic for the dlopen→sidecar swap. - **t-of-included semantics** (§6): engine-side subset verification at Round2 (own membership, subset-of-included, size t) so safety never depends on coordinator honesty; signing packages ride #4040-style signed-body envelopes extending the #4044 equivocation-evidence retention; silent members cost zero attempts. - **Deletion trigger made precise** (§7): three conditions defining "interactive production path validated e2e" (Phase-5-equivalent suites incl. consumed-nonce-marker persist-fault cases; a real testnet t-of-included finalize through the full retry machinery; pinned cross-language vectors). The `nonce.rs` freeze marker now points at this section — the only code change in this PR, comment-only on the frozen file. - **Reserved, not built** (§8): bounded n−t+1 concurrency hooks (attempt-scoped keys/handles). - **Phasing** (§9): PR-sized 7.0–7.6 with repo-side mapping; #4007 sidecar scoping folds into 7.0 as an addendum. - **Open questions with proposed defaults** (§10): package-distribution channel, round-1 transport shape, responsive-subset policy, markers-only durability — to be decided at freeze sign-off and recorded in the Decision Log. ## Freeze process Status is Proposed; it freezes on signer + keep-core owner sign-off per §11, with the §10 defaults ratified or overridden in the gates-doc Decision Log. The audit scope statement should reference this document and name the §5 API as in-scope (decision 1 interaction). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This was referenced Jun 13, 2026
mswilkison
added a commit
that referenced
this pull request
Jun 14, 2026
… blame (#4054) ## What A design note scoping Phase 7.2b **before** the implementation, matching the spec-first discipline that preceded 7.1 (and that 7.2a's blame-deferral made the case for: don't ship the feature ahead of its foundation). Now includes a companion **open-questions discussion doc** with the three design questions **resolved** post-review. ## Why a note first 7.2a (#4052) ships InteractiveAggregate but fails closed *without* attributable blame, because the engine can't yet bind the aggregate's inputs to what each member signed — so a coordinator aggregating against a different package/root could frame honest members (the #4052 P1). The frozen spec ties trustworthy blame to **signed package envelopes** (§6). 7.2b builds the foundation and the feature together. It spans Go (wire/distribution/equivocation/**blame adjudication**) and Rust (engine **candidate culprits**/FFI), so it needs design before code. ## Covers - **Signed-body signing-package envelope** mirroring the #4040 pattern (`SigningPackageBody` / `SignedSigningPackage`): signed once, embedded verbatim, re-verified against exactly what was received. Members verify the coordinator signature, the `attempt_context_hash`, **and `taproot_merkle_root` against their session root** before signing. - **Equivocation detection** extending #4044: a coordinator distributing different bodies (incl. different roots) to different members is self-incriminating. - **Blame split (corrected post-review — see below):** the **Rust engine does pure FROST math** and returns mathematically-failing members as *candidate* culprits; **all envelope verification + authoritative blame** runs **Go-side at the f+1 accuser quorum**, re-checking each accused share against that member's retained envelope. `culprits == full subset` is treated as suspect (coordinator misconfig, not universal cheating). - **FFI structured-culprit payload** (#4052 P2): typed `culprits: Option<Vec<u16>>` on the error response so candidate culprits are machine-readable. - **Completion marker** + **cross-language vectors**. ## Post-review correction The companion discussion doc resolves the three open questions. The substantive change: an earlier draft had the *engine* verify envelopes — **wrong**, on a converging Gemini+Codex P1. The engine has no operator-key registry (can't verify operator signatures), and a coordinator-signed envelope is the wrong authentication direction for *member* blame. The resolution **realigns to the frozen spec** (§5.4/§6 — no spec amendment): engine = pure math → candidate culprits; Go = envelope verification + authoritative blame at the f+1 quorum vs the member's retained bytes. Two follow-up Codex P1s folded in: members must verify the taproot root before signing, and **member-authenticated share submission is a hard prerequisite** before blame is enabled. ## Plan Go/Rust split spelled out, a 5-step sub-PR sequence where **authoritative blame (7.2b-4) lands only after the envelope (7.2b-2) and the engine's candidate culprits (7.2b-3), and is gated on member-authenticated share submission**. Three open questions resolved (recorded in the discussion doc's Decision Log; pending owner sign-off to record in the gates-doc). Not a frozen contract — a scoping doc to review before the 7.2b-1 implementation PR. 🤖 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.
Post-merge follow-up #4 from the June 2026 review stack (#4028–#4035): replace
json.Marshalas the canonical signed-bytes encoding for evidence snapshots/bundles — explicitly scheduled to land before Phase 7 wiring ossifies the format. (Items 2 and 3 landed as #4036/#4037 on the mirror branch; this is the Go-side sibling on the scaffold branch.)Why now
The RFC-21 Layer B evidence signatures were computed over canonical JSON. That byte stability is a Go-implementation accident — field-order-stable
encoding/jsonoutput — not a portable contract. The moment Phase 7 wires evidence verification into the Rust signer (or any second implementation appears), every verifier would need to replicate Go's exact JSON emission. No persisted or cross-component evidence exists yet, so the format can still change for free.Design: sign what you transmit, verify what you received
New
pkg/frost/roast/gen/pb/evidence.proto:SignedLocalEvidenceSnapshot{body, operator_signature}wherebodyis the serializedLocalEvidenceSnapshotBody— the operator signs those exact bytes.SignedTransitionMessage{body, coordinator_signature}whoseTransitionMessageBodyembeds every member's signed snapshot envelope verbatim (repeated bytes signed_snapshots) — the coordinator attests to the exact signed snapshots it assembled, in order.Marshalof a received message returns the received envelope verbatim — evidence bytes survive re-broadcast, including wire-legal but non-canonical encodings (pinned by a handcrafted reversed-field-order test).CanonicalSnapshotBytes/CanonicalBundleBytes→SignableBytes()accessors; the coordinator's first-write-wins conflict check now compares exact signed bytes.Tests
wire_test.gopins the format's core properties: byte-preservation through unmarshal→re-marshal, verbatim snapshot-envelope embedding inside bundle bodies, producer-signed bytes == receiver-verified bytes, non-canonical-encoding survival, tampered-body verification failure.go build ./...,go vet,gofmtclean; frost + tbtc package tests green. Generated with protoc 33.4 / protoc-gen-go v1.36.3 (matches the go.mod protobuf runtime v1.36.3).Docs
RFC-21 "Evidence message format" decision rewritten: signed-body protobuf envelopes, with the retirement rationale for canonical JSON recorded.
Notes for reviewers
LocalEvidenceSnapshot,TransitionMessage) are unchanged apart from two unexported byte caches; all call sites kept their shapes.SignableBytes()is first computed (documented on the cache fields); the aggregation flow already treats snapshots as immutable post-receipt.bodybytes and parse them with any protobuf implementation — no canonicalization requirements transfer.🤖 Generated with Claude Code