Skip to content

feat(tbtc/signer): FROST/ROAST Rust signer#4005

Open
mswilkison wants to merge 205 commits into
mainfrom
extraction/frost-signer-mirror-2026-05-26
Open

feat(tbtc/signer): FROST/ROAST Rust signer#4005
mswilkison wants to merge 205 commits into
mainfrom
extraction/frost-signer-mirror-2026-05-26

Conversation

@mswilkison

@mswilkison mswilkison commented May 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Lands the Rust FROST/ROAST signer at pkg/tbtc/signer/ alongside the existing Go DKG coordinator. The initial import was extracted from tlabs-xyz/tbtc:feat/frost-schnorr-migration at frozen signed tag frost-extraction-source-v1, then this PR applied targeted canonical-path, CI, and reviewer hardening follow-ups in the canonical keep-core repo.

This is no longer a pure byte-for-byte mirror PR. The post-freeze changes are intentional canonical keep-core changes (canonical-path normalization, CI wiring, and reviewer hardening). After this PR lands, threshold-network/keep-core is the source of truth for the signer; the extracted monorepo tag is provenance for the initial import only.

Source-PR Provenance

The monorepo provenance above is used only to audit the initial import. Ongoing signer development, fixes, CI, and reviews happen in threshold-network/keep-core.

Why This Lives In keep-core

Per extraction plan v38 section 3.1, the signer co-locates with keep-core because:

  • HSM enforcement is external to signer code via standard PKCS#11/KMIP and does not force a separate audit lifecycle.
  • Coordinator coupling dominates; the B-2 Go DKG coordinator already lives in this repo via PR FROST/ROAST readiness branch #3866.
  • TEE adoption later is reversible without forcing a repo split.

Layout Introduced

pkg/tbtc/signer/
├── Cargo.toml + Cargo.lock + build.sh
├── src/
├── tests/
├── benches/
├── include/
├── docs/
│   └── formal/models/
├── scripts/formal/
└── test/vectors/

CI Coverage

This PR now runs:

  • cargo fmt --manifest-path pkg/tbtc/signer/Cargo.toml -- --check
  • cargo clippy --manifest-path pkg/tbtc/signer/Cargo.toml --all-targets -- -D warnings
  • cargo test --manifest-path pkg/tbtc/signer/Cargo.toml
  • cargo test --manifest-path pkg/tbtc/signer/Cargo.toml formal_verification_
  • pkg/tbtc/signer/scripts/formal/run_tla_models.sh

Verification

Latest local verification after review fixes:

  • cargo fmt --manifest-path pkg/tbtc/signer/Cargo.toml -- --check
  • cargo clippy --manifest-path pkg/tbtc/signer/Cargo.toml --all-targets -- -D warnings
  • TBTC_SIGNER_STATE_PATH=/tmp/tbtc-signer-ci-state-local-4005.json cargo test --manifest-path pkg/tbtc/signer/Cargo.toml
  • cargo test --manifest-path pkg/tbtc/signer/Cargo.toml formal_verification_

Latest GitHub CI on this branch is green, including Signer Rust checks, Signer formal invariants, and TLA model checks.

Relationship To PR #3866

PR #3866 lands the Go-side DKG coordinator. This PR lands the Rust-side signer. They are complementary protocol implementations:

  • DKG: keep-core Go side coordinates; signer participates; DKG result digest is computed on both sides and checked for parity.
  • Signing: signer produces FROST signature shares; coordinator aggregates.

PR #3866 and this PR can land independently in either order.

Plan Context

This is one of the mergeable mirror PRs comprising the FROST extraction, alongside tbtc-v2, tbtc-subgraph, and tbtc-v3-indexer mirror PRs.

Lands the Rust signer at pkg/tbtc/signer/ alongside the existing Go DKG
coordinator. Mirrors the signer slice of tlabs-xyz/tbtc:feat/frost-schnorr-migration
(PR #10) at frozen tag frost-extraction-source-v1.

Per extraction plan v38 §3.1, the signer co-locates with keep-core
because: (a) HSM enforcement is external to signer code (standard
PKCS#11/KMIP client), doesn't force a separate audit lifecycle;
(b) coordinator coupling dominates (B-2 DKG coordinator already lives
in this repo via PR #3866); (c) TEE adoption later is reversible
without forcing a repo split.

Layout
- pkg/tbtc/signer/ — Rust crate with own Cargo.toml
- pkg/tbtc/signer/docs/ — signer + ROAST + TEE specs
- pkg/tbtc/signer/docs/formal/models/ — ROAST + TEE TLA+ models
- pkg/tbtc/signer/scripts/formal/ — ROAST vector + TLA runner
- pkg/tbtc/signer/test/vectors/ — roast-attempt-context-v1.json

Files (49 total)
- 47 mirror status (Rust source, signer docs, ROAST docs, TLA models,
  test vector, etc.)
- 2 allowlisted-divergence:
  - pkg/tbtc/signer/scripts/formal/check_roast_attempt_context_vectors.mjs
    (path normalization to signer-repo paths)
  - pkg/tbtc/signer/scripts/formal/run_tla_models.sh (MODELS_PATH env
    var refactor; default pkg/tbtc/signer/docs/formal/models/)

Provenance
- Source repository: tlabs-xyz/tbtc
- Source branch: feat/frost-schnorr-migration
- Source tag (frozen): frost-extraction-source-v1
- Source commit (H): 52389bd5cccb5daeef195671feb7ca46be6e2f37
- Source manifest: extraction/frost-extraction-source-manifest.json
  (manifestSha256: f7295fb738104501eb6c0c2447a42122ceb5f684c7a7c5dfb50ecb0bde3a0ea0)
- Source PR(s): #425 (tbtc-signer error codes) + the full FROST migration
  series; complete list per source manifest's commit range over
  tools/tbtc-signer/ and signer-adjacent docs/scripts.

Build wiring (follow-up)
This PR lands the signer source code, docs, vectors, and scripts. Rust
toolchain CI integration (cargo build/test/clippy/fmt as a separate CI
job alongside the existing Go jobs) is a follow-up — Go workspace
ignores non-Go subdirectories automatically; the cargo crate at
pkg/tbtc/signer/Cargo.toml is independently buildable.

Known TBD (resolved pre-merge)
- 2 allowlisted-divergence files have expectedTargetSha256 = <TBD> in
  the source manifest. Path normalization transformations need to be
  applied to make the scripts run in pkg/tbtc/signer/ context; this PR
  ships the source verbatim, follow-up commits on this branch apply
  the transformations before merge.
- Dual signoff required on each allowlisted-divergence entry per plan
  v38 §4.2 (extraction lead + canonical repo maintainer).

Verification (pre-merge per plan v38 §7.2)
- 47 mirror files: sha256 equality between
  git show frost-extraction-source-v1:<sourcePath> and this PR's content
  at pkg/tbtc/signer/<targetPath>.
- 2 allowlisted-divergence files: sha256 == expectedTargetSha256
  (recorded post-transformation in source manifest).
- PR-scoped rogue-file check: git diff --name-only main...HEAD only
  contains files in manifest's fileMap with targetKey "signer".

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR introduces pkg/tbtc/signer, a Rust tbtc-signer bootstrap crate with a C ABI JSON-over-buffer interface, FFI translation helpers, API types, error semantics, deterministic coordinator selection, a comprehensive lib.rs FFI surface with tests, an admission-checker CLI, Criterion benchmarks, TLA+ formal models, conformance vectors/checkers, and extensive design/runbook documentation.

Changes

Core Signer Implementation

Layer / File(s) Summary
FFI & API contract definition
include/frost_tbtc.h, src/api.rs
C header declares TbtcBuffer and TbtcSignerResult structs plus ~18 frost_tbtc_* function signatures; Rust API types define serde-serialized request/response payloads for DKG, sign-round lifecycle, transcript audit, quarantine, refresh, emergency rekey, differential fuzzing, and canary rollout.
Error classification and recovery semantics
src/errors.rs
EngineError enum with thiserror integration; code() maps variants to stable string identifiers; recovery_class() classifies errors as "recoverable" or "terminal" with explicit handling for consumed attempt/round replays.
FFI translation layer
src/ffi.rs
C-compatible buffer structs, JSON serialization/deserialization helpers, ffi_entry for panic containment via catch_unwind, and safe free_buffer memory management for the FFI boundary.
Deterministic coordinator selection
src/go_math_rand.rs
Rust port of Go's math/rand with fixed cooked constants table, seedrand, bounded sampling, and shuffle; select_coordinator_identifier returns deterministic coordinators matching keep-core outputs per test vectors.
Lib.rs FFI endpoints & bootstrap gating
src/lib.rs
Module declarations, signer version, bootstrap-mode environment/profile parsing with strict production gating, test-only override, ~18 #[no_mangle] extern "C" endpoints (DKG, sign-round, transcript audit, emergency rekey, canary rollout, refresh-shares), and comprehensive test suite covering idempotency, lifecycle transitions, Taproot validation, and emergency/rollout semantics.

Admission Checking & Policy Enforcement

Layer / File(s) Summary
Admission checker binary
src/bin/admission_checker.rs
JSON-configured operator admission evaluator with policy constraints (per-provider/region caps, custody/attestation/SLA requirements), optional DAO override verification (Schnorr signature validation over SHA-256 payload digest), and replay registry persistence to prevent re-use of override decisions.

Benchmarking & Formal Test Vectors

Layer / File(s) Summary
Phase 5 ROAST benchmark suite
benches/phase5_roast.rs
Criterion benchmarks exercising DKG, start/finalize sign-round, state reload/recovery, and attempt transitions (timeout/invalid-share/stale-replay); includes FFI helpers, hashing/fingerprinting utilities, deterministic coordinator probing, and error validation.
Taproot signature fraud formal verification
tests/p2tr_signature_fraud_vectors.rs
Integration test loading p2tr-signature-fraud-v0.json vectors, computing BIP-341 key-path sighashes, verifying Schnorr signatures, deriving dual challenge identities, and asserting mutation-based identity uniqueness.
ROAST attempt-context test vectors
test/vectors/roast-attempt-context-v1.json
JSON schema and fixtures for deterministic fingerprint/attempt-id computation across multiple participant sets and edge cases, used by formal verification checkers.

TLA+ Formal Verification Models

Layer / File(s) Summary
ROAST attempt state machine model
docs/formal/models/RoastAttemptStateMachine.tla, .cfg
TLA+ module defining attempt numbering, active participants, deterministic coordinator, consumed-attempt registries (transient and durable), transitions (advance/restart/reload), and safety invariants for monotonicity and replay safety.
ROAST rollout policy state machine
docs/formal/models/RoastRolloutPolicy.tla, .cfg
TLA+ module specifying five-stage rollout progression (bootstrap→canary→broad→rollback↔halted) with control signals and temporal properties governing promotion/hold/rollback and emergency stop.
State-key provider policy & production gating model
docs/formal/models/StateKeyProviderPolicy.tla, .cfg, .production.cfg
TLA+ module modeling provider/key selection lifecycle and load outcomes; production-gate enforcement; properties asserting fail-closed behavior and exact binding enforcement.
TEE enforcement modes state machine
docs/formal/models/TeeEnforcementModes.tla, .cfg
TLA+ module for TE admission control with three enforcement modes, attestation states, break-glass override, and properties enforcing valid attestation and admission stability.
Formal models documentation & runner
docs/formal/models/README.md, scripts/formal/run_tla_models.sh
README documenting model coverage and bounds; Bash runner downloading/verifying TLA+ toolchain and executing TLC model checks.

Design Documentation & Specifications

Layer / File(s) Summary
ROAST protocol phase specifications
docs/roast-phase-{0-spec-freeze,1.5-consumed-registry,2-coordinator,3-transcript,4-liveness,5-baseline,5-runbook,5-security-gates}.md
Phase 0 spec defining attempt-context, coordinator validation, strict-mode gating, and error taxonomy; Phases 1.5–5 covering consumed-registry integration, coordinator enforcement, transcript replay hardening, liveness policy with recovery classification, baseline calibration, rollout runbook, and security gates.
ROAST implementation roadmap
docs/roast-implementation-plan.md
Complete ROAST migration plan covering baseline, goals/non-goals, design principles, threat model, target semantics (deterministic attempt/coordinator, authenticated retry, replay-safety), phased deliverables (Phase 0–5), cross-repo dependencies, and Definition-of-Done.
Hardening, security & operational plans
docs/permissioned-signer-hardening-rfc.md, docs/tbtc-signer-secret-material-hardening-plan.md, docs/tee-whitelisted-signer-enforcement-plan.md
RFC with P0–P2 hardening phases; secret-material plan (encrypted-at-rest XChaCha20-Poly1305 envelope); TEE operator enforcement covering admission tokens, runtime behavior, break-glass, and failure modes.
API contract and design decisions
docs/signer-api-contract-decision-brief.md, docs/rust-rewrite-bootstrap.md
Decision brief comparing round-level vs coarse session API (recommending session); bootstrap status documenting implemented features and production gates.
Future considerations documentation
docs/true-late-t_of_n_finalize_considerations.md
Discussion draft on true late t-of-n finalize tradeoffs and current recommendation to defer as future consideration.

Build Configuration & Scripts

Layer / File(s) Summary
Crate metadata and build setup
pkg/tbtc/signer/Cargo.toml, pkg/tbtc/signer/build.sh, pkg/tbtc/signer/.gitignore
Cargo.toml defines tbtc-signer crate (edition 2021, MIT), library config (cdylib+rlib), bench-restart-hook feature, frost-secp256k1-tr dependency; build.sh with strict bash and release cargo build; .gitignore ignores target/.
Comprehensive README documentation
pkg/tbtc/signer/README.md
Full README covering exposed C ABI endpoints, not-yet-implemented items, build/test commands, admission checker CLI, encrypted state provider contracts, benchmark/chaos suite invocation, FFI JSON contract, error codes, and buffer management.
Admission checker sample configurations
pkg/tbtc/signer/scripts/admission-*.sample.json
JSON samples for policy (constraints, DAO overrides), candidates, existing operators, override signatures, and replay registries.
Formal verification & testing scripts
pkg/tbtc/signer/scripts/formal/check_roast_attempt_context_vectors.mjs, pkg/tbtc/signer/scripts/run_phase5_chaos_suite.sh, pkg/tbtc/signer/scripts/formal/run_tla_models.sh
Node.js CLI for ROAST vector conformance; Bash runners for chaos suite scenarios and TLA+ model checking.

🎯 4 (Complex) | ⏱️ ~75 minutes

🐰 A bootstrap springs forth, with FFI calls so bright,
Formal models carved in TLA+ light,
ROAST coordinators dancing in Go-lang's stride,
Secrets encrypted with cryptographic pride,
From signer to harness, one system unified!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 47.28% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: adding the FROST/ROAST Rust signer under tbtc/signer.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch extraction/frost-signer-mirror-2026-05-26

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

…ripts

Per extraction plan v38 §4.4, allowlisted-divergence files require
content normalization for the canonical context. This commit applies
the transformations declared in the source manifest for the 2 signer-
side allowlisted-divergence entries.

Transformations
- pkg/tbtc/signer/scripts/formal/check_roast_attempt_context_vectors.mjs:
  Rewrite vector path from
    `docs/frost-migration/test-vectors/roast-attempt-context-v1.json`
  to canonical signer layout
    `test/vectors/roast-attempt-context-v1.json`
  (both relative to rootDir which is two levels up from the script
  location; in canonical context that's pkg/tbtc/signer/)

- pkg/tbtc/signer/scripts/formal/run_tla_models.sh:
  Rewrite MODEL_DIR default from
    `$ROOT_DIR/docs/frost-migration/formal-verification/models`
  to canonical signer layout
    `$ROOT_DIR/docs/formal/models`
  Plus MODELS_PATH env-var override for alternate environments (CI
  matrices, local dev trees). ROOT_DIR is unchanged
  (`$(dirname $BASH_SOURCE)/../..` resolves to pkg/tbtc/signer/ here).

Verification
- Both files retain identical behavior to their monorepo counterparts
  when invoked from canonical signer layout
- Comments added documenting the path normalization with reference back
  to the source manifest's allowlisted-divergence status

Recompute expectedTargetSha256 for both entries in the source manifest
and collect dual signoff before merge per plan v38 §4.2.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 13

🧹 Nitpick comments (3)
pkg/tbtc/signer/src/api.rs (1)

196-202: 💤 Low value

Missing #[serde(default, skip_serializing_if)] on script_tree_hex.

All other Option<T> fields in this file use #[serde(default, skip_serializing_if = "Option::is_none")], but script_tree_hex does not. This inconsistency means the field will serialize as null when absent, rather than being omitted.

Suggested fix
 pub struct BuildTaprootTxRequest {
     pub session_id: String,
     pub inputs: Vec<TxInput>,
     pub outputs: Vec<TxOutput>,
+    #[serde(default, skip_serializing_if = "Option::is_none")]
     pub script_tree_hex: Option<String>,
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/tbtc/signer/src/api.rs` around lines 196 - 202, The BuildTaprootTxRequest
struct's script_tree_hex Option field lacks the serde attributes used elsewhere;
update the declaration of BuildTaprootTxRequest so the script_tree_hex field is
annotated with #[serde(default, skip_serializing_if = "Option::is_none")] to
match other Option<T> fields (preserving Clone/Debug/Deserialize/Serialize
behavior) so it is omitted from serialized output when None instead of
serializing as null.
pkg/tbtc/signer/src/lib.rs (1)

391-421: 💤 Low value

std::env::set_var and std::env::remove_var are not thread-safe.

These functions are unsound in multi-threaded contexts and deprecated since Rust 1.66. While the tests appear to serialize access via lock_test_state(), this guard must be held across all env mutations and checks within a test to prevent races with parallel test threads.

Current usage appears safe given the locking pattern, but this is fragile. Consider using a dedicated test configuration mechanism that doesn't rely on process-wide environment mutation.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/tbtc/signer/src/lib.rs` around lines 391 - 421, EnvVarGuard's methods
(EnvVarGuard::set, EnvVarGuard::unset) and its Drop rely on
std::env::set_var/remove_var which are process-wide and not thread-safe; replace
this pattern with a test-scoped, non-global solution such as using a crate that
provides scoped environment variables (e.g., temp_env or similar) or refactor
tests to accept an injected configuration object instead of mutating process
env; update usages to acquire and hold the new scoped guard for the entire
duration of tests that need env changes (or pass a Config struct into functions
under test) and remove direct calls to std::env::set_var/remove_var and the
EnvVarGuard Drop behavior to avoid races.
pkg/tbtc/signer/src/bin/admission_checker.rs (1)

257-276: 💤 Low value

Consider cleaning up the temp file if rename fails.

If fs::rename fails (e.g., cross-filesystem move or permissions issue), the temp file remains on disk. Adding a cleanup attempt in the error path would improve robustness.

♻️ Proposed cleanup on error
     fs::write(&tmp_path, serialized).map_err(|error| {
         format!(
             "failed to write override replay registry temp file [{}]: {error}",
             tmp_path.display()
         )
     })?;
-    fs::rename(&tmp_path, path).map_err(|error| {
-        format!(
-            "failed to persist override replay registry [{}]: {error}",
-            path.display()
-        )
-    })
+    fs::rename(&tmp_path, path).map_err(|error| {
+        let _ = fs::remove_file(&tmp_path); // Best-effort cleanup
+        format!(
+            "failed to persist override replay registry [{}]: {error}",
+            path.display()
+        )
+    })
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/tbtc/signer/src/bin/admission_checker.rs` around lines 257 - 276,
persist_override_replay_registry currently leaves the temporary file (tmp_path)
if fs::rename fails; modify the rename error path to attempt cleanup of tmp_path
before returning the error. Specifically, call fs::remove_file(&tmp_path)
(ignoring or logging its result) inside the Err branch that handles the rename
failure so the function still returns the original formatted error for
fs::rename but also tries to remove the leftover tmp file; reference
persist_override_replay_registry, path, tmp_path, and fs::rename when making the
change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/tbtc/signer/docs/formal/models/README.md`:
- Around line 32-51: Update the incorrect repository paths in the traceability
matrix of pkg/tbtc/signer/docs/formal/models/README.md so links point to the
actual implementation and docs in this repo: change references to
tools/tbtc-signer/src/engine.rs to pkg/tbtc/signer/src/engine.rs for the entries
mentioning RoastAttemptStateMachine.tla (validate_attempt_context, replay
guards) and StateKeyProviderPolicy.tla (decode_encrypted_state_envelope,
encode_encrypted_state_envelope); change
docs/frost-migration/tee-whitelisted-signer-enforcement-plan.md to
pkg/tbtc/signer/docs/tee-whitelisted-signer-enforcement-plan.md for
TeeEnforcementModes.tla; and change
docs/frost-migration/roast-phase-5-security-rollout-gates.md to
pkg/tbtc/signer/docs/roast-phase-5-security-rollout-gates.md for
RoastRolloutPolicy.tla so readers can locate the referenced code and policy
docs.

In `@pkg/tbtc/signer/docs/roast-implementation-plan.md`:
- Line 93: Update the broken doc links in
pkg/tbtc/signer/docs/roast-implementation-plan.md by replacing repo-external
paths like `docs/frost-migration/roast-phase-0-spec-freeze.md` and any
`tools/tbtc-signer/...` references with the correct repo-local paths under
pkg/tbtc/signer/docs (or use correct relative paths from this markdown file);
search the file for all occurrences of `docs/frost-migration/...` and
`tools/tbtc-signer/...` (including the instances similar to the shown
`docs/frost-migration/roast-phase-0-spec-freeze.md`) and normalize each link so
it points to the new location in this package, preserving anchor fragments and
updating any link text as needed.

In `@pkg/tbtc/signer/docs/roast-phase-5-rollout-runbook.md`:
- Around line 26-29: Update the incorrect crate path used in the runbook
commands: replace occurrences of "cd tools/tbtc-signer" with "cd
pkg/tbtc/signer" for the benchmark command (`cargo bench --features
bench-restart-hook --bench phase5_roast`) and the chaos suite script invocation
(`./scripts/run_phase5_chaos_suite.sh`) so the commands run from the correct
crate directory.

In `@pkg/tbtc/signer/docs/roast-phase-5-security-rollout-gates.md`:
- Around line 92-99: Update stale repository paths in
roast-phase-5-security-rollout-gates.md: replace any occurrences of the old
tooling path "tools/tbtc-signer" with the new location "pkg/tbtc/signer" (e.g.,
update the run command `cd tools/tbtc-signer && cargo bench --features
bench-restart-hook --bench phase5_roast` to `cd pkg/tbtc/signer ...`), and
update any links referencing
`docs/frost-migration/roast-phase-5-baseline-calibration.md` to the document’s
new location in the repo (search for the exact link text and swap to the correct
path). Ensure all instances at the reported locations (around the run command
and the listed links) are changed consistently so operators following the
runbook hit the correct files and commands.

In `@pkg/tbtc/signer/docs/rust-rewrite-bootstrap.md`:
- Around line 10-13: The documentation still references the old crate path
"tools/tbtc-signer" and the validation command using that path; update every
occurrence to "pkg/tbtc/signer" in rust-rewrite-bootstrap.md (including the
header lines that list the crate, the C ABI include path `include/frost_tbtc.h`,
and any validation/build commands) so links and commands point to the colocated
pkg/tbtc/signer location; search for "tools/tbtc-signer" and replace with
"pkg/tbtc/signer" and verify the validation command and any examples reference
the new path.

In `@pkg/tbtc/signer/docs/signer-api-contract-decision-brief.md`:
- Around line 43-47: Update the two referenced paths in
signer-api-contract-decision-brief.md so they point to the mirrored crate
locations: replace `docs/frost-migration/rust-rewrite-bootstrap.md` with
`pkg/tbtc/signer/docs/frost-migration/rust-rewrite-bootstrap.md` and replace
`tools/tbtc-signer/src/lib.rs` with `pkg/tbtc/signer/src/lib.rs` (look for the
occurrences shown around the paragraph mentioning the bootstrap Rust crate and
file: `tools/tbtc-signer/src/lib.rs` and update those strings accordingly).

In `@pkg/tbtc/signer/docs/tbtc-signer-secret-material-hardening-plan.md`:
- Around line 6-7: The doc still uses the old crate path string
`tools/tbtc-signer`; update that scope reference to `pkg/tbtc/signer` throughout
the file (tbtc-signer-secret-material-hardening-plan.md) so the plan points to
the mirrored crate location, and scan for any other occurrences of
`tools/tbtc-signer` in this document to replace with `pkg/tbtc/signer`.

In `@pkg/tbtc/signer/docs/tee-whitelisted-signer-enforcement-plan.md`:
- Around line 296-298: Update the two broken cross-doc links in
pkg/tbtc/signer/docs/tee-whitelisted-signer-enforcement-plan.md (currently
referencing docs/frost-migration/roast-phase-5-security-rollout-gates.md and
docs/frost-migration/roast-phase-5-rollout-runbook.md on lines ~296–297) to
point to their correct locations inside this PR:
pkg/tbtc/signer/docs/roast-phase-5-security-rollout-gates.md and
pkg/tbtc/signer/docs/roast-phase-5-rollout-runbook.md so cross-document
navigation resolves correctly.

In `@pkg/tbtc/signer/README.md`:
- Around line 53-54: Update the README.md occurrence(s) that reference the old
path string "tools/tbtc-signer" to the new crate location "pkg/tbtc/signer":
search for and replace that path in all command snippets, code blocks, and file
references (e.g., cargo build/cd commands and any path bullets) so every
instance uses "pkg/tbtc/signer" consistently; ensure both shell commands and
prose file paths are updated, and run a quick grep for "tools/tbtc-signer" to
confirm no remaining references.

In `@pkg/tbtc/signer/scripts/admission-policy-v1.sample.json`:
- Line 8: Replace the non-hex placeholder value for the JSON key
dao_override_trust_root_pubkey_hex with a syntactically valid 32-byte hex string
(64 lowercase hex characters) so sample files and copy/paste validation don't
break; update the sample value to something like a 64-character hex placeholder
and leave replacement guidance in the README or nearby comment explaining it
must be replaced with the real x-only pubkey hex.

In `@pkg/tbtc/signer/scripts/formal/check_roast_attempt_context_vectors.mjs`:
- Around line 15-18: vectorsPath currently resolves to
"docs/frost-migration/test-vectors/roast-attempt-context-v1.json" and will fail
because the vectors live under "test/vectors"; update the path.join call that
constructs vectorsPath (using rootDir and the filename) to point to
"test/vectors/roast-attempt-context-v1.json" so the script loads the correct
file; ensure the change is made where vectorsPath is declared and used in this
module.

In `@pkg/tbtc/signer/scripts/formal/run_tla_models.sh`:
- Around line 4-5: The MODEL_DIR assignment in run_tla_models.sh is pointing to
the wrong path; update the MODEL_DIR variable (currently computed relative to
ROOT_DIR) to "$ROOT_DIR/docs/formal/models" so the directory existence check in
the script (around the directory existence test near line 20) succeeds; modify
the MODEL_DIR definition in the script (look for the MODEL_DIR variable
assignment and references) to use the corrected path and ensure any subsequent
uses of MODEL_DIR still reference this updated variable.

In `@pkg/tbtc/signer/tests/p2tr_signature_fraud_vectors.rs`:
- Around line 375-376: The test constructs vectors_path using the vectors_path
variable in pkg/tbtc/signer/tests/p2tr_signature_fraud_vectors.rs which
currently joins
"../../docs/frost-migration/test-vectors/p2tr-signature-fraud-v0.json" relative
to CARGO_MANIFEST_DIR and points to a non-existent file; fix by either adding
the missing p2tr-signature-fraud-v0.json to the repo at that path or update the
vectors_path join to the correct relative path where the JSON actually lives
(adjust the "../" segments or point to the canonical test-vectors location),
ensuring the variable name vectors_path and its usage remain unchanged.

---

Nitpick comments:
In `@pkg/tbtc/signer/src/api.rs`:
- Around line 196-202: The BuildTaprootTxRequest struct's script_tree_hex Option
field lacks the serde attributes used elsewhere; update the declaration of
BuildTaprootTxRequest so the script_tree_hex field is annotated with
#[serde(default, skip_serializing_if = "Option::is_none")] to match other
Option<T> fields (preserving Clone/Debug/Deserialize/Serialize behavior) so it
is omitted from serialized output when None instead of serializing as null.

In `@pkg/tbtc/signer/src/bin/admission_checker.rs`:
- Around line 257-276: persist_override_replay_registry currently leaves the
temporary file (tmp_path) if fs::rename fails; modify the rename error path to
attempt cleanup of tmp_path before returning the error. Specifically, call
fs::remove_file(&tmp_path) (ignoring or logging its result) inside the Err
branch that handles the rename failure so the function still returns the
original formatted error for fs::rename but also tries to remove the leftover
tmp file; reference persist_override_replay_registry, path, tmp_path, and
fs::rename when making the change.

In `@pkg/tbtc/signer/src/lib.rs`:
- Around line 391-421: EnvVarGuard's methods (EnvVarGuard::set,
EnvVarGuard::unset) and its Drop rely on std::env::set_var/remove_var which are
process-wide and not thread-safe; replace this pattern with a test-scoped,
non-global solution such as using a crate that provides scoped environment
variables (e.g., temp_env or similar) or refactor tests to accept an injected
configuration object instead of mutating process env; update usages to acquire
and hold the new scoped guard for the entire duration of tests that need env
changes (or pass a Config struct into functions under test) and remove direct
calls to std::env::set_var/remove_var and the EnvVarGuard Drop behavior to avoid
races.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c0224eef-499a-42a2-a346-f06ef353278b

📥 Commits

Reviewing files that changed from the base of the PR and between 980587b and 335ce60.

⛔ Files ignored due to path filters (1)
  • pkg/tbtc/signer/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (48)
  • pkg/tbtc/signer/.gitignore
  • pkg/tbtc/signer/Cargo.toml
  • pkg/tbtc/signer/README.md
  • pkg/tbtc/signer/benches/phase5_roast.rs
  • pkg/tbtc/signer/build.sh
  • pkg/tbtc/signer/docs/formal/models/README.md
  • pkg/tbtc/signer/docs/formal/models/RoastAttemptStateMachine.cfg
  • pkg/tbtc/signer/docs/formal/models/RoastAttemptStateMachine.tla
  • pkg/tbtc/signer/docs/formal/models/RoastRolloutPolicy.cfg
  • pkg/tbtc/signer/docs/formal/models/RoastRolloutPolicy.tla
  • pkg/tbtc/signer/docs/formal/models/StateKeyProviderPolicy.cfg
  • pkg/tbtc/signer/docs/formal/models/StateKeyProviderPolicy.production.cfg
  • pkg/tbtc/signer/docs/formal/models/StateKeyProviderPolicy.tla
  • pkg/tbtc/signer/docs/formal/models/TeeEnforcementModes.cfg
  • pkg/tbtc/signer/docs/formal/models/TeeEnforcementModes.tla
  • pkg/tbtc/signer/docs/permissioned-signer-hardening-rfc.md
  • pkg/tbtc/signer/docs/roast-implementation-plan.md
  • pkg/tbtc/signer/docs/roast-phase-0-spec-freeze.md
  • pkg/tbtc/signer/docs/roast-phase-1.5-consumed-registry-integration.md
  • pkg/tbtc/signer/docs/roast-phase-2-coordinator-policy-enforcement.md
  • pkg/tbtc/signer/docs/roast-phase-3-attempt-transcript-replay-hardening.md
  • pkg/tbtc/signer/docs/roast-phase-4-liveness-policy-recovery.md
  • pkg/tbtc/signer/docs/roast-phase-5-baseline-calibration.md
  • pkg/tbtc/signer/docs/roast-phase-5-rollout-runbook.md
  • pkg/tbtc/signer/docs/roast-phase-5-security-rollout-gates.md
  • pkg/tbtc/signer/docs/rust-rewrite-bootstrap.md
  • pkg/tbtc/signer/docs/signer-api-contract-decision-brief.md
  • pkg/tbtc/signer/docs/tbtc-signer-secret-material-hardening-plan.md
  • pkg/tbtc/signer/docs/tee-whitelisted-signer-enforcement-plan.md
  • pkg/tbtc/signer/docs/true-late-t-of-n-finalize-considerations.md
  • pkg/tbtc/signer/include/frost_tbtc.h
  • pkg/tbtc/signer/scripts/admission-candidate.sample.json
  • pkg/tbtc/signer/scripts/admission-existing.sample.json
  • pkg/tbtc/signer/scripts/admission-override-registry.sample.json
  • pkg/tbtc/signer/scripts/admission-override.sample.json
  • pkg/tbtc/signer/scripts/admission-policy-v1.sample.json
  • pkg/tbtc/signer/scripts/formal/check_roast_attempt_context_vectors.mjs
  • pkg/tbtc/signer/scripts/formal/run_tla_models.sh
  • pkg/tbtc/signer/scripts/run_phase5_chaos_suite.sh
  • pkg/tbtc/signer/src/api.rs
  • pkg/tbtc/signer/src/bin/admission_checker.rs
  • pkg/tbtc/signer/src/engine.rs
  • pkg/tbtc/signer/src/errors.rs
  • pkg/tbtc/signer/src/ffi.rs
  • pkg/tbtc/signer/src/go_math_rand.rs
  • pkg/tbtc/signer/src/lib.rs
  • pkg/tbtc/signer/test/vectors/roast-attempt-context-v1.json
  • pkg/tbtc/signer/tests/p2tr_signature_fraud_vectors.rs

Comment thread pkg/tbtc/signer/docs/formal/models/README.md
Comment thread pkg/tbtc/signer/docs/roast-implementation-plan.md Outdated
Comment thread pkg/tbtc/signer/docs/roast-phase-5-rollout-runbook.md Outdated
Comment thread pkg/tbtc/signer/docs/roast-phase-5-security-rollout-gates.md Outdated
Comment thread pkg/tbtc/signer/docs/rust-rewrite-bootstrap.md Outdated
Comment thread pkg/tbtc/signer/README.md Outdated
Comment thread pkg/tbtc/signer/scripts/admission-policy-v1.sample.json Outdated
Comment thread pkg/tbtc/signer/scripts/formal/run_tla_models.sh Outdated
Comment thread pkg/tbtc/signer/tests/p2tr_signature_fraud_vectors.rs Outdated
Adds a focused workflow that runs the Rust signer's formal-invariant
test suite + TLA model checks. Moved from
threshold-network/tbtc-v2/.github/workflows/ci-formal-verification.yml
(jobs `signer-formal-invariants` + `tla-model-checks`) per extraction
plan v38 §3.1 — the signer code lives here at pkg/tbtc/signer/, not
in tbtc-v2, so the CI jobs that exercise it belong here too.

Jobs
- signer-formal-invariants: cargo test --manifest-path pkg/tbtc/signer/
  Cargo.toml formal_verification_ (filter to formal-only cases)
- tla-model-checks: pkg/tbtc/signer/scripts/formal/run_tla_models.sh
  (iterates over .cfg files in pkg/tbtc/signer/docs/formal/models/ and
  runs TLC against each; MODELS_PATH env var allows override per the
  path-normalization commit b84b574c on this branch)

Triggers
- pull_request on pkg/tbtc/signer/** changes + this workflow file
- schedule nightly at 05:23 UTC (mirrors monorepo's pattern of running
  formal invariants both on PRs and nightly)
- workflow_dispatch for manual runs

Related changes in companion PR threshold-network/tbtc-v2#971:
- Removed these jobs from canonical tbtc-v2's ci-formal-verification.yml
- Added a comment in that file pointing here

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/tbtc-signer-formal.yml:
- Around line 25-26: The checkout steps using actions/checkout@v4 persist git
credentials by default; update each Checkout step (the uses: actions/checkout@v4
entries) to add with: persist-credentials: false so credentials are not stored
in the runner after checkout. Ensure both occurrences of actions/checkout@v4 in
the workflow are modified accordingly.
- Line 26: Update the GitHub Actions workflow to pin action versions and harden
checkout credentials: replace occurrences of actions/checkout@v4 with
actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 and add with:
persist-credentials: false to both checkout steps that use checkout, replace
dtolnay/rust-toolchain@stable with
dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8, and replace
actions/setup-java@v4 with
actions/setup-java@c1e323688fd81a25caa38c78aa6df2d33d3e20d9 so the workflow pins
SHA-based commits and disables persisting credentials on checkout.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 88553e79-6db4-4e95-98fe-00c56e1bc640

📥 Commits

Reviewing files that changed from the base of the PR and between bfd7658 and 551bd42.

📒 Files selected for processing (1)
  • .github/workflows/tbtc-signer-formal.yml

Comment thread .github/workflows/tbtc-signer-formal.yml Outdated
Comment thread .github/workflows/tbtc-signer-formal.yml Outdated
Resolves CI failures on PR #4005 (signer mirror):

1. TLA model checks: run_tla_models.sh lacked executable bit at
   canonical HEAD. CI ran the script directly (no `bash` prefix),
   which fails with `Permission denied`. Fixed via
   `git update-index --chmod=+x`.

2. Signer formal invariants: engine.rs's
   formal_verification_roast_attempt_context_shared_vectors_match_
   expected_values test referenced vectors at a path stale from
   the umbrella's docs/frost-migration/test-vectors/ layout. The
   manifest places the vector at the canonical-signer test/vectors/
   subdir (pkg/tbtc/signer/test/vectors/roast-attempt-context-v1.json
   per the source-to-target map). Updated the
   `PathBuf::from(env!("CARGO_MANIFEST_DIR")).join(...)` argument from
   `../../docs/frost-migration/test-vectors/roast-attempt-context-v1.json`
   (umbrella-relative) to `test/vectors/roast-attempt-context-v1.json`
   (signer-CARGO_MANIFEST_DIR-relative, where the vector actually
   lives at canonical HEAD).

Verified locally:
- ls -l shows executable bit set on run_tla_models.sh
- engine.rs path now resolves to the correct mirror location
- Vector exists at pkg/tbtc/signer/test/vectors/roast-attempt-context-v1.json

Same fix needs to be applied to PR #4007 (stacked on #4005)
in a follow-up commit on its branch.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mswilkison added a commit that referenced this pull request May 26, 2026
PR #4005 signer formal invariants test
formal_verification_p2tr_signature_fraud_vectors_match_bitcoin_crate
was failing because:

1. The umbrella source manifest only mapped p2tr-signature-fraud-v0.json
   to the tbtc-v2 target (docs/test-vectors/). It was not mirrored to
   the keep-core (signer) target.

2. tests/p2tr_signature_fraud_vectors.rs referenced the vector at the
   umbrella-relative path
   `../../docs/frost-migration/test-vectors/p2tr-signature-fraud-v0.json`
   (CARGO_MANIFEST_DIR-relative -> repo-root + docs/frost-migration/...).
   That directory does not exist on canonical keep-core.

This is a structural omission in the manifest, not a divergence: the
cross-language vector test exists in both Solidity (tbtc-v2 side) and
Rust (signer side), and the vector is genuinely needed in both places
to verify cross-implementation consistency.

Fix
- Mirror p2tr-signature-fraud-v0.json (598 lines, byte-identical
  content from tbtc-v2 mirror at docs/test-vectors/) to
  pkg/tbtc/signer/test/vectors/p2tr-signature-fraud-v0.json.
- Update the test path in tests/p2tr_signature_fraud_vectors.rs from
  `../../docs/frost-migration/test-vectors/p2tr-signature-fraud-v0.json`
  to `test/vectors/p2tr-signature-fraud-v0.json` (CARGO_MANIFEST_DIR =
  pkg/tbtc/signer/, so test/vectors/... resolves correctly).

Two stale comment references remain in
- pkg/tbtc/signer/docs/roast-implementation-plan.md:265
- pkg/tbtc/signer/scripts/formal/check_roast_attempt_context_vectors.mjs:19

Both are comment-only doc pointers to the source layout; they do not
affect runtime. Left as-is to preserve the umbrella -> canonical
provenance trail.

Manifest update follows in a stacked PR on tlabs-xyz/tbtc#10:
- Add p2tr-signature-fraud-v0.json -> signer target mapping
  (test/vectors/p2tr-signature-fraud-v0.json).
- Reclassify tests/p2tr_signature_fraud_vectors.rs from mirror to
  allowlisted-divergence (path differs from umbrella).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Stacked on extraction/frost-signer-mirror-2026-05-26 / PR #4005. Adds
optional taproot_merkle_root_hex to start/finalize signing rounds, binds
it into request fingerprints and round IDs, signs and aggregates with
frost-secp256k1-tr Taproot tweaks, and verifies tweaked aggregates in
tests. Verification: cargo test in pkg/tbtc/signer.
mswilkison and others added 8 commits June 30, 2026 18:24
…ntine recheck)

Codex re-review of the signer FFI surface:

1. [P1] Secret request fields not zeroized (api.rs): SignShareRequest.nonces_hex
   and key_package_hex deserialized into plain Strings dropped without
   zeroization, retaining the one-time nonce / private key package in freed heap.
   Wrap both in Zeroizing<String> (wiped on drop) + a redacting Debug so the
   secrets no longer leak via {:?} either.

2. [P1] Panic hook defeats payload redaction (ffi.rs): catch_unwind does not
   suppress Rust's default panic hook, so a panic payload (path/config/secret)
   prints to stderr before being converted to the redacted ErrorResponse. Install
   a panic hook at the FFI boundary that redacts the payload outside development.

3. [P2] Quarantine not rechecked on cached-round reuse (signing.rs): the matched-
   fingerprint reuse branch returned a fresh share without the new-round path's
   enforce_not_quarantined_identifiers check, so a participant quarantined after
   the round opened could still obtain a share. Apply the check on reuse too.

Verified: cargo test (301 passed) + clippy -D warnings + fmt clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…pty history

Codex re-review: legacy state written before refresh_history existed can carry
refresh_request_fingerprint/refresh_result with an EMPTY refresh_history. The
fingerprint backfill used refresh_history.last_mut(), which is None for empty
history, so the previous fingerprint was not recorded and a delayed retry of that
pre-upgrade refresh was re-executed as a new refresh.

When there is no record to backfill onto, synthesize one from the cached
refresh_result (prior epoch, keeping history strictly increasing) carrying the
previous fingerprint, so stale retries stay rejected. Adds a regression test for
the empty-history legacy shape.

Verified: cargo test (302 passed) + clippy -D warnings + fmt clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…h env

Two Codex-flagged issues in the non-interactive signing path:

1. start_sign_round fingerprinted and derived the round id from the raw
   request.message_hex string. hex::decode accepts mixed casing, so two calls
   carrying identical message bytes but different hex casing derived different
   fingerprints/round ids and a semantically identical retry was rejected as a
   SessionConflict. Lowercase message_hex right after it validates as hex,
   mirroring the interactive path (interactive.rs), so the fingerprint and
   derive_round_id are casing-independent.

2. The README-documented `cargo bench --features bench-restart-hook --bench
   phase5_roast` failed in a clean shell: ensure_benchmark_environment never set
   TBTC_SIGNER_PROFILE (missing -> production) or TBTC_SIGNER_STATE_ENCRYPTION_KEY_HEX
   (required by the default `env` state-key provider), so the first RunDkg
   persist aborted. Seed profile=development, provider=env, and a fixed dev key.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The prior commit lowercases message_hex before computing every canonical and
legacy fingerprint. That regressed the upgrade path: when a node upgrades with
an active sign round that a previous build started using mixed/uppercase
message_hex, the persisted sign_request_fingerprint was computed over that
original casing. The same retry then matched none of the fingerprints and fell
through to SessionConflict instead of reusing the cached round.

Capture the pre-canonicalization casing and, only when it differs from the
lowercased form, compute the four legacy fingerprint shapes over the original
casing and accept them in the cached-round match. A match migrates the stored
fingerprint to the canonical lowercase form, so the compatibility cost is paid
once per in-flight round. Adds a regression test that persists a mixed-case
fingerprint and asserts the retry reuses the round and migrates it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…d result

The empty-history synthesize branch was guarded by `else if let Some(previous_result)`,
so a legacy/degraded state that kept refresh_request_fingerprint but whose
refresh_result deserialized to None (and whose refresh_history is empty) fell
through: neither the last_mut() backfill nor the synthesize branch ran. The next
refresh then overwrote the only copy of the old fingerprint, so a delayed retry
of the pre-upgrade request was treated as fresh and advanced the epoch — a replay.

Make the branch unconditional: synthesize a history record carrying the previous
fingerprint whether or not a cached result is present. Prefer the result's epoch
(when non-zero and below the new refresh) and share count; otherwise fall back to
refresh_epoch-1 (min 1) and a zero share count. refresh_epoch_counter is
persisted, so a prior accepted refresh implies refresh_epoch >= 2 and the fallback
stays non-zero and strictly below the new record, keeping history monotonic.

Adds a regression test (empty history + refresh_result=None) verified to fail
against the pre-fix code (the stale retry was re-executed instead of rejected).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…riodic reshare (#4124)

## Summary

Three findings from the #4005 review (the multi-agent custody review +
Codex's review), fixed and verified.

### 1. [HIGH] Plaintext state fallback was accepted unconditionally —
`persistence.rs`
`decode_persisted_state_storage_format` fell back to parsing an
**unencrypted, unauthenticated** `PersistedEngineState` whenever the
bytes weren't a valid AEAD envelope — with no gate at all. The AEAD
envelope is the only integrity control for state-at-rest, so anyone who
could write the state file could forge it (cleared `consumed_*` replay
markers → resume-into-double-sign, or an attacker `dkg_key_package` →
admission bypass) **without holding the state-encryption key**.

Per the secret-material hardening plan, the plaintext path is now
**emergency-rollback-only**: compile-time disabled in release builds
(`cfg!(debug_assertions)`), rejected in production profiles, and gated
behind an explicit `TBTC_SIGNER_PERMIT_PLAINTEXT_STATE_ROLLBACK` opt-in.
The two tests that load plaintext (migration + schema-mismatch) opt in,
and a negative assertion pins the fail-closed default.

### 2. [Codex P1] Secret FFI buffers left unwiped on free — `ffi.rs`
`free_buffer` now `zeroize`s the buffer before deallocation, so secret
material returned by nonce/DKG/key-package endpoints (e.g.
`frost_tbtc_generate_nonces_and_commitments`) is wiped rather than left
in allocator memory when a caller forgets to clear it. (`zeroize` is a
volatile wipe the optimizer can't elide.)

### 3. [Codex P2] Periodic reshare was a one-shot — `lifecycle.rs`
After the first `refresh_shares`, any later reshare with updated
`current_shares` had a different fingerprint and was rejected as
`SessionConflict`, so a long-lived session could never advance
`refresh_history`/`refresh_count`/cadence past the first refresh. A
different fingerprint is now treated as a new periodic refresh;
idempotent replay of the *same* fingerprint is unchanged.

## Deferred (flagged for the team)
- **Signing-policy firewall production force-on** (my MEDIUM finding):
mirroring the provenance gate is correct in principle, but
force-enabling the firewall makes its *entire* policy config (allowed
script classes, output caps, …) mandatory in production — a
deployment-contract change. The verifier flagged it as needing product
confirmation, so it's left as-is pending that decision.
- **State-at-rest anti-rollback freshness counter** and
**quarantine-reset replay-marker loss**: design-level (needs an external
monotonic anchor) / inherent to that opt-in policy (default is
fail-closed).

## Verification
- `cargo test` — **297 passed, 0 failed**.
- `cargo clippy --all-targets -- -D warnings` — **clean (exit 0)**.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

🤖 Generated with [Claude Code](https://claude.com/claude-code)
…t credential persistence

Resolves the zizmor/CodeRabbit findings on the signer formal-verification
workflow: unpinned mutable action tags and persisted git credentials in
checkout steps. All `uses:` references now pin full commit SHAs with
human-readable version comments (actions/checkout v4.3.1,
dtolnay/rust-toolchain stable head, EmbarkStudios/cargo-deny-action
v2.0.20, actions/setup-java v4.8.0), and every checkout step sets
persist-credentials: false so the ephemeral token is not written to the
local git config.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mswilkison and others added 21 commits July 1, 2026 16:33
…ction

The stateless FROST FFI primitives `generate_nonces_and_commitments` and
`sign_share` in `frost_ops.rs` hand a one-time signing nonce pair out to
the host and later accept it back as opaque `nonces_hex`. That leaves
nonce custody -- and the single-use invariant -- entirely with the caller
(the `SignShareRequest` contract even states the caller "is
cryptographically responsible for single use"). A host bug or compromise
that replays one `nonces_hex` across two distinct signing packages
produces two Schnorr shares under the same nonce, which algebraically
solves for the long-term secret share.

The deterministic StartSignRound/FinalizeSignRound path is already fenced
off in production by `enforce_transitional_signing_disabled_in_production`,
but these stateless primitives gated only on the provenance attestation
and had no production-profile guard -- a fail-closed asymmetry. A
production signer could therefore be driven through the host-custody nonce
path and inherit that catastrophic blast radius.

Add `enforce_stateless_nonce_primitives_disabled_in_production`, mirroring
the deterministic guard's style (same `signer_profile_is_production()`
predicate, same `EngineError::LifecyclePolicyRejected` variant and message
shape), and invoke it in both primitives immediately after the provenance
gate -- before any secret key package or nonce is deserialized. Under the
production profile both refuse with reason_code
`stateless_nonce_primitives_disabled_in_production`; production signing
must use the interactive FROST path (`interactive.rs`), where the engine
keeps nonce custody and enforces durable single-use consumption markers.
Non-production/test profiles retain current behavior.

No current production caller reaches these paths (defense-in-depth before
activation). Adds a focused test proving both primitives fail closed under
the production profile with valid provenance -- feeding the same secret
inputs that succeed under development -- and still work under the
development profile.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex P2 review on #4127 flagged that SHA-pinning dtolnay/rust-toolchain could
make it install the SHA as a toolchain, since older versions of the action
derive the default toolchain from the action ref. Verified against the pinned
version (4be7066): its action.yml already defaults `toolchain` to `stable`, so
the Setup Rust / formal jobs were not actually broken (they pass on this
branch). Still, name the toolchain explicitly on both Setup Rust steps: it is
self-documenting and independent of the pinned action's default, which is the
safe form for a SHA-pinned toolchain action.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…t credential persistence (#4127)

## Summary

Hardens `.github/workflows/tbtc-signer-formal.yml` (introduced in #4005)
against supply-chain and credential-exposure risks:

- Every `actions/checkout` step now sets `persist-credentials: false`,
so the ephemeral `GITHUB_TOKEN` is not persisted into the local git
config after checkout. None of the jobs perform authenticated git
operations after checkout, so nothing relies on the persisted
credential.
- Every `uses:` reference is pinned to a full 40-hex commit SHA with a
trailing comment recording the human-readable version, replacing mutable
tag/branch refs. No action major versions were changed — each pin is the
current commit behind the ref that was already in use.

This addresses the two open CodeRabbit review threads on #4005
(persisted checkout credentials + unpinned action refs on
`.github/workflows/tbtc-signer-formal.yml`).

## Pinned actions

| Action | Old ref | Pinned SHA (version) |
| --- | --- | --- |
| `actions/checkout` | `v4` | `34e114876b0b11c390a56381ad16ebd13914f8d5`
(v4.3.1) |
| `dtolnay/rust-toolchain` | `stable` |
`4be7066ada62dd38de10e7b70166bc74ed198c30` (stable branch head) |
| `EmbarkStudios/cargo-deny-action` | `v2` |
`bb137d7af7e4fb67e5f82a49c4fce4fad40782fe` (v2.0.20) |
| `actions/setup-java` | `v4` |
`c1e323688fd81a25caa38c78aa6df2d33d3e20d9` (v4.8.0) |

## Notes

- Stacked on `extraction/frost-signer-mirror-2026-05-26` (PR #4005);
only the workflow file is touched.
- YAML validated locally after the edit.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
…e production profile (#4129)

## Finding (MEDIUM, fail-closed asymmetry — defense-in-depth)

The stateless FROST FFI primitives `generate_nonces_and_commitments` and
`sign_share` in `pkg/tbtc/signer/src/engine/frost_ops.rs` hand a
one-time signing nonce pair out to the host and later accept it back as
opaque `nonces_hex`. Nonce custody — and the single-use invariant — is
left entirely with the caller. The `SignShareRequest` contract even
documents this: the caller *"is cryptographically responsible for single
use."*

A host bug or compromise that replays one `nonces_hex` across two
distinct signing packages yields two Schnorr shares under the same
nonce, which algebraically solves for the long-term secret share.
Catastrophic blast radius if reached.

The **deterministic** StartSignRound/FinalizeSignRound path is already
fenced off in production by
`enforce_transitional_signing_disabled_in_production`. The **stateless**
primitives, by contrast, gated only on the provenance attestation
(`enforce_provenance_gate`) and had **no** production-profile guard — a
fail-closed asymmetry. A production signer could therefore be driven
through the host-custody nonce path and inherit that blast radius.

No current production caller reaches these paths; this is
defense-in-depth before activation.

## Guard added

New
`enforce_stateless_nonce_primitives_disabled_in_production(operation)`
in `frost_ops.rs`, mirroring the deterministic guard's style:
- same predicate: `signer_profile_is_production()`
- same error type/shape: `EngineError::LifecyclePolicyRejected`
- reason_code: `stateless_nonce_primitives_disabled_in_production`

Under the production profile it fails closed (returns the error);
non-production/test profiles keep current behavior.

## The two call sites

Invoked immediately after `enforce_provenance_gate()?` — before any
secret key package or nonce is deserialized — in:
- `generate_nonces_and_commitments`
(`enforce_stateless_nonce_primitives_disabled_in_production("GenerateNoncesAndCommitments")`)
- `sign_share`
(`enforce_stateless_nonce_primitives_disabled_in_production("SignShare")`)

The deterministic path is unchanged.

## Test


`engine::tests::stateless_nonce_primitives_reject_under_production_profile`:
- Under the development profile, builds real key packages + nonces +
signing package and confirms both primitives succeed.
- Flips to the production profile (with valid provenance so the new gate
— not the provenance gate — is what fires) and feeds the **same** secret
inputs: both primitives return `LifecyclePolicyRejected` with
reason_code `stateless_nonce_primitives_disabled_in_production`, before
any secret is deserialized (the gate is the second statement of each
primitive).

## Validation (from `pkg/tbtc/signer`)

- `cargo fmt -- --check` — clean
- `cargo clippy --all-targets -- -D warnings` — clean
- `cargo test` — 319 passed (lib), 25 passed (admission_checker bin), 1
passed (p2tr integration), 0 failed

🤖 Generated with [Claude Code](https://claude.com/claude-code)
…aterial

The interactive signing path loads keys from the engine's persisted session
state (dkg_key_packages by member_identifier + the public key package). The
dealer run_dkg persists ALL key packages because it generates them; a REAL
distributed FROST DKG runs Part1/2/3 across nodes and each node's Part3 returns
only its OWN secret key package, which was previously discarded - so a
distributed-DKG wallet could not sign.

- Add frost_tbtc_persist_distributed_dkg_key_package (engine
  persist_distributed_dkg_key_package): store this node's single key package
  (keyed by its participant identifier) + the group public key package into the
  session, derive the key group from the verifying key, and persist. Idempotent
  for the same key group, conflicting for a different one. No production gate:
  this is the real distributed path, unlike the dealer run_dkg.
- Interactive signing OPEN validated the included participants against
  dkg_key_packages, which a distributed node only has its OWN entry in. Validate
  against the public key package's verifying shares (the full participant set)
  instead - equivalent for the dealer, correct for distributed.

Rust dkg unit tests pass; the rebuilt lib passes the keep-core real-cgo
interactive-signing and distributed-DKG tests. Go bridge + node wiring follow.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ession

persist_distributed_dkg_key_package overwrote dkg_key_packages with a single
entry and returned early on a repeat call, so a multi-seat operator persisting
several local seats of the SAME distributed DKG kept only the first seat's key
package - the others could not open an interactive signing session. Merge each
seat's key package into the session instead (same key group -> accumulate; a
different key group for the session is still a conflict). A single-seat node is
unchanged (one entry); this makes a multi-seat operator - and the single-process
end-to-end test - able to sign with every local seat.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… unit tests

Review follow-ups on the persist op:

- Enforce enforce_provenance_gate() before decoding or persisting any key
  material - the same gate run_dkg and every interactive op enforce. The op
  writes signing material to durable state that interactive signing trusts after
  restart, so an unattested runtime (e.g. production without a valid attestation)
  must not be able to install distributed-DKG signing material. (Codex/review P1.)
- Add engine unit tests: multi-seat key packages accumulate under one session;
  an identifier/participant mismatch and a conflicting key group are rejected;
  and the provenance gate rejects persistence when attestation is required. The
  test helper normalizes DKG material to even-Y exactly as dkg_part3 does.
- rustfmt.

Full Rust suite (303) green; the rebuilt lib keeps the keep-core distributed-DKG
persist->interactive-sign e2e green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…or distributed persist (review)

Three review follow-ups on the distributed-DKG persist op:

- Enforce the DKG admission policy (min participants/threshold, required and
  allowlisted identifiers) over the participant set DERIVED from the public key
  package's verifying shares, the same gate run_dkg enforces. Refactor
  enforce_admission_policy into a shared enforce_admission_policy_for over the raw
  primitives so both paths use identical logic. Otherwise a caller could persist a
  group that omits a required participant or includes a non-allowlisted one, and
  interactive signing would later trust it. (Codex P1.)
- Validate the key package against the group public key package before storing:
  matching identifier, embedded min_signers == threshold, group verifying key, and
  this participant's verifying share. An inconsistent package (e.g. a 3-of-N key
  package stored under threshold 2) previously passed persist and burned the
  attempt at interactive Round2 share release. (Codex P2.)
- Bump the FFI ABI minor to 1 (additive symbol
  frost_tbtc_persist_distributed_dkg_key_package) and update the pinned ABI test,
  so a bridge that needs the symbol can require abi_minor >= 1 and fail-close
  against an older lib instead of failing at symbol lookup. (Codex P2.)

Adds engine tests for the threshold mismatch and the admission rejection. Full
Rust suite (304) green; the rebuilt lib keeps the keep-core distributed-DKG
persist->interactive-sign e2e green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…Ds in persist (review)

Three more review follow-ups on the distributed-DKG persist op:

- Reject any public-key-package verifying-share identifier that does not
  round-trip to a u16 participant identifier. Previously filter_map silently
  dropped a non-canonical identifier from the admission allowlist/required checks
  while it still counted toward the group, so a package could carry allowed u16
  members plus an extra non-allowlisted FROST identifier and still persist. A
  non-canonical identifier cannot be a real group member, so fail closed. (Codex P2.)
- Validate participant_count against the authoritative public-package participant
  set. A 3-member DKG could previously be installed as participant_count=2,
  leaving downstream consumers of DkgResult with the wrong group size. (Codex P2.)
- Clear the global admission-policy env at the end of the admission test (and
  before, for isolation); reset_for_tests does not clear these overrides, so the
  leak could make a later test exercise the wrong policy. (Codex P2.)

Full Rust suite (304) green; the rebuilt lib keeps the keep-core
distributed-DKG persist->interactive-sign e2e green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… public package on accumulate (review)

Two more correctness fixes on the distributed-DKG persist op:

- Verify the SECRET signing share derives to the key package's PUBLIC verifying
  share (VerifyingShare::from(signing_share) == verifying_share). The previous
  checks only trusted the embedded verifying share, but Round2 signs with the
  signing share and deserialization does not prove the scalar matches; a corrupt
  key package could persist with a public share matching the group while storing
  an unrelated signing share, then open and burn signing attempts producing shares
  that never verify. (Codex P2.)
- On accumulate (a second local seat of the same session), require the SAME
  threshold, participant count, and public key package - not just the same group
  verifying key. Otherwise a second seat validated against a different submitted
  public package would leave the session's stored public material inconsistent
  with the newly inserted key, breaking later signing. (Codex P2.)

Adds regression tests: a crafted signing/verifying-share mismatch and a
same-key-group-different-shares accumulate. Full Rust suite (306) green; the
rebuilt lib keeps the keep-core distributed-DKG e2e + multi-node tests green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ibuted DKG (review)

When auto-quarantine is configured and an operator is already quarantined, the
dealer run_dkg refuses to include it (enforce_not_quarantined_identifiers), but
the distributed-DKG persist path applied only admission policy. So a distributed
DKG whose group includes a quarantined operator could be persisted and then
trusted by later interactive signing. Run the same quarantine check over the
participant set derived from the public key package before storing, honoring the
DAO allowlist exactly as run_dkg does. Adds a regression test. (Codex P2.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…cross sessions

The interactive signing path required the DKG key material to live under the
per-signing session_id (SessionNotFound / DkgNotReady otherwise). But DKG persists
under the wallet's DKG session, while interactive ROAST signing runs under a fresh
per-message RoastSessionID - so a distributed-DKG wallet (signable ONLY via the
interactive path; the dealer path uses coarse re-derive) could never sign: the
signing session never held the material. Single-session tests missed it by
persisting and signing under one id.

Fix: treat DKG key material as the WALLET-level asset it is, keyed by key_group,
not by session_id. InteractiveSessionOpen resolves the wallet session by key_group
(its own session when co-located, else the session whose completed DKG produced the
key_group), reads the key material AND the wallet-level policy gates (emergency
rekey / finalization / tx-binding) from it, and creates the per-signing session on
first Open - storing ONLY per-signing state there (no secret copy). The attempt
context is still validated against request.session_id, so coordinator/attempt
derivation is unchanged (per the Go ROAST layer's model). InteractiveAggregate and
verify_share resolve the same wallet material by key_group (via the session's
bound_key_group), reading per-signing completion markers from the signing session.
DkgNotReady now means "no wallet key for this key_group".

- state.rs: SessionState.bound_key_group (transient; set at Open, not persisted -
  the in-memory attempt it serves does not survive restart either).
- interactive.rs: resolve_wallet_session_id helper; Open/Aggregate split wallet vs
  per-signing state; verify_share.rs likewise.
- New cross-session regression test (persist under session A, sign under session B,
  same key_group -> valid BIP-340 signature, and the signing session holds NO copy
  of the DKG material). Updated two tests whose SessionNotFound expectation became
  DkgNotReady under the new semantics.

Approach vetted with Codex. 308 engine tests green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…session at Round2

Follow-up to the cross-session key_group resolution: InteractiveSessionOpen was
updated to read the wallet-level policy gates from the resolved wallet session, but
interactive_round2 - the moment this node's secret FROST share is actually released
- was NOT. Round2 fed enforce_interactive_signing_gates the emergency_rekey_event
and finalize_request_fingerprint from the PER-SIGNING session (request.session_id).
For a distributed-DKG wallet that session is a fresh SessionState created at Open
with no policy state, while emergency rekey / finalization are recorded on the
WALLET (DKG) session. So the Round2 kill-switch re-check - which exists precisely to
stop a rekey/finalization recorded AFTER Open - silently FAILED OPEN on the only
signing path distributed-DKG wallets have: a watchtower could trigger an emergency
rekey after Open and the share would still be released.

Fix: at Round2, resolve the wallet session by the signing session's bound_key_group
(as Open/Aggregate already do) and read emergency_rekey_event /
finalize_request_fingerprint / tx_result from it. Co-located sessions resolve to
themselves, so no behavior change there.

New regression test interactive_round2_rechecks_gates_at_share_release_across_sessions:
DKG under a wallet session, sign under a DISTINCT session, record the rekey on the
wallet session -> Round2 blocks with emergency_rekey_required and does NOT consume
the nonce; clearing it lets the attempt complete. (Without the fix Round2 would
fail open and the test's expect_err would fail.) 309 engine tests green; Go
cross-session e2e still signs.

Caught by adversarial review. The refuted duplicate-key_group scan nondeterminism
(P3) stays a documented non-issue: DKG yields a fresh key per run, so at most one
session ever carries a given key_group.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-rekey writer

CI fixes:
- rustfmt: reflow interactive.rs to satisfy `cargo fmt --check` (whitespace only;
  the cross-session changes were not fmt-clean).
- dependency audit: bump crossbeam-epoch 0.9.18 -> 0.9.20 for RUSTSEC-2026-0204
  (invalid pointer deref in fmt::Pointer). Dev-only transitive dep
  (criterion -> rayon -> crossbeam-deque); lockfile-only, no code/API impact.

Defense-in-depth (the writer-side counterpart to the Round2 gate fix): emergency
rekey is a WALLET-level kill switch, and interactive readers resolve it from the
wallet session by key_group. trigger_emergency_rekey keyed the event by the literal
request.session_id; if a caller passed a per-signing session id (a distinct
RoastSessionID bound to a wallet key) the event would land where no signing path
looks. Now it resolves the target to the WALLET session by key_group (a session that
already holds the DKG resolves to itself, so co-located callers are unchanged), so
writer and reader can never diverge. New test
trigger_emergency_rekey_on_signing_session_records_on_wallet_session. 310 engine
tests green.

Note: the "TLA model checks" CI failure is an unrelated infra issue - the pinned
tla2tools-v1.8.0.jar checksum drifted upstream (download hash != expected); no TLA
or script change in these PRs touches it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…session

InteractiveSessionOpen creates the per-signing session on first Open (for a distinct
RoastSessionID) via entry().or_insert(), but skipped ensure_session_insert_capacity
- unlike every other session-creating path (run_dkg, persist, refresh, build_tx).
So with a fresh RoastSessionID per message the registry could grow past
TBTC_SIGNER_MAX_SESSIONS, and Round2 could then persist an over-limit registry that
the reload path rejects (persisted_engine_state_rejects_session_registry_over_limit)
- stranding the node's state. The per-member interactive cap bounds live nonces, not
total sessions, so it did not cover this.

Call ensure_session_insert_capacity before the insert (a reopen of an existing
session is exempt, so co-located callers are unchanged). New test
interactive_open_cross_session_respects_the_session_cap. 311 engine tests green.

Caught by review (Codex).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…clone; refuse cross-wallet session rebinding

Secret-lifetime scrubbing (completeness sweep of the sign path). serde deserializes
FFI request hex into OWNED Strings, independent of the C buffer the Go caller scrubs;
several held secret material and were left to drop un-wiped:
- persist_distributed_dkg_key_package: zeroize request.key_package.data_hex (the
  secret share hex) after decode, and bind+zeroize the Copy SigningShare extracted for
  the verifying-share derivation check (frost SigningShare is Copy + DefaultIsZeroes,
  not ZeroizeOnDrop).
- generate_nonces_and_commitments / sign_share: zeroize the request key_package_hex
  (secret share) and nonces_hex (one-time nonces) on success AND error paths. Mirrors
  the decoded-buffer scrubbing these ops already do.

Session isolation. A per-signing session is keyed by RoastSessionID
(message/root/start-block), NOT key_group, so two wallets signing the same digest at
the same block on a node holding members of both can collide on one session id.
InteractiveSessionOpen rebound bound_key_group unconditionally; a live member of the
first wallet would then resolve the wrong wallet material at Round2/Aggregate. Now
Open refuses to rebind a session to a different key group while any member is
mid-signing under the current one (idle sessions and same-key-group co-signers
unaffected). Test interactive_open_refuses_to_rebind_a_live_session_to_a_different_key_group.

Caught by review (Codex). 312 engine tests green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t request hex on all paths

P1 (session key-group isolation). InteractiveSessionOpen bound/rebound a per-signing
session to request.key_group without fully checking the session's existing wallet.
Two gaps: (a) the rebind guard keyed on live interactive_signing entries, but that set
is empty in the consumed-but-unaggregated window after a member's Round2, so a
colliding Open could still rebind bound_key_group; (b) opening a session that already
holds a DKG result for wallet A with request.key_group = wallet B installed B while
dkg_result stayed A, and later resolution prefers dkg_result, so Round2/Aggregate/
verify_share would enforce A's policy and material while signing B's share - bypassing
B's rekey/finalization gates and mis-verifying honest B shares. Both roast-session ids
are message/root/block-derived, not key_group-derived, so two wallets can collide on
one id. Fix: a session belongs to exactly ONE key group for its lifetime - reject an
Open whose key_group differs from the session's established one (dkg_result key group
if co-located, else the bound one), regardless of live members. Keeps dkg_result and
bound_key_group mutually consistent so later resolution is always correct. Tests:
...refuses_to_rebind_a_live_session... and ...refuses_to_bind_through_another_wallets_dkg_session.

P2 (secret request-String scrubbing on all paths). serde deserializes FFI request hex
into owned Strings that hold secret material (signing share, one-time nonces). The
manual scrubs landed AFTER earlier fallible checks (persist: admission/quarantine;
sign_share: signing-package/nonces decode), so an early return dropped the secret
un-wiped. Move each secret field into a Zeroizing holder up front (mem::take) so it is
wiped on EVERY return path. Applies to persist_distributed_dkg_key_package,
generate_nonces_and_commitments, and sign_share. (Per the coarse-path usage map, the
latter two have no production caller today - the interactive path supersedes them -
but the leak was real; closed uniformly.)

Caught by review (Codex). 313 engine tests green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s a restart

bound_key_group was reset to None on reload, on the reasoning that the live attempt
it serves does not survive a restart anyway. But InteractiveAggregate/verify_share
run AFTER a member's Round2 frees the live entry, using the durable consumed markers
plus coordinator-held shares - and they resolve the wallet by key_group. For a
distributed-DKG wallet the signing session has no dkg_result, so bound_key_group is
the ONLY link back to the wallet DKG. A restart between Round2 (shares consumed,
markers written) and InteractiveAggregate left both None -> DkgNotReady -> the
collected shares are stranded and the attempt must be fully re-run.

Persist bound_key_group alongside the consumed/aggregate markers (it is public - a key
group id, not secret; serde(default) keeps old state loadable), and restore it on
reload. This also keeps the one-key-group-per-session guard effective across a restart.
Test persisted_session_state_round_trip_preserves_bound_key_group.

Caught by review (Codex). 314 engine tests green; cross-session e2e green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…on_copy)

CI's `cargo clippy --all-targets -- -D warnings` (Signer Rust checks) failed on lints
a newer clippy enforces:
- resolve_wallet_session_id used .map_or(false, |dkg| dkg.key_group == kg); switch to
  .is_some_and(...) (unnecessary_map_or).
- the corrupt-key-package test cloned a Copy SigningShare/VerifyingKey; dereference
  instead (clone_on_copy).

No behavior change. 314 engine tests green; clippy clean locally.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ble across sessions (#4136)

## What

Makes a real **distributed FROST DKG** produce usable signing material,
and makes that material signable through the interactive path — the
signer half of the distributed-DKG node wiring (#4135).

- **Persist op** `frost_tbtc_persist_distributed_dkg_key_package`:
stores this node's own `Part3` key package (keyed by its participant
identifier) + the group public key package into engine state, derives
the key group from the verifying key, and **accumulates** across a
node's local seats. Hardened with a provenance gate, admission policy
(canonical big-endian identifiers, participant-count match), quarantine
enforcement, key-package consistency (identifier / `min_signers` ==
threshold / verifying-key / verifying-share-from-signing-share
derivation), and idempotent-vs-conflict semantics per key group. ABI
bumped to **1.1** (additive symbol; the Go bridge fail-closes against an
older lib).
- **Wallet-keyed cross-session signing**: DKG material is a
**wallet-level asset keyed by key group**, not by session id.
Interactive ROAST signing runs under a fresh per-message session
distinct from the DKG session, so `InteractiveSessionOpen` / `Round2` /
`interactive_aggregate` / `verify_share` resolve the wallet material by
key group (via the signing session's `bound_key_group`) and create the
per-signing session on Open holding only per-signing state — **no secret
is copied in**. Without this, distributed-DKG wallets, signable *only*
via the interactive path, could never sign.
- **Wallet-level policy gates** (emergency rekey / finalization /
tx-binding) are read from the resolved wallet session at **both** Open
and the Round2 share-release moment, so a kill switch recorded after
Open still fires cross-session; `trigger_emergency_rekey` resolves the
wallet session too (writer/reader symmetry).
- **Robustness**: interactive Open honors the total-session cap like
every other session-creating path; the included-participants check
validates against the public key package (the full participant set),
correct for a distributed node that caches only its own secret share.

## Test

311 engine tests, including a cross-session persist→sign round trip
(persist under session A, sign under session B → valid BIP-340
signature, signing session holds no material copy), the wallet-session
kill-switch re-check across sessions, the writer redirect, the
session-cap enforcement on Open, and the full persist hardening
(quarantine, admission, key-package consistency, accumulate). Also
proven end-to-end from Go via #4135's multi-node real-transport e2e.
`cargo fmt` / dependency audit clean.

## Companion

The Go orchestrator, node wiring, FFI bridge, and multi-node
real-transport e2e are #4135. Draft until the end-to-end path lands
there.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants