feat: self-healing wedged-behind-network watchdog + snapshot completeness checks#125
Conversation
The payload checksum only proves the download matches what the serving node serialized; it cannot detect that the serving node serialized INCOMPLETE state. A single missing account on the serving side passes the checksum, imports cleanly, then wedges the node the moment a canonical block references that account (2026-07-13 rpc.viz.cx incident: out_of_range "viz-social-bot" at database get_account). Add three post-import checks inside the import write-lock: - per-section object-count reconciliation: header.object_counts (recorded at export from the serialized section arrays) vs the live index sizes for all multi-instance indices. Catches a lossy import. - referential integrity: every account_authority must reference an existing account (the exact invariant the wedge violated). - critical singletons present: dynamic_global_property, validator_schedule, hardfork_property. On failure FC_ASSERT aborts the import — loud and retryable: the P2P snapshot-sync path rejects the incomplete snapshot and tries another trusted peer instead of silently starting on corrupt state.
A snapshot-synced node whose state is missing an object rejects every canonical block that references it (out_of_range at apply_block). This is a rejection livelock, distinct from the #117-#120 write-lock deadlock monitors: push_block fails fast, so those monitors never fire. The divergence sits at/below LIB, so fork-switch cannot recover — only wipe + snapshot re-import does, and that was gated on empty state, so it never self-triggered. The 2026-07-13 rpc.viz.cx outage needed a manual docker compose --force-recreate after ~14h frozen. Detection (dlt_p2p_node::check_wedge_watchdog, in periodic_task): declare WEDGED when, sustained for WEDGE_CONFIRM_SEC (900s): behind > WEDGE_BEHIND_THRESHOLD (200) AND head flat AND gap-fill rejections still climbing. The triple-AND rules out a healthy syncing node (head advancing) and a partition (behind+flat but no rejections). A new monotonic _gap_rejected_total drives the "still rejecting" signal because _gap_rejected_count oscillates (resets on blacklist). The pure predicate is_wedged() is factored out for table-testing. Action (gated behind auto-resync-on-wedge, default OFF for this release so we observe the elog/marker in the wild first): elog, write a force_resync marker next to shared_memory.bin, then std::_Exit(2) (precedent: undo_all watchdog). With OFF, the watchdog only logs. Recovery (chain plugin_startup): a force_resync marker wipes state so head becomes 0, and the existing empty-state gate re-bootstraps from the trusted snapshot peer, then deletes the marker — the exact path a manual --force-recreate triggers today. The marker is removed only after the wipe, so a crash mid-wipe re-triggers recovery; it composes with the resize marker (wipe clears it too). The marker lives in the shared-memory dir (exposed via chain::plugin::get_state_dir) so it persists exactly as long as the state it condemns.
chiliec
force-pushed
the
feat/self-heal-wedge-watchdog
branch
from
f3ffe3f to
455a767
Compare
…cro-arg UB
The completeness-check block lives inside the db.with_strong_write_lock([&]{...})
lambda, and with_strong_write_lock is itself a function-like macro. The local
#define/#undef CHECK_SECTION therefore sat inside that macro's argument list,
which is undefined behavior — GCC silently dropped the #define, leaving
CHECK_SECTION undeclared and breaking the build.
Replace the macro with 28 direct check_section(...) calls via a small local
lambda; identical behavior, no preprocessor directives inside the macro arg.
| // 2026-07-13 rpc.viz.cx incident). Fork-switch can't recover state below | ||
| // LIB — only wipe + snapshot re-import can. Honor the marker by wiping | ||
| // state so head becomes 0; the empty-state gate later in this function then | ||
| // re-bootstraps from a trusted snapshot peer. This is the exact recovery a |
There was a problem hiding this comment.
[major] Recovery can re-import a self-created snapshot and re-wedge (crash loop). This comment says the empty-state gate "re-bootstraps from a trusted snapshot peer", but the gate further down first honors my->snapshot_path (set from --snapshot / --snapshot-auto-latest). db.wipe(..., true) deletes shared_memory.bin + block_log but not the snapshots/ dir. On a node that serves/creates periodic snapshots (rpc.viz.cx runs allow-snapshot-serving), auto-latest can rediscover a snapshot generated from the already-corrupt state, re-import it, and wedge again. The new completeness checks (commit 1) may not break the loop — see the count-reconciliation note. Suggest forcing trusted-peer bootstrap on force_resync: skip the local snapshot_path (or ignore snapshots/) for this boot so recovery pulls a fresh snapshot from a trusted peer, matching the comment's stated intent.
There was a problem hiding this comment.
Fixed in 5b1d4cd. On a force_resync boot the recovery block now clears my->snapshot_path and sets my->auto_recover_from_snapshot = false (plugins/chain/plugin.cpp:559-560) before the snapshot-load gate. So neither --snapshot / --snapshot-auto-latest nor a node-local snapshots/ file (which db.wipe(..., true) does not remove) can re-import state serialized from the corrupt db. The node comes up empty and re-bootstraps from a trusted snapshot peer (or full P2P resync), matching the comment's stated intent. Confirmed the production config.ini relies on sync-snapshot-from-trusted-peer + trusted-snapshot-peer for exactly this path (no in-wire snapshot fetch otherwise).
| return; | ||
| } | ||
|
|
||
| bool rejects_climbed = (_gap_rejected_total > _wedge_reject_baseline); |
There was a problem hiding this comment.
[major] rejects_climbed is sticky for the whole window, weakening the partition safeguard. _wedge_reject_baseline is captured once at window open (L3793), so a single rejection anywhere in the 900s window keeps rejects_climbed == true until WEDGE_CONFIRM_SEC. The design intent is "rejections still climbing" — to tell genuine divergence apart from a partition. As written, a node that took one early rejection and then went silent (partial isolation) still satisfies the predicate; with the flag ON it would wipe good state. Suggest re-baselining per relog interval (or requiring a rejection within the last WEDGE_RELOG_SEC) so "climbing" means sustained, not "happened at least once".
There was a problem hiding this comment.
Fixed in 5b1d4cd. Replaced the once-per-window baseline with recency: _wedge_last_reject is stamped on each observed rejection, and rejects_climbed is true only when a rejection occurred within the last WEDGE_RELOG_SEC (dlt_p2p_node.cpp:3844-3845). A node that took one early rejection and then went silent no longer satisfies the predicate — "climbing" now means sustained, which is what distinguishes genuine divergence from a partial isolation.
| uint32_t our_head = _delegate->get_head_block_num(); | ||
| if (our_head == 0) return; // not bootstrapped yet (or mid snapshot import) | ||
|
|
||
| uint32_t behind = (_highest_seen_block_num > our_head) |
There was a problem hiding this comment.
[major] _highest_seen_block_num is monotonic and peer-influenced — one bogus advertised head permanently arms the watchdog. It only ever increases (set from peer messages at ~L1506/1691/1920) and is never validated or clamped. A single malicious/buggy peer advertising a head far above the real tip makes behind huge forever, so the "behind" gate stays permanently open. Combined with head-flat + any rejection (flag ON), this is an operator-invisible route to an auto-wipe under an eclipse-style scenario. Suggest clamping an implausibly-far tip (e.g. vs local wall-clock/expected rate) or requiring corroboration from >=2 peers before feeding it into the wedge decision.
There was a problem hiding this comment.
Fixed in 5b1d4cd. The wedge decision no longer feeds on the monotonic _highest_seen_block_num. network_tip is now the second-highest head across established (SYNCING/ACTIVE) peers, and requires at least WEDGE_MIN_CORROBORATING_PEERS of them (dlt_p2p_node.cpp:3782-3794); when under-corroborated the tip collapses to 0, behind collapses to 0, and the watchdog disarms. A single outlier/malicious advertised head can no longer move the tip or arm an eclipse-style auto-wipe.
| // multi-instance index is cleared before import (see clear block above), | ||
| // so the live index size must equal the recorded count. A mismatch | ||
| // means the import silently dropped objects. | ||
| auto expect_count = [&](const std::string& section, size_t actual) { |
There was a problem hiding this comment.
[medium] Count-reconciliation cannot detect export-side incompleteness — which is the actual incident class. header.object_counts is computed at export from the same serialized arrays (plugin.cpp:1177). If the serving node serialized incomplete state (the viz-social-bot incident), the recorded count already equals the short array, so on import live == recorded and this passes. A whole section emptied by a chainbase type-enum shift reads 0 == 0 too. The only real net for the incident is the account_authority -> account referential check below, and only if the dangling authority survived. The stronger Finding-A check (dgp.current_supply vs summed balances) is not implemented — consider adding it. Also worth softening the commit message, which presents count-reconciliation as catching the incident.
There was a problem hiding this comment.
Acknowledged — this is the real incident class and count-reconciliation cannot catch it. The in-code comment now documents that the section counts only catch lossy import, and that the account_authority -> account referential check is the only existing net for export-side gaps (and only if a dangling ref survives).
The proper detector is a value invariant (current_supply vs summed balances; total_vesting_shares vs Σ account.vesting_shares). database::validate_invariants() is declared-only — no definition anywhere in git history — so this needs the accounting written from scratch and, critically, validated against a real mainnet snapshot before arming; a wrong invariant would reject valid snapshots, which is worse than the status quo. Deferred to a follow-up on a build machine (no build env / real snapshot in this round). The full accounting model is designed: the TOKEN identity (current_supply == Σ balances + reserved + escrow + invite + vesting_fund + reward_fund + committee_fund + Σ validator.pending_stakeholder_reward) plus a safe SHARES-only fallback (total_vesting_shares == Σ account.vesting_shares, delegations net to zero). It'll be implemented as a soft ++invariant_failures in the same post-import block so a mismatch triggers the retry-another-peer path.
| if (boost::filesystem::exists(shm)) boost::filesystem::remove(shm); | ||
| } | ||
| // Remove the marker only after the wipe so a crash mid-wipe re-triggers recovery. | ||
| boost::filesystem::remove(force_resync_marker); |
There was a problem hiding this comment.
[minor] This remove() can throw and abort plugin_startup. It sits outside the surrounding try/catch. If it throws (file locked, permission), recovery aborts here; with the crash-safe re-trigger design that can turn into a crash loop. Use the non-throwing error_code overload.
| boost::filesystem::remove(force_resync_marker); | |
| boost::system::error_code _rm_ec; | |
| boost::filesystem::remove(force_resync_marker, _rm_ec); |
There was a problem hiding this comment.
Fixed in 5b1d4cd — the marker removal now uses the non-throwing error_code overload and logs on failure (plugins/chain/plugin.cpp:566-570), so a locked/permission-denied marker can no longer throw out of plugin_startup.
| // preprocessor directive inside a macro argument list is undefined | ||
| // behavior (GCC silently drops the #define, leaving CHECK_SECTION | ||
| // undeclared and the build broken). | ||
| auto check_section = [&](const std::string& section, size_t actual) { |
There was a problem hiding this comment.
[minor] check_section is a trivial pass-through to expect_count. It duplicates behavior (future changes must touch two lambdas). Now that the #define/#undef is gone, the 28 call sites can call expect_count(...) directly and this wrapper can be dropped.
There was a problem hiding this comment.
Fixed in 5b1d4cd — dropped the check_section pass-through; the call sites now invoke expect_count(...) directly (plugins/snapshot/plugin.cpp:1832+).
…t, no re-wedge loop Addresses On1x review on PR #125: [major] Recovery re-import loop (plugins/chain/plugin.cpp): db.wipe() does not clear the snapshots/ dir, so on a snapshot-serving node --snapshot-auto-latest could rediscover a snapshot serialized FROM the corrupt state and re-wedge. On force_resync, drop the local snapshot_path and disable auto_recover_from_snapshot so no node-local snapshot is silently re-imported; the node comes up empty and resyncs from the network. Corrected the comments: there is no in-wire snapshot-fetch-from-peer, recovery is network resync or an operator-supplied snapshot. [major] Sticky rejects (dlt_p2p_node): _wedge_reject_baseline was captured once at window open, so a single early rejection kept rejects_climbed true for the whole 900s window (a node that rejected once then went silent would falsely confirm). Replace with recency tracking: "climbing" now means a rejection was observed within the last WEDGE_RELOG_SEC — sustained, not "happened once". [major] Unvalidated network tip (dlt_p2p_node): _highest_seen_block_num is monotonic and peer-influenced, so one bogus advertised head armed the watchdog forever (eclipse-style auto-wipe). Derive a corroborated tip = 2nd-highest head across established (SYNCING/ACTIVE) peers, requiring >= WEDGE_MIN_CORROBORATING_PEERS; a single outlier can no longer move it, and an under-corroborated node never arms. [medium] Count-reconciliation scope (plugins/snapshot/plugin.cpp): documented that header.object_counts is derived at export from the same arrays, so check (1) catches lossy IMPORT only, not export-side incompleteness; the referential check is the real net for the incident. The stronger supply-vs-balances invariant is deferred (database::validate_invariants() is declared but not defined; a hand-rolled partial accounting would risk rejecting valid snapshots). [minor] Marker remove() could throw outside the try/catch and abort plugin_startup (crash loop) — use the non-throwing error_code overload (also for the fallback shared_memory.bin remove). [minor] Dropped the trivial check_section pass-through; the 28 sites now call expect_count directly.
The wedge watchdog only takes its destructive wipe+resync action when the pure is_wedged() predicate returns true, so pin its boolean contract with a regression guard. The predicate's semantics changed in the review-fix round (rejects_climbed is now recency-based, behind is corroborated-tip-based) though its signature did not. - Promote is_wedged() and its WEDGE_BEHIND_THRESHOLD / WEDGE_CONFIRM_SEC thresholds into a public: block so the test can assert boundaries symbolically. The predicate was documented as table-testable but was actually private; out-of-line constexpr defs and other .cpp references are unaffected by the access change. - Add tests/consensus_sim/scenarios/test_wedge_predicate.cpp: 8 cases covering the sole confirming combination, each dimension vetoed individually, and both boundaries (behind strictly >, elapsed inclusive >=). Wired into the existing Boost.Test runner and linked graphene::network. Runs only under -DBUILD_CONSENSUS_TESTS=ON + ctest (opt-in, as with the other consensus_sim scenarios); the current Docker CI builds only vizd/cli_wallet so it compiles the header change but not this test.
|
@On1x thanks for the review — all 6 threads addressed and replied inline. Summary: Fixed in
Deferred (with sign-off) to a build-machine follow-up:
Also added this round: a truth-table unit test for the |
…riant feat(snapshot): supply/value conservation invariants on import (#125 follow-up)
…126) Brings the wedged-behind-network watchdog (#125) and the snapshot post-import completeness/supply invariants (#126) onto the PM branch. The #125 checks (count reconciliation, referential integrity, singletons) and the #126 SHARES conservation check are PM-agnostic and stay fatal. The #126 TOKEN supply invariant is extended for PM: PM is zero-sum, so a bet / liquidity / lazy-deposit / commit-escrow / dispute-fee / oracle- insurance / leverage-collateral moves TOKEN out of account.balance into a PM object field. Those pools are now summed (minimal non-overlapping set, duplicates such as bets_sum/reserves, empty-provider lazy LP, and deposit.principal excluded). Because this PM accounting is not yet satoshi-validated against real PM snapshots, the TOKEN check is LOG-ONLY on this branch (per-component delta logged, import NOT failed) until reconciled to delta==0 and re-armed as a fatal FC_ASSERT. Tracked in #127 with a TODO(pm) at the check. Conflicts: tests/consensus_sim/CMakeLists.txt (kept both test_pm_lifecycle and test_wedge_predicate scenarios).
2 participants
Background
A snapshot-synced node whose committed state is missing an object rejects every canonical block that references it (
out_of_rangeatapply_block). This is a rejection livelock, distinct from the #117–#120 write-lock deadlock monitors:push_blockfails fast, so those monitors never fire. The divergence sits at/below LIB, so fork-switch can't recover — only wipe + snapshot re-import does, and that was gated on empty state, so it never self-triggered. The 2026-07-13 rpc.viz.cx outage sat frozen ~14h and needed a manualdocker compose up -d --force-recreate.Design spec:
docs/self-heal-wedge-watchdog-plan.md(included).What's in this PR (2 commits)
1.
feat(snapshot)— post-import completeness/invariant checks (independent, lands first per rollout §5)header.object_countsvs live index sizes for all multi-instance indices (catches a lossy import).account_authoritymust reference an existing account — the exact invariant the wedge violated.dynamic_global_property,validator_schedule,hardfork_property).FC_ASSERTaborts the import — loud + retryable: the P2P snapshot-sync path rejects the incomplete snapshot and tries another trusted peer.2.
feat(p2p)— wedged-behind-network watchdog + recoverydlt_p2p_node::check_wedge_watchdog, inperiodic_task): WEDGED when, sustainedWEDGE_CONFIRM_SEC(900s):behind > 200AND head flat AND gap-fill rejections still climbing. Triple-AND rules out a healthy syncing node (head advancing) and a partition (no rejections). New monotonic_gap_rejected_totaldrives the "still rejecting" signal (_gap_rejected_countoscillates on blacklist). Pure predicateis_wedged()factored out for table-testing.auto-resync-on-wedge(default OFF this release; observe elog/marker in the wild first): elog + writeforce_resyncmarker next toshared_memory.bin+std::_Exit(2). OFF ⇒ logs only.plugin_startup): aforce_resyncmarker wipes state so head→0, the existing empty-state gate re-bootstraps from the trusted snapshot peer, then deletes the marker — the exact path a manual--force-recreatetriggers. Marker removed only after wipe (crash-safe re-trigger); composes with the resize marker. Lives in the shared-memory dir (via newchain::plugin::get_state_dir) so it persists exactly as long as the state it condemns.The full build could not be run locally (dev machine has Boost 1.90, which removed
boost::asio::io_service; project pins Boost ≤1.86 — an environment mismatch unrelated to these changes). Verified locally as far as possible:dlt_p2p_node.cpp,p2p_plugin.cpp:-fsyntax-onlyclean.chain/plugin.cpp: additions clean (only pre-existing Boost-1.90 API noise on untouched lines).snapshot/plugin.cpp: new block compiles clean in isolation against faithful API mocks (full-file check is parser-desynced by Boost-1.90 fc-reflection errors); all APIs verified against real headers.Please confirm the Docker/Linux CI build is green before merge.
Not in scope
is_wedged— need the build environment; the predicate is already factored public/static for that.🤖 Generated with Claude Code