mirror of
https://github.com/luxfi/node.git
synced 2026-07-27 03:39:39 +00:00
zeekayandHanzo Dev
49e6958664
Two pre-existing reliability bugs surfaced by the v1.32.1 roll (both present
identically in v1.31.5 — NOT caused by QCv2), plus a snapshot-inconsistency
diagnostic.
BUG 1 (HIGH) — concurrent-map crash in getPostForkBlock. The consensus engine
drives the proposervm from multiple handler goroutines concurrently (a
PullQuery/Put verifying a block WRITES verifiedBlocks via verifyAndRecordInnerBlk
while a Qbit handler READS it via GetBlock -> getPostForkBlock; the retry loop in
PullQuery even documents "allow concurrent Put to complete"). The verifiedBlocks
map was read/written with no lock -> the Go runtime aborts with "fatal error:
concurrent map read and map write" (exit 2) under heavy verify load. Fix: a
dedicated verifiedBlocksLock (RWMutex) behind three accessors (cachedVerifiedBlock
/ recordVerifiedBlock / forgetVerifiedBlock) at ALL seven sites; the sibling
Tree.nodes map (touched on the same Verify/Accept paths) gets its own RWMutex,
with Accept restructured to make its inner-VM Accept/Reject callouts OUTSIDE the
lock. Both are leaf locks, never nested, never held across a callout — the lock
graph is a strict DAG (vm.lock > verifiedBlocksLock; tree.lock disjoint), so
deadlock-free. innerBlkCache (lru.SizedCache) is already internally locked.
Regression: TestVerifiedBlocksConcurrentAccess + TestTreeConcurrentAccess hammer
readers vs writers under -race; both FAIL on the pre-fix code with the exact
production signatures ("concurrent map read and map write" / "concurrent map
writes") and pass after.
BUG 3 — a proposervm height index BELOW the inner VM tip (the devnet-C
"index 7 < inner 8" from a snapshot restored inconsistently across the proposervm
and EVM databases) had a cryptic fatal ("should never be lower"). It is now a
LOUD, ACTIONABLE fatal with a recovery runbook. It is deliberately NOT
"self-healed" by dropping the finality pointer: after DeleteLastAccepted,
proposervm.LastAccepted() falls back to the INNER-namespace id, whose ParentID is
contiguity-incompatible with the network's OUTER wrappers — that permanently
wedges bootstrap (first-block anchor), catch-up (parent==tip guard) and live
Verify (parent lookup) at the inner tip (blocks at height <= tip are skipped, so
the missing wrapper is never rebuilt): a SILENT wedge strictly worse than the
loud stop. The only correct remedy is operator action (restore a consistent
snapshot or full resync), which the error now states. The reconciliation decision
is decomplected into a pure classifyHeightRepair(pro, inner) -> heightRelation,
regression-locked by TestClassifyHeightRepair so a revert to a silent reset fails
the test; the fork height is read lazily (only the ahead/rollback path needs it),
so the match and behind paths gain no new failure mode.
RED-reviewed: CHANGE 1 (locks) cleared (deadlock-free DAG, fail-on-old proven);
the original bug-3 silent-reset was caught as a wedge and reworked to this
fail-loud form. Full vms/proposervm/... suite green under -race.
Co-authored-by: Hanzo Dev <dev@hanzo.ai>