feat(tbtc/signer): Phase 7.2a InteractiveAggregate (tweaked, self-verifying)#4052
Merged
mswilkison merged 4 commits intoJun 13, 2026
Conversation
…blame
Coordinator-side InteractiveAggregate per the frozen spec: collect the
responsive subset's signature shares, verify each against its verifying
share, and produce the BIP-340 signature - then self-verify it against
the (taproot-tweaked) group key before release, matching the coarse
finalize path.
Verifying material is resolved from the session's own DKG public key
package, never the request, consistent with the no-secret-on-the-FFI
discipline (the session must exist with completed DKG). Aggregation
operates on public material only, so no policy gate runs here: the
secret-bearing step is each signer's Round2, where lifecycle /
quarantine (full subset) / firewall were already enforced.
frost::aggregate already verifies every share and reports the culprit
identifiers on failure; instead of flattening that to a string, a bad
share now surfaces as the structured EngineError::InvalidSignatureShare
{ culprits } (code invalid_signature_share, recoverable) naming the
offending member(s), so the coordinator has attributable blame and can
exclude them on the next attempt.
FFI export frost_tbtc_interactive_aggregate + C header declaration;
call/success counters and latency telemetry consistent with the other
interactive ops. Tests: interactive member + stateless member shares
aggregate to a verified BIP-340 signature through the engine; a corrupt
co-signer share fails with attributable blame naming the culprit; FFI
dispatch smoke. Full suite 268 passed / 1 ignored, clippy -D warnings
clean, header parses, chaos green.
Deferred to 7.2b (needs persistence plumbing, not security-load-bearing
since aggregate is deterministic over public data): the "mark session
complete" marker, plus the signed-body package envelopes and
cross-language vectors.
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 |
… bound Review finding (Codex P1): emitting per-member InvalidSignatureShare blame from InteractiveAggregate is forgeable. The engine cannot yet bind the aggregate's public inputs (signing package, taproot root) to what each member actually signed at Round2, so a buggy or malicious coordinator aggregating against a different package/root makes honest shares fail verification and frames their members. Binding "to what the members signed" requires the per-member signed-package envelopes (frozen spec section 6), which are Phase 7.2b. So this drops the premature attributable blame: a share-verification failure is now a generic fail-closed Validation error (no signature, no culprit naming). The InvalidSignatureShare error variant and the culprit-mapping helper are removed and will be reintroduced in 7.2b together with the envelope binding that makes the attribution sound - and with the FFI structured-culprit payload (Codex P2), since the current ffi error_result carries only code/message/recovery_class and would drop a culprit vector anyway. The aggregate still verifies every share (frost) and self-verifies the result against the tweaked group key; only the blame OUTPUT is deferred. Test renamed to assert the fail-closed generic error. Full suite 268 passed / 1 ignored, clippy -D warnings clean, chaos green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mswilkison
changed the title
…behavior Review finding (Codex P2): the previous commit removed the structured invalid_signature_share error but left the public C header and the InteractiveAggregateRequest doc still advertising attributable blame / the invalid_signature_share code. A coordinator built against that contract would wait for an error code that can no longer be returned. Both now state the actual 7.2a behavior: an invalid share (or a mismatched package/root) fails closed with the generic validation_error code and no signature, and per-member attributable blame is deferred to Phase 7.2b where the signed-package envelopes make the attribution unforgeable. The frozen spec's design statement is unchanged - it describes the completed feature, and the deferral is tracked. Doc-only: header parses, crate builds clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…gate Review finding (Codex P2): aggregate took the engine lock but, unlike open/round1/round2/abort, never called sweep_expired_interactive_state. A process receiving only InteractiveAggregate calls would let expired Round1 nonce handles linger in memory past the TTL until some other interactive endpoint happened to run, breaking the documented TTL guarantee for secret nonce handles. Aggregate now sweeps right after acquiring the lock, like every other interactive entry point. Test: an aged Round1 handle in one session is cleared by an aggregate call targeting a different (missing) session - proving the sweep runs regardless of which interactive endpoint takes the lock. Full suite 269 passed / 1 ignored, clippy -D warnings clean, chaos green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mswilkison
merged commit f96a54c
into
extraction/frost-signer-mirror-2026-05-26
19 checks passed
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.
What
Phase 7.2a of the frozen spec: coordinator-side InteractiveAggregate, the step that combines the responsive subset's signature shares into the final taproot-tweaked BIP-340 signature. Builds on the merged 7.1 session layer (#4051).
Design
finalize. Message binding is enforced cryptographically: a share produced over one message won't verify against a package over another.frost_tbtc_interactive_aggregate+ C header; call/success counters + latency telemetry consistent with the other interactive ops.Attributable blame is deferred to 7.2b (review outcome)
The first cut emitted a per-member
InvalidSignatureShare { culprits }on a bad share. Review (Codex P1) correctly flagged that as forgeable: the engine can't yet bind the aggregate's public inputs (signing package, taproot root) to what each member actually signed at Round2, so a coordinator aggregating against a different package/root would make honest shares fail and frame their members. Binding "to what the members signed" needs the per-member signed-package envelopes (spec §6) — which are 7.2b.So this PR fails closed with a generic error on a verification failure (no signature, no culprit naming). Attributable blame returns in 7.2b together with the envelope binding that makes the attribution sound, and with the FFI structured-culprit payload (Codex P2 — the current FFI error carries only code/message/recovery_class).
Verification
-D warningsclean, C header parses, Phase 5 chaos green.🤖 Generated with Claude Code