C2 — Carve persistence into seams

[piotr-roslaniec]

C2 — Carve persistence into seams

Targets origin/extraction/frost-signer-mirror-2026-05-26 (the source branch for PR #4005). NOT independent from PR #5 (C5 SessionState grouping) — both rewrite the bodies of the same two TryFrom impls in persistence.rs. Sequencing note in both PRs; hand-merge required at the TryFrom bodies.

This PR ships two commits: a spec/doc commit (spec(frost-signer): carve persistence into seams) and the implementation commit (feat(frost-signer): carve persistence into seams) that performs the 4-module persistence split described in the spec. The implementation is byte-for-byte stable on the wire (on-disk file format and persisted schemas unchanged), and the full test suite passes; the body of this PR is the design doc + the matching restructuring. Subsequent commits beyond these two are doc-only corrections to the spec doc and the PR description itself.

What this PR is

Deepening spec for pkg/tbtc/signer/src/engine/persistence.rs (currently 2,389 lines). Splits one monolithic file into 4 pub(crate) modules under engine::persistence/:

schema_codec    // pure TryFrom + encode/decode + schema-version validation
envelope_io     // file I/O, lock, atomic rename, corrupt recovery, backup retention
key_provider    // StateKeyProvider trait + EnvKeyProvider + CommandKeyProvider
                //   + CachedStateKeyProvider (real production 3rd adapter)
                //   + state_encryption_key_material() thin wrapper
pending_ops     // registry + snapshot covering + durable retry

The 4 concerns are independently testable but currently require each other in the test environment. The boundary between them is invisible — and the highest-stakes functions in the codebase (AEAD envelope verification, key-provider subprocess management, schema-version migration) are the ones that most need clean per-concern test surfaces.

Trait shape (verified against current code)

pub(crate) trait StateKeyProvider: Send + Sync {
    fn material(&self) -> Result<StateEncryptionKeyMaterial, EngineError>;
    fn key_id(&self) -> &str;
}

Three real production adapters live in key_provider:

• EnvKeyProvider — reads TBTC_SIGNER_STATE_ENCRYPTION_KEY_HEX env.
• CommandKeyProvider — runs the configured TBTC_SIGNER_STATE_KEY_COMMAND, with a timeout enforced by state_key_command_timeout_secs().
• CachedStateKeyProvider — process-lifetime cache decorator wrapping any Box<dyn StateKeyProvider>; keyed on key_id() so a fresh material() is only invoked when the provider identity changes. New in this PR — the pre-split code re-resolved the key (and re-spawned TBTC_SIGNER_STATE_KEY_COMMAND for the command provider) on every state_encryption_key_material() call. The caching is a deliberate, beneficial behavior change; see the spec doc's key_provider section and Risks subsection for details.

No test-only fake type moves into this module. Tests continue to define their own StateKeyProvider impls locally (current convention: TrackingMockKeyProvider + StdArcMockProvider newtype), wrapped through CachedStateKeyProvider for cache-coverage tests.

Wired data flow

load_engine_state_from_storage (envelope_io)
  └── decode_persisted_state_storage_format (envelope_io)
      └── decode_encrypted_state_envelope (envelope_io)
          └── state_encryption_key_material (key_provider)  ← decrypts the envelope
  └── decode_state (schema_codec)  ← pure

persist_engine_state_to_storage (envelope_io)
  └── state_encryption_key_material (key_provider)  ← encrypts the envelope
  └── persist_engine_state_to_storage_with_key (envelope_io)
      └── encode_state (schema_codec)  ← pure
      └── atomic state-file replacement (envelope_io)
      └── sync_state_file_parent_directory (envelope_io)
      └── clear_snapshot_covered_operations (pending_ops)  ← only on successful replacement, not on load

persist_engine_state_to_storage_with_key is a 2nd pub(crate) envelope_io entry point that takes an already-resolved key — callers that need the key resolved under a specific lock ordering (e.g. round2's durable-marker write, which must resolve the key before writing the marker) call it directly and skip the wrapper's own key resolution.

What stays unchanged

• The on-disk file format. A state file written by the previous code loads cleanly with the new code, and vice versa.
• The persisted schema: PersistedEncryptedEngineStateEnvelope, PersistedEngineState, PersistedSessionState, PersistedKeyPackage — field names, ordering, and serde tags preserved byte-for-byte.
• The TBTC_SIGNER_ABI_* constants are not bumped.
• The STATE_FILE_LOCK mutex and the #[cfg(unix)]/#[cfg(not(unix))] subprocess-group-keying branching are preserved.

Sequencing with PR #5 (C5)

Both PRs rewrite the bodies of TryFrom<PersistedSessionState> for SessionState (persistence.rs:1917-2233) and TryFrom<&SessionState> for PersistedSessionState (persistence.rs:2235 onward). This PR moves those bodies into schema_codec.rs unchanged in shape; PR #5 reshapes what they project into (flat fields → 6 substructures).

Merge order: land whichever is easier to rebase the other onto (either order works structurally), then hand-merge the shared TryFrom bodies. Do not apply both to a worktree in parallel and expect a clean merge.

Verification

Spec was verified against current code. Verified facts that the spec encodes (corrections from earlier draft):

• persistence.rs is 2,389 lines (not 2,256 as in the earlier draft).
• StateKeyProvider trait methods are material() and key_id() -> &str (the earlier draft had fictional resolve() / key_id() -> String).
• 3 real production adapters exist; no InMemoryKeyProvider (the earlier draft invented one).
• clear_snapshot_covered_operations is fn (private) with parameter engine_state: &EngineState. Called from persist_engine_state_to_storage (success path), not from load_engine_state_from_storage. Earlier draft had it on the load path with the wrong caller.
• legacy_plaintext_state_permitted stays fn (private); only called inside envelope_io.
• pending_build_taproot_tx_operation (single session_id arg, name with -taproot- not -tx_persistence_pending-); the earlier draft name was wrong.
• clear_persistence_pending_operations is cfg(any(test, feature = "bench-restart-hook")), not just cfg(test).
• state_lock_rejects_multi_process_contention lives at tests.rs:5787 (not state_file_lock_rejects_multi_process_contention at line ~5700 as in the earlier draft).

Files added

Spec doc:

• docs/specs/frost-signer-persistence-deepening.md (design doc, ~238 lines)

Implementation (omitted from base commit 555f2c514; added by the second commit on this branch):

• pkg/tbtc/signer/src/engine/persistence/mod.rs (re-exports the four submodules)
• pkg/tbtc/signer/src/engine/persistence/schema_codec.rs (pure TryFrom + encode/decode + schema-version validation)
• pkg/tbtc/signer/src/engine/persistence/envelope_io.rs (file I/O, lock, atomic rename, corrupt recovery, backup retention)
• pkg/tbtc/signer/src/engine/persistence/key_provider.rs (StateKeyProvider trait + EnvKeyProvider + CommandKeyProvider + CachedStateKeyProvider + subprocess machinery)
• pkg/tbtc/signer/src/engine/persistence/pending_ops.rs (registry + snapshot covering + durable retry)

Removed:

• pkg/tbtc/signer/src/engine/persistence.rs (2,256 lines replaced by the four submodules above + mod.rs; contents preserved byte-for-byte across the split)

Revisited risks (now landed in this PR)

The 4 split modules talk to each other without a circular dependency: envelope_io calls key_provider::state_encryption_key_material() for both load and persist paths, and the cross-module clear_snapshot_covered_operations call from envelope_io::persist_engine_state_to_storage (success path only) to pending_ops is one-directional. The resolution path is envelope_io → key_provider and envelope_io → pending_ops; key_provider and pending_ops do not call back into envelope_io. The mod.rs orchestration preserves the pre-split call ordering.

[coderabbitai]

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: 734259dd-92cc-42cb-a117-9f76ddad8dbe

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

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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