Peer-exchange serves stale ENRs: PeerStore copy is never liveness-pruned

[igor-sirotin]

Summary

Peer-exchange responds with ENRs read from the PeerStore ENRBook, which is fed add-only from discv5 and is only ever pruned on capacity, never on staleness/liveness. As a result a node keeps advertising ENRs of disconnected/dead discv5 peers via PX more or less indefinitely (as long as the store is under capacity). PX clients (e.g. go-waku light clients) then repeatedly try to dial unreachable peers.

Permalinks below pinned to master @ 86e424c (no develop branch exists).

The chain

1. PX serves from the PeerStore, and its only liveness gate is CannotConnect. getEnrsFromStore iterates peerStore[ENRBook], skips only CannotConnect, and poolFilter then keeps only origin == Discv5:

   • logos-delivery/waku/waku_peer_exchange/protocol.nim

     Lines 96 to 122 in 86e424c

     	proc getEnrsFromStore(
     	wpx: WakuPeerExchange, numPeers: uint64
     	): seq[enr.Record] {.gcsafe.} =
     	# Reservoir sampling (Algorithm R)
     	var i = 0
     	let k = min(MaxPeersCacheSize, numPeers.int)
     	let enrStoreLen = wpx.peerManager.switch.peerStore[ENRBook].len
     	var enrs = newSeqOfCap[enr.Record](min(k, enrStoreLen))
     	wpx.peerManager.switch.peerStore.forEnrPeers(
     	peerId, peerConnectedness, peerOrigin, peerEnrRecord
     	):
     	if peerConnectedness == CannotConnect:
     	debug "Could not retrieve ENR because cannot connect to peer",
     	remotePeerId = peerId
     	continue
     	poolFilter(wpx.cluster, peerOrigin, peerEnrRecord).isOkOr:
     	debug "Could not get ENR because no peer matched pool", error = error
     	continue
     	if i < k:
     	enrs.add(peerEnrRecord)
     	else:
     	# Add some randomness
     	let j = rand(i)
     	if j < k:
     	enrs[j] = peerEnrRecord
     	inc(i)
     	return enrs

   • logos-delivery/waku/waku_peer_exchange/protocol.nim

     Lines 79 to 88 in 86e424c

     	proc poolFilter*(
     	cluster: Option[uint16], origin: PeerOrigin, enr: enr.Record
     	): Result[void, string] =
     	if origin != Discv5:
     	trace "peer not from discv5", origin = $origin
     	return err("peer not from discv5: " & $origin)
     	if cluster.isSome() and enr.isClusterMismatched(cluster.get()):
     	trace "peer has mismatching cluster"
     	return err("peer has mismatching cluster")
     	return ok()

2. The ENRBook is populated add-only by the discv5 search loop (addPeer(peer, PeerOrigin.Discv5)), with no reverse path that removes an entry when discv5 later evicts it:

   • logos-delivery/waku/discovery/waku_discv5.nim

     Lines 252 to 290 in 86e424c

     	proc searchLoop(wd: WakuDiscoveryV5) {.async.} =
     	## Continuously add newly discovered nodes
     	let peerManager = wd.peerManager.valueOr:
     	return
     	info "Starting discovery v5 search"
     	while wd.listening:
     	trace "running discv5 discovery loop"
     	let discoveredRecords = await wd.findRandomPeers()
     	var discoveredPeers: seq[RemotePeerInfo]
     	var wrongRecordsReasons: seq[tuple[record: string, errorDescription: string]]
     	## this is to store the reasons why certain records could not be converted to RemotePeerInfo
     	for record in discoveredRecords:
     	let peerInfo = record.toRemotePeerInfo().valueOr:
     	## in case of error, we keep track of it for debugging purposes
     	wrongRecordsReasons.add(($record, $error))
     	waku_discv5_errors.inc(labelValues = [$error])
     	continue
     	discoveredPeers.add(peerInfo)
     	trace "discv5 discovered peers",
     	num_discovered_peers = discoveredPeers.len,
     	peers = toSeq(discoveredPeers.mapIt(shortLog(it.peerId)))
     	trace "discv5 discarded wrong records",
     	wrong_records =
     	wrongRecordsReasons.mapIt("(" & it.record & "," & it.errorDescription & ")")
     	for peer in discoveredPeers:
     	# Peers added are filtered by the peer manager
     	peerManager.addPeer(peer, PeerOrigin.Discv5)
     	# 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.

3. The only removal path is capacity-based, not staleness-based. prunePeerStore returns early unless numPeers > capacity, runs every 10 min, and when it does fire it evicts by failed-conn-count / shard balancing — never by "this ENR is stale":

   • logos-delivery/waku/node/peer_manager/peer_manager.nim

     Lines 1014 to 1090 in 86e424c

     	proc prunePeerStore*(pm: PeerManager) =
     	let peerStore = pm.switch.peerStore
     	let numPeers = peerStore[AddressBook].book.len
     	let capacity = peerStore.getCapacity()
     	if numPeers <= capacity:
     	return
     	trace "Peer store capacity exceeded", numPeers = numPeers, capacity = capacity
     	let pruningCount = numPeers - capacity
     	var peersToPrune: HashSet[PeerId]
     	# prune failed connections
     	for peerId, count in peerStore[NumberFailedConnBook].book.pairs:
     	if count < pm.maxFailedAttempts:
     	continue
     	if peersToPrune.len >= pruningCount:
     	break
     	peersToPrune.incl(peerId)
     	var notConnected = peerStore.getDisconnectedPeers().mapIt(it.peerId)
     	# Always pick random non-connected peers
     	shuffle(notConnected)
     	var shardlessPeers: seq[PeerId]
     	var peersByShard = initTable[uint16, seq[PeerId]]()
     	for peer in notConnected:
     	if not peerStore[ENRBook].contains(peer):
     	shardlessPeers.add(peer)
     	continue
     	let record = peerStore[ENRBook][peer]
     	let rec = record.toTyped().valueOr:
     	shardlessPeers.add(peer)
     	continue
     	let rs = rec.relaySharding().valueOr:
     	shardlessPeers.add(peer)
     	continue
     	for shard in rs.shardIds:
     	peersByShard.mgetOrPut(shard, @[]).add(peer)
     	# prune not connected peers without shard
     	for peer in shardlessPeers:
     	if peersToPrune.len >= pruningCount:
     	break
     	peersToPrune.incl(peer)
     	# calculate the avg peers per shard
     	let total = sum(toSeq(peersByShard.values).mapIt(it.len))
     	let avg = min(1, total div max(1, peersByShard.len))
     	# prune peers from shard with higher than avg count
     	for shard, peers in peersByShard.pairs:
     	let count = max(peers.len - avg, 0)
     	for peer in peers[0 .. count]:
     	if peersToPrune.len >= pruningCount:
     	break
     	peersToPrune.incl(peer)
     	for peer in peersToPrune:
     	peerStore.delete(peer)
     	let afterNumPeers = peerStore[AddressBook].book.len
     	trace "Finished pruning peer store",
     	beforeNumPeers = numPeers,
     	afterNumPeers = afterNumPeers,
     	capacity = capacity,
     	pruned = peersToPrune.len

   • interval:

     logos-delivery/waku/node/peer_manager/peer_manager.nim

     Line 78 in 86e424c

     PrunePeerStoreInterval = chronos.minutes(10)

Why the existing CannotConnect filter doesn't catch it

A discv5-discovered or recently-disconnected peer sits at NotConnected; it only becomes CannotConnect if this node actively dials it and fails — which doesn't happen for peers it never tries to dial. discv5's own routing-table revalidation does drop dead nodes quickly (~seconds), but that eviction is not propagated to the PeerStore that PX reads. #3652 (enrCache removal) reduced this but did not close it.

Impact

Amplified in CI / ephemeral-node fleets (random keys + constant teardown on a shared bootnode). Concretely it breaks status-go functional light-client tests, where light clients fetch dead full-node ENRs via PX and time out on filter/lightpush: status-im/status-go#7513.

Suggested directions

• Serve PX from (or reconcile against) the live, revalidated discv5 routing table instead of the add-only PeerStore copy; or
• Add last-seen TTL / staleness eviction to ENRBook (or propagate discv5 evictions into the PeerStore); or
• Also exclude NotConnected in getEnrsFromStore — but carefully, since discv5 peers are legitimately NotConnected.

[igor-sirotin]

Copilot analysis: this is a correctly identified, real bug.

---

1. getEnrsFromStore only gates on CannotConnect — NotConnected passes through

In waku/waku_peer_exchange/protocol.nim, the only liveness check is:

if peerConnectedness == CannotConnect:
  continue

In waku/node/peer_manager/waku_peer_store.nim, the forEnrPeers template defaults any peer absent from the ConnectionBook to NotConnected:

let peerConnectedness {.inject.} = connBook.book.getOrDefault(pid, NotConnected)

A peer only reaches CannotConnect when this node actively dials it and fails (peer_manager.nim:371). Discv5-discovered peers that are never dialed remain NotConnected indefinitely and pass the PX filter.

---

2. Discv5 evictions are never propagated to the PeerStore

waku/discovery/waku_discv5.nim's searchLoop only ever calls peerManager.addPeer(peer, PeerOrigin.Discv5). There is no callback, hook, or reverse path anywhere in waku_discv5.nim that removes a peer from the ENRBook when discv5's own routing table evicts it.

---

3. prunePeerStore is capacity-gated, not staleness-gated

peer_manager.nim:1018:

if numPeers <= capacity:
  return

In normal operation the store stays under capacity, so the pruning loop never fires and stale ENRs accumulate without bound.

---

Net effect

Discv5 maintains a live, revalidated routing table and does drop dead nodes quickly — but that eviction is completely invisible to the PeerStore. PX reads from the stale ENRBook copy, which is add-only for Discv5 peers. In ephemeral/CI fleets or high-churn environments this reliably produces a pool of dead ENRs being handed to PX clients.

The suggested directions in the issue are sound. Reconciling against the live discv5 routing table (option 1) would be the most principled fix; a last-seen TTL on ENRBook entries (option 2) would also close the gap without restructuring the data flow.

[igor-sirotin]

@Ivansete-status Can you please take a look? I didn't dive deep in the code, but this seem to be a real issue.

Found it while investigating this:

• Flaky functional tests (light-client only): filter subscription churns on dead peer-exchange peers status-im/status-go#7513.

I'm sorry for the AI-generated description, but this was the simplest way to get all the details into the ticket. And it will simplify next AI agent to fix the issue 🙂

[Ivansete-status]

@igor-sirotin - I don't remember if we agreed that I need to tackle this or you, in the end.

[igor-sirotin]

@Ivansete-status we agreed that I'll check it. I've got #3939.

Unfortunately, it didn't resolve the issue with status-go tests, so I was hesitating to open it for reivew.
But I think it looks ok to go, will open now.