From 38a98ac7c10866e51c1ca7fc0b3bdf9b97d85be1 Mon Sep 17 00:00:00 2001 From: Ivan FB Date: Mon, 22 Jun 2026 13:42:11 +0200 Subject: [PATCH] fix(discovery): age stale discv5 ENRs out of peer-exchange via freshness TTL PX serves ENRs from the add-only PeerStore ENRBook, whose only liveness gate was CannotConnect. Disconnected/never-dialed discv5 peers stayed NotConnected and were advertised indefinitely (#3933). Instead of deleting peer-store state against the bounded discv5 routing table (which can false-prune a live peer evicted from a full k-bucket), track an ENR last-seen Moment and gate the PX read path on it: - EnrLastSeenBook + EnrFreshnessTTL; touchEnrSeen stamps freshness. - Refresh-only liveness: stamped on ENR (re)write, on connect, and from the revalidated routing table each discv5 round. Membership never deletes state, so a live-but-bucket-evicted peer just ages out of PX rather than being purged. - getEnrsFromStore skips non-Connected ENRs older than the TTL; a Connected peer is always considered fresh. Co-Authored-By: Claude Opus 4.8 --- logos_delivery/waku/discovery/waku_discv5.nim | 16 +++++++++++++- .../waku/node/peer_manager/peer_manager.nim | 13 ++++++++++++ .../node/peer_manager/waku_peer_store.nim | 21 ++++++++++++++++++- .../waku/waku_peer_exchange/protocol.nim | 8 ++++++- 4 files changed, 55 insertions(+), 3 deletions(-) diff --git a/logos_delivery/waku/discovery/waku_discv5.nim b/logos_delivery/waku/discovery/waku_discv5.nim index ade874733..c4b38dbd1 100644 --- a/logos_delivery/waku/discovery/waku_discv5.nim +++ b/logos_delivery/waku/discovery/waku_discv5.nim @@ -12,7 +12,8 @@ import libp2p/multiaddress, eth/keys as eth_keys, eth/p2p/discoveryv5/node, - eth/p2p/discoveryv5/protocol + eth/p2p/discoveryv5/protocol, + eth/p2p/discoveryv5/routing_table import logos_delivery/waku/ [net/auto_port, node/peer_manager/peer_manager, waku_core, waku_enr] @@ -291,6 +292,19 @@ proc searchLoop(wd: WakuDiscoveryV5) {.async.} = # Peers added are filtered by the peer manager peerManager.addPeer(peer, PeerOrigin.Discv5) + # Refresh ENR liveness from discv5's revalidated routing table so stale + # ENRs age out of peer-exchange. This only extends freshness; it never + # deletes peer-store state, so a live peer absent from a full k-bucket is + # not wrongly purged (it just stops being advertised until rediscovered). + var liveIds = initHashSet[PeerId]() + for peer in discoveredPeers: + liveIds.incl(peer.peerId) + for node in wd.protocol.routingTable.randomNodes(int.high): + let info = node.record.toRemotePeerInfo().valueOr: + continue + liveIds.incl(info.peerId) + peerManager.refreshEnrLiveness(liveIds) + # Discovery `queryRandom` can have a synchronous fast path for example # when no peers are in the routing table. Don't run it in continuous loop. # diff --git a/logos_delivery/waku/node/peer_manager/peer_manager.nim b/logos_delivery/waku/node/peer_manager/peer_manager.nim index 60bd2acb1..622a72343 100644 --- a/logos_delivery/waku/node/peer_manager/peer_manager.nim +++ b/logos_delivery/waku/node/peer_manager/peer_manager.nim @@ -796,6 +796,9 @@ proc onPeerEvent(pm: PeerManager, peerId: PeerId, event: PeerEvent) {.async.} = of PeerEventKind.Joined: direction = if event.initiator: Outbound else: Inbound connectedness = Connected + # A live connection is the strongest liveness signal; keep the ENR fresh so + # the freshness window after a later disconnect starts from now. + peerStore.touchEnrSeen(peerId) ## Check max allowed in-relay peers let inRelayPeers = pm.connectedPeers(WakuRelayCodec)[0] @@ -1013,6 +1016,16 @@ proc manageRelayPeers*(pm: PeerManager) {.async.} = trace "Connecting to Peers", peerIds = $uniquePeers[i ..< stop] await pm.connectToNodes(uniquePeers[i ..< stop]) +proc refreshEnrLiveness*(pm: PeerManager, liveIds: HashSet[PeerId]) = + ## Extend the ENR freshness window for peers discv5 still considers reachable. + ## Refresh-only on purpose: routing-table membership never deletes peer-store + ## state, so a live peer evicted from a full k-bucket simply ages out of + ## peer-exchange rather than being wrongly purged. + let peerStore = pm.switch.peerStore + for peerId in liveIds: + if peerStore[ENRBook].contains(peerId): + peerStore.touchEnrSeen(peerId) + proc prunePeerStore*(pm: PeerManager) = let peerStore = pm.switch.peerStore let numPeers = peerStore[AddressBook].book.len diff --git a/logos_delivery/waku/node/peer_manager/waku_peer_store.nim b/logos_delivery/waku/node/peer_manager/waku_peer_store.nim index e4e48f20e..9811a8312 100644 --- a/logos_delivery/waku/node/peer_manager/waku_peer_store.nim +++ b/logos_delivery/waku/node/peer_manager/waku_peer_store.nim @@ -42,9 +42,19 @@ type # Keeps track of the ENR (Ethereum Node Record) of a peer ENRBook* = ref object of PeerBook[enr.Record] + # Keeps track of when a peer's ENR was last seen alive (discv5 revalidation + # or active connection). Drives staleness gating of peer-exchange responses. + EnrLastSeenBook* = ref object of PeerBook[Moment] + # Keeps track of peer shards ShardBook* = ref object of PeerBook[seq[uint16]] +const EnrFreshnessTTL* = chronos.minutes(5) + ## A Discv5-origin ENR is served by peer-exchange only if seen-live within + ## this window, unless the peer is currently Connected. discv5 revalidates + ## its routing table within seconds and the discv5 search loop runs every + ## few seconds, so this is comfortably above the refresh cadence. + proc getPeer*(peerStore: PeerStore, peerId: PeerId): RemotePeerInfo = let addresses = if peerStore[LastSeenBook][peerId].isSome(): @@ -82,6 +92,11 @@ proc delete*(peerStore: PeerStore, peerId: PeerId) = # Delete all the information of a given peer. peerStore.del(peerId) +proc touchEnrSeen*(peerStore: PeerStore, peerId: PeerId) = + ## Mark a peer's ENR as seen-live now. Called from liveness signals (discv5 + ## revalidation, active connection) so stale ENRs eventually age out of PX. + peerStore[EnrLastSeenBook][peerId] = Moment.now() + proc peers*(peerStore: PeerStore): seq[RemotePeerInfo] = let allKeys = concat( toSeq(peerStore[LastSeenOutboundBook].book.keys()), @@ -147,6 +162,8 @@ proc addPeer*(peerStore: PeerStore, peer: RemotePeerInfo, origin = UnknownOrigin peerStore[NumberFailedConnBook].book.hasKeyOrPut(peer.peerId, peer.numberFailedConn) if peer.enr.isSome(): peerStore[ENRBook][peer.peerId] = peer.enr.get() + # A freshly (re)discovered ENR starts its freshness window now. + peerStore.touchEnrSeen(peer.peerId) proc setShardInfo*(peerStore: PeerStore, peerId: PeerID, shards: seq[uint16]) = peerStore[ShardBook][peerId] = shards @@ -232,14 +249,16 @@ proc getPeersByCapability*( template forEnrPeers*( peerStore: PeerStore, - peerId, peerConnectedness, peerOrigin, peerEnrRecord, body: untyped, + peerId, peerConnectedness, peerOrigin, peerEnrRecord, peerEnrSeen, body: untyped, ) = let enrBook = peerStore[ENRBook] let connBook = peerStore[ConnectionBook] let sourceBook = peerStore[SourceBook] + let seenBook = peerStore[EnrLastSeenBook] for pid, enrRecord in tables.pairs(enrBook.book): let peerId {.inject.} = pid let peerConnectedness {.inject.} = connBook.book.getOrDefault(pid, NotConnected) let peerOrigin {.inject.} = sourceBook.book.getOrDefault(pid, UnknownOrigin) let peerEnrRecord {.inject.} = enrRecord + let peerEnrSeen {.inject.} = seenBook.book.getOrDefault(pid, Moment.low) body diff --git a/logos_delivery/waku/waku_peer_exchange/protocol.nim b/logos_delivery/waku/waku_peer_exchange/protocol.nim index a389938f8..5f6e2e06f 100644 --- a/logos_delivery/waku/waku_peer_exchange/protocol.nim +++ b/logos_delivery/waku/waku_peer_exchange/protocol.nim @@ -102,13 +102,19 @@ proc getEnrsFromStore( let k = min(MaxPeersCacheSize, numPeers.int) let enrStoreLen = wpx.peerManager.switch.peerStore[ENRBook].len var enrs = newSeqOfCap[enr.Record](min(k, enrStoreLen)) + let now = Moment.now() wpx.peerManager.switch.peerStore.forEnrPeers( - peerId, peerConnectedness, peerOrigin, peerEnrRecord + peerId, peerConnectedness, peerOrigin, peerEnrRecord, peerEnrSeen ): if peerConnectedness == CannotConnect: debug "Could not retrieve ENR because cannot connect to peer", remotePeerId = peerId continue + # A connected peer is alive by definition; otherwise require a recent + # liveness refresh so we stop advertising ENRs discv5 has already evicted. + if peerConnectedness != Connected and (now - peerEnrSeen) > EnrFreshnessTTL: + debug "Could not retrieve ENR because it is stale", remotePeerId = peerId + continue poolFilter(wpx.cluster, peerOrigin, peerEnrRecord).isOkOr: debug "Could not get ENR because no peer matched pool", error = error continue