Skip to content

fix(dig-node): guard long-lived background loops against silent panic-death (#2067) - #174

Merged
MichaelTaylor3d merged 2 commits into
mainfrom
harden/2067-spawn-loop-panic-guard
Aug 3, 2026
Merged

fix(dig-node): guard long-lived background loops against silent panic-death (#2067)#174
MichaelTaylor3d merged 2 commits into
mainfrom
harden/2067-spawn-loop-panic-guard

Conversation

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor

Closes dig_ecosystem#2067. Extends the #2044 panic-guard from the tier-0 precache loop to the other long-lived tokio::spawn background loops in dig-node-core, so a transient per-iteration panic no longer silently kills a subsystem until process restart.

Approach

A single shared combinator crate::shared::catch_iteration(loop_name, fut) (new shared/panic_guard.rs) is now the ONE place the AssertUnwindSafe(fut).catch_unwind() machinery lives — Ok passes the value through transparently; Err (a caught panic) logs a fixed-shape tracing::warn!(loop_name, …) and returns None so the loop continues. tier0_live::run_round_catching (#2044) was refactored to delegate to it (DRY — no copy-pasted unwind machinery; its two #2044 tests stay green, behaviour unchanged).

Triage (every tokio::spawn in crates/dig-node-core/src/ classified)

  • A — long-lived subsystem loop → GUARDED: chainwatch.rs run_loop (subscription-read + gap-fill tick), pex.rs run_tick_loop (delta flow), dht.rs run_maintenance (republish/refresh/gc). Plus tier0_live.rs (already #2044, now via the shared combinator).
  • B — one-shot / per-connection / per-stream handler → left as-is: serve_one_stream, run_send_direction, per-capsule single-flight pull, spawn_capsule_warm, ctrl-c withdraw, etc. A panic kills one unit of work, not a subsystem — task-death is the correct fail-safe.
  • C — test-only spawns → skipped.
  • A-survive but DEFERRED (scope focus): a set of inline loops inside large peer.rs bring-up fns / helper fns (selector-registry feed, DHT-routing feed, pool-stats, relay-acceptor, spawn_pool_feeder, spawn_gc, run_first_peer_announcer, run_holdings_ingest) — each needs its per-iteration body extracted into a testable helper first. Tracked as a follow-up (see below).

AssertUnwindSafe soundness (per guarded site)

Verified per site: no MutexGuard crosses the catch boundary in any guarded loop — each iteration carries only Arc<…> seam handles, and every awaited call locks-and-releases internally (returns a plain value), so a caught mid-iteration panic can never observe a poisoned/half-held lock. No loop was left unguarded for lock-safety reasons; no site was genuinely ambiguous.

Real (non-panic) errors are never masked — the combinator intercepts ONLY unwinds; each loop's normal Result/outcome handling is untouched (a dedicated test asserts a plain Err flows through unchanged). Log is fixed-shape; the panic payload is never logged (#1603).

How verified (TDD)

  • shared::panic_guard::a_panicking_iteration_is_contained_as_none — panic → None; non-vacuous (removing the catch_unwind aborts the test).
  • shared::panic_guard::a_completed_iteration_passes_its_value_through_unchanged — transparent happy path.
  • shared::panic_guard::a_normal_error_value_is_not_intercepted — a plain Err is NOT swallowed.
  • chainwatch::a_panicking_tick_is_caught_and_does_not_propagate — a PanickingResolver makes a REAL run_tick unwind; the guard contains it (non-vacuous — unwrapping run_tick directly aborts). The pex/dht loop bodies share the same combinator mechanism, fully covered by the panic_guard panic-injection test.
  • cargo test -p dig-node-core --lib662 passed, 0 failed; fmt + clippy -D warnings clean; build OK.

Follow-up (to be filed)

Guard the remaining inline (A)-survive loops (the deferred peer.rs/pex.rs/download.rs set above) by extracting each per-iteration body and wrapping it in crate::shared::catch_iteration. peer.rs run_relay_connection's loop lives in the dig_nat crate — panic containment there is a dig_nat concern.

Version

root [workspace.package].version 0.93.5 → 0.93.6 (patch, fix:); Cargo.lock regenerated.


Generated by Claude Code

claude added 2 commits August 3, 2026 17:37
Refs #2067

Co-Authored-By: Claude <noreply@anthropic.com>
…-death (#2067)

Extend the #2044 per-iteration panic-guard pattern to the other long-lived
tokio::spawn background subsystem loops in dig-node-core, so a single
iteration's panic can no longer unwind out of the spawned task and silently
disable a whole subsystem for the process's lifetime.

- Add crate::shared::catch_iteration — the ONE shared unwind combinator (fixed-
  shape WARN, never the panic payload #1603); refactor tier0's run_round_catching
  to delegate to it (DRY, no copy-pasted unwind machinery).
- Guard the (A)-survive loops: chain-watch + gap-fill (chainwatch::run_loop, the
  named target), PEX tick (pex::run_tick_loop), DHT maintenance
  (dht::run_maintenance). Each carries only plain values + Arc handles across the
  catch boundary (no lock/guard held across the await), so AssertUnwindSafe is
  sound per site.
- Panics only are intercepted; each loop's normal Result/error handling is
  unchanged.

Per-iteration one-shot / per-connection / per-stream handlers and test-only
spawns are intentionally left unguarded (task-death is the correct fail-safe
there). The remaining inline (A) event/accept loops in peer.rs bring-up are a
scoped follow-up.

Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d
MichaelTaylor3d merged commit 625534a into main Aug 3, 2026
15 checks passed
@MichaelTaylor3d
MichaelTaylor3d deleted the harden/2067-spawn-loop-panic-guard branch August 3, 2026 18:27
MichaelTaylor3d added a commit that referenced this pull request Aug 3, 2026
…ot Ok(None)

The production `MeltChain` derived a melt from `AnchoredRootResolver::anchored_root()
== Ok(None)`, which was wrong in both directions and made a remote-triggered,
irreversible delete unsafe:

- `Ok(None)` is the node's fail-closed sentinel for "no confirmed generation".
  `CoinsetResolver` produces it for "launcher coin is unspent (store not minted
  yet)" — a store whose lineage has not STARTED. Deleting on it wrongfully erases
  live data and broadcasts a false melt network-wide, correlated across holders.
- A genuine melt never produces it. A melted tip is spent without a datastore
  child, so the lineage walk returns Err("singleton spend did not yield a store"),
  which fail-closed to Unknown — the gate could not fire on a real melt.

Replace it with `confirm_melt_via_chain(&dyn ChainReads, store_id)`, composed from
two POSITIVE chain facts, neither of which re-walks the lineage (#747):

1. The launcher coin whose `coin_id == store_id` exists and is SPENT. `coin_id ==
   store_id` is a 256-bit hash preimage that cannot be ground, so identity is
   pinned to the one unforgeable on-chain anchor, never to a look-alike singleton
   that merely curries `launcher_id == store_id` (#1473). An UNSPENT launcher is
   Live — "not minted yet" is the opposite of melted.
2. The launcher's hint index is NON-EMPTY and every generation under it is SPENT.
   One `coin_records_by_hint(store_id, include_spent = true)` read gives both. The
   non-empty half is load-bearing: an empty index is indistinguishable from an
   un-indexed store, so it resolves to Unknown rather than authorizing a delete.

Every error, absence, or ambiguity resolves away from deletion, and fact 2 is
deliberately conservative — candidates are not launcher-anchored before they count
as live, so an adversary who can plant hints can only suppress a deletion, never
cause one.

Tests: 8 new cases drive the real `ChainReads` trait with crafted coin records —
the link the spy-driven policy tests could not reach. Unused trait methods on the
mock are `unimplemented!()` so the gate cannot silently grow a chain dependency,
and the mock asserts `include_spent = true`. All six inverting mutations of the
gate were confirmed to fail their test.

Also: both new background loops wrap their per-iteration body in
`shared::catch_iteration` (#173/#174/#175) with `recv()`/`tick()` outside the
guard, so a persistently-panicking iteration paces instead of hot-spinning;
`TombstoneSet` recovers from lock poisoning rather than panicking forever after
one contained panic; SPEC.md 14.5 corrected — it documented the `Ok(None)` rule.

root [workspace.package].version -> 0.94.0 (minor, new capability).

Co-Authored-By: Claude <noreply@anthropic.com>
MichaelTaylor3d added a commit that referenced this pull request Aug 3, 2026
…ot Ok(None)

The production `MeltChain` derived a melt from `AnchoredRootResolver::anchored_root()
== Ok(None)`, which was wrong in both directions and made a remote-triggered,
irreversible delete unsafe:

- `Ok(None)` is the node's fail-closed sentinel for "no confirmed generation".
  `CoinsetResolver` produces it for "launcher coin is unspent (store not minted
  yet)" — a store whose lineage has not STARTED. Deleting on it wrongfully erases
  live data and broadcasts a false melt network-wide, correlated across holders.
- A genuine melt never produces it. A melted tip is spent without a datastore
  child, so the lineage walk returns Err("singleton spend did not yield a store"),
  which fail-closed to Unknown — the gate could not fire on a real melt.

Replace it with `confirm_melt_via_chain(&dyn ChainReads, store_id)`, composed from
two POSITIVE chain facts, neither of which re-walks the lineage (#747):

1. The launcher coin whose `coin_id == store_id` exists and is SPENT. `coin_id ==
   store_id` is a 256-bit hash preimage that cannot be ground, so identity is
   pinned to the one unforgeable on-chain anchor, never to a look-alike singleton
   that merely curries `launcher_id == store_id` (#1473). An UNSPENT launcher is
   Live — "not minted yet" is the opposite of melted.
2. The launcher's hint index is NON-EMPTY and every generation under it is SPENT.
   One `coin_records_by_hint(store_id, include_spent = true)` read gives both. The
   non-empty half is load-bearing: an empty index is indistinguishable from an
   un-indexed store, so it resolves to Unknown rather than authorizing a delete.

Every error, absence, or ambiguity resolves away from deletion, and fact 2 is
deliberately conservative — candidates are not launcher-anchored before they count
as live, so an adversary who can plant hints can only suppress a deletion, never
cause one.

Tests: 8 new cases drive the real `ChainReads` trait with crafted coin records —
the link the spy-driven policy tests could not reach. Unused trait methods on the
mock are `unimplemented!()` so the gate cannot silently grow a chain dependency,
and the mock asserts `include_spent = true`. All six inverting mutations of the
gate were confirmed to fail their test.

Also: both new background loops wrap their per-iteration body in
`shared::catch_iteration` (#173/#174/#175) with `recv()`/`tick()` outside the
guard, so a persistently-panicking iteration paces instead of hot-spinning;
`TombstoneSet` recovers from lock poisoning rather than panicking forever after
one contained panic; SPEC.md 14.5 corrected — it documented the `Ok(None)` rule.

root [workspace.package].version -> 0.94.0 (minor, new capability).

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants