in reviewhumanPR #189Auth
Redeem_Grimm: extract reusable heartbeat authenticator, make the forgery discriminator a component swap
This is a direct follow-up to the review note on #152. That review approved the signed-heartbeat forgery defense but flagged two structural points: the authenticated-heartbeat logic lived inside the failure_detection scenario builtin…
Author
@Redeem-Grimm-Satoshi
github profile →- Status
- In review
- Opened on
- Jul 11
- Branch
- hackathon/redeem-grimm-reusable-heartbeat-auth
Description
The pitch.
## Problem
This is a direct follow-up to the review note on #152. That review approved the signed-heartbeat forgery defense but flagged two structural points: the authenticated-heartbeat logic lived inside the `failure_detection` scenario builtin rather than a reusable component, and the forgery discriminator was a `verify_heartbeats` boolean toggle rather than a swapped component like the rest of the framework uses. Both are fair. No other scenario could reuse the authentication without copy-pasting it, and the "does authentication matter?" question was answered by a config flag instead of by swapping an implementation, which is the pattern everything else here follows (a scenario picks `phi_accrual` or `heartbeat` for the detector, `signed` or `noop` for privacy, and so on).
This PR extracts the authentication into a reusable module and turns the discriminator into a component swap, with no change to observable behavior.
## What is in the diff
| Path | Purpose |
|---|---|
| `packages/nest-plugins-reference/nest_plugins_reference/failure_detection/heartbeat_auth.py` | New reusable module, co-located with the detector plugins and following the same `*_wire` pattern as `coordination/hotstuff_wire.py`. It holds the wire format (`heartbeat_payload`, `claimed_peer`, `HB_PREFIX`), a `HeartbeatAuthenticator` protocol, two verifiers, and a `make_heartbeat_authenticator` selector. |
| `packages/nest-core/nest_core/scenarios_builtin/failure_detection.py` | The scenario now imports the wire format and verifiers from the reusable module instead of defining them. `FailureMonitorAgent` holds an injected authenticator and has a single heartbeat path, so it no longer branches on a verify flag. The factory selects the authenticator by name. |
| `packages/nest-plugins-reference/tests/test_failure_detection.py` | Component unit tests for both verifiers and the selector, plus an end-to-end component-swap test and a test that the new selector and the legacy alias produce byte-identical traces. |
| `packages/nest-plugins-reference/tests/test_negotiation_determinism.py` | Wraps one pre-existing over-length line. See the note at the end. |
## The reusable component
`heartbeat_auth.py` packages three things any scenario can now reuse:
- The wire format `FDHB|<id>|<ts>|<sig-hex>`, unchanged, with the signature reconstructed from the raw wire text so traces stay deterministic.
- `SignedHeartbeatAuthenticator`: accepts a beat only if the signature verifies against the claimed peer's registered key and the timestamp is fresh (strictly newer than the last accepted, and not from the future). It owns its per-peer last-accepted timestamp internally, so the replay state is encapsulated rather than threaded through the caller.
- `TrustingHeartbeatAuthenticator`: the naive baseline that believes the claimed id. This is the real foil, the behavior a spoofable observer exhibits.
Both satisfy a small `HeartbeatAuthenticator` protocol (`accept(payload, *, now) -> (peer, ts) | None`, plus `name` and `verifies`), checked structurally at import the same way the detector plugins are.
## The discriminator is now a component swap
The observer no longer has a verify-or-trust branch. It holds one `HeartbeatAuthenticator` and feeds the detector whatever that authenticator accepts. Turning authentication on or off is selecting the component:
```yaml
task:
config:
hb_auth: signed # or: trusting
```
This mirrors how the detector itself is chosen (`fd_plugin: phi_accrual | heartbeat`). The forgery scenario's `no_forged_liveness` validator fails under `hb_auth: trusting` and passes under `hb_auth: signed`, exactly as it did with the boolean.
The older `verify_heartbeats: true|false` is kept as an exact alias (`true` maps to `signed`, `false` to `trusting`), so the merged forgery scenario and any existing config keep working with no change.
## Verification
`make ci-local` passes all five gates:
```
ruff check: All checks passed!
ruff format --ch
…Try it
Open PR on GitHubView diffCheckout locally
git fetch origin pull/189/head:pr-189
git checkout pr-189