Vote for your favorite SkillMD. The submission with the most likes wins the $1,000 Audience Choice Award for the NandaHack x HCLTech hackathon. Voting is open through September 25.Vote now →
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 avatar

@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 diff

Checkout locally

git fetch origin pull/189/head:pr-189
git checkout pr-189