feat(frost): carry the chosen signing subset in the signed package (7.3 t-of-included PR1/3)#4092
Merged
mswilkison merged 1 commit intoJun 19, 2026
Conversation
….3 t-of-included PR1/3) First of the t-of-included finalize stack: expose WHICH members the FROST signing package was built over, so a non-coordinator can learn the t-subset to await round-2 shares from when the package covers fewer than the full included set. Add a signed signer_ids field (member indices, ascending + distinct) to SigningPackageBody -- it rides the coordinator-signed body, so it is authenticated, not an unsigned sidecar. The SigningPackageBytes stays the cryptographic source of truth: the engine verifies shares against it, so a coordinator that lies in signer_ids causes only a LIVENESS failure (aggregate fails closed), never a wrong signature or false blame (Gemini's Option-B adjudication). Validate enforces structure (each a real member index so SignerIDs() cannot truncate; strictly ascending hence distinct); empty stays valid for the full-included flow, so this is fully back-compatible (the body-travels-verbatim discipline means old nodes still verify new packages over the received bytes). No behaviour change yet -- nothing sets or reads signer_ids. PR2 (runner subset) populates + consumes it; PR3 makes the done-check threshold-aware. The included-set oversizing that activates it is a deferred policy knob. Design locked via Codex + Gemini consult (both PROCEED). Pre-prod, behind frost_native tags; .pb.go regenerated with protoc-gen-go v1.36.3. 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 |
mswilkison
merged commit 80cf249
into
feat/frost-schnorr-migration-scaffold
15 of 16 checks passed
mswilkison
added a commit
that referenced
this pull request
Jun 19, 2026
… committers (7.3 t-of-included PR2/3) (#4093) ## What RFC-21 Phase 7.3, the **runner subset** of t-of-included finalize. Today the interactive ROAST signing runner (`pkg/frost/signing/roast_runner_frost_native.go`) waits for **all** included members in round 1 (`collectCommitments`) and round 2 (`collectShares`), so one slow or offline member stalls the attempt to its ctx deadline → fail → ROAST retry. This makes an attempt finalize over the **first `t` responsive committers**. ### Roles (decided by the package's signed `signer_ids`, PR1's field #4092) - **Coordinator** — `collectCommitments` collects until **exactly `t`** (its own commitment seeded), proceeds the instant `t` have arrived, and builds the FROST package over that `t`-subset. It sets `signer_ids` (ascending, distinct) in `signSigningPackage` to the chosen members. If fewer than `t` ever commit, the ctx deadline fires and the run fails into the existing retry path. - **Non-coordinator signer** (in `signer_ids`) — runs round 2 and collects round-2 shares from the **package's signer set**, not the full included set. - **Observer** (committed in round 1 but **not** in `signer_ids`) — does **not** run round 2. It aggregates the subset's broadcast shares publicly, `MarkSucceeded`, and returns the signature — and **still aborts the engine session on success** to drop its unconsumed round-1 nonces. The success path previously suppressed the abort, which leaked an observer's nonces; fixed via a `signedRound2` flag. A round-2-silent member among the chosen `t` still fails the attempt → retry (existing path). Only the elected coordinator builds the package, so there is no honest divergence (receivers already filter `sender==elected` + attempt hash). The FROST `SigningPackageBytes` remains the cryptographic source of truth, so a coordinator that lies in `signer_ids` causes only a liveness failure (aggregate fails closed), never a wrong signature or false blame. ## Inert until oversizing (MacLane's policy knob) t-of-included is **dormant** until participant selection oversizes the included set past the threshold — it currently trims to exactly `honestThreshold` (`signing_loop_legacy_selector.go:91`), so the chosen subset equals the full included set and every member signs (today's behavior). The machinery is harmless and inert until then. ## Hardening The runner constructor now rejects `threshold > len(includedSet)` — the new round-1 loop's termination invariant (a well-formed attempt always selects ≥ threshold members; fail fast rather than silently degrade into timeout-driven retries). Found by the inline adversarial review. ## Tests (all `frost_native`, behind pre-prod tags) - Extended `buildInteractiveSigningHarness` with `runMembers` to drive **silence** (a non-responsive member). - `FinalizesOverResponsiveThresholdSubset` — a non-coordinator offline member is excluded; the two responsive members finalize over `t`; the coordinator's package omits the offline member. - `OversizedAllOnline_FinalizesOverThreshold` — all online; exactly `t` sign (round-2 count), `n−t` observe and abort their nonces, all reach Succeeded; the broadcast package carries exactly `t` ascending signer ids. - `ObserverAggregatesAndAbortsWithoutSigning` — focused observer: aggregates, Succeeded, **0** round-2 calls, **1** abort. - Existing `UsesEngineDerivedFrostIdentifiers` retargeted to a full-included (`n==t`) attempt for determinism. Validated: build+vet+test across the 5 tag combos (default / `frost_native` / `frost_roast_retry` / `frost_native frost_roast_retry` / `frost_native frost_tbtc_signer` w/ CGO), `-race` on `pkg/frost/signing`, the `frost_roast_retry` drive tests, repo-wide `frost_native` vet, `pkg/tbtc` build, and gofmt — all clean. ## Follow-up (PR3, lands with/right after this) `signingDoneCheck` (`pkg/tbtc/signing_done.go:93`, called from `signing_loop.go:458`) sets `expectedSignersCount = len(attemptMembersIndexes)`; once oversizing is enabled, a successful attempt that omitted an offline member would hang the outer done-check. PR3 makes it threshold/package-set aware. 🤖 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.
RFC-21 Phase 7.3 — t-of-included finalize, PR1/3: signer-set exposure
First of the t-of-included finalize stack (a robustness win: finalize an interactive attempt with
tof the included set instead of requiring all of them, absorbing slow/offline members in-attempt instead of forcing a ROAST retry). This PR is the pure data-structure prerequisite: expose which members the FROST signing package was built over, so a non-coordinator can learn thet-subset to await round-2 shares from.Change
Add a signed
signer_idsfield (member indices, ascending + distinct) toSigningPackageBody. Because it rides the coordinator-signed body (domain_tag || SigningPackageBody), it's authenticated — not an unsigned sidecar.Why it's safe (Gemini's Option-B adjudication)
The opaque
SigningPackageBytesremains the cryptographic source of truth: the engine verifies shares against it, so a coordinator that lies insigner_idscauses only a liveness failure (InteractiveAggregatefails closed when shares don't match), never a wrong signature or false blame. That's why a signed envelope field suffices and no engine-side parser/cross-check is needed.Validateenforces structure (each a real member index — soSignerIDs()can't truncateuint32→uint8; strictly ascending hence distinct).Back-compatible
signer_idsempty stays valid (the full-included flow carries no subset), and the body-travels-verbatim discipline means old nodes still verify new packages over the received bytes. No behaviour change yet — nothing sets or reads the field. PR2 (runner subset) populates + consumes it; PR3 makessigningDoneCheckthreshold-aware. The included-set oversizing that activates the feature is a deferred policy knob.Verification
frost_native frost_roast_retry/frost_native frost_tbtc_signerw/ CGO) — additive proto change, downstream compilessigner_idsround-trip-and-signed (different lists → different signable bytes) +Validaterejects zero/out-of-range/non-ascending/duplicate;-race; gofmt clean.pb.goregenerated withprotoc-gen-go v1.36.3(matching the existing toolchain marker)🤖 Generated with Claude Code