alerting: StatusEvent wire schema + status category (bit 16) - #480
Conversation
Adds a node-health `StatusEvent` body (field 31) and the explicit-only `status` category bit, carried on the mask-bearing streaming transports. Round-3 review fixes folded in: - The silent-payment deep-replay exemption tested the raw category mask for equality against `tweaks`, so adding the new `status` bit silently forfeited it. A BIP-352 wallet asking for "my tweaks, and tell me if the node is sick" would have had its cold sync clamped to the most recent 10k blocks — and the `Subscribe` path has no way to signal a clamp in-band, so the wallet would never learn it was truncated. Mask `status` out before the test. - Do not emit `status` on the ZMQ `nodeevent` topic. ZMQ has no per-subscriber category mask, only an all-or-nothing topic toggle, so it cannot honor "explicit-only": every already-deployed `nodeevent` consumer would start receiving a new body type on upgrade (detectors are on by default, so `peer_floor` alone would do it), and the only escape would be `eventszmqnodeevent=0`, which silences every other envelope too. This is unlike the tweak categories, which reach the bus only while a gRPC tweaks subscriber is attached. Health remains available via webhooks and `-alertnotify`; an opt-in `eventszmqstatus` topic stays possible later. - `details` was documented as "decimal strings" on both the proto and in §7.8, but not every value is numeric. Correct the claim and repoint the per-kind key reference at streaming.md §7.8 (it named docs/api/webhooks.md, which does not exist at this point in the stack). Both behavioral fixes have tests verified to fail without them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AfAm3VVN8gBeYcTL5BJqH
15b229f to
c20643a
Compare
Deep-review fixes for the status wire schema.
- **`StatusEvent::raised`/`cleared`/`edge` are now total.** They asserted the
kind/state pairing with `debug_assert!`, and both call sites in `health.rs`
take the kind as a runtime parameter — so a mismatch was reachable and a
release build would ship it silently: `state: "raised"` for an edge kind is a
standing condition a consumer waits forever to see cleared, with
`satd_alert_active{kind=...}` stuck at 1. The state is now derived from the
kind rather than asserted against it.
- **`StatusKind::ALL` has a compile-time exhaustiveness guard.** It is a
hand-maintained array that `from_str_exact` scans, so a variant added to the
enum but not to the array compiled clean and then rejected the operator's
`kinds = ["new_kind"]` at alertfile load while the docs advertised it. No test
could catch that — `kind_names_round_trip` iterates `ALL`, so a missing entry
is invisible to it. Adding a variant now fails the build.
- **`message` and `details` are bounded** (1024 / 256 bytes, truncated on a
UTF-8 boundary). Every producer is in-tree and emits short tokens today, but
this body rides a 4096-slot broadcast to every subscriber and goes inside an
HMAC-signed webhook payload, so the first detector to interpolate
peer-supplied text should not be able to size either.
- **The changelog and release notes said status is served on the ZMQ
`nodeevent` topic. It is not** — `events/src/zmq.rs` excludes it deliberately
(ZMQ has no per-subscriber category mask) and has a regression test for the
exclusion. An operator wiring a `nodeevent` consumer to page on `disk_low`
would have tested it against a healthy node and shipped something that never
fires. Release notes are the primary discovery surface for this feature.
- `details` is documented as "decimal strings" in three places, but a `cleared`
event carries a `reason` token. Corrected: a consumer must not parse the map
uniformly as integers.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AfAm3VVN8gBeYcTL5BJqH
Deep review — findings (#480 + #481)Independent review; these are reviewer-reported and I have not personally re-verified them — worth confirming before acting. Wire-schema compatibility is clean. Bit 16 is genuinely explicit-only end to end: 1.
|
…cribe Every other category on this surface describes the chain, which is public and is what `stream:subscribe` exists to hand out. `StatusEvent` describes the host: free bytes on the node's volume, connected peers split inbound/outbound, the mempool byte cap with its occupancy and `mempoolminfee`, tip height, IBD state, reorg depth and fork heights. That is the content of `getblockchaininfo`, `getpeerinfo`, `getmempoolinfo` and `getwarnings`. `satd-auth` separates `rpc:read` from `stream:subscribe` precisely so an operator can issue a streaming token to a wallet backend, an indexer, or a tenant *without* also granting node read RPC. Serving status on `stream:subscribe` alone routed around that split rather than extending it — the token delivered, over the stream, the telemetry it was issued to withhold. Refused at the handshake (`PERMISSION_DENIED` on gRPC Subscribe, 403 on WS/SSE) rather than stripped, because bit 16 is never in the `categories=0` default so asking for it is always deliberate, and a category that silently never arrives reads to a health dashboard as "nothing is wrong". The mid-stream control paths — SetCategories and SetWatchSet on both carriers — strip the bit instead, since they have no per-message error channel; that keeps a control message from being a second door around the gate. No new capability: the vocabulary is a Tier 1 surface, and the data already has an owner in it. `status` is new this release, so no deployed token loses access to anything it has today. Defaults are untouched. With no token store `may_receive_status` returns true (the Core-compatible loopback-trust path), and the operator principal holds `CapabilitySet::ALL`. Both are asserted. Also corrects the proto comment on STATUS_KIND_TIP_STALL, which claimed the alert is suppressed in IBD; `check_tip_stall_values` deliberately ignores IBD and explains why at length. All three gate tests were run against the ungated code and fail there.
Tier 1 + Tier 2 review findings fixedWorking bottom-up across the stack; every branch was rebased and force-pushed with On this PR (#480)Finding 4 —
New Finding 6 (part) — the Finding 2(a) — Finding 3 — Deferred (Tier 3), with reasonsFinding 5 — Full deferral list with rationale is on #486. |
PR 1 of the A3 alerting stack (
SATD_ALERTING_DESIGN.md§10). Wire schema only — no emitters, no config, no behavior change.Why
Every body on the streaming API describes the chain or the mempool. None describes the node, so an operator who wanted to know their daemon had stalled, filled its disk, or lost its peers had to poll
getblockchaininfoon a timer and diff the results.Per design decision D2, health events are first-class on the firehose rather than internal to the webhook dispatcher. Every carrier and both SDKs then get them with zero extra plumbing, and the dispatcher landing in PR 3 is just another bus consumer.
What
StatusEvent+StatusKind/StatusState/StatusSeverityenums;NodeEvent.status = 31(28/29/30 taken by the SP work). NoSubscribeRequestfield — the category bit is the opt-in.node/src/events/status.rs(the Rust types),NodeEventBody::Status,CATEGORY_STATUS = 16joiningEXPLICIT_ONLY_CATEGORIES,EventPublisher::publish_status.status_event_to_protomapping arm; WS/SSE serde passthrough; ZMQnodeevent-only (no Core-compat topic — Core's equivalent is-alertnotify, which satd drives from the same detectors in PR 2).docs/api/streaming.md§7.8, CHANGELOG +0.5.0-pre.md.Decisions worth reviewing
categories=0subscriber written against an older node never starts receiving a body it can't parse after an upgrade.CATEGORY_TWEAKSfrom every mask because tweaks are firehose-scale and gRPC-only; status is low-volume JSON with no index prerequisite, so an explicit bit-16 request survives the mask. Asserted in a test.detailsis aBTreeMap, not aHashMap. The webhook body is HMAC-signed; golden signature vectors would be unreproducible under map-iteration order.node/src/events/schema.rs).Acceptance (design §10 PR 1)
categories=0subscriber receives nothing new — asserted instatus_category_is_explicit_onlyand the WS mask test.cargo check --workspace --tests(the recurring proto-literal gotcha — noSubscribeRequestfield was added this time, so only one exhaustive-match test site needed a new arm).--workspace --all-targets --all-featuresclean; full test suite green.Local test run: everything passes except the 10 pre-existing
test_address_index_backfill_*regtest cases, which fail on this machine's disk-space guard (need ~80 GB, have 68 GB free) and are unrelated to this change.Stack
PR 1 (this) → PR 2 detectors + warnings + metrics → PR 3
satd-alertcrate + webhook dispatcher → PR 4 watch-set hooks → PR 5 bench + docs → PR 6 SDK.🤖 Generated with Claude Code
https://claude.ai/code/session_018AfAm3VVN8gBeYcTL5BJqH