C4 — Funnel the policy reject_* family

[piotr-roslaniec]

C4 — Funnel the policy reject_* family

Targets origin/extraction/frost-signer-mirror-2026-05-26 (the source branch for PR #4005). Independent, can run in parallel with the other 4 spec PRs. This PR ships BOTH the design doc and its implementation: a new docs/specs/frost-signer-policy-reject-funnel.md plus a 442-line Rust refactor that introduces the reject_with core in pkg/tbtc/signer/src/engine/policy.rs and wires it through interactive.rs, telemetry.rs, and transaction.rs.

Note: an earlier draft of the PR description labeled this a "pure doc/spec change — no Rust source touched". That was incorrect: this branch contains two commits, the second of which (a25710457 feat(frost-signer): funnel the policy reject_* family) is a real implementation change. The framing has been corrected; the rest of the substance below is unchanged.

What this PR is

Deepening spec for pkg/tbtc/signer/src/engine/policy.rs, plus the corresponding implementation. The module exposes 7 reject_* helpers across 44 production call sites. The signing family (3 helpers, 3 thin wrappers around reject_signing_policy_with_metric) already collapses into one funnel. The remaining duplication is concentrated in 3 uncompressed helpers:

• reject_admission_policy (policy.rs:236) — 17 lines, own metric (run_dkg_admission_reject_total).
• reject_quarantine_policy (policy.rs:510) — 13 lines.
• reject_lifecycle_policy<T> (policy.rs:524) — 13 lines, generic over T.

Plus 2 hand-constructed EngineError::LifecyclePolicyRejected sites that previously bypassed the log hook entirely:

• interactive.rs:1916 — emergency-rekey kill switch.
• transaction.rs:62 — build_taproot_tx entry.

Proposed change

Extract a private core reject_with(stage, session_id, reason_code, detail) -> EngineError inside policy.rs. reject_with returns EngineError directly, not Result<(), EngineError> — every stage always rejects, so returning the bare error is what makes a single-dispatch-point core actually fit (each of the 3 uncompressed helpers already returns the error directly today).

The 7 existing helpers stay as pub(crate) 1-line wrappers, each Err(reject_with(...)):

pub(crate) fn reject_admission_policy(...)
pub(crate) fn reject_quarantine_policy(...)
pub(crate) fn reject_lifecycle_policy<T>(...)        // unifies against any T
fn reject_signing_policy_with_metric(...)
pub(crate) fn reject_signing_policy(...)
fn reject_heartbeat_signing_policy(...)
fn reject_interactive_rate_limit_signing_policy(...)

The 2 hand-constructed sites route through reject_lifecycle_policy(...) so the lifecycle_policy log stage fires. Each caller's own Result<T, EngineError> unifies against the helper's return type: T = () for enforce_interactive_signing_gates, T = TransactionResult for build_taproot_tx.

Err(reject_with(...)) type-checks for every wrapper — there is no ? and no dead-code branch to reconcile. (An earlier draft wrote Err(reject_with(...)?), which would have been a type bug: the ? would always return early on the helper's Err, leaving the outer Err(...) unreachable.)

Bug premise correction (kept in spec body)

An earlier draft's ## Blocked section claimed interactive_rate_limit_reject_total is referenced but not declared in HardeningTelemetryState. That premise was wrong: the field is added by THIS PR at telemetry.rs:144 (pub(crate) interactive_rate_limit_reject_total: u64), and the implementation wires it through reject_with for the SigningPolicyFirewall { metric: InteractiveRateLimit } stage so the metric is ready before the parallel candidate's enforcement path lands. The ## Blocked section was removed from the spec; the field is a new addition, not a pre-existing one.

The PR also surfaces the new counter on the public SignerHardeningMetricsResult snapshot (with #[serde(default)] so existing JSON consumers do not break), so operators can observe the metric via frost_tbtc_hardening_metrics once the parallel candidate activates the enforcement path.

Hand-constructed site corrections

The hand-constructed sites use these exact fields (verified against the actual inline Err constructs):

• reason_code: "emergency_rekey_required" (NOT "emergency_rekey_active" as in the earlier draft).
• detail: a format! string carrying emergency_rekey_event.triggered_at_unix and emergency_rekey_event.reason, distinct per site (one prefixed with "emergency rekey required for session [{}] since [{}]: {}", the other with "build_taproot_tx blocked: emergency rekey required since [{}]: {}").

What stays unchanged

• The public EngineError variants (AdmissionPolicyRejected, QuarantinePolicyRejected, LifecyclePolicyRejected, SigningPolicyRejected, ...) — the bridge does not need to bump its ABI version.
• The log_policy_decision function signature and behavior — the new core forwards to it.
• The enforce_signing_policy_firewall_inner charge_rate_limit: bool parameterization — preserved, not collapsed.
• The record_canary_policy_outcome call at transaction.rs:254.
• The TBTC_SIGNER_ABI_* constants are not bumped.

Test invariants

• The 7 helper names are preserved, so the existing test assertions (metrics.build_taproot_tx_policy_reject_total, etc.) continue to work.
• The lifecycle_policy log stage is now emitted for the emergency-rekey kill switch AND for the build_taproot_tx entry — the invariant "all rejections go through log_policy_decision" is enforced for the first time at those 2 sites.
• New coverage in engine::tests::lifecycle_rejection_fires_log_for_kill_switch_blocking_build_taproot_tx and engine::tests::lifecycle_rejection_fires_log_for_kill_switch_blocking_interactive_session captures the eprintln! output of log_policy_decision and asserts the stage=lifecycle_policy marker is emitted with the emergency_rekey_required reason code at each of the two formerly hand-constructed sites.
• The chaos suite (scripts/run_phase5_chaos_suite.sh) passes.

Verification

Spec was verified against current code. Verified facts that the spec encodes:

• policy.rs:236/510/524 are the 3 uncompressed helpers' definition sites.
• interactive.rs:1916 (not 1914 as in earlier draft) is the emergency-rekey kill-switch inline Err.
• transaction.rs:62 is the build_taproot_tx entry inline Err.
• transaction.rs:253 is the matches!(error, EngineError::SigningPolicyRejected { .. }) matches-arm — preserved by the change.
• telemetry.rs:144 (this PR) is the new interactive_rate_limit_reject_total field on HardeningTelemetryState, plus the corresponding field on SignerHardeningMetricsResult.

Dependencies

None of the other 4 spec PRs (C1, C2, C5, C6) introduces a dependency on this one. The C1 spec touches interactive.rs for a different reason — its only "touch" at interactive.rs:1916 is the hand-constructed Err site that C4 rewrites; C1's restructuring is scoped to the lock-prologue and marker-durability blocks at the top/end of each of the 5 pub fn phase handlers, NOT to internal business-logic helpers like enforce_interactive_signing_gates (interactive.rs:1895) that C1 does not touch.

Files added

• docs/specs/frost-signer-policy-reject-funnel.md (214 lines, design doc)

Files modified

• pkg/tbtc/signer/src/engine/policy.rs — new reject_with core, 7 wrapper helpers, 1-line each.
• pkg/tbtc/signer/src/engine/interactive.rs — emergency-rekey kill-switch site routes through reject_lifecycle_policy.
• pkg/tbtc/signer/src/engine/transaction.rs — build_taproot_tx entry routes through reject_lifecycle_policy.
• pkg/tbtc/signer/src/engine/telemetry.rs — new interactive_rate_limit_reject_total field on HardeningTelemetryState; field surfaced on the public SignerHardeningMetricsResult snapshot in api.rs.
• pkg/tbtc/signer/src/api.rs — interactive_rate_limit_reject_total added to SignerHardeningMetricsResult (with #[serde(default)]).
• pkg/tbtc/signer/src/engine/tests.rs — two new tests for the kill-switch log line; see "Test invariants" above.

Open follow-ups (not in this PR)

The parallel candidate that activates the interactive-rate-limit enforcement path is the only thing this PR does not ship. The funnel, the wrapper, the metric, the snapshot field, and the tests are all in place; the dead-code allowance on reject_interactive_rate_limit_signing_policy lifts the moment that candidate lands its caller.

[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: 0a663d8d-393f-46ed-a769-7b2a8c9f1aca

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