mirror of
https://github.com/luxfi/node.git
synced 2026-07-27 03:39:39 +00:00
The uptime event-delivery plumbing (node/chain_router.go + chains/manager.go blockHandler) dropped two invariants avalanchego's handler upholds — the consensus lock and the peer version — each a mainnet-fleet crash. The uptime tracker itself (vms/platformvm/uptime_tracker.go) is RED-cleared and UNCHANGED. CRITICAL #2 — P-chain state race (concurrent map writes -> fatal): chainRouter dispatched Connected/Disconnected on the peer-lifecycle goroutine, where VM.Disconnected -> tracker.Disconnect (state.SetUptime) + state.Commit (state.write) ran concurrently with the block acceptor's state.CommitBatch (state.write) on the engine accept goroutine — no shared lock, so ordinary peer churn triggered Go "concurrent map writes" and crashed the P-chain node. Fix: one VM-owned stateLock serializes every commit of shared platform state that originates outside the engine's lock-free VM.Accept call-out — - block DECISION: block/executor Block.Accept/Reject hold &vm.stateLock (supplied to executor.NewManager) around the whole acceptor visit; - peer/lifecycle: VM.Disconnected and the onReady/onBootstrapStarted/Shutdown Start/StopTracking uptime flushes hold vm.stateLock. This is avalanchego's ctx.Lock invariant (accept serialized with engine.Connected/Disconnected), scoped to the state the platform VM owns and implemented at the VM: the Lux engine invokes VM.Accept as a lock-free call-out (no ctx.Lock over accept exists) and a chain-agnostic blockHandler cannot reach a per-VM accept lock, so routing "through the engine" is not feasible. CRITICAL #1 — C-Chain nil-version panic on state sync: blockHandler.Connected passed connector.Connected(ctx, nodeID, nil). proposervm promotes Connected to coreth, whose state-sync peer tracker compares peer versions; a nil version deref panics any C-Chain node running state sync (a fresh join OR a validator rejoining after falling behind — the launch's core invariant). Fix: plumb the REAL peer version through a versionedConnector capability — chainRouter.Connected converts the node peer version (luxfi/node/version) to the VM boundary type (luxfi/version = chain.VersionInfo) and delivers it via blockHandler.ConnectedWithVersion -> connector.Connected. Audit: geth in-process Connected is a no-op stub (nil-safe); xvm stores the pointer without deref (nil-safe); the real coreth plugin derefs -> fixed by feeding the real version. Tests (SDKROOT + CGO_ENABLED=1, -race): - vms/platformvm/uptime_state_race_test.go: state.Commit (VM.Disconnected path) concurrent with state.CommitBatch (acceptor path) under the shared lock — no race/fatal; verified meaningful (an unlocked probe trips DATA RACE). - chains/blockhandler_connected_version_test.go + node/chain_router_connected_version_test.go: the real, converted version reaches the connector non-nil (dedup covered). Uptime tracker 13/13 and the block/executor reward gate untouched and green; go build ./... and go vet clean. Co-authored-by: Hanzo Dev <dev@hanzo.ai>
108 lines
3.9 KiB
Go
108 lines
3.9 KiB
Go
// Copyright (C) 2019-2025, Lux Industries Inc. All rights reserved.
|
|
// See the file LICENSE for licensing terms.
|
|
|
|
package chains
|
|
|
|
import (
|
|
"context"
|
|
"sync"
|
|
"testing"
|
|
|
|
"github.com/stretchr/testify/require"
|
|
|
|
"github.com/luxfi/ids"
|
|
"github.com/luxfi/log"
|
|
version "github.com/luxfi/version"
|
|
"github.com/luxfi/vm/chain"
|
|
)
|
|
|
|
// recordingConnector is a chain.ChainVM that records the version it is handed on
|
|
// Connected. Every other method comes from the embedded (nil) interface and is
|
|
// never invoked by the connect path.
|
|
type recordingConnector struct {
|
|
chain.ChainVM
|
|
|
|
mu sync.Mutex
|
|
called bool
|
|
gotVersion *version.Application
|
|
}
|
|
|
|
func (c *recordingConnector) Connected(_ context.Context, _ ids.NodeID, nodeVersion *chain.VersionInfo) error {
|
|
c.mu.Lock()
|
|
defer c.mu.Unlock()
|
|
c.called = true
|
|
c.gotVersion = nodeVersion
|
|
return nil
|
|
}
|
|
|
|
func (c *recordingConnector) Disconnected(context.Context, ids.NodeID) error { return nil }
|
|
|
|
// TestBlockHandlerConnectedWithVersionDeliversRealVersion is the regression
|
|
// guard for RED CRITICAL #1 (C-Chain nil-version panic).
|
|
//
|
|
// Before the fix blockHandler.Connected forwarded connector.Connected(ctx,
|
|
// nodeID, nil) — a hardcoded nil version. proposervm promotes Connected to the
|
|
// inner C-Chain VM (coreth), whose state-sync peer tracker compares peer
|
|
// versions; a nil version there dereferences nil (version.Application.Compare)
|
|
// and PANICS a state-syncing node (a fresh join, or a validator rejoining after
|
|
// falling behind — the launch's core invariant).
|
|
//
|
|
// The fix plumbs the REAL peer version through ConnectedWithVersion to the
|
|
// connector. This test drives the real blockHandler with a real version and
|
|
// asserts the connector receives that non-nil version — never nil.
|
|
func TestBlockHandlerConnectedWithVersionDeliversRealVersion(t *testing.T) {
|
|
require := require.New(t)
|
|
|
|
rc := &recordingConnector{}
|
|
bh := newBlockHandler(
|
|
nil, // BlockBuilder — unused by the connect path
|
|
rc, // connector under test
|
|
log.Noop(), // logger
|
|
nil, // engine
|
|
nil, // net
|
|
nil, // msgCreator
|
|
ids.Empty, // chainID
|
|
ids.Empty, // networkID
|
|
nil, // beacons
|
|
ids.NodeID{}, // selfNodeID
|
|
false, // expectsStakedBeacons
|
|
)
|
|
|
|
nodeID := ids.GenerateTestNodeID()
|
|
peerVersion := &version.Application{Name: "lux", Major: 1, Minor: 36, Patch: 27}
|
|
|
|
require.NoError(bh.ConnectedWithVersion(context.Background(), nodeID, peerVersion))
|
|
|
|
rc.mu.Lock()
|
|
defer rc.mu.Unlock()
|
|
require.True(rc.called, "connector.Connected must be invoked")
|
|
require.NotNil(rc.gotVersion, "connector must receive a NON-nil version (coreth dereferences it in state-sync)")
|
|
require.Equal("lux", rc.gotVersion.Name)
|
|
require.Equal(1, rc.gotVersion.Major)
|
|
require.Equal(36, rc.gotVersion.Minor)
|
|
require.Equal(27, rc.gotVersion.Patch)
|
|
}
|
|
|
|
// TestBlockHandlerConnectedDedupsButStillCarriesVersion confirms the version
|
|
// survives the once-only dedup: the first (versioned) dispatch reaches the
|
|
// connector; a duplicate is a no-op (not a nil-version overwrite).
|
|
func TestBlockHandlerConnectedDedupsButStillCarriesVersion(t *testing.T) {
|
|
require := require.New(t)
|
|
|
|
rc := &recordingConnector{}
|
|
bh := newBlockHandler(nil, rc, log.Noop(), nil, nil, nil, ids.Empty, ids.Empty, nil, ids.NodeID{}, false)
|
|
|
|
nodeID := ids.GenerateTestNodeID()
|
|
peerVersion := &version.Application{Name: "lux", Major: 1, Minor: 36, Patch: 27}
|
|
|
|
require.NoError(bh.ConnectedWithVersion(context.Background(), nodeID, peerVersion))
|
|
// A duplicate connect (e.g. dispatched again per tracked network) must be a
|
|
// no-op — never a second call that could clobber the stored version with nil.
|
|
require.NoError(bh.ConnectedWithVersion(context.Background(), nodeID, nil))
|
|
|
|
rc.mu.Lock()
|
|
defer rc.mu.Unlock()
|
|
require.NotNil(rc.gotVersion, "the deduped duplicate must not overwrite the real version with nil")
|
|
require.Equal(27, rc.gotVersion.Patch)
|
|
}
|