fix(tbtc/signer): zeroize DKG wire secrets#4158
Merged
mswilkison merged 3 commits intoJul 15, 2026
Merged
Conversation
|
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 |
This was referenced Jul 14, 2026
mswilkison
force-pushed
the
fix/signer-dkg-secret-zeroization-2026-07-13
branch
3 times, most recently
from
5176e56 to
c189fdc
Compare
mswilkison
force-pushed
the
fix/signer-dkg-secret-zeroization-2026-07-13
branch
from
c189fdc to
c717a5a
Compare
## Stack Rust signer review-fix stack 3 of 3. - Depends on: #4158, which depends on #4157 - Ultimate base: `extraction/frost-signer-mirror-2026-05-26` (#4005) Merge #4157 and #4158 first, then retarget this PR down the stack. ## What changed - Serialize the complete first fallible state load and migration behind a dedicated process-local initialization mutex. - Double-check the OnceLock after acquiring the initializer mutex so concurrent first callers invoke the loader and any in-place migration only once. - Keep the initializer separate from `STATE_FILE_LOCK`, avoiding lock-order recursion when the loader resolves the active state path. - Recover the initializer token after mutex poisoning and leave the OnceLock unset after load errors so later calls can retry. - Add deterministic concurrency and failure-retry regressions against the same helper used by production `state()`. ## Review finding addressed - P2: serialize first-time state loading and migration. ## Verification - `cargo fmt --manifest-path pkg/tbtc/signer/Cargo.toml -- --check` - `cargo check --locked --manifest-path pkg/tbtc/signer/Cargo.toml --all-targets` - `cargo clippy --locked --manifest-path pkg/tbtc/signer/Cargo.toml --all-targets -- -D warnings` - `cargo test --locked --manifest-path pkg/tbtc/signer/Cargo.toml` (263 passed, 1 ignored; the ignored multiprocess helper passes in its child process; 28 binary and 1 integration test passed) - `cargo test --locked --manifest-path pkg/tbtc/signer/Cargo.toml formal_verification_` - `pkg/tbtc/signer/scripts/run_phase5_chaos_suite.sh` (all 6 scenarios passed) - `git diff --check` CI is authoritative for the TLA+ model and dependency-audit gates.
mswilkison
merged commit bcdd852
into
fix/signer-session-registry-2026-07-13
20 checks passed
mswilkison
added a commit
that referenced
this pull request
Jul 15, 2026
## Stack Rust signer review-fix stack 1 of 3. - Base: `extraction/frost-signer-mirror-2026-05-26` (#4005) - Follow-up: #4158 — DKG secret zeroization - Final: #4159 — startup load serialization ## What changed - Require Aggregate attempt IDs to be canonical SHA-256 hex, and bind fixed-size Aggregate authorization to the validated attempt, signing package, and taproot root. - Allow the elected coordinator to aggregate a validated attempt without requiring that coordinator to be part of the frozen signing subset. - Keep consumed markers readable by the immediately previous schema-1 binary, and pin a retired session for the full unlocked Aggregate operation. - Reclaim completed per-message sessions through bounded retirement while keeping every persisted schema-1 snapshot within the legacy total `TBTC_SIGNER_MAX_SESSIONS` limit. - Reserve new session slots by transactionally evicting the oldest eligible retired tombstone; protect pending and in-flight Aggregate sessions, and restore evicted replay state after pre-replacement Build/DKG/Open failures. - Persist the first per-message Open binding before success, and make Abort plus full or partial TTL expiry crash-durable. Post-replacement uncertainty remains fail-closed behind a repair marker. - Stage retirement before registering a post-rename Aggregate repair, so either the exact retry or an unrelated full-state writer durably covers completion and retirement before clearing pending protection. - Compact and immediately rewrite oversized intermediate schema-1 state during load so emergency rollback remains available. ## Review findings addressed - P1: reclaim per-message sessions after completion. - P2: validate Aggregate attempt IDs before persistence. - P1: allow non-signing coordinators to aggregate. - P1: preserve consumed markers across binary rollback. - P2: pin sessions while aggregation is in flight. - P1: preserve the previous total-session bound on disk. - P2: make Abort-driven retirement crash-durable. - P2: retire the session after an Aggregate repair. Retired per-message tombstones use only the unoccupied portion of the shared legacy-compatible total session budget. Live wallet/DKG sessions are never classified as retireable. ## Verification - `cargo fmt --manifest-path pkg/tbtc/signer/Cargo.toml -- --check` - `cargo check --locked --manifest-path pkg/tbtc/signer/Cargo.toml --all-targets` - `cargo clippy --locked --manifest-path pkg/tbtc/signer/Cargo.toml --all-targets -- -D warnings` - `cargo test --locked --manifest-path pkg/tbtc/signer/Cargo.toml` (259 passed, 1 ignored; the ignored multiprocess helper passes in its child process; 28 binary and 1 integration test passed) - `cargo test --locked --manifest-path pkg/tbtc/signer/Cargo.toml formal_verification_` - `pkg/tbtc/signer/scripts/run_phase5_chaos_suite.sh` (all 6 scenarios passed) - `git diff --check` CI is authoritative for the TLA+ model and dependency-audit gates.
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.
Stack
Rust signer review-fix stack 2 of 3.
extraction/frost-signer-mirror-2026-05-26(feat(tbtc/signer): Rust FROST/ROAST signer (distributed DKG, interactive-only signing, FFI ABI 2.0) #4005)Merge #4157 first, then retarget this PR to the extraction branch.
What changed
SecretHexholder backed byZeroizing<String>with redacted Debug output and zeroize-on-drop semantics.Review finding addressed
Caller-owned FFI request memory remains the caller responsibility; Rust-owned request/result allocations and returned buffers now have explicit wipe paths.
Verification
cargo fmt --manifest-path pkg/tbtc/signer/Cargo.toml -- --checkcargo check --locked --manifest-path pkg/tbtc/signer/Cargo.toml --all-targetscargo clippy --locked --manifest-path pkg/tbtc/signer/Cargo.toml --all-targets -- -D warningscargo test --locked --manifest-path pkg/tbtc/signer/Cargo.tomlgit diff --checkThe TLA+ runner and
cargo-denyare unavailable locally; CI remains authoritative for those gates.