mirror of
https://github.com/luxfi/node.git
synced 2026-07-27 03:39:39 +00:00
fix(consensus): wire the engine's catch-up transport — un-strand behind followers
node built the consensus engine with netCfg.Catchup unset, so the engine's requestCatchup() hit a nil interface: a follower that fell behind DURING consensus (it drives blocks by gossip, not Put/PushQuery) could never fetch the missing ancestors and looped on an unfinalizable orphan forever. Observed live — after the mainnet RLP recovery, luxd-0/2 stranded at 1082780 while peers reached 1082793. (Bootstrap had GetAncestors wired, so startup sync worked; only the live-engine transport was unplugged — the same class of gap as the GossipOp vote-transport bug.) Fix is the decomplected bridge avalanche's design implies: the engine DECIDES when to catch up (chain.Catchup); the blockHandler owns the WIRE — it already has requestContext (send GetAncestors) and handleContext->Put->engine (deliver the fetched ancestors). networkCatchup routes one to the other through a one-method ancestorRequester interface (testable, narrow, no consensus/network cross-import), late-bound at chainInfo construction since the handler wraps the engine. Behind followers now self-heal without a restart. RED test: a nil wire leaves calls=0 (the bug); the wired bridge routes RequestAncestors -> requestContext once, for the missing block and the advertising peer; + a compile-time guard that *blockHandler stays a valid ancestorRequester.
This commit is contained in:
@@ -0,0 +1,56 @@
|
||||
// Copyright (C) 2019-2025, Lux Industries, Inc. All rights reserved.
|
||||
// See the file LICENSE for licensing terms.
|
||||
|
||||
package chains
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/luxfi/ids"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// *blockHandler must satisfy ancestorRequester, or the catch-up wire silently
|
||||
// loses its transport. Catch it at compile time, not in production.
|
||||
var _ ancestorRequester = (*blockHandler)(nil)
|
||||
|
||||
// catchupSpy records the requestContext calls networkCatchup bridges to.
|
||||
type catchupSpy struct {
|
||||
calls int
|
||||
lastFrom ids.NodeID
|
||||
lastBlock ids.ID
|
||||
}
|
||||
|
||||
func (s *catchupSpy) requestContext(_ context.Context, from ids.NodeID, blockID ids.ID) {
|
||||
s.calls++
|
||||
s.lastFrom = from
|
||||
s.lastBlock = blockID
|
||||
}
|
||||
|
||||
// TestNetworkCatchup_BridgesEngineSignalToWire is the regression for the
|
||||
// stranded-follower bug: node never set netCfg.Catchup, so the engine's
|
||||
// requestCatchup hit a nil interface and a follower that fell behind during
|
||||
// consensus never fetched the missing ancestors — it looped on an unfinalizable
|
||||
// orphan forever (observed live: mainnet luxd-0/2 stuck at 1082780 while peers
|
||||
// reached 1082793). The wire must (a) be nil-safe before its handler is
|
||||
// late-bound and (b) route the engine's RequestAncestors to the handler's
|
||||
// GetAncestors transport (requestContext), once, for the right block and peer.
|
||||
func TestNetworkCatchup_BridgesEngineSignalToWire(t *testing.T) {
|
||||
missing := ids.GenerateTestID()
|
||||
peer := ids.GenerateTestNodeID()
|
||||
|
||||
// (a) Before late-binding (handler nil): a harmless no-op, never a panic.
|
||||
c := &networkCatchup{}
|
||||
require.NoError(t, c.RequestAncestors(ids.Empty, ids.Empty, missing, peer))
|
||||
|
||||
// (b) Once wired: the engine's catch-up signal reaches the GetAncestors wire
|
||||
// exactly once — for the missing block, addressed to the peer that advertised
|
||||
// its child. RED before the fix: handler is never set, calls stays 0.
|
||||
spy := &catchupSpy{}
|
||||
c.handler = spy
|
||||
require.NoError(t, c.RequestAncestors(ids.Empty, ids.Empty, missing, peer))
|
||||
require.Equal(t, 1, spy.calls, "RequestAncestors must route to requestContext — a nil wire IS the stranded-follower bug")
|
||||
require.Equal(t, missing, spy.lastBlock)
|
||||
require.Equal(t, peer, spy.lastFrom)
|
||||
}
|
||||
+50
-5
@@ -1287,6 +1287,12 @@ func (m *manager) buildChain(chainParams ChainParameters, sb nets.Net) (*chainIn
|
||||
// to Start; without the gossiper Mode() reports degraded and value-DEX is
|
||||
// refused. For K==1 these stay nil (no quorum, no signatures).
|
||||
gossiper := &networkGossiper{net: m.Net, msgCreator: m.MsgCreator, networkID: networkID}
|
||||
// catchup is the behind-follower self-heal wire. The engine signals through
|
||||
// it (chain.Catchup) when a gossiped child references a parent we lack; it
|
||||
// routes to the blockHandler's GetAncestors transport. The handler wraps the
|
||||
// engine built just below, so its handler ref is late-bound at chainInfo
|
||||
// construction (a no-op until then — see networkCatchup).
|
||||
catchup := &networkCatchup{}
|
||||
netCfg := consensuschain.NetworkConfig{
|
||||
ChainID: chainParams.ID,
|
||||
NetworkID: networkID,
|
||||
@@ -1294,6 +1300,7 @@ func (m *manager) buildChain(chainParams ChainParameters, sb nets.Net) (*chainIn
|
||||
Validators: m.Validators, // CRITICAL: Pass validator sampler for k-peer polling
|
||||
Logger: m.Log,
|
||||
Gossiper: gossiper,
|
||||
Catchup: catchup, // fetch missing ancestors so a behind follower converges
|
||||
VM: blockBuilder,
|
||||
Params: &consensusParams,
|
||||
}
|
||||
@@ -1435,17 +1442,23 @@ func (m *manager) buildChain(chainParams ChainParameters, sb nets.Net) (*chainIn
|
||||
if chainName == "" {
|
||||
chainName = chainRuntime.ChainID.String()
|
||||
}
|
||||
// Handler parses inbound blocks through the SAME builder the engine emits
|
||||
// through (blockBuilder == the P-chain-height wrapper on K>1, the inner VM
|
||||
// on K==1), so the container bytes it parses match the bytes the engine
|
||||
// framed — one codec, no raw-vs-wrapped split.
|
||||
bh := newBlockHandler(blockBuilder, m.Log, consensusEngine, m.Net, m.MsgCreator, chainParams.ID, networkID)
|
||||
// Late-bind the handler as the engine's catch-up wire: it owns requestContext
|
||||
// (send GetAncestors) and handleContext -> Put -> engine (deliver the fetched
|
||||
// ancestors), so a follower that falls behind now self-heals instead of
|
||||
// looping on an unfinalizable orphan.
|
||||
catchup.handler = bh
|
||||
createdChain = &chainInfo{
|
||||
Name: chainName,
|
||||
VMID: chainParams.VMID,
|
||||
Runtime: chainRuntime,
|
||||
VM: vmTyped, // Use the real VM directly
|
||||
Engine: consensusEngine, // Use real consensus engine directly
|
||||
// Handler parses inbound blocks through the SAME builder the engine emits
|
||||
// through (blockBuilder == the P-chain-height wrapper on K>1, the inner VM
|
||||
// on K==1), so the container bytes it parses match the bytes the engine
|
||||
// framed — one codec, no raw-vs-wrapped split.
|
||||
Handler: newBlockHandler(blockBuilder, m.Log, consensusEngine, m.Net, m.MsgCreator, chainParams.ID, networkID),
|
||||
Handler: bh,
|
||||
}
|
||||
default:
|
||||
return nil, fmt.Errorf("unsupported VM type: %T", vmImpl)
|
||||
@@ -3225,6 +3238,38 @@ func (n *noopWarpSender) SendGossip(ctx context.Context, config warp.SendConfig,
|
||||
// networkGossiper implements consensuschain.Gossiper for Lux consensus integration.
|
||||
// It adapts the node's network layer to the minimal Gossiper interface used by
|
||||
// the integrated consensus engine.
|
||||
// ancestorRequester is the one capability networkCatchup needs from the inbound
|
||||
// handler: ask a peer for the chain of blocks ending at a missing block.
|
||||
// *blockHandler provides it via requestContext (GetAncestors out; the Ancestors
|
||||
// response flows handleContext -> Put -> engine). Depending on this one-method
|
||||
// interface (not the concrete handler) keeps the bridge testable and narrow.
|
||||
type ancestorRequester interface {
|
||||
requestContext(ctx context.Context, from ids.NodeID, blockID ids.ID)
|
||||
}
|
||||
|
||||
// networkCatchup is the node-side wire for the consensus engine's chain.Catchup
|
||||
// interface. The engine DECIDES when to catch up — followVerifiedBlock sees a
|
||||
// gossiped child whose parent it lacks and calls requestCatchup; networkCatchup
|
||||
// ROUTES that to the handler's GetAncestors transport. Splitting "decide" (engine,
|
||||
// no network types) from "transport" (handler, no consensus policy) keeps each in
|
||||
// its lane. Without this wire a follower that falls behind during consensus loops
|
||||
// on an unfinalizable orphan forever — the stranded-follower bug.
|
||||
//
|
||||
// The handler wraps the engine and is built after it, so `handler` is late-bound
|
||||
// once both exist. Until then RequestAncestors is a harmless no-op: no block can
|
||||
// be missing before the engine has even started.
|
||||
type networkCatchup struct{ handler ancestorRequester }
|
||||
|
||||
// RequestAncestors satisfies chain.Catchup. chainID/networkID are already fixed
|
||||
// per-handler (one handler per chain), so the wire needs only the missing block
|
||||
// and the peer that advertised its child.
|
||||
func (c *networkCatchup) RequestAncestors(_ ids.ID, _ ids.ID, missingBlockID ids.ID, from ids.NodeID) error {
|
||||
if c.handler != nil {
|
||||
c.handler.requestContext(context.Background(), from, missingBlockID)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
type networkGossiper struct {
|
||||
net network.Network
|
||||
msgCreator message.OutboundMsgBuilder
|
||||
|
||||
Reference in New Issue
Block a user