Skip to content

fix(tbtc/signer): bound interactive session persistence#4157

Merged
mswilkison merged 10 commits into
extraction/frost-signer-mirror-2026-05-26from
fix/signer-session-registry-2026-07-13
Jul 15, 2026
Merged

fix(tbtc/signer): bound interactive session persistence#4157
mswilkison merged 10 commits into
extraction/frost-signer-mirror-2026-05-26from
fix/signer-session-registry-2026-07-13

Conversation

@mswilkison

@mswilkison mswilkison commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Stack

Rust signer review-fix stack 1 of 3.

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.

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b653ea2a-49b3-460b-8cfd-e270a7db81e8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/signer-session-registry-2026-07-13

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.

## 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.
## 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
mswilkison merged commit 1ef344c into extraction/frost-signer-mirror-2026-05-26 Jul 15, 2026
19 checks passed
@mswilkison
mswilkison deleted the fix/signer-session-registry-2026-07-13 branch July 15, 2026 02:27
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.

1 participant