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>
59 lines
2.4 KiB
Go
59 lines
2.4 KiB
Go
// Copyright (C) 2019-2025, Lux Industries Inc. All rights reserved.
|
|
// See the file LICENSE for licensing terms.
|
|
|
|
package proposervm
|
|
|
|
import "testing"
|
|
|
|
// TestClassifyHeightRepair locks the init-time reconciliation policy between the
|
|
// proposervm finality index and the inner VM's accepted tip.
|
|
//
|
|
// The load-bearing case is heightBehind: proposervm BELOW the inner (e.g. the
|
|
// devnet-C "index 7 < inner 8" from a snapshot restored inconsistently across
|
|
// the proposervm and EVM databases). It MUST classify as heightBehind — which
|
|
// the caller turns into a LOUD, actionable fatal — and must NEVER be treated as
|
|
// heightMatch (a no-op that leaves the node inconsistent) or heightAhead (a
|
|
// rollback). It must in particular never be "self-healed" by dropping the
|
|
// finality pointer: that leaves proposervm.LastAccepted() in the inner-id
|
|
// namespace and permanently wedges bootstrap/catch-up/live at the inner tip.
|
|
// A regression back to that silent reset would have to reclassify this case and
|
|
// fail here.
|
|
func TestClassifyHeightRepair(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
proHeight, innerHeight uint64
|
|
want heightRelation
|
|
}{
|
|
{"heights match => nothing", 8, 8, heightMatch},
|
|
{"heights match at genesis", 0, 0, heightMatch},
|
|
|
|
// The bug-3 case: proposervm index behind the inner tip is unrecoverable
|
|
// locally and must be a loud fatal, never a silent reset or a rollback.
|
|
{"behind by one (devnet-C 7<8)", 7, 8, heightBehind},
|
|
{"behind by many", 100, 4242, heightBehind},
|
|
{"behind at genesis boundary", 0, 1, heightBehind},
|
|
|
|
// Proposervm ahead: the inner rolled back; roll the proposervm back.
|
|
{"ahead by one", 9, 8, heightAhead},
|
|
{"ahead by many", 4242, 100, heightAhead},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
got := classifyHeightRepair(tt.proHeight, tt.innerHeight)
|
|
if got != tt.want {
|
|
t.Fatalf("classifyHeightRepair(%d,%d) = %d, want %d",
|
|
tt.proHeight, tt.innerHeight, got, tt.want)
|
|
}
|
|
})
|
|
}
|
|
|
|
// Explicit safety assertion: the behind-index case must be heightBehind (loud
|
|
// fatal), NEVER heightMatch (silent inconsistency) or heightAhead (rollback).
|
|
// This is the exact regression RED flagged: a silent finality-pointer reset
|
|
// here wedges the node forever.
|
|
if got := classifyHeightRepair(7, 8); got != heightBehind {
|
|
t.Fatalf("behind-index (7<8) must classify heightBehind (loud fatal), got %d", got)
|
|
}
|
|
}
|