fix(tbtc/signer): sign-store followup - local compaction, hash-chain hardening, review fixes - #4271
Open
piotr-roslaniec wants to merge 269 commits into
Open
fix(tbtc/signer): sign-store followup - local compaction, hash-chain hardening, review fixes#4271piotr-roslaniec wants to merge 269 commits into
piotr-roslaniec wants to merge 269 commits into
Conversation
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.
## Summary - add stateless Rust C ABI endpoints for interactive FROST DKG part1/part2/part3 - add stateless signing endpoints for nonce generation, signing-package construction, share signing, and aggregation - normalize DKG outputs to the even-Y BIP340 convention and export the group key as x-only material - cover the full Rust FFI path with a 3-member DKG, threshold signing, aggregation, and BIP340 verification test ## Validation - cargo test --lib - cargo build
The order-independence test asserted only that two input orderings agree, not what they agree on. The Go side now pins the concrete result for the same (members, seed, attempt) tuple, so pin Some(4) here as well to keep the cross-language vector sets symmetric. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nce test (#4027) Stacked on #4005 (base: `extraction/frost-signer-mirror-2026-05-26`). Companion to #4026 on the Go branch. ## What `select_coordinator_is_input_order_independent` in `pkg/tbtc/signer/src/go_math_rand.rs` asserted only that two input orderings of the member set agree — not what they agree on. This pins the concrete result (`Some(4)` for `(members=[1..6], seed=333, attempt=4)`), matching the value now pinned on the Go side in `TestSelectCoordinator_CrossLanguagePinnedVectors` (#4026). ## Why Coordinator selection must agree byte-for-byte between Go's `SelectCoordinator` and this Rust port of Go's `math/rand` shuffle, or honest nodes elect different coordinators and the ROAST liveness path fractures. With the vector sets symmetric across both test suites, either implementation drifting fails its own unit tests instead of surfacing in mixed-version soak testing. Verified locally: `cargo test --lib go_math_rand` passes, `cargo fmt --check` clean.
…transitional signing out of production The transitional StartSignRound/FinalizeSignRound flow derives round-1 nonces deterministically. Its nonce-reuse safety previously rested on two indirect properties: (1) every transcript-affecting input staying bound into the seed via the round_id derivation schema, and (2) consumed-round registry integrity on durable state, which rollback/restore/replication can silently violate. Remove the failure class instead of guarding it: - Introduce RoundNonceBinding with a documented invariant: every value entering the FROST binding factor, challenge, Lagrange set, or key material selection feeds the nonce seed directly. The seed now also binds the group verifying key, the Taproot tweak root, and the canonical signing-participant set (domain bumped to round-nonce-v2). Nonce safety no longer depends on round_id schema evolution or on registry integrity: any transcript variation yields a fresh nonce, so state rollback can only repeat identical transcripts (yielding the identical signature), never the same nonce under a new challenge. - Gate the deterministic-nonce entry points out of the production profile. Dealer DKG was already production-blocked, but persisted state created under a development profile could be carried into a production-profile process and signed with. StartSignRound and FinalizeSignRound now reject with transitional_deterministic_signing_disabled_in_production; production signing is the interactive FROST path with OS-random nonces only. A per-boot RAM-only salt was considered and rejected: the transitional flow has every member independently derive all participants' commitments with no round-1 exchange, so cross-machine determinism is load-bearing; a per-machine salt would break every multi-member bootstrap session. Mirror note: port back to the tBTC monorepo signer alongside the next extraction sync. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…+ cross-language vectors The engine's attempt-context validation derived the coordinator-shuffle seed from the first 8 bytes of the raw message digest with the 1-based wire attempt number -- the legacy signingAttemptSeed convention -- while the Go RFC-21 layer derives fold(SHA256(KeyGroup || SessionID || MessageDigest)) with 0-based attempt numbers (divergence flagged in #4026). At Phase-7 wiring every Go-derived attempt context would have failed strict-mode validation. Adopt the RFC-21 Annex A derivation as normative: - roast_attempt_shuffle_seed(key_group, session_id, message_digest) replaces roast_attempt_seed_from_message_digest_hex; the key-group handle's UTF-8 bytes feed the hash as an opaque string, exactly matching keep-core's attempt.DeriveAttemptSeed + foldAttemptSeed. - validate_attempt_context takes the session's key group and composes the shuffle source with the 0-based attempt number (wire encoding stays 1-based; the engine subtracts one), so both layers select the same coordinator for the same logical attempt. - testdata/coordinator_seed_vectors.json is a byte-identical copy of the canonical vector file generated from the Go implementation; coordinator_seed_derivation_matches_cross_language_vectors pins the seed, the coordinator, the wire mapping, and full strict-mode validate_attempt_context acceptance for all ten vectors (including negative folded seeds and the n=100 production set). - docs/roast-coordinator-seed-derivation.md mirrors the normative annex for signer-side readers. The coordinator-mismatch test now derives the provably-wrong coordinator instead of hardcoding one, so it stays valid under any seed derivation. Pairs with the Go-side PR on feat/frost-schnorr-migration-scaffold (RFC-21 Annex A + canonical vector file + Go conformance test). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…pin to =3.0.0 final frost-secp256k1-tr 3.0.0 (with frost-core and frost-rerandomized 3.0.0) is published and unyanked; the engine was pinned to the =3.0.0-rc.0 release candidate. Pin the final release instead -- release candidates receive no post-release fixes and are the wrong long-term anchor for the curve/ciphersuite layer under custody code. Exact-pin discipline is retained. Full suite passes unchanged against the final (244 tests; clippy clean), confirming no API or behavior drift from rc.0. Audit-trail follow-up for the rollout gates: record which ZF/external audit reports cover frost-core 3.x and the secp256k1-tr ciphersuite specifically, alongside the existing audit-lineage notes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ffle corpus Consumes the byte-identical copy of the differential corpus generated from keep-core's Go implementation (pkg/frost/roast/testdata/coordinator_shuffle_corpus.json on the RFC-21 branch): 216 integer-boundary cases (0/+-1/i64 MIN/MAX seeds, wrapping seed+attempt composition up to u32::MAX, unsorted and reversed member inputs) plus 384 generated sweeps over set sizes 1..255 with full-range seeds. select_coordinator_matches_cross_language_differential_corpus replays every case through the go_math_rand port, so any drift in source seeding, Fisher-Yates order, int31n bounds, sign handling, wrapping, or internal sorting fails this suite directly instead of fracturing coordinator agreement in a mixed deployment. Pairs with the Go-side corpus PR on feat/frost-schnorr-migration-scaffold. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ssage, not the transcript digest Review finding on the seed unification: validate_attempt_context fed roast_attempt_shuffle_seed with the engine's internal transcript digest (hash_hex(message_bytes) = SHA256(message)), but the Go RFC-21 layer derives the seed from messageDigestFromBigInt(request.Message) -- the raw 32-byte signing digest itself. Valid Phase-7 Go attempt contexts would have selected a different coordinator and been rejected in strict mode; the conformance test missed it because its end-to-end leg called validate_attempt_context directly, bypassing start_sign_round's digest computation. - rfc21_message_digest mirrors messageDigestFromBigInt exactly: leading zero bytes insignificant (big.Int round-trip), big-endian left-pad to 32 bytes, more than 32 significant bytes rejected. - validate_attempt_context now takes the raw message bytes and seeds the shuffle from the padded message; the SHA256 transcript digest keeps feeding only the attempt_id check. StartSignRound passes its request message; FinalizeSignRound passes the cached sign_message_bytes stored by the same StartSignRound. - start_sign_round_accepts_go_derived_attempt_context_in_strict_mode reimplements the Go-side derivation inline (DeriveAttemptSeed + foldAttemptSeed + 0-based SelectCoordinator) and proves acceptance through the real strict-mode StartSignRound call path -- the test shape that would have caught the original finding. - Cross-language conformance test now treats the vector digest as the raw message (the exact production relationship) and binds attempt_id to the transcript digest, exercising the two-digest split. - Attempt-context proptests constrained to 32-byte messages, matching the RFC-21 bound the Go bridge enforces. Annex A on the Go branch is amended in lockstep to state the input unambiguously. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The widened round-nonce-v2 binding mixes encodings: the participants set serializes big-endian while participant_identifier keeps the v1 little-endian encoding. Harmless (fixed-width parts, length-framed by deterministic_seed) but part of the derived value -- note that any encoding change requires a new seed domain, never an in-place edit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Closes a completeness gap in the v2 RoundNonceBinding found in third-pass review (P1). The seed bound only the *group* verifying key, not the individual verifying shares. In the transitional flow every member re-derives ALL participants' round-1 commitments from the held key packages, so each other participant's verifying share enters the commitment list -> this member's binding factor and challenge. Two key packages can share a group verifying key while differing in a non-target share (any threshold t>=3 admits two polynomials with identical f(0) and target share but a different non-target share). Consequence under the old binding: a rolled-back/restored/cloned state (exactly #4028's threat model) could present an identical nonce seed under a *different* challenge -> the same member signs two different challenges with one deterministic nonce -> share extraction. The in-process run_dkg SessionConflict guard does not cover this, by design: nonce safety must not depend on registry integrity, since durable state can be rolled back or replicated. The production hard-gate still blocks this transitional flow in production, so the exposure is confined to the dealer-DKG dev/staging path; the interactive production path draws from OS randomness and is unaffected. Fix: bind the full serialized PublicKeyPackage (group key AND every verifying share); domain round-nonce-v2 -> round-nonce-v3. Regression: deterministic_round_nonce_and_commitment_binds_full_transcript now includes a variant with the baseline group key but a non-target verifying share swapped; it produces an identical seed (and asserts an identical group key) under the old binding, a different commitment under the new one. Full signer suite 246 pass; clippy/rustfmt clean. Mirror note: v3 domain + the widened binding port back to the tBTC monorepo signer with the next extraction sync, alongside the rest of #4028. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review follow-up (F2). Sync the byte-identical 648-case corpus (adds the +/-MaxInt32 source-seed normalization collision from the Go side) and document the two go_math_rand port branches the differential corpus cannot reach -- int63n (dead for any u16 member set) and the int31n_fast rejection loop (fires with probability ~set_size/2^31 per draw) -- as accepted faithful 1:1 ports of Go's math/rand covered by Go's own stdlib tests. Full signer suite passes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…inal (#4033) Stacked on #4005 (base: `extraction/frost-signer-mirror-2026-05-26`). Implements the dependency-pin item from the review feedback ("get off the `=3.0.0-rc.0` pin"). `frost-secp256k1-tr 3.0.0` final (plus `frost-core`/`frost-rerandomized` 3.0.0) is published and unyanked on crates.io; the engine was anchored to the release candidate, which receives no post-release fixes. This moves the exact pin to the final release — same exact-pin discipline, correct anchor. Verified: full signer suite passes unchanged against the final (244 tests, clippy clean), confirming no rc.0→final API or behavior drift. Remaining half of the review item for the rollout gates: record which ZF/external audit reports cover `frost-core` 3.x and the `secp256k1-tr` ciphersuite specifically (the audited lineage claim in the readiness docs should cite the exact report and version range). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
…ffle corpus (#4035) Stacked on #4005 (base: `extraction/frost-signer-mirror-2026-05-26`). Rust half of the corpus-based differential parity item from the review; pairs with the Go-side PR #4034. Adds `testdata/coordinator_shuffle_corpus.json` — a byte-identical copy of the canonical 600-case corpus generated from keep-core's Go `SelectCoordinator` — and `select_coordinator_matches_cross_language_differential_corpus`, which replays every case through the `go_math_rand` port: 216 integer-boundary cases (seeds 0/±1/`i64::MIN`/`i64::MAX`/the #4026 pin seed; wrapping `seed + attempt` composition up to `u32::MAX`; unsorted and reversed member inputs pinning the internal sort) plus 384 generated sweeps over set sizes 1..255 with full-range seeds. All 600 cases replay identically today — direct evidence the `math/rand` port is bit-exact across the boundary regions where ports diverge first. Any future drift in source seeding, Fisher-Yates order, `int31n` bounds, sign handling, wrapping, or sorting fails this suite on the drifting side. Full signer suite passes (245 tests); clippy/rustfmt clean. Mirror note: port back to the tBTC monorepo signer with the next extraction sync. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
…+ cross-language conformance vectors (#4031) Stacked on #4005 (base: `extraction/frost-signer-mirror-2026-05-26`). Implements item 3 of the review feedback (duplicated, divergent protocol constants) — Rust half; pairs with the Go-side PR #4030 stacked on #3866. ## Problem Flagged in #4026: the engine validated attempt contexts using `int64_be(MessageDigest[0..8])` with the 1-based wire attempt number (the legacy `signingAttemptSeed` convention), while the Go RFC-21 layer derives `fold(SHA256(KeyGroup ‖ SessionID ‖ MessageDigest))` with 0-based attempt numbers. At Phase-7 wiring, every Go-derived attempt context would fail the engine's strict-mode `validate_attempt_context` — a deterministic, network-wide liveness failure invisible to either side's property tests. ## What changed - **`roast_attempt_shuffle_seed(key_group, session_id, message_digest_hex)`** implements the normative RFC-21 Annex A derivation (see #4030). The key-group handle — this engine's hex-encoded serialized group verifying key — feeds the hash as an opaque UTF-8 string, exactly matching keep-core's `attempt.DeriveAttemptSeed` + `foldAttemptSeed` composition, including the strict 32-byte digest requirement. - **`validate_attempt_context` now takes the session's key group** (threaded from `dkg.key_group` at StartSignRound and the session's `DkgResult` at FinalizeSignRound) and composes the shuffle source with the **0-based** RFC-21 attempt number. The FFI wire encoding stays 1-based (`attempt_number >= 1` still enforced; `wire = AttemptNumber + 1`); the engine subtracts one before composition, per the annex. - **`testdata/coordinator_seed_vectors.json`** — byte-identical copy of the canonical file generated from the Go implementation. `coordinator_seed_derivation_matches_cross_language_vectors` pins, for all ten vectors: the folded seed (including negative values, so an unsigned port cannot pass), the selected coordinator (including the n=100 production-shape set), the 0-/1-based wire mapping, and end-to-end strict-mode `validate_attempt_context` acceptance of a context built from the wire encoding. Either language drifting now fails its own unit suite. - **`docs/roast-coordinator-seed-derivation.md`** mirrors the normative annex for signer-side readers, with the regen/copy procedure. - The coordinator-mismatch test derives the provably-wrong coordinator instead of hardcoding member 1 (which, under the new seed, happened to become the correct selection — exactly the class of silent assumption these vectors exist to catch). ## Notes - Mixed-version note: engines on the old derivation reject contexts produced under the new one (and vice versa) — strict-mode attempt contexts are not yet produced by the Go layer in any deployment, so this is pre-wiring cleanup with no live-fleet impact. - The attempt-context vector suite (`roast-attempt-context-v1.json`) is unaffected: it pins fingerprint/attempt-id domains with the coordinator as an *input*. - Port back to the tBTC monorepo signer alongside the next extraction sync. ## Tests Full suite: 245 passed, 0 failed; clippy and rustfmt clean. New conformance test exercises all ten cross-language vectors. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
…transitional signing out of production (#4028) Stacked on #4005 (base: `extraction/frost-signer-mirror-2026-05-26`). Implements item 1 of the review feedback on the FROST/ROAST stack (nonce rollback safety). ## Problem The transitional `StartSignRound`/`FinalizeSignRound` flow derives round-1 nonces deterministically from `H("round-nonce", signing_share, session_id, round_id, message, participant_id)`. Nonce-reuse safety therefore rested on two indirect properties: 1. **`round_id` schema integrity** — the participants set, Taproot tweak, and attempt context were bound only through `derive_round_id`. Any future change to that derivation (or an encoding collision in it) could let two different FROST transcripts share a nonce seed, which is the share-extraction condition. 2. **Consumed-round registry integrity** — the on-disk registries were the replay boundary. A VM snapshot restore, backup restore, or state replicated to a second host silently re-arms consumed rounds. ## What changed **1. Total direct binding (`RoundNonceBinding`, domain `round-nonce-v2`).** The nonce seed now directly binds every value that enters the FROST binding factor, challenge, Lagrange interpolation set, or key-material selection: group verifying key, Taproot merkle root, canonical signing-participant set — in addition to the existing signing share, session, round, message, and participant id. The struct carries a documented invariant so the next transcript input added to this flow gets added to the seed in the same change. Consequence: a rolled-back or cloned state can only ever repeat an *identical* transcript (producing the identical signature — no new information), never the same nonce under a different challenge. Nonce safety no longer depends on `round_id` schema or registry integrity at all. **2. Production hard-gate on the deterministic-nonce entry points.** Dealer DKG was already blocked in production, but that gate only fires at session *creation*: persisted state created under a development profile could be carried into a production-profile process and signed with. `StartSignRound`/`FinalizeSignRound` now reject in the production profile with reason `transitional_deterministic_signing_disabled_in_production`, making the OS-random interactive FROST path the only production signing path regardless of how on-disk state was created. ## Why not the per-boot RAM-only salt suggested in the review Exploration showed the transitional flow has every member independently derive **all** participants' commitments with zero round-1 exchange (members exchange only signature shares), so cross-machine determinism is load-bearing: a per-machine salt would break every multi-member bootstrap session. The two changes above implement the same goal — *rollback costs liveness, never keys* — within the flow's actual architecture: (1) removes the nonce-reuse class structurally, (2) removes production exposure structurally. The stateless interactive path (`GenerateNoncesAndCommitments`) already draws from OS randomness and holds nonces only in caller RAM. ## Tests - `deterministic_round_nonce_and_commitment_binds_full_transcript`: identical binding re-derives identical commitments; each of 7 binding inputs (message, tweak root, participants set, group key, session, round, participant) independently changes the commitment. - `start_sign_round_rejects_transitional_signing_in_production_profile` / `finalize_sign_round_rejects_transitional_signing_in_production_profile`: the state-smuggling scenario — dev-created dealer session, production-profile process — rejects at both entry points, even with the strict-mode env flag explicitly disabled. - `production_profile_forces_roast_strict_mode_without_env_flag` repurposed to assert the strict-mode forcing at the helper level (the FFI-level path it previously exercised is now unreachable in production by design). - Full suite: 246 passed, 0 failed; clippy and rustfmt clean. ## Notes for the mirror - The seed domain bump (`round-nonce` → `round-nonce-v2`) changes derived commitments for identical inputs; a mixed-version fleet cannot co-sign transitional rounds mid-rollout. Dev/staging-only flow, so the cost is a failed attempt until the fleet converges. - Port back to the tBTC monorepo signer alongside the next extraction sync.
…submodules Post-merge follow-up #2 from the June 2026 review stack (#4028-#4035): engine.rs absorbed four merges plus the round-nonce-v3 fix and every new PR was contending for the same 18,248-line file. Pure code move - no behavior change: - production code -> 16 thematic submodules under src/engine/ (state, persistence, config, policy, provenance, telemetry, lifecycle, audit, codec, frost_ops, nonce, roast, dkg, signing, transaction, testsupport); formerly-private items widened to pub(crate), and `mod engine` itself stays private in lib.rs, so the crate-external surface is identical - `mod tests` moved verbatim to engine/tests.rs: the module path engine::tests::* is unchanged, so run_phase5_chaos_suite.sh --exact filters and phase-doc test references stay valid - only semantic edit: the coordinator-seed-vectors include_str! path gains one ../ (the file now sits one directory deeper) Verified: cargo fmt --check; clippy --all-targets -D warnings; full suite 223 passed + 1 ignored / 24 / 1 - counts identical to the pre-split HEAD; formal_verification_ filter passes; all five chaos-suite --exact paths pass; testdata untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- point roast-coordinator-seed-derivation.md and formal/models/README.md at the functions' new submodule homes (roast.rs, signing.rs, persistence.rs); the formal README lines also dropped their stale monorepo tools/tbtc-signer/ path prefix - drop the per-file "Split from the former single-file engine.rs" provenance comments (mod.rs and git history record the split); keep the one-line module descriptions - tests.rs header now states the constraint instead of provenance: the file stays a single module because the chaos suite pins engine::tests::<name> paths with cargo test -- --exact Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…submodules (#4036) Post-merge follow-up #2 from the June 2026 review stack (#4028–#4035). `engine.rs` was deferred-split to avoid conflicting with the open stack; with the stack merged it had grown to 18,248 lines (absorbing four merges plus the round-nonce-v3 fix), and every new PR contends for the same file. This lands the split before anything new piles onto the monolith. ## What this is A **pure code move** — no behavior change, no API change, no test-path change. | Module | Lines | Contents | |---|---|---| | `state` | 434 | in-memory engine/session state, state-file lock, registry capacity guards | | `persistence` | 1,421 | encrypted state envelope, key providers/commands, corruption recovery, persisted↔live conversions | | `config` | 392 | the `TBTC_SIGNER_*` env surface: const names, defaults, parsers, profile detection | | `policy` | 633 | admission, signing-policy firewall, rate limiting, auto-quarantine config | | `provenance` | 353 | runtime provenance attestation gate | | `telemetry` | 313 | hardening latency trackers + metrics | | `lifecycle` | 468 | canary rollout, refresh cadence/shares, emergency rekey, quarantine status | | `audit` | 376 | transcript audit, blame-proof verification, differential fuzzing | | `codec` | 430 | hex/struct codecs, Go↔frost identifier conversions | | `frost_ops` | 303 | stateless `dkg_part1..3`, nonces, signing package, share, aggregate | | `nonce` | 99 | **`RoundNonceBinding` + deterministic round-nonce derivation (round-nonce-v3), isolated for audit** | | `roast` | 1,003 | RFC-21 attempt machinery: request fingerprints, round/attempt ids, attempt-context + transition-evidence validation | | `dkg` | 257 | `run_dkg` flow + transitional-dealer production gates | | `signing` | 970 | `start_sign_round` / `finalize_sign_round` flows, bootstrap synthetic contributions | | `transaction` | 227 | taproot tx building | | `testsupport` | 88 | cfg(test) cross-module helpers (`lock_test_state`, `reset_for_tests`, …) | | `tests` | 10,558 | the former inline `mod tests`, moved **verbatim** | ## Design decisions - **`engine::tests::*` paths are preserved.** `mod tests` moved as a single child module (`engine/tests.rs`), so `scripts/run_phase5_chaos_suite.sh`'s five `cargo test … -- --exact` filters and every `engine::tests::<name>` reference in the phase docs remain valid. Splitting tests further would force rewriting those contracts — left as an explicit team decision. - **Visibility:** formerly-private items are now `pub(crate)`; each submodule opens with `use super::*;` against glob re-exports in `mod.rs`. Since `lib.rs` keeps `mod engine;` **private**, the crate-external surface is byte-identical. Per-module visibility tightening can happen incrementally later. - **`config.rs` deliberately concentrates the env surface** — it pre-stages follow-up #3 (move `TBTC_SIGNER_*` env vars into an init-time FFI config struct) as a mostly-one-file change. - **Only semantic edit in the whole diff:** the `include_str!("../testdata/coordinator_seed_vectors.json")` in the tests gains one `../` because the file now sits one directory deeper. ## Verification - `cargo fmt --check` ✅, `cargo clippy --all-targets -- -D warnings` ✅ - Full suite: **223 passed + 1 ignored / 24 / 1 — counts identical to the pre-split HEAD** (verified by stashing the split and re-running on d47f009) - `cargo test formal_verification_` ✅ (5/5); all five chaos-suite `--exact` paths ✅ - `testdata/` untouched — seed vectors and shuffle corpus remain byte-identical - Review aid: `git diff d47f009 --color-moved=zebra --color-moved-ws=ignore-all-space` renders nearly the entire diff as moved lines; `git blame -C -C` follows history across the split. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Post-merge follow-up #3 from the June 2026 review stack: shrink the ops/audit surface by letting the host install the signer's operational configuration once at startup (frost_tbtc_init_signer_config) instead of exporting ~40 TBTC_SIGNER_* environment variables. Design (parity by construction): - every operational env read now goes through one chokepoint, engine::signer_env_var; with no config installed it falls through to the process environment, so existing env-driven behavior (and the entire pre-existing test suite) is unchanged - an installed config wins wholesale: the environment is no longer consulted for covered knobs, and an unset field means the built-in default - no per-knob source mixing - typed InitSignerConfigRequest (field = lowercased env suffix) converts to the same canonical strings the existing parsers consume, so every clamp/warn/reject path runs unchanged on identical inputs - deny_unknown_fields: a typo'd knob fails the init instead of silently running on defaults; enforcement-gated policy combinations (admission, firewall, auto-quarantine) are validated at install with rollback - re-init is idempotent for an identical request, rejected on conflict - secrets never ride the config FFI: TBTC_SIGNER_STATE_ENCRYPTION_KEY_HEX stays on the dedicated env/command key-provider channel (deliberate std::env::var exception, commented) - deletes lib.rs's duplicated profile/truthy parsing in favor of the engine's single implementation Verified: fmt --check; clippy --all-targets -D warnings; full suite 235 passed + 1 ignored / 24 / 1 (11 new tests incl. FFI round-trip); --features bench-restart-hook builds; chaos suite and formal_verification_ filter pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Addresses the converged Codex/Gemini review finding plus two findings from my own review of #4037: - candidate configs are now validated through a thread-local resolver override visible only to the validating thread, and published to the global slot only after validation succeeds. Previously the candidate was installed first and rolled back on failure, so a concurrent identical init could report idempotent success while the twin rolled the slot back to None, and concurrent readers could briefly act on a config that never legally installed. Both impossible now: failed init has no observable side effects, and idempotent success is only ever reported against a validated, installed config. - init now validates state_file_path(): a production config (explicit, or by profile-omission default) without state_path fails at init instead of installing and then failing at first state access with an env-var-oriented message; the state-path error now also names the state_path config field. - new end-to-end test: installed config's state_path is honored by run_dkg persistence after a process (re)start, and the existing in-process state-path-switch refusal is pinned as the contract for installing a config after state has been touched. - README: do not inline key material into state_key_command (the command string rides the config FFI); documented the no-side-effects init guarantee and the production state_path requirement. Suite: 237 passed + 1 ignored / 24 / 1; clippy -D warnings, fmt, chaos suite, bench-restart-hook feature build all green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Addresses Codex's re-review P2: the init validated state_path but not the adjacent key-provider knobs, so a production config that omitted state_key_provider (wholesale-defaulting to the env provider, which production forbids) - or that selected the command provider without a command - installed successfully and then failed at the first state access. - extract the structural head of state_encryption_key_material into resolve_state_key_provider_plan (provider selection, the production env-provider prohibition, command-spec presence; error strings unchanged) and have both the runtime key path and init validation consume it - one source of truth, no drift - init validation rejects: production defaulting to the env provider, command provider without state_key_command, unknown provider values; all WITHOUT reading the secret or executing the key command (pinned by a test whose key command points at a nonexistent binary) - README documents that production configs must carry the command key-provider pair Suite: 241 passed + 1 ignored / 24 / 1 (4 new tests); clippy -D warnings, fmt, chaos suite, bench-restart-hook build all green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Addresses Codex's third-round P2 (the family member my own review had deferred): production forces the provenance gate, so a production config without a complete, verifiable attestation set installed successfully and then failed every protected operation. - validate_candidate_config now runs enforce_provenance_gate(): self- gating (no-op when unenforced, so dev configs are unaffected), reads only candidate values plus local crypto - no secrets, no command execution, no network. Full verification at init (status, payload signature against trust root, runtime-version minimum, TTL); runtime calls still re-check, so an init-time pass does not exempt TTL aging. - production-config tests now carry a complete signed attestation (reusing the existing build_signed_provenance_attestation fixture); new tests pin: production-without-attestation rejected at init, enforced-gate-with-unparseable-trust-root rejected, and a complete production config (state path + command key provider + valid attestation + min version) installs. - README documents the production attestation requirement and the TTL caveat. Suite: 244 passed + 1 ignored / 24 / 1 (3 new tests); clippy -D warnings, fmt, chaos suite all green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ig (#4037) Post-merge follow-up **#3** from the June 2026 review stack (#4028–#4035; #4036 landed the engine split that stages this change): move the `TBTC_SIGNER_*` env-var surface into an init-time FFI config struct, shrinking the ops/audit surface from ~40 scattered `std::env::var` reads to one explicit, validated installation at startup. ## What this adds New FFI entry `frost_tbtc_init_signer_config(request_ptr, request_len)` taking a typed JSON `InitSignerConfigRequest` (40 optional fields; field name = lowercased `TBTC_SIGNER_*` suffix). The host installs it once at startup. ## Semantics - **Wholesale source of truth.** Once installed, the environment is *not consulted* for any covered knob; an unset field means the built-in default. No per-knob mixing of config and env — split-brain configs can't exist. - **Fail-closed init.** `deny_unknown_fields` rejects typo'd knobs; enforcement-gated policy combinations (admission, signing-policy firewall, auto-quarantine) are validated at install by running the same loaders the runtime gates use, with rollback on rejection — a misconfigured signer fails at startup, not at first signing. - **Idempotent re-init** for an identical request (fingerprint match); conflicting re-init rejected. - **Secrets never ride the config FFI.** `TBTC_SIGNER_STATE_ENCRYPTION_KEY_HEX` is read exclusively from the dedicated env/command key-provider channel even when a config is installed (the one deliberate `std::env::var` left outside the chokepoint, commented at the read). - **Transitional compatibility.** With no config installed, `engine::signer_env_var` falls through to the process environment — existing hosts and the entire pre-existing test suite run unchanged; non-development profiles log a one-time warning suggesting the init FFI. ## Why parity is safe by construction The typed request converts to the same canonical strings the existing env parsers consume (`"true"`/`"false"`, decimal ints, comma-joined identifier lists), and every existing clamp/warn/reject path runs unchanged on identical inputs. The diff swaps `std::env::var(X)` → `signer_env_var(X)` at 31 sites and changes nothing else about how values are interpreted. Also deletes `lib.rs`'s duplicated profile/truthy parsing in favor of the engine's single implementation. Thanks to #4036, this lands as one new ~400-line module (`engine/init_config.rs`) plus one-line touches across `config/lifecycle/persistence/policy/provenance/state` — not an 18k-line-file churn. `engine/config.rs` remains the single home of the env-name constants. ## Verification - `cargo fmt --check` ✅; `cargo clippy --all-targets -- -D warnings` ✅ - Full suite **235 passed + 1 ignored / 24 / 1** — all 224 pre-existing tests pass unchanged (env-fallback parity), plus 11 new tests: config-over-env precedence, wholesale env-ignoring for unset fields, idempotent/conflicting re-init, invalid-profile rejection, install rollback on incomplete firewall policy, complete-admission-policy validation, secret-stays-on-env-channel, production-profile-forces-strict via config, `reset_for_tests` clearing, `deny_unknown_fields`, list/bool canonicalization, and an FFI round-trip - `--features bench-restart-hook` builds; chaos suite (5/5 `--exact` paths) and `formal_verification_` filter pass - `include/frost_tbtc.h` gains the symbol; README documents the contract ## Notes for reviewers - Knobs the runtime warn-and-defaults on (e.g. out-of-range timeouts) keep that behavior under config values — init validation only rejects what the runtime gates would reject. Tightening init further is possible later without breaking the contract. - Go-host adoption is a follow-up: this is additive ABI; nothing changes for hosts until they call the new entry. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
…ence Post-merge follow-up #5 from the June 2026 review stack (the remaining half of #4033's item): cite the exact external audit coverage of the pinned FROST stack in the readiness/rollout docs, and record the attestation-rotation operational requirement surfaced by #4037. - rollout gates doc gains a "Cryptographic Dependency Audit Status" section: NCC Group's Zcash FROST Security Assessment (2023-10-20) covered v0.6.0 (commit 5fa17ed) of frost-core and five ciphersuites; upstream states explicitly that frost-secp256k1-tr and rerandomized FROST are NOT included; Least Authority's Q1 2025 audit covered demo tooling only. Bottom line recorded honestly: the pinned frost-secp256k1-tr =3.0.0 and the v0.6.0->3.0.0 frost-core evolution have no external audit coverage, so Gate 1 must either commission an audit or record written risk acceptance scoped to canary - a team decision this section now gives a factual basis. - rollout runbook prerequisites gain the attestation rotation cadence: init-time config is immutable per process and attestation TTL caps at 7 days, so signers must restart with fresh attestation within every window; live re-attestation is deliberately unsupported. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
## Stack 1 of 3. Base: `extraction/frost-signer-mirror-2026-05-26` (#4005). Next: #4161. ## Summary - Normalize one-component signer state paths to `.` for directory creation, fsync, and corrupt-backup enumeration. - Make completed canary rollback retries true no-ops for rollout state, config version, metrics, and promotion evidence. - Repair uncertain post-rename directory durability on a restarted rollback retry without rewriting the state file; keep fresh missing-state no-ops side-effect free. ## Review findings addressed - P1: bare state paths used an empty parent for directory operations. - P2: completed canary rollback retries were not idempotent. ## Validation - `cargo test --locked --manifest-path pkg/tbtc/signer/Cargo.toml` on this branch tip: 270 library tests passed, 1 ignored; 28 admission-checker tests passed; 1 integration test passed. - Fault-injection regressions cover pre/post-rename behavior, restarted retry repair, and no state rewrite on idempotent retry.
## Stack 2 of 3. Parent: #4160. Next: #4162. ## Summary - Reject unknown top-level fields when deserializing `AdmissionPolicyV1`. - Keep supported policy files compatible while turning misspelled security controls into explicit input errors instead of omitted limits. - Add a malicious typo regression and a supported-field positive control. ## Review finding addressed - P2: unknown admission-policy fields could silently disable enforcement and fail open. ## Security invariant A policy field that the checker does not recognize cannot silently alter or weaken admission behavior. ## Validation - `cargo test --locked --manifest-path pkg/tbtc/signer/Cargo.toml` on this branch tip: 270 library tests passed, 1 ignored; 30 admission-checker tests passed; 1 integration test passed. - The real checker rejects a misspelled `max_operator_per_provider` field and accepts the shipped policy sample.
## Stack 3 of 3. Parent: #4161. ## Summary - Track each live interactive signing member from its last accepted activity using monotonic `Instant` state instead of wall-clock Open time. - Refresh activity for exact Open retries, accepted Round1 calls, and retry-preserving Round2 failures; rejected traffic does not extend lifetime. - Add a forward-only test clock and regressions for active-at-boundary attempts, idle siblings, invalid requests, retry-preserving failures, expiry, and reset behavior. ## Review finding addressed - P2: interactive inactivity TTL was measured from Open and could sweep an attempt used moments earlier. ## Lifecycle invariant Only accepted or retry-preserving member activity extends the TTL; successful terminal operations and post-replacement failures still retire the nonce handle. ## Validation - `cargo test --locked --manifest-path pkg/tbtc/signer/Cargo.toml`: 273 library tests passed, 1 ignored; 30 admission-checker tests passed; 1 integration test passed. - `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 fmt --manifest-path pkg/tbtc/signer/Cargo.toml -- --check` and `git diff --check`. - `pkg/tbtc/signer/scripts/run_phase5_chaos_suite.sh`: all 6 fault-injection scenarios passed.
…-mirror-main # Conflicts: # pkg/bitcoin/electrum/electrum_integration_test.go
…ABI 3.5) Descriptor-bound stable .store-id, O_NOFOLLOW/openat with inode and path checks, atomic replacement, an append-only PREPARE/COMMIT/ABORT state-witness journal, retained key-package inventory with dkg_share_epoch, and ABI 3.5 symbols for durable identity, inventory and witness proof, with cross-language vectors. Closes seven previously audited blockers: legacy-migration fixtures now drop the lock and remove .state-witness before writing a true pre-witness fixture; open_or_create_state_witness truncates only an incomplete trailing record and syncs before parsing, while a complete-but-malformed record still fails closed; ensure_state_file_lock fully validates a newly acquired store; a cfg(not(unix)) stub for reconcile_pending_witness; three clippy fixes; and a rollback test proving a coherent generation-N snapshot is rejected against an independently retained N+1 anchor. Verified: cargo fmt --check, cargo check, cargo clippy --all-targets --all-features -- -D warnings all clean; cargo test --lib --test-threads=1 263 passed / 0 failed / 1 ignored (baseline was 260 passed / 2 failed plus 3 clippy errors). KNOWN DEFECTS - an independent adversarial review confirmed three issues in this design that the above gates do not detect. Do not merge before addressing: 1. (medium) store.rs:232 - lock_fingerprint hashes the advisory lock file's st_dev/st_ino and canonical_path_fingerprint hashes the parent directory's path string and inode. Both feed durable_store_fingerprint, the first field of every state_commitment. Deleting the zero-byte lock file, restoring from backup at the same path, renaming the directory, or remounting on btrfs/ZFS/overlayfs/NFS/tmpfs therefore makes every committed record unverifiable and the signer unstartable. The failure is raised inside acquire(), so TBTC_SIGNER_STATE_CORRUPTION_POLICY=quarantine_and_reset never reaches it; the only remaining action is deleting .state-witness, which re-genesises at generation 1 - exactly the rollback this feature exists to detect - while frost_tbtc_durable_store_identity keeps returning a byte-identical fingerprint, so the host gets no signal. 2. (low) store.rs:1210 - the truncation-repair comment asserts "a record is written and fsynced as one unit", but creation writes a 258-byte header+PREPARE+COMMIT blob non-atomically before syncing. A hard kill during first initialisation leaves a short file that is fatal and unrecoverable on next start. A torn append is repaired; a torn genesis is not. 3. (medium) store.rs:609 - the witness journal grows ~210 bytes per persist with no compaction or rotation, and is fully re-read and re-verified roughly four times per persist. Cost grows without bound in lifetime persist count, degrading toward missed signing and DKG deadlines. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012KZb3W6TwYtKTw1mb2sfpS
The v1 store fingerprint mixed the stable `.store-id` bytes with three
volatile filesystem descriptors: the canonical parent path string plus the
directory's st_dev/st_ino, the filesystem st_dev, and the lock file's name
plus its st_dev/st_ino. That fingerprint is the first field of every state
commitment, and `parse_state_witness_journal` rejects any record whose
commitment does not recompute under the CURRENT fingerprint.
Failure mode: deleting the zero-byte lock file (tmp reaper, "stale lock"
cleanup, container rebuild), restoring the data directory from backup at the
same path (tar/rsync assigns new inodes), renaming or moving the directory,
or any remount that moves st_dev (btrfs/ZFS subvolumes, overlayfs, NFS,
tmpfs - note the non-production default state path is std::env::temp_dir())
made 100% of committed records unverifiable and the signer unstartable. The
error is raised inside StateFileLock::acquire, so
TBTC_SIGNER_STATE_CORRUPTION_POLICY=quarantine_and_reset never reaches it,
leaving `rm .state-witness` - a generation-1 re-genesis, exactly the rollback
the journal exists to detect - as the only operator action.
Fix: separate verification-time checks from commitment-time binding.
- `durable_store_fingerprint` now binds the schema constant, the backend
constant, and the 32 stable store-ID bytes, and nothing else. It anchors
`state_commitment` and `state_witness_genesis`.
- The canonical-path, filesystem, and lock descriptors are still computed and
still enforced on every access by `revalidate_store_entries` (symlink,
inode, device, nlink, ownership, and mode checks are untouched), and are
still reported by frost_tbtc_durable_store_identity for diagnostics. They
no longer enter any committed transcript.
- This is a break in a frozen cross-language transcript, so the fingerprint,
genesis, and commitment domains move to v2, the identity schema moves to
/v2, and the journal magic moves to TBTCWITNESSv2. A v1 journal is
recognized by its magic alone - which works even when the v1 fingerprint
can no longer be recomputed - and fails closed with an actionable migration
error instead of a generic "invalid commitment". A v1 journal is never
rewritten, repaired, or reset.
- Transcript v2 changes the bytes of existing wire fields, so the FFI
contract moves to ABI major 4 (minor resets to 0; the symbol set is
unchanged from 3.5). Go bridges must move in lockstep.
New v2 cross-language vectors (Go must reproduce these exactly):
durable_store_fingerprint([0x11;32])
= 8bb8d21c69e78916e8f165b0c861c0d84c5d7af5393f75b0321fe048f772abba
durable_store_fingerprint([0x24;32])
= 52fcbfc4b2c6a93645106a32c62113192cac30b934b905e1ad357792c4ce8628
state_witness_genesis([0x11;32])
= 44085b42d29bf25f06207142f9e2db58eaf86f88d92b6e18104161ce59e98a89
state_commitment([0x11;32], 42, [0x22;32], [0x33;32])
= ea5eb04a4776357e59875f683390a2ff4b7dd511ad394e588dfab147f94fa867
The frozen v1 vectors are retained as regression tests for the rejection
path, alongside new tests proving that a deleted lock file and a whole-store
restore to a different directory with fresh inodes both keep every committed
record verifiable and let the chain advance from the restored tip.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012KZb3W6TwYtKTw1mb2sfpS
`open_or_create_state_witness` opened the journal O_CREAT|O_EXCL and then
wrote the 258-byte header+PREPARE+COMMIT image with one non-atomic
`write_file_at` before fsyncing it; `open_or_create_store_id` had the same
shape for its 32 bytes. A hard kill inside that window left a short file at
the final name, and every short length is fatal on the next start:
.store-id 0 or 1-31 bytes -> "invalid length"
.state-witness 0-47 -> header incomplete
48-152 -> missing/partial record
153-257-> no committed genesis record
All of these are raised inside `StateFileLock::acquire`, so
TBTC_SIGNER_STATE_CORRUPTION_POLICY=quarantine_and_reset cannot help. Note
the asymmetry the previous comment had backwards: a torn APPEND after genesis
is correctly repaired by `truncate_incomplete_witness_record`, while a torn
GENESIS - the one write that is not a single fixed-width record - was
unrecoverable.
Fix: `create_entry_atomically` writes the complete image to a temp entry in
the same directory, fsyncs it, checks the target name is still absent,
renames over it, and then fsyncs the DIRECTORY so the rename itself is
durable. Both genesis writers use it. Mode 0600 and the openat/O_EXCL/
O_NOFOLLOW discipline are preserved end to end - the temp entry is created
through the held no-follow directory descriptor with O_EXCL, so it can never
be a symlink-following hazard, and the descriptor that survives the rename is
the one the store keeps, so nothing is reopened by name. A failed publish
unlinks the temp entry. A crash before the rename now leaves only a temp
entry and an absent target, which the opener already handles by creating it.
Recovery for a short file at the FINAL name is deliberately not offered: this
build cannot produce one, so it is damage from outside the signer, and a
zero-length or half-written journal is indistinguishable from a deliberate
truncation. Re-creating it would re-genesis the anti-rollback chain at
generation 1 over whatever state image is present - the rollback the journal
exists to detect. Those bands now fail closed with errors that name the file,
state that a short file is not a torn write, and tell the operator to restore
rather than delete.
Tests cover both crash windows (a planted orphan temp entry for .store-id and
.state-witness, after which the store comes up clean and advances) and every
fatal short length band of both files.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012KZb3W6TwYtKTw1mb2sfpS
`revalidate_store_entries` read the ENTIRE journal and `parse_state_witness_journal` recomputed a SHA-256 commitment for every record ever written. `replace_state` invokes it three times per persist and `ensure_state_file_lock` -> `identity()` adds a fourth, while the journal grows ~210 bytes per persist with no compaction, rotation, or checkpointing. Cost therefore grew without bound in LIFETIME persist count: at ~1e6 persists that is a ~210 MB journal re-read and re-hashed ~4x per persist, degrading toward missed signing and DKG deadlines. (A stale 336 KB journal left in the temp dir by an earlier run of this crate's own test suite already accounted for roughly two thirds of its wall-clock time.) The journal is append-only, so verification is incremental. The verified prefix is now cached per `StateFileLock` as (file identity, change stamp, verified offset, history length, last verified generation + commitment, exact trailing record bytes). A later access re-checks that anchor in O(1): - the journal's dev/ino must be the descriptor the store holds, - its size/mtime/ctime stamp must be untouched since the last verification - any write moves at least one, and ctime cannot be back-dated by an unprivileged writer, - the 48-byte header must still bind this store ID, - the trailing record must still be byte-identical. Bytes appended since the last verification are read BACK from disk and compared with what was written, at append time, so no journal byte is ever trusted without having been read. Any mismatch - including a moved file identity - falls through to the full parse, which is what still produces the precise error, so nothing is weakened: a tampered prefix is caught in-process through the change stamp and unconditionally on a fresh open, where the cache does not exist. The cache is in-memory only and is never a trust anchor across process restarts. Compaction and rotation are deliberately NOT implemented here: preserving the anchor chain and the anti-rollback property across a rewrite is a separate, riskier change. The resident `witness_history` therefore still grows with lifetime persist count. Tests: a benchmark-style assertion that per-persist journal read volume stays constant (< 1 KiB) and below the journal length while 60 persists grow the journal to ~12.9 KB, with exactly one full parse for the whole run; and a tampering test that flips a byte deep inside the already-verified prefix and requires detection both in-process and on a fresh open, plus recovery when the committed bytes are restored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012KZb3W6TwYtKTw1mb2sfpS
…ABI 4.3) Add the two-stage Ed25519 trust-transition engine (anchor_trust) with a durable prepare/commit journal, crash-recovery from a descriptor-held transition intent, typed state_anchor_trust_recovery_required reporting, and bootstrap-facts provisioning for the offline ceremony. New additive FFI: frost_tbtc_transition_state_witness_anchor, frost_tbtc_state_anchor_trust_head, frost_tbtc_state_anchor_bootstrap_facts. Anchored interactive calls are bounded to three witness generations (reconcile + sweep/retirement snapshot + own write) under the production barrier model; the restart-history descendant check pins the exact floor+4096 accept / +4097 reject boundary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DpftF6aYHh56KgiHU6D2NQ
The interrupted multi-snapshot retry parks the witness journal at the rotation terminal reservation as a legitimate steady state, waiting for the next checkpoint acknowledgement. A state image that stops decoding while the journal is parked there had no exit: quarantine-and-reset appends its absence witness through prepare_witness, which refused at the terminal limit, and the acknowledgement that would rotate the segment revalidates the state image against the committed tip first, so the corrupt image blocked its own repair. The store stayed wedged until a byte-exact image matching the committed tip was restored. Reserve two records above the terminal band for the quarantine PREPARE/COMMIT pair and let only the absence commit draw on them. Quarantine never extends usable state and cannot repeat - the second call finds no state file - so the reserve cannot become an escape hatch for ordinary writes, which still stop at the terminal band exactly as before. The local anchor configuration and the certified trust endpoint both now require the reserve to fit below the hard record ceiling; without that, quarantine would have to choose between the rotation bound and a journal that no longer parses on reopen. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The production cfg(not(unix)) storage stubs build, but the two lock-replacement fault injectors are cfg(test) only: they displace and recreate directory entries through openat/renameat/as_raw_fd and take a cfg(unix) recovery guard, so `cargo test` could not compile for a non-Unix target even though every test that arms them is already cfg(unix). Gate the injectors and their switches on all(test, unix). Their only call sites are cfg(unix) functions guarded by an inner cfg(test) statement attribute, so Unix test coverage is byte-identical. Verified by substituting the cfg predicates in a scratch copy of the crate (cfg(unix) -> cfg(any()), cfg(not(unix)) -> cfg(all())) and running `cargo check --all-targets`: it reproduces the reported E0425/E0599 failures before this change and is clean after. That exercises the cfg wiring, not a real non-Unix toolchain; no non-Unix std target is installed locally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
frost_tbtc_retire_distributed_dkg_key_packages was exported from lib.rs but never declared in include/frost_tbtc.h, leaving it the only one of the 39 exported entry points missing from the header. The header is the documented C contract for the library, so a consumer generating bindings from it could not see the symbol at all. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
D1 (Split): Drop ABI minor bump (4 -> 0) and remove the 11 new FFI symbols added by this branch. The internal engine work (anchor.rs, anchor_trust.rs, inventory.rs, store.rs additions) is preserved; the new FFI exports and ABI major version are deferred to a follow-up PR that re-exposes them with proper Go-side coordination. The PR's 'ABI 4.3 coordinated merge with #4199' framing is now moot. D2 (Rename): durable -> descriptor_bound on DurableStoreIdentityResult. Matches the existing C header doc which says 'fails closed if a live path, lock, store-ID, or state entry no longer matches its held no-follow descriptor.' P0 #3 (v1->v2 re-anchor): Error message at retired_v1_state_witness_journal_error no longer references a 'documented v1->v2 witness re-anchor' that did not exist. The recovery procedure is now inline in the error. New retired_v1_state_witness_journal_recovery_steps() exposes the same procedure as a Rust function for test coverage. New pkg/tbtc/signer/docs/signer-store-v1-to-v2-migration-runbook.md is the operator runbook for v1 stores that hit this code path. P1 (validate_state_image 3x): New validate_state_image_with_digest helper accepts a precomputed digest, avoiding 2 redundant state-file reads + 2 SHA-256 hashes per persist. The first two revalidates in replace_state use the new helper; the third (after rename) still uses the original. P1/P2 (Hardening): cfg(not(unix)) guards on StateFileLock::acquire now return a clear 'requires Unix; not supported on this platform' error at construction time (distinguishable from 'store corrupted'). validate_entry_name now rejects '.' and '..' (preventing path traversal if a future caller bypasses canonicalize). unique_temp_name drops PID from the temp-name format (16-byte OsRng suffix is sufficient to prevent name collisions; PID leak no longer useful for same-uid attackers). P1/P2 (Tests): 16 new tests in tests.rs + 2 new tests in store.rs, plus 2 existing tests strengthened. Covers: orphan temp recovery, short file non-recovery, cross-mount restore, mid-journal modification, cache invalidation, O_NOFOLLOW symlink rejection for all entry paths, mode 0600 on all entries, dkg_share_epoch rollback prevention, inventory retention (rotation/eviction/multi-wallet), FFI symbol exports match ABI, 0x24*32 fingerprint vector, mid-record torn-repair, production-realistic state_witness_max_records test, production default rotation threshold test. 4 tests marked #[ignore] with explanation pending setup clarification. Test results: cargo test --lib -- --test-threads=1 -> 345 passed, 7 ignored, 0 failed. Clippy: cargo clippy --all-targets --all-features -- -D warnings -> No issues found. cargo fmt --check -> clean. Ref: agent-docs/reviews/pr-4198/findings.json (multi-agent-review of this branch) Ref: agent-docs/gap-inventory.md (D1, D2, C1 decisions)
Lists the four P0/P1 items deferred to this branch: - Per-record hash chain for witness journal (P0 #2) - Compaction implementation for witness journal (P0 #4) - Trust journal rotation/compaction (P1) - witness_history unbounded growth (P1) Each item references its multi-agent-review location and a brief implementation plan. Reference to the implementation commit on the main PR branch (b976a46) and the gap-inventory decisions (D1, D2, C1).
Every fixed-width witness record now carries a 32-byte chain_hash field that commits to all preceding records via a domain-separated SHA-256 link (chain_hash[i+1] = H(domain || chain_hash[i] || record[i+1])), making any historical tamper with the journal detectable on reload even when the state-commitment chain itself is unchanged. Records grow from 105 to 137 bytes; the segment header layout is unchanged so the frozen Go/Rust cross-language 472-byte header vector is preserved and cross-segment chains are anchored by anchoring the first record of each new segment to the previous segment's header_commitment, rather than to zeros. Bump the plain witness magic from TBTCWITNESSv2 to TBTCWITNESSv3 and add an actionable v2 retirement error so any v2-format journal left on disk by an older build is rejected with a one-time migration runbook instead of failing closed with a generic partial-record error. Old v2 journals must be renamed aside (NOT deleted) and the signer restarted; the new build regenerates at generation 1, accepting the v2->v3 break as a migration event.
Companion to the per-record hash chain implementation: the v2 record layout (105-byte records, no per-record chaining) is now retired and rejected at startup with an actionable migration error. This runbook mirrors the v1-to-v2 runbook format and gives an operator the exact shell commands to verify the magic, rename the retired journal aside, restart under the new ABI, and confirm the new v3 fingerprint. It also calls out the Go-side pin requirement so the rollout is coordinated across the threshold set, and points at the unchanged segment header layout so the cross-language byte vector stays valid.
…d advisory flock - Remove DurableStoreIdentityResult: the FFI symbol was already dropped in D1 (ABI reversal) so the 12-field result struct is unreferenced. - Add redacted_internal_error helper and apply it to the lock-file and state-directory error paths in acquire_with_mode, so production profiles do not leak absolute on-disk paths through the FFI error channel. - Add advisory_exclusive_lock (best-effort flock) on the durable store ID and the state witness journal, complementing the existing state lock with a defense-in-depth guard against a second process that bypasses the lock file. - Cover the redaction helper with a unit test that exercises both production (redacted) and development (verbose) branches.
…through F-34)
Addresses the second review pass tracked under agent-docs/reviews/ for
this branch (findings.json).
P0:
- F-01: implement local witness-journal compaction for unanchored signers
(compact_witness_journal_local, recover_state_witness_compaction). Fixes
a permanent write-lockout once the record ceiling is reached with no
signed anchor configured. Retires the previous segment immediately after
publish, matching the existing signed-rotation convention, so
revalidate_store_entries's steady-state invariant holds.
- F-08 folded into F-01: the ceiling error message no longer points at a
checkpoint ABI with zero FFI exports.
P1:
- F-02: add hash-chain tamper/domain/v2-rejection/cross-segment tests.
- F-17: correct signer-api-contract-decision-brief.md/README.md's FFI
surface claims (round-level DKG/signing-package, not coarse session API)
and reconcile the ABI major version number.
P2:
- F-03: move error redaction to the FFI boundary (ffi_redacted_message),
scoped to Internal only -- Validation is user-facing business-rule text
and must not be redacted.
- F-07: add a records-based ceiling to the trust journal alongside the
existing byte cap.
- F-09: harden backup/quarantine directory access with openat/O_NOFOLLOW,
with relative-path (AT_FDCWD) support for the bare-state-path case.
- F-14: pin a frozen cross-language vector for the record chain-hash domain.
- F-16/F-27/F-34: rewrite FOLLOWUP.md to reflect actual landed state.
- F-21: document the frozen 472-byte segment header wire format instead of
refactoring it (cross-language contract, not safe to change).
- F-22: collapse v1/v2 retired-journal helpers into parameterized versions.
- F-23: rename rotation tests/comments that misused 'compaction'.
- F-24: narrow module-wide #![allow(dead_code)] to the specific orphaned
FFI symbols in policy/state/dkg/codec/inventory/api.
- F-05/F-06/F-18/F-26: new compaction runbook, hash-chain security-model
and filesystem-dependency notes, mark the secret-material plan superseded.
P3:
- F-11: gate the corrupted-state eprintln! behind the production profile.
- F-15: add the missing post-rename DKG-retirement crash-injection test.
- F-30: encode_state_witness_record returns a stack array, not a Vec.
- F-31: rewrite two evergreen-comment violations.
- F-32: add Status: fields to decision-brief.md and rust-rewrite-bootstrap.md.
Bugs found and fixed during verification (not in the original findings):
- Recursive stack overflow: compact_witness_journal_local's own PREPARE/
COMMIT append re-entered ensure_witness_record_capacity, re-triggering
compaction. Split append_witness_record into a checked wrapper plus an
unchecked core the compaction path uses for its own terminal records.
- prepare_witness computed the next witness before ensuring capacity, so a
mid-call compaction (which advances the tip) made the pre-computed
witness stale. prepare_witness now takes the state-image digest and
computes the witness after ensuring capacity/compaction.
- A test asserted the rotated segment's second record chained directly
from header_commitment; the chain is sequential (record 1 chains from
record 0's hash), not every record independently from the header.
- open_state_directory_nofollow required an absolute path, breaking the
bare-filename ('.'-parent) state path test; added an AT_FDCWD-based
traversal for relative paths.
cargo fmt --check, cargo clippy --all-targets -- -D warnings, cargo test,
cargo test formal_verification_, and cargo deny check advisories all pass.
|
Warning Review limit reached
Next review available in: 25 minutes Limit details: You’ve used all 2 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (84)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 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 |
The v1.8.0 tla2tools.jar release asset was rebuilt upstream (updated 2026-08-11 per the GitHub release API), so the pinned SHA-256 no longer matched the download and CI's TLA model checks job failed closed as designed. Verified the new hash against the official release URL (https://github.com/tlaplus/tlaplus/releases/download/v1.8.0/tla2tools.jar) independently before re-pinning it. Ran pkg/tbtc/signer/scripts/formal/run_tla_models.sh locally with the updated pin; all models (RoastAttemptStateMachine, RoastRolloutPolicy, StateKeyProviderPolicy incl. production config, TeeEnforcementModes) pass with no errors found.
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
2 participants
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.
Summary
Follow-up to #4198 (
codex/signer-store-identity-abi). Implements the four items originally deferred from that review's findings, plus a second multi-agent-review pass over the resulting branch, with all confirmed findings fixed.P0 (was a permanent write-lockout): unanchored signers now perform local witness-journal compaction when the record ceiling is reached, instead of failing closed forever with no recovery path.
P0 #2 (per-record hash chain): already landed earlier in this branch; this PR adds the missing tamper/domain/cross-segment/frozen-vector test coverage for it.
P1 trust journal / P1 witness_history: trust journal gets a records-based ceiling alongside its byte cap; witness_history growth was already mitigated by the pre-existing rotation path (documented, not a separate code change).
Plus: FFI-boundary error redaction moved to a single chokepoint (scoped to
Internalonly --Validationis user-facing text, not path-bearing); backup/quarantine directory access hardened against symlink swaps; segment-header/retired-journal-helper simplifications; narrowed#[allow(dead_code)]scoping; corrected FFI-surface documentation; new compaction runbook.Full finding-by-finding detail is in the commit message.
Verification
cargo fmt --manifest-path pkg/tbtc/signer/Cargo.toml -- --checkcargo clippy --locked --manifest-path pkg/tbtc/signer/Cargo.toml --all-targets -- -D warningscargo test --locked --manifest-path pkg/tbtc/signer/Cargo.toml(386 passed, 7 ignored)cargo test --locked --manifest-path pkg/tbtc/signer/Cargo.toml formal_verification_cargo deny check advisoriesAll pass locally; CI runs the same gates plus TLA model checks.