docs(tbtc/signer): Phase 7.2b design note — package envelopes + bound blame#4054
Merged
mswilkison merged 15 commits intoJun 14, 2026
Conversation
… blame Scopes the 7.2b implementation before code, matching the spec-first discipline that preceded 7.1. Covers the signed-body signing-package envelope (frozen spec section 6, mirroring the #4040 pattern), the envelope-bound attributable blame deferred from 7.2a (so blame is never emitted without the binding that makes it unforgeable - the direct answer to the #4052 P1), the FFI structured-culprit payload (#4052 P2), the InteractiveAggregate completion marker, and cross-language vectors. Lays out the Go (scaffold: wire envelope, distribution, equivocation retention extending #4044, bridge decoder) vs Rust (mirror: Round2 binding record, bound blame, FFI payload, completion marker, vectors) split, a 5-step sub-PR sequence where blame (7.2b-3) only lands after its binding (7.2b-1) and the envelope (7.2b-2) exist, and three open questions (envelope transport, equivocation-comparison location, FFI error shape) for sign-off before implementation. 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 |
…wers Companion to the 7.2b design note. Works the three open questions (Q1 envelope binding, Q2 equivocation-comparison location, Q3 FFI error shape) into options tables, tradeoffs, and a recommendation each, with a per-question "Reviewer ask" for Gemini/Claude. Carries the coordinator-as-adversary framing (refuse-to-blame can run in the coordinator engine; prove-equivocation must run at members) and corrects the design note's body-hash lean on Q1 toward member-transmitted SignedSigningPackage envelopes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Converging P1 from both reviewers on Q1: the Rust engine must NOT verify envelopes. It has no operator-key registry (Gemini), and a coordinator signature is the wrong authentication direction for member blame (Codex's framing attack: coordinator distributes P' to A, aggregates against P with its own P-envelope, frames honest A). This realigns the design docs to the FROZEN spec (§5.4/§6), which already specified: engine does pure FROST math returning *candidate* culprits; all envelope verification, retention, and authoritative blame re-check live in the Go host at the f+1 accuser quorum, against each member's retained received bytes. No spec amendment needed. Q2 (both concur): retention now; compare at the f+1 quorum step (Option B); gossip deferred. Q3 (both concur): typed optional culprits: Option<Vec<u16>> on ErrorResponse, not a generic details map. Design-note corrections: §1/§4 (engine pure-math + Go adjudication, not "engine verifies the coordinator signature"), §5 ([]string -> []u16, Codex P2), §6 (Round2 record is engine-local bookkeeping), §7 (blame adjudication in the Go column), §9 (7.2b-3 engine has no envelope handling; 7.2b-4 adds quorum adjudication), §10 (superseded body-hash proposal withdrawn, Codex P2), §11 (candidate-vs-authoritative split). Discussion doc adds a post-review Decision Log; pending owner sign-off to record in the gates-doc. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ixes Two valid Codex P1s on the 7.2b design (no change to the Q1/Q2/Q3 decisions): - Taproot root binding: members MUST verify taproot_merkle_root against their live session root before signing. The attempt context does NOT carry the root but Round2 signs under the session root, so without the check a coordinator can hand an envelope with the right package/attempt but a divergent root — the retained envelope then misdescribes what was signed and the quorum re-check misattributes blame. Added to design note §2 (member verify), §3 (root-equivocation is caught by the same machinery since the root is a body field), §11 (acceptance test). - Member-authenticated share submission promoted from "confirm" to a hard prerequisite gating authoritative blame: without it a coordinator submits garbage as "A's share", the engine returns A as a candidate, and the quorum re-checks bytes A never sent. Added to §9 (7.2b-2 scope + 7.2b-4 gate) and §11 (acceptance), and the discussion doc's Q1 prerequisite section. Self-review consistency fixes: discussion-doc framing block no longer claims job (a) can run in the coordinator engine (it's enforced at the quorum, per the resolved Q1); Q1 title drops "the engine"; Q3 options table Vec<String> -> Vec<u16>; design note §4/§9 drop the engine-local Round2 package-hash record (the corrected design has no consumer); Decision Log records both re-review P1s. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codex P2 (valid concern, corrected remedy): the `culprits == entire
subset` rule is unreachable if 7.2b-3 uses the default aggregate path.
Verified against frost-core 3.0.0 / frost-secp256k1-tr 3.0.0:
- `frost::aggregate` and `aggregate_with_tweak` hard-code
CheaterDetection::FirstCheater, so they report only the first invalid
share (Codex right that the rule would never fire + multi-share
failures under-reported).
- But no reimplementation is needed (Codex's proposed remedy): frost-core
3.0.0's detect_cheater already collects every culprit under
CheaterDetection::AllCheaters. The wrinkle is the taproot wrapper
aggregate_with_tweak does NOT expose the mode, so the engine applies
the tweak itself (public_key_package.tweak(merkle_root)) and calls
frost_core::aggregate_custom(.., AllCheaters). Pinned in §4/§9 + the
discussion-doc Decision Log; soundness still rests on the Go quorum
re-check, not this engine heuristic.
Self-review consistency fixes (my own /review pass): purge the two
remaining references to the dropped engine-local Round2 record (design
note §6, §7 Rust column); rewrite the discussion-doc intro, which still
described a soliciting-input structure ("Reviewer ask" lines) the
RESOLVED conversion removed; de-densify design note §2 step 1 (root-check
rationale lifted below the list).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…re quorum re-check Two valid Codex P2s on the member-side acceptance logic, plus a held self-review finding on the quorum re-check: - Elected-coordinator check (Codex P2): attempt_context_hash is public, so "valid operator signature + right attempt hash + right root" would accept a body from any operator. §2 now requires coordinator_id == the elected coordinator AND signature verified under that key. The §3 equivocation proof also requires both divergent bodies under the same elected coordinator's signature. - Retain-on-reject (Codex P2): §2 previously rejected a root-divergent envelope before retention, so §3's cross-member comparison lost the signed bytes proving root equivocation. §2 reordered: authenticate -> retain (genuine-coordinator evidence) -> then sign-or-refuse; a refused divergent envelope is retained as equivocation evidence. - Tweak-aware, engine-delegated quorum re-check (self-review): the quorum's per-share crypto re-verify is FROST math and must be tweak-consistent (taproot is the production case). Delegated to a stateless engine verify-share (inputs: signing_package, taproot_merkle_root, verifying_share, share - never the envelope or operator keys), not reimplemented in Go. §4/§7 split adjudication into Go policy + engine crypto; §9 adds the verify-share FFI to 7.2b-3; §11 adds tweak-consistency + retain-on-reject + elected-coordinator acceptance tests. Discussion-doc Decision Log records all three. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Last round's retain-on-reject rule (§2 step 2) means a member retains envelopes it received and refused to sign; §3's opening "received and signed over" contradicted that. Corrected to "received (whether or not it signed over it)". Wording only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two valid Codex findings (the loop had NOT converged — premature to call it closed last pass): - [P1] Context-bound share authentication: member-authenticated share submission proving only "these bytes are A's" is replayable — a coordinator can resubmit an old A-signed share for a different attempt/package, and the quorum re-checks bytes A never submitted for that context, framing A. The signed body MUST cover (attempt_context, signing-package/envelope hash, share). §4/§9/§11 + discussion-doc prerequisite. - [P2] Group key in verify-share inputs: FROST share verification computes the challenge + binding factors from the GROUP verifying key (tweaked for taproot), not just the per-member verifying share. The proposed FFI input list omitted it. Fixed: the engine resolves the public key package (group key + verifying shares) from session DKG state and applies the tweak (consistent with 7.2a); caller passes only public inputs (signing_package, taproot_merkle_root, identifier, share). §4/§7/§9. Discussion-doc Decision Log records both. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codex P2 and my own pass converged on the same verify-share resolution gap, two facets: - Selector (Codex P2): the listed inputs (signing_package, root, identifier, share) don't identify WHICH session/DKG public key package the engine resolves; engine state is keyed by session_id, so multi- session lookups are ambiguous or could verify against the wrong group key. Add a session_id/wallet selector. - Lifetime (self-review): the f+1 quorum re-check can run after the interactive session is TTL-swept (frozen §5), so the group key must resolve from durably-retained wallet DKG material that outlives the signing session, not the ephemeral session object. Resolution covers both: selector + durable wallet-scoped key, with the explicit-pass alternative noted (the public key package is public, so passing it keeps verify-share a pure function, sound under the f+1-independent-accuser model). §4/§7/§9 + §11 multi-session/post-sweep test; discussion-doc Decision Log updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…firmation Decision Log entry 9 (roast-phase-5-security-rollout-gates.md) records sign-off of the Phase 7.2b package-envelope + Go-side bound-blame design: the Q1 engine-pure-math correction plus the eight resolutions folded across the seven adversarial review passes (Q2 retain-now/f+1 compare, Q3 typed culprits, AllCheaters, taproot root binding, context-bound member-auth share submission, elected-coordinator + retain-on-reject, verify-share FFI contract). Also resolves the design's §9 durable-wallet-pubkey-package-retention question as already satisfied: dkg_public_key_package persists on the session and survives the interactive-attempt TTL sweep, so the 7.2b-3 verify-share FFI resolves the group key by session_id with no new persistence. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Match the 7.2b-1 implementation change (PR #4055): the InteractiveAggregate completion marker becomes a completion record (attempt_id -> aggregate signature hex), and a repeat aggregate returns the stored signature (idempotent) rather than an 'already aggregated' error. Addresses a Codex review finding that the reject-based marker made a successfully-computed signature unrecoverable from the engine if the host lost the FFI response. §11 acceptance updated accordingly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- open-questions doc: drop the stale 'pending owner sign-off' status; the design is signed off and recorded as gates-doc Decision Log entry 9 in the same PR, so the binding docs no longer contradict each other. - design note: reconcile terminology - the section 6 retitle to 'completion record' had left six 'completion marker' references in sections 1/4/7/9; all now read 'completion record'. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Match the 7.2b-1 fix (PR #4055): re-emission of the completion record is validated against the request's tweaked key/message - attempt_id does not bind the taproot root, so a reused attempt_id with different aggregate inputs is rejected rather than handed a non-verifying signature (Codex re-review finding). Drops the now-inaccurate 'or erroring' on the same-inputs path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The open-questions doc's closing paragraph still said owner sign-off was pending and 7.2b-1 could only start afterward, contradicting the signed-off status header + Decision Log entry 9 added in the same PR. Updated to reflect the sign-off and that 7.2b-1 is implemented (PR #4055). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Match the implementation decision (PR #4055): section 6 reverts from idempotent signature re-emission to a completion MARKER that rejects re-aggregation; recovery of a lost signature is via a fresh ROAST attempt. Records why the re-emit variant was dropped (concurrent-race divergence, request-input binding, lost-shares recovery gap). section 11 acceptance + 'completion record'->'completion marker' terminology updated across the design + open-questions docs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
mswilkison
merged commit 9d0accf
into
extraction/frost-signer-mirror-2026-05-26
19 checks passed
mswilkison
deleted the
docs/phase-7-2b-package-envelope-design-2026-06-13
branch
mswilkison
added a commit
that referenced
this pull request
Jun 14, 2026
#4055) ## Phase 7.2b-1: InteractiveAggregate completion marker First implementation PR of Phase 7.2b (design signed off in #4054 — gates-doc Decision Log entry 9). **Engine-side only — no blame, no envelopes** — preserving the crypto-only engine boundary (the Q1 correction): all envelope verification and authoritative blame remain Go-side. Targets the mirror branch directly and is independent of #4054 (the code touches no docs; #4054 touches no source). ### What it does Adds a durable per-attempt "aggregated" marker so re-aggregating a completed interactive attempt is rejected (`InteractiveAttemptAlreadyAggregated`) rather than recomputed (frozen Phase 7 spec; design §6). This is the only engine-side state 7.2b adds. ### Changes - **`errors.rs`**: new `EngineError::InteractiveAttemptAlreadyAggregated { session_id, attempt_id }` — code `interactive_attempt_already_aggregated`, recovery class `recoverable` (benign duplicate; the engine is healthy). - **`engine/state.rs`**: `aggregated_interactive_attempt_markers: HashSet<String>` on `SessionState`. - **`engine/persistence.rs`**: mirrored `Vec<String>` on `PersistedSessionState` with `#[serde(default, skip_serializing_if = "Vec::is_empty")]` (loads pre-7.2b state; writes nothing when empty), bounded via the existing consumed-registry helpers, wired through both `TryFrom` conversions. - **`engine/interactive.rs`**: `interactive_aggregate` pre-checks the marker before recomputing; keeps the aggregation lock-free; then re-acquires the lock, **re-checks** (a concurrent-duplicate race guard), inserts the marker, and persists with rollback-on-failure — mirroring the Round2 consume-before-release pattern — before reporting success. ### Durable-retention confirmation (design §9) The §9 question — is the group public key package durably retained beyond the signing-session TTL (needed by the later 7.2b-3 verify-share FFI)? — is **already satisfied**: `dkg_public_key_package` lives on the persisted session and `sweep_expired_interactive_state` clears only the live attempt's nonces. No new persistence is added here. ### Tests - Re-aggregating a completed attempt is rejected with the correct code. - The completion marker survives `simulate_process_restart_for_tests()` + reload (persistence round-trip). - Error code / recovery class / message format pinned. ### Verification `cargo fmt`, `cargo clippy --all-targets --all-features -- -D warnings` clean, full suite **274 lib pass / 0 fail / 1 ignored**, Phase-5 chaos suite all scenarios pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
mswilkison
added a commit
that referenced
this pull request
Jun 14, 2026
… foundation) (#4056) ## 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 - **Protos** (`pkg/frost/roast/gen/pb/signing_package.proto`): `SigningPackageBody {attempt_context_hash, coordinator_id, signing_package, taproot_merkle_root}` and `SignedSigningPackage {body, coordinator_signature}` — generated into the existing `gen/pb` dir (protoc-gen-go v1.36.3; same dir as `evidence.proto`, so no new Dockerfile gen-allowlist entry). - **`SigningPackage` Go type** (`signing_package.go`): the same signed-body, sign-what-you-transmit / verify-what-you-received discipline as the evidence envelopes (`wire.go`). `SignableBytes` marshals the body once and caches it; `Unmarshal` retains the received body + envelope verbatim (the coordinator signature is checked over exactly those bytes); `Validate` does 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, `Validate` rejects malformed packages, and `Marshal` refuses an unsigned package. ### Not in this increment (follows on this branch) - Coordinator-side: sign `SigningPackageBody` with the operator key + distribute. - Member-side: authenticate (elected `coordinator_id` per 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). - Context-bound member-authenticated Round2 share submission (signed body covers attempt_context_hash + package/envelope hash + share) — the hard prerequisite gating blame (7.2b-4). - The engine never sees this envelope; blame adjudication stays Go-side (frozen spec §6). ### Verification `gofmt`, `go build`, `go vet`, and the full `pkg/frost/roast/...` test suite are green. 🤖 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.
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
SigningPackageBody/SignedSigningPackage): signed once, embedded verbatim, re-verified against exactly what was received. Members verify the coordinator signature, theattempt_context_hash, andtaproot_merkle_rootagainst their session root before signing.culprits == full subsetis treated as suspect (coordinator misconfig, not universal cheating).culprits: Option<Vec<u16>>on the error response so candidate culprits are machine-readable.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