fix(tbtc/signer): serialize initial state loading#4159
Merged
mswilkison merged 1 commit intoJul 15, 2026
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
from
July 14, 2026 16:31
6e924e4 to
21fc16c
Compare
mswilkison
force-pushed
the
fix/signer-state-load-serialization-2026-07-13
branch
from
July 14, 2026 16:31
54d425b to
d1e23ec
Compare
mswilkison
force-pushed
the
fix/signer-dkg-secret-zeroization-2026-07-13
branch
from
July 14, 2026 16:35
21fc16c to
5176e56
Compare
mswilkison
force-pushed
the
fix/signer-state-load-serialization-2026-07-13
branch
from
July 14, 2026 16:35
d1e23ec to
56dcf9d
Compare
mswilkison
force-pushed
the
fix/signer-dkg-secret-zeroization-2026-07-13
branch
from
July 14, 2026 21:36
5176e56 to
c189fdc
Compare
mswilkison
force-pushed
the
fix/signer-state-load-serialization-2026-07-13
branch
from
July 14, 2026 21:36
56dcf9d to
3c4aed6
Compare
mswilkison
force-pushed
the
fix/signer-dkg-secret-zeroization-2026-07-13
branch
from
July 15, 2026 00:51
c189fdc to
c717a5a
Compare
mswilkison
force-pushed
the
fix/signer-state-load-serialization-2026-07-13
branch
from
July 15, 2026 00:51
3c4aed6 to
136e85c
Compare
mswilkison
merged commit Jul 15, 2026
19a9fa8
into
fix/signer-dkg-secret-zeroization-2026-07-13
20 checks passed
mswilkison
added a commit
that referenced
this pull request
Jul 15, 2026
## Stack Rust signer review-fix stack 2 of 3. - Depends on: #4157 - Ultimate base: `extraction/frost-signer-mirror-2026-05-26` (#4005) - Follow-up: #4159 — startup load serialization Merge #4157 first, then retarget this PR to the extraction branch. ## What changed - Introduce a transparent `SecretHex` holder backed by `Zeroizing<String>` with redacted Debug output and zeroize-on-drop semantics. - Use it for DKG secret packages, confidential round-2 packages, and the long-lived native FROST key package signing share. - Preserve the existing JSON string wire shape and FFI ABI while avoiding independent unwiped Rust-owned secret string allocations. - Continue wiping decoded and serialized byte buffers and returned FFI buffers through their existing paths. ## Review finding addressed - P2: treat DKG package fields as secret-bearing types. 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 -- --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` - DKG serde, Debug-redaction, zeroize-on-drop, distributed-DKG persistence, and FFI round-trip tests - `cargo test --locked --manifest-path pkg/tbtc/signer/Cargo.toml` - `git diff --check` The TLA+ runner and `cargo-deny` are unavailable locally; CI remains authoritative for those gates.
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
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 3 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 and #4158 first, then retarget this PR down the stack.
What changed
STATE_FILE_LOCK, avoiding lock-order recursion when the loader resolves the active state path.state().Review finding addressed
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.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 --checkCI is authoritative for the TLA+ model and dependency-audit gates.