C5 — Split SessionState into 6 named substructures

[piotr-roslaniec]

C5 — Split SessionState into 6 named substructures

Targets origin/extraction/frost-signer-mirror-2026-05-26 (the source branch for PR #4005). NOT independent from PR #2 (C2 persistence split) — 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 both the deepening spec (docs/specs/frost-signer-sessionstate-grouping.md) AND the full implementation: 10 files, +842/-616, splitting SessionState into 6 named substructures and migrating every call site. Behaviorally a pure refactor (persisted schema byte-identical; cross-field invariants preserved verbatim); the risk lives in the migration's correctness, not its scope, so reviewers expecting a doc-only change should read the diff before relying on that label.

What this PR is

This PR splits pkg/tbtc/signer/src/engine/state.rs's SessionState (a 31-field flat struct at state.rs:111-183 holding 6 unrelated concerns — Round2, Aggregate, Refresh, Audit, etc.) into 6 named substructures, still under engine::state (no new module):

SessionState {
    dkg:           DkgSessionState,            // 5 fields (request fingerprint,
                                             //   key packages, public key package,
                                             //   result, policy snapshot version)
    signing:       LegacySigningSessionState,  // 12 fields
    interactive:   InteractiveSessionState,    // 5 fields
    audit:         AuditTrail,                 // 1 field
    lifecycle:     LifecycleState,             // 5 fields
    capacity_pins: OperationalState,           // 3 fields
}

The grouping is grounded in call-site co-location: each group is co-read at the call sites that exclusively touch that concern.

What stays unchanged

• The persisted schema PersistedSessionState is unchanged. Field names, ordering, serde tags preserved byte-for-byte. A state file written by the previous code loads cleanly with the new code, and vice versa.
• The TryFrom<PersistedSessionState> for SessionState impl projects into the new substructures; the inverse TryFrom<&SessionState> for PersistedSessionState reads them and flattens back to the same 31 fields.
• The refresh_count = persisted.refresh_count.max(history.len() as u64) legacy semantics are preserved (lifecycle.rs ignores this until a versioned cryptographic refresh protocol exists; the field is retained only for schema compatibility).
• The wire schema version is not bumped.
• The TBTC_SIGNER_ABI_* constants are not bumped.
• The Drop for InteractiveSigningState implementation (state.rs:105-109) is unchanged — only its containing field path changes.

Migration cost

Roughly 147 production .field read/write sites spread across production consumers:

File	.field sites	Notes
interactive.rs	~89	heaviest consumer; all 5 content groups touched
dkg.rs	~17	dkg.* + interactive.bound_key_group
state.rs	~13	mostly the exhaustive destructuring in per_message_interactive_session (state.rs:553-621)
lifecycle.rs	~13	lifecycle.* + dkg.result + interactive.bound_key_group
transaction.rs	~9	signing.* + lifecycle.emergency_rekey_event
verify_share.rs	~3	interactive.bound_key_group + dkg.public_key_package
audit.rs	~2	audit.attempt_transition_records at audit.rs:335 and audit.rs:397
telemetry.rs	~1	lifecycle.emergency_rekey_event at telemetry.rs:521
Total (excl. persistence.rs, tests.rs)	~147

persistence.rs is excluded from this count: its TryFrom impls read SessionState AND PersistedSessionState under the same field names verbatim, so a plain-text grep cannot distinguish the two. Both TryFrom bodies are rewritten regardless (see "TryFrom projection" below and PR #2's "Sequencing with PR #5").

Struct literals: 21 in tests.rs + 1 inside TryFrom<PersistedSessionState> impl in persistence.rs = 22 total rewrite to the nested form. The PersistedSessionState { ... } literal at tests.rs:623 is the wire schema and intentionally unchanged.

TryFrom projection (key invariant)

The TryFrom<PersistedSessionState> for SessionState impl (persistence.rs:1917-2233 today) validates the consumed-marker registries and hex-decodes the DKG/sign-message fields exactly as it does today, then projects into the new substructures in a single literal.

OperationalState is NOT wholesale-defaulted. Three real fields with mixed persistence:

• retired_interactive_at_unix — persisted (round-tripped through both TryFrom impls).
• heartbeat_rate_limiter — transient (not serialized; resets on restart).
• aggregate_eviction_pin — transient (Arc<()> refcount pin; an in-flight Aggregate clones it under the engine lock).

Defaulting the whole OperationalState to ::default() would silently drop retired_interactive_at_unix on every restart — a wire-schema-breaking behavior change.

Cross-field retirement invariant

The existing invariant if session.capacity_pins.retired_interactive_at_unix.is_some() && !per_message_interactive_session(&session) { return Err(...) } (persistence.rs:2122-2129 post-implementation) spans 3 groups (capacity_pins for the retired timestamp, interactive and dkg via the discriminator). The spec's DoD promised a dedicated unit test pinning this exact TryFrom-level check; this PR ships that test (engine::tests::persisted_session_state_rejects_retired_interactive_on_non_per_message_session) so a future migration cannot silently drop or invert the invariant. A second test, persisted_session_state_round_trip_preserves_capacity_pins_retired_interactive_at_unix, pins that OperationalState::retired_interactive_at_unix survives a TryFrom<&SessionState> -> TryFrom<PersistedSessionState> round-trip (defense against a future maintainer wholesale-defaulting OperationalState in either direction).

Sequencing with PR #2 (C2)

Both PRs rewrite the bodies of TryFrom<PersistedSessionState> for SessionState and TryFrom<&SessionState> for PersistedSessionState. PR #2 moves those bodies into schema_codec.rs unchanged in shape; this PR reshapes what they project into.

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.

Sequencing with PR #1 (C1)

Both touch interactive.rs, on disjoint lines: PR #1 rewrites only the lock-prologue and marker-durability blocks at the top/end of each phase handler; this PR rewrites the ~89 field-access sites in the phase-specific business logic between them, which PR #1 leaves untouched. A rebase is mechanical, not a semantic conflict.

Verification

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

• SessionState has 31 fields (state.rs:111-183), including policy_snapshot_version: u32 at state.rs:161 — added since the original draft was written. Placed in DkgSessionState by name and doc comment (no other production reader besides policy::current_policy_snapshot_version() which is currently unreferenced; groups with dkg rather than interactive).
• Legacy-signing group is 12 fields not 10 (4 consumed-marker registries + sign request fingerprint, message bytes, round state, active attempt context, finalize request fingerprint, signature result, build-tx request fingerprint, transaction result).
• 21 SessionState{...} literals in tests.rs (not 22 as in earlier draft); 1 SessionState{...} literal inside TryFrom<PersistedSessionState> impl in persistence.rs (not 2; the 2 reverse TryFrom<&SessionState> constructs PersistedSessionState{...} which is the wire schema and unchanged).
• Cross-field retirement invariant at persistence.rs:2222-2230 (was 2093-2099 in earlier draft).
• engine::tests::persisted_session_state_round_trip_preserves_bound_key_group at tests.rs:4615 (was ~4729 in earlier draft).
• Audit trail reads at audit.rs:335 and audit.rs:397 (was audit.rs:302 in earlier draft, which was wrong).
• Various other line citations corrected.

Files added

• docs/specs/frost-signer-sessionstate-grouping.md (design doc, the deepening spec)
• 9 Rust source files updated by the implementation commit (10 files total including the spec): the new substructures plus all call-site migrations. See "Migration cost" above for the per-file site count and "Verification" below for the file:line citations.

[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: 8f009758-259e-46bb-a84c-82ecba547288

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.}