From f9f9818f81bd87934de713e97193a88300092955 Mon Sep 17 00:00:00 2001 From: iurii Date: Thu, 6 Aug 2026 18:20:30 +0300 Subject: [PATCH 1/3] network/discovery: add TimedConn to track discv5 socket-read liveness --- network/discovery/timed_conn.go | 84 ++++++++++++++++++++ network/discovery/timed_conn_test.go | 110 +++++++++++++++++++++++++++ 2 files changed, 194 insertions(+) create mode 100644 network/discovery/timed_conn.go create mode 100644 network/discovery/timed_conn_test.go diff --git a/network/discovery/timed_conn.go b/network/discovery/timed_conn.go new file mode 100644 index 0000000000..3e019e4b55 --- /dev/null +++ b/network/discovery/timed_conn.go @@ -0,0 +1,84 @@ +package discovery + +import ( + "net" + "net/netip" + "sync/atomic" + "time" + + "github.com/ethereum/go-ethereum/p2p/discover" +) + +// TimedConn wraps a discv5 UDP socket and records when it was last read from. +// +// A discv5 listener drains its socket only through ReadFromUDPAddrPort, so a +// wedged socket — bound but no longer read, leaving discovery silently dead — +// shows up as a last-read timestamp that stops advancing. StaleFor turns that +// into a liveness signal, used by both the operator and boot nodes. +// +// Only ReadFromUDPAddrPort is overridden; writes, Close and LocalAddr fall +// through to the embedded conn. +type TimedConn struct { + discover.UDPConn + + lastReadUnixNano atomic.Int64 + read atomic.Bool // set once the socket has been read at least once + now func() time.Time // overridable in tests; nil means time.Now +} + +var _ discover.UDPConn = (*TimedConn)(nil) + +// NewTimedConn wraps conn, seeding the last-read time so LastRead reports a +// sensible value before the first read. Staleness only arms once the socket has +// actually been read (see StaleFor). +func NewTimedConn(conn *net.UDPConn) *TimedConn { + c := &TimedConn{UDPConn: conn} + c.lastReadUnixNano.Store(c.nowFn().UnixNano()) + return c +} + +func (c *TimedConn) nowFn() time.Time { + if c.now != nil { + return c.now() + } + return time.Now() +} + +// ReadFromUDPAddrPort delegates and, on success, records the read time and marks +// the socket as having been read. Errors leave both untouched, so a closed or +// failing socket can't look freshly drained. +func (c *TimedConn) ReadFromUDPAddrPort(b []byte) (n int, addr netip.AddrPort, err error) { + n, addr, err = c.UDPConn.ReadFromUDPAddrPort(b) + if err == nil { + c.lastReadUnixNano.Store(c.nowFn().UnixNano()) + c.read.Store(true) + } + return n, addr, err +} + +// LastRead reports when a packet was last read from the socket. +func (c *TimedConn) LastRead() time.Time { + return time.Unix(0, c.lastReadUnixNano.Load()) +} + +// ReadStaleness reports how long the socket has gone unread, and whether it has +// ever been read. Before the first successful read there is nothing to measure, +// so ok is false: a socket that has never delivered a packet is not wedged, it +// has simply had no inbound discv5 traffic yet (unreachable bootnodes, blocked +// UDP), which a restart cannot fix. +func (c *TimedConn) ReadStaleness() (age time.Duration, ok bool) { + if !c.read.Load() { + return 0, false + } + return c.nowFn().Sub(c.LastRead()), true +} + +// StaleFor reports whether the socket has wedged: it was being drained, then +// went unread for longer than d. A socket that has never been read is never +// stale — a wedge can only arise after draining has started, so requiring a +// prior read rules out the false positive (a healthy node that has simply had +// no inbound traffic yet) without missing any real wedge. +func (c *TimedConn) StaleFor(d time.Duration) bool { + age, ok := c.ReadStaleness() + return ok && age > d +} diff --git a/network/discovery/timed_conn_test.go b/network/discovery/timed_conn_test.go new file mode 100644 index 0000000000..d2c6f0c5fd --- /dev/null +++ b/network/discovery/timed_conn_test.go @@ -0,0 +1,110 @@ +package discovery + +import ( + "errors" + "net" + "net/netip" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// fakeUDPConn is a minimal discover.UDPConn so the read path can be exercised +// without a real socket. +type fakeUDPConn struct { + readErr error +} + +func (f *fakeUDPConn) ReadFromUDPAddrPort(b []byte) (int, netip.AddrPort, error) { + if f.readErr != nil { + return 0, netip.AddrPort{}, f.readErr + } + n := copy(b, []byte{0x1}) + return n, netip.MustParseAddrPort("1.2.3.4:30303"), nil +} + +func (f *fakeUDPConn) WriteToUDPAddrPort(b []byte, _ netip.AddrPort) (int, error) { + return len(b), nil +} + +func (f *fakeUDPConn) Close() error { return nil } +func (f *fakeUDPConn) LocalAddr() net.Addr { return nil } + +// TestTimedConn_SeededNotStale: a freshly constructed conn is not stale — it has +// not been read yet, so there is nothing to flag at startup. +func TestTimedConn_SeededNotStale(t *testing.T) { + c := NewTimedConn(nil) + require.False(t, c.StaleFor(3*time.Minute)) +} + +// TestTimedConn_NeverReadNeverStale: a socket that has never delivered a packet +// is not a wedge — it's a node with no inbound discv5 traffic (unreachable +// bootnodes, blocked UDP), which a restart cannot fix — so it must never report +// stale, however much time passes. Once it has been read, staleness arms as +// usual. +func TestTimedConn_NeverReadNeverStale(t *testing.T) { + c := NewTimedConn(nil) + t0 := c.LastRead() + + c.now = func() time.Time { return t0.Add(time.Hour) } + require.False(t, c.StaleFor(3*time.Minute), "never-read socket must not be stale") + _, ok := c.ReadStaleness() + require.False(t, ok, "never-read socket has no staleness to report") + + // A successful read arms the signal; from the read time on, it goes stale. + c.UDPConn = &fakeUDPConn{} + _, _, err := c.ReadFromUDPAddrPort(make([]byte, 16)) + require.NoError(t, err) + age, ok := c.ReadStaleness() + require.True(t, ok, "a read arms the signal") + require.Zero(t, age, "staleness resets to the read time") + + c.now = func() time.Time { return t0.Add(time.Hour + 4*time.Minute) } + require.True(t, c.StaleFor(3*time.Minute), "past grace after a read") +} + +// TestTimedConn_StaleForTransitions pins the staleness boundary using an +// injected clock, so no real time passes. +func TestTimedConn_StaleForTransitions(t *testing.T) { + c := NewTimedConn(nil) + c.read.Store(true) // staleness only arms once the socket has been read + t0 := c.LastRead() + + c.now = func() time.Time { return t0.Add(2 * time.Minute) } + require.False(t, c.StaleFor(3*time.Minute), "within grace") + + c.now = func() time.Time { return t0.Add(3 * time.Minute) } + require.False(t, c.StaleFor(3*time.Minute), "exactly at grace is not yet stale") + + c.now = func() time.Time { return t0.Add(4 * time.Minute) } + require.True(t, c.StaleFor(3*time.Minute), "past grace") +} + +// TestTimedConn_ReadStampsLastRead: a successful read advances the timestamp, +// so an actively drained socket stays fresh. +func TestTimedConn_ReadStampsLastRead(t *testing.T) { + c := &TimedConn{UDPConn: &fakeUDPConn{}} + t0 := time.Now() + c.lastReadUnixNano.Store(t0.UnixNano()) + c.now = func() time.Time { return t0.Add(time.Hour) } + + n, addr, err := c.ReadFromUDPAddrPort(make([]byte, 16)) + require.NoError(t, err) + require.Equal(t, 1, n) + require.Equal(t, netip.MustParseAddrPort("1.2.3.4:30303"), addr) + require.Equal(t, t0.Add(time.Hour).UnixNano(), c.LastRead().UnixNano()) +} + +// TestTimedConn_FailedReadDoesNotStamp: a read error must leave the timestamp +// untouched, so a wedged/closed socket cannot look freshly drained. +func TestTimedConn_FailedReadDoesNotStamp(t *testing.T) { + c := &TimedConn{UDPConn: &fakeUDPConn{readErr: errors.New("boom")}} + t0 := time.Now() + c.lastReadUnixNano.Store(t0.UnixNano()) + c.now = func() time.Time { return t0.Add(time.Hour) } + + _, _, err := c.ReadFromUDPAddrPort(make([]byte, 16)) + require.Error(t, err) + require.Equal(t, t0.UnixNano(), c.LastRead().UnixNano(), "errored read must not stamp") +} From 578dc5486504542399b104af626e9dc6e353a22a Mon Sep 17 00:00:00 2001 From: iurii Date: Thu, 6 Aug 2026 18:20:31 +0300 Subject: [PATCH 2/3] network/p2p: fail the health check when the discv5 socket wedges --- network/discovery/dv5_service.go | 27 +++++++++++++++- network/discovery/dv5_service_stale_test.go | 35 +++++++++++++++++++++ network/discovery/local_service.go | 6 ++++ network/discovery/observability.go | 10 ++++++ network/discovery/service.go | 3 ++ network/discovery/service_test.go | 1 + network/discovery/shared_conn_test.go | 27 ++++++++++++++++ network/p2p/p2p.go | 15 +++++++++ network/p2p/p2p_health_test.go | 28 ++++++++++++++++- 9 files changed, 150 insertions(+), 2 deletions(-) create mode 100644 network/discovery/dv5_service_stale_test.go diff --git a/network/discovery/dv5_service.go b/network/discovery/dv5_service.go index 3023a62508..0b9ad10c51 100644 --- a/network/discovery/dv5_service.go +++ b/network/discovery/dv5_service.go @@ -78,6 +78,9 @@ type DiscV5Service struct { conn *net.UDPConn sharedConn *SharedUDPConn + // socketConn wraps conn to time socket reads; the post-fork listener drains + // through it, so DiscoveryStale can detect a wedged socket. + socketConn *TimedConn ssvConfig *networkconfig.SSV subnets commons.Subnets @@ -159,6 +162,21 @@ func (dvs *DiscV5Service) close() error { return nil } +// DiscoveryStale reports whether the discv5 socket has gone unread for longer +// than grace — the sign discovery has wedged. False before the listener is built +// (or after a failed init), when there's nothing to judge. +// +// The operator health check polls this periodically, so it doubles as the +// sampling point for the read-staleness gauge. +func (dvs *DiscV5Service) DiscoveryStale(grace time.Duration) bool { + if dvs.socketConn == nil { + return false + } + age, ok := dvs.socketConn.ReadStaleness() + recordDiscoveryReadStaleness(dvs.ctx, int64(age.Seconds())) + return ok && age > grace +} + // Self returns self node func (dvs *DiscV5Service) Self() *enode.LocalNode { return dvs.dv5Listener.LocalNode() @@ -402,6 +420,12 @@ func (dvs *DiscV5Service) initDiscV5Listener(discOpts *Options) (err error) { } dvs.conn = udpConn + // Wrap the socket for liveness (see DiscoveryStale): only the post-fork + // listener drains it, so only it gets the wrapped conn — the pre-fork + // listener reads sharedConn's buffer, not the socket. + socketConn := NewTimedConn(udpConn) + dvs.socketConn = socketConn + // Registered before anything else can fail, so every error path below // releases the socket. Runs last of the deferred cleanups, by which point a // listener may already have closed it. @@ -409,6 +433,7 @@ func (dvs *DiscV5Service) initDiscV5Listener(discOpts *Options) (err error) { if err != nil { _ = udpConn.Close() dvs.conn = nil + dvs.socketConn = nil } }() @@ -453,7 +478,7 @@ func (dvs *DiscV5Service) initDiscV5Listener(discOpts *Options) (err error) { return err } - dv5PostForkListener, err := listenV5(udpConn, localNode, *dv5PostForkCfg) + dv5PostForkListener, err := listenV5(socketConn, localNode, *dv5PostForkCfg) if err != nil { return fmt.Errorf("could not create discV5 listener: %w", err) } diff --git a/network/discovery/dv5_service_stale_test.go b/network/discovery/dv5_service_stale_test.go new file mode 100644 index 0000000000..19d04078d5 --- /dev/null +++ b/network/discovery/dv5_service_stale_test.go @@ -0,0 +1,35 @@ +package discovery + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// TestDiscV5Service_DiscoveryStale covers the operator-side liveness accessor: +// nil before the listener is built, never stale until the socket has actually +// been read, and driven by the wrapped socket's last-read time thereafter. +func TestDiscV5Service_DiscoveryStale(t *testing.T) { + // No socket wrapped yet (not-yet / failed init) → never stale. + require.False(t, (&DiscV5Service{}).DiscoveryStale(3*time.Minute)) + + sc := NewTimedConn(nil) + t0 := sc.LastRead() + dvs := &DiscV5Service{ctx: t.Context(), socketConn: sc} + + // Never read → never stale, however long it sits idle: an operator that has + // simply had no inbound discv5 traffic yet must not be flagged as wedged. + sc.now = func() time.Time { return t0.Add(time.Hour) } + require.False(t, dvs.DiscoveryStale(3*time.Minute), "never-read socket is not a wedge") + + // A read arms the signal, stamped at t0. + sc.read.Store(true) + sc.lastReadUnixNano.Store(t0.UnixNano()) + + sc.now = func() time.Time { return t0.Add(2 * time.Minute) } + require.False(t, dvs.DiscoveryStale(3*time.Minute), "read within grace → not stale") + + sc.now = func() time.Time { return t0.Add(4 * time.Minute) } + require.True(t, dvs.DiscoveryStale(3*time.Minute), "read then unread past grace → stale") +} diff --git a/network/discovery/local_service.go b/network/discovery/local_service.go index c9cb9cabba..f0187b64fc 100644 --- a/network/discovery/local_service.go +++ b/network/discovery/local_service.go @@ -114,6 +114,12 @@ func (md *localDiscovery) PublishENR() { // TODO } +// DiscoveryStale implements Service. Local discovery has no discv5 socket to +// wedge, so it's never stale. +func (md *localDiscovery) DiscoveryStale(time.Duration) bool { + return false +} + // discoveryNotifee gets notified when we find a new peer via mDNS discovery type discoveryNotifee struct { handler HandleNewPeer diff --git a/network/discovery/observability.go b/network/discovery/observability.go index e7042ce121..73f6cacf1f 100644 --- a/network/discovery/observability.go +++ b/network/discovery/observability.go @@ -65,12 +65,22 @@ var ( observability.InstrumentName(observabilityNamespace, "unhandled_packets.dropped"), metric.WithUnit("{packet}"), metric.WithDescription("total number of packets forwarded to the pre-fork listener that were dropped because its buffer was full"))) + + discoveryReadStalenessGauge = metrics.New( + meter.Int64Gauge( + observability.InstrumentName(observabilityNamespace, "socket.read_staleness"), + metric.WithUnit("s"), + metric.WithDescription("seconds since the discv5 socket was last read; 0 until the first read"))) ) func recordUnhandledPacketDropped(ctx context.Context) { unhandledPacketsDroppedCounter.Add(ctx, 1) } +func recordDiscoveryReadStaleness(ctx context.Context, seconds int64) { + discoveryReadStalenessGauge.Record(ctx, seconds) +} + func recordPeerSkipped(ctx context.Context, reason skipReason) { peerRejectionsCounter.Add(ctx, 1, metric.WithAttributes(peerSkipReasonAttribute(reason))) } diff --git a/network/discovery/service.go b/network/discovery/service.go index 565835dffd..16f4be12b3 100644 --- a/network/discovery/service.go +++ b/network/discovery/service.go @@ -62,6 +62,9 @@ type Service interface { DeregisterSubnets(subnets ...uint64) (updated bool, err error) Bootstrap(handler HandleNewPeer) error PublishENR() + // DiscoveryStale reports whether discovery has stopped draining its socket + // for longer than grace (the "wedge"). + DiscoveryStale(grace time.Duration) bool } type DiscoveredPeer struct { diff --git a/network/discovery/service_test.go b/network/discovery/service_test.go index fbaf018a8a..212aadefaa 100644 --- a/network/discovery/service_test.go +++ b/network/discovery/service_test.go @@ -37,6 +37,7 @@ func TestNewDiscV5Service(t *testing.T) { assert.NotNil(t, dvs.dv5Listener) assert.NotNil(t, dvs.conns) + assert.NotNil(t, dvs.socketConn) assert.NotNil(t, dvs.subnetsIdx) assert.NotNil(t, dvs.ssvConfig) diff --git a/network/discovery/shared_conn_test.go b/network/discovery/shared_conn_test.go index 4c796dd8b1..cac1b227f1 100644 --- a/network/discovery/shared_conn_test.go +++ b/network/discovery/shared_conn_test.go @@ -312,6 +312,33 @@ func TestInitDiscV5Listener_CleansUpOnError(t *testing.T) { } } +// TestInitDiscV5Listener_WrapsPostForkConn pins the wiring the wedge detector +// depends on: the post-fork listener — the only one that drains the real socket — +// must read through the TimedConn, and the pre-fork listener must read the +// SharedUDPConn buffer. Every other test builds these conns by hand, so without +// this a refactor that passed the wrong conn to ListenV5 would leave the detector +// silently reporting "healthy" forever while the whole suite still passed. +// +// Swaps the package-level listenV5, so the test must stay non-parallel. +func TestInitDiscV5Listener_WrapsPostForkConn(t *testing.T) { + orig := listenV5 + t.Cleanup(func() { listenV5 = orig }) + + var conns []discover.UDPConn + listenV5 = func(conn discover.UDPConn, ln *enode.LocalNode, cfg discover.Config) (Listener, error) { + conns = append(conns, conn) + return orig(conn, ln, cfg) + } + + dvs := testingDiscovery(t) + t.Cleanup(func() { require.NoError(t, dvs.Close()) }) + + require.Len(t, conns, 2, "init builds the post-fork listener, then the pre-fork one") + require.IsType(t, &TimedConn{}, conns[0], "post-fork listener must read through the TimedConn") + require.Same(t, dvs.socketConn, conns[0].(*TimedConn), "and it must be the service's own socketConn") + require.IsType(t, &SharedUDPConn{}, conns[1], "pre-fork listener must read the SharedUDPConn buffer") +} + // TestSharedUDPConn_DrainsWhileClosing guards the shutdown ordering: the // producer keeps forwarding while the reader shuts down, and must not block or // panic. Previously Close() closed Unhandled, so an in-flight send panicked diff --git a/network/p2p/p2p.go b/network/p2p/p2p.go index 65bf49a66d..7b8ee29b67 100644 --- a/network/p2p/p2p.go +++ b/network/p2p/p2p.go @@ -545,6 +545,11 @@ func (n *p2pNetwork) isReady() bool { return atomic.LoadInt32(&n.state) == stateReady } +// discoveryStaleGrace is how long the discv5 socket may go unread before Healthy +// treats discovery as wedged — comfortably above normal quiet on a live network, +// where inbound traffic and query responses keep the socket busy. +const discoveryStaleGrace = 3 * time.Minute + // Healthy reports whether the p2p network is operating normally. // It satisfies the health-check interface from hprobe package. func (n *p2pNetwork) Healthy(ctx context.Context) error { @@ -557,6 +562,16 @@ func (n *p2pNetwork) Healthy(ctx context.Context) error { if n.discoveryFailed.Load() { return fmt.Errorf("discovery bootstrap failed") } + // A wedged discv5 socket leaves discovery silently dead while bootstrap keeps + // looping; surface it so the hprobe watchdog restarts the node. n.disc is nil + // in tests and briefly at startup, hence the guard. + if n.disc != nil && n.disc.DiscoveryStale(discoveryStaleGrace) { + // Surface the wedge in the log alongside the exit reason, so a restart + // isn't the only evidence that discovery died. + n.logger.Warn("discv5 socket wedged: not drained within grace, discovery stalled", + zap.Duration("grace", discoveryStaleGrace)) + return fmt.Errorf("discv5 socket not drained for >%s (discovery wedged)", discoveryStaleGrace) + } return nil } diff --git a/network/p2p/p2p_health_test.go b/network/p2p/p2p_health_test.go index 62e62ce096..0c14ecd2a7 100644 --- a/network/p2p/p2p_health_test.go +++ b/network/p2p/p2p_health_test.go @@ -4,8 +4,12 @@ import ( "context" "sync/atomic" "testing" + "time" "github.com/stretchr/testify/require" + "go.uber.org/zap" + + "github.com/ssvlabs/ssv/network/discovery" ) func TestP2PNetwork_Healthy(t *testing.T) { @@ -14,6 +18,7 @@ func TestP2PNetwork_Healthy(t *testing.T) { state int32 discoveryFailed bool cancelCtx bool + disc discovery.Service wantErr string }{ { @@ -43,11 +48,23 @@ func TestP2PNetwork_Healthy(t *testing.T) { cancelCtx: true, wantErr: "context canceled", }, + { + name: "discovery wedged", + state: stateReady, + disc: staleDiscovery{stale: true}, + wantErr: "discovery wedged", + }, + { + name: "ready with live discovery", + state: stateReady, + disc: staleDiscovery{stale: false}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - n := &p2pNetwork{} + n := &p2pNetwork{logger: zap.NewNop()} + n.disc = tt.disc atomic.StoreInt32(&n.state, tt.state) if tt.discoveryFailed { n.discoveryFailed.Store(true) @@ -69,3 +86,12 @@ func TestP2PNetwork_Healthy(t *testing.T) { }) } } + +// staleDiscovery is a discovery.Service whose only real method is DiscoveryStale; +// Healthy calls nothing else, so the embedded nil Service is never dereferenced. +type staleDiscovery struct { + discovery.Service + stale bool +} + +func (d staleDiscovery) DiscoveryStale(time.Duration) bool { return d.stale } From a16e6da5a4bf4f330d102298edb18743004e10a9 Mon Sep 17 00:00:00 2001 From: iurii Date: Thu, 6 Aug 2026 18:20:32 +0300 Subject: [PATCH 3/3] utils/boot_node: add /healthz tied to discv5 draining --- utils/boot_node/health.go | 97 ++++++++++++++++++++++++++++++++++ utils/boot_node/health_test.go | 85 +++++++++++++++++++++++++++++ utils/boot_node/node.go | 16 +++--- 3 files changed, 191 insertions(+), 7 deletions(-) create mode 100644 utils/boot_node/health.go create mode 100644 utils/boot_node/health_test.go diff --git a/utils/boot_node/health.go b/utils/boot_node/health.go new file mode 100644 index 0000000000..9402779e4e --- /dev/null +++ b/utils/boot_node/health.go @@ -0,0 +1,97 @@ +package bootnode + +import ( + "fmt" + "net/http" + "sync/atomic" + "time" + + "github.com/ethereum/go-ethereum/p2p/enode" +) + +const ( + // bootReadStaleGrace is how long the socket may go unread before /healthz + // fails. A boot node is queried continuously, so a wedge stops reads well + // inside this. + bootReadStaleGrace = 3 * time.Minute + + // bootEmptyTableGrace tolerates cold start, when the table is briefly empty + // before the boot node discovers peers. + bootEmptyTableGrace = 10 * time.Minute +) + +// nodeLister is the slice of discovery.Listener the health check needs. +type nodeLister interface { + AllNodes() []*enode.Node +} + +// socketDrainState reports socket-read staleness; satisfied by *discovery.TimedConn. +type socketDrainState interface { + StaleFor(time.Duration) bool +} + +// bootNodeHealth backs /healthz. A boot node's whole job is discovery, so it's +// broken if its socket has wedged or its routing table has been empty past cold +// start; either fails /healthz closed so a liveness probe restarts the pod. +type bootNodeHealth struct { + lister nodeLister + socket socketDrainState + readStaleGrace time.Duration + emptyTableGrace time.Duration + now func() time.Time + lastNonEmpty atomic.Int64 // unix nanos of the last observation with a non-empty table +} + +func newBootNodeHealth(lister nodeLister, socket socketDrainState) *bootNodeHealth { + h := &bootNodeHealth{ + lister: lister, + socket: socket, + readStaleGrace: bootReadStaleGrace, + emptyTableGrace: bootEmptyTableGrace, + now: time.Now, + } + h.lastNonEmpty.Store(h.now().UnixNano()) + return h +} + +// check returns a reason when discovery looks broken, or nil when healthy. +// +// The socket-staleness check only applies while the routing table is populated: +// discv5 revalidates those peers and should be reading their responses, so a +// stale socket then means the read loop has wedged. An empty table generates no +// such traffic, so staleness there is expected (cold start, or a quiet node with +// no peers) and is judged only by the empty-table grace, which tolerates cold +// start. lastNonEmpty is seeded at startup, so it catches both "never populated" +// and "populated then emptied". +// +// One case slips past the fast path: a wedge present from boot. On restart discv5 +// loads seed nodes from the persistent enode DB straight into the table, so +// AllNodes() can be >0 before the socket has been read once — and StaleFor stays +// disarmed until that first read. Those seeds then fail revalidation and age out, +// the table empties, and the empty-table grace trips instead. A runtime wedge +// (socket drained, then stopped) is unaffected: a prior read has already armed +// StaleFor. +func (h *bootNodeHealth) check() error { + now := h.now() + if len(h.lister.AllNodes()) > 0 { + h.lastNonEmpty.Store(now.UnixNano()) + if h.socket.StaleFor(h.readStaleGrace) { + return fmt.Errorf("discv5 socket not drained for >%s while routing table is populated", h.readStaleGrace) + } + return nil + } + if emptyFor := now.Sub(time.Unix(0, h.lastNonEmpty.Load())); emptyFor > h.emptyTableGrace { + return fmt.Errorf("discv5 routing table empty for >%s", h.emptyTableGrace) + } + return nil +} + +func (h *bootNodeHealth) handler() http.HandlerFunc { + return func(w http.ResponseWriter, _ *http.Request) { + if err := h.check(); err != nil { + http.Error(w, err.Error(), http.StatusServiceUnavailable) + return + } + _, _ = w.Write([]byte("ok")) + } +} diff --git a/utils/boot_node/health_test.go b/utils/boot_node/health_test.go new file mode 100644 index 0000000000..5059553880 --- /dev/null +++ b/utils/boot_node/health_test.go @@ -0,0 +1,85 @@ +package bootnode + +import ( + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/ethereum/go-ethereum/p2p/enode" + "github.com/stretchr/testify/require" +) + +type fakeLister struct{ count int } + +func (f fakeLister) AllNodes() []*enode.Node { return make([]*enode.Node, f.count) } + +type fakeSocket struct{ stale bool } + +func (f fakeSocket) StaleFor(time.Duration) bool { return f.stale } + +func newTestHealth(lister nodeLister, socket socketDrainState, now func() time.Time) *bootNodeHealth { + h := &bootNodeHealth{ + lister: lister, + socket: socket, + readStaleGrace: bootReadStaleGrace, + emptyTableGrace: bootEmptyTableGrace, + now: now, + } + h.lastNonEmpty.Store(now().UnixNano()) + return h +} + +func TestBootNodeHealth_Check(t *testing.T) { + base := time.Now() + fixed := func() time.Time { return base } + + // Populated table + fresh socket → healthy. + require.NoError(t, newTestHealth(fakeLister{count: 5}, fakeSocket{stale: false}, fixed).check()) + + // Populated table + stale socket → wedged (discv5 should be revalidating those + // peers and reading responses, but isn't). + require.ErrorContains(t, + newTestHealth(fakeLister{count: 5}, fakeSocket{stale: true}, fixed).check(), + "socket not drained") + + // Empty table + stale socket, within cold-start grace → healthy: a quiet node + // with no peers produces no reads, so staleness must not flag it (regression + // for the "quiet socket looks wedged" false positive). + require.NoError(t, + newTestHealth(fakeLister{count: 0}, fakeSocket{stale: true}, fixed).check(), + "empty/quiet table must not trip the socket-staleness check") + + // Empty table but still within cold-start grace → healthy; once it stays + // empty past the grace → unhealthy. + now := base + h := newTestHealth(fakeLister{count: 0}, fakeSocket{stale: false}, func() time.Time { return now }) + require.NoError(t, h.check(), "empty table within cold-start grace is tolerated") + now = base.Add(bootEmptyTableGrace + time.Minute) + require.ErrorContains(t, h.check(), "routing table empty") + + // A non-empty observation resets the empty-table clock, so a brief later + // emptiness is tolerated again. + now = base + h = newTestHealth(fakeLister{count: 3}, fakeSocket{stale: false}, func() time.Time { return now }) + now = base.Add(bootEmptyTableGrace + time.Minute) // long after, but table is non-empty here + require.NoError(t, h.check(), "a non-empty table refreshes the clock") + h.lister = fakeLister{count: 0} // now empties, but only briefly + now = now.Add(time.Minute) + require.NoError(t, h.check(), "just-emptied table is within grace of the last non-empty observation") +} + +func TestBootNodeHealth_Handler(t *testing.T) { + base := time.Now() + fixed := func() time.Time { return base } + + rr := httptest.NewRecorder() + newTestHealth(fakeLister{count: 5}, fakeSocket{stale: false}, fixed). + handler()(rr, httptest.NewRequest(http.MethodGet, "/healthz", nil)) + require.Equal(t, http.StatusOK, rr.Code) + + rr = httptest.NewRecorder() + newTestHealth(fakeLister{count: 5}, fakeSocket{stale: true}, fixed). + handler()(rr, httptest.NewRequest(http.MethodGet, "/healthz", nil)) + require.Equal(t, http.StatusServiceUnavailable, rr.Code) +} diff --git a/utils/boot_node/node.go b/utils/boot_node/node.go index 198f72b233..cc77427b3c 100644 --- a/utils/boot_node/node.go +++ b/utils/boot_node/node.go @@ -106,13 +106,11 @@ func (n *bootNode) Start(ctx context.Context) error { } ipAddr, err := network.ExternalIP() - // ipAddr = "127.0.0.1" - n.logger.Info("TEST Ip addr----", zap.String("ip_addr", ipAddr)) if err != nil { n.logger.Fatal("Failed to get ExternalIP", zap.Error(err)) } - listener := n.createListener(ipAddr, n.discv5port, privKey) + listener, socketConn := n.createListener(ipAddr, n.discv5port, privKey) node := listener.LocalNode().Node() n.logger.Info("Running", zap.Stringer("node", node), @@ -124,8 +122,10 @@ func (n *bootNode) Start(ctx context.Context) error { logger: n.logger, listener: listener, } + health := newBootNodeHealth(listener, socketConn) mux := http.NewServeMux() mux.HandleFunc("/p2p", handler.httpHandler()) + mux.HandleFunc("/healthz", health.handler()) const timeout = 3 * time.Second @@ -143,7 +143,7 @@ func (n *bootNode) Start(ctx context.Context) error { return nil } -func (n *bootNode) createListener(ipAddr string, port uint16, privateKey *ecdsa.PrivateKey) discovery.Listener { +func (n *bootNode) createListener(ipAddr string, port uint16, privateKey *ecdsa.PrivateKey) (discovery.Listener, *discovery.TimedConn) { // Create the UDP listener and the LocalNode record. ip := net.ParseIP(ipAddr) if ip.To4() == nil { @@ -169,20 +169,22 @@ func (n *bootNode) createListener(ipAddr string, port uint16, privateKey *ecdsa. if err != nil { n.logger.Fatal("Failed to create UDP server", zap.Error(err)) } + // Wrap the socket so /healthz can tell whether discv5 is still draining it. + socketConn := discovery.NewTimedConn(conn) localNode, err := n.createLocalNode(privateKey, ip, port) if err != nil { n.logger.Fatal("Failed to create local node", zap.Error(err)) } - listener, err := discover.ListenV5(conn, localNode, discover.Config{ + listener, err := discover.ListenV5(socketConn, localNode, discover.Config{ PrivateKey: privateKey, V5ProtocolID: &n.ssvConfig.DiscoveryProtocolID, }) if err != nil { - n.logger.Fatal("Filed to create UDPv5 listener", zap.Error(err)) + n.logger.Fatal("failed to create UDPv5 listener", zap.Error(err)) } - return listener + return listener, socketConn } func (n *bootNode) createLocalNode(privKey *ecdsa.PrivateKey, ipAddr net.IP, port uint16) (*enode.LocalNode, error) {