From e126e44c48e82f45c69240ba8fadf50f23d83abb Mon Sep 17 00:00:00 2001 From: Ivan FB Date: Fri, 10 Jul 2026 22:03:11 +0200 Subject: [PATCH] fix: don't apply reconnect backoff to discovered relay peers Discovery populates the peer store fast enough that the backgrounded reconnect future treats freshly discovered relay peers as if they came from persistent storage, delaying their connection by a serial ~1min backoff. Tag storage-loaded peers with a new PeerOrigin.Cache and have reconnectPeers act only on those; discovered peers are left to the connectivity loop, which dials them right away. The reconnect dials are also parallelized so a slow peer can't stall the rest. Co-Authored-By: Claude Opus 4.8 --- .../waku/node/peer_manager/peer_manager.nim | 38 +++++++++------ logos_delivery/waku/waku_core/peers.nim | 1 + tests/test_peer_manager.nim | 48 +++++++++++++++++++ 3 files changed, 73 insertions(+), 14 deletions(-) diff --git a/logos_delivery/waku/node/peer_manager/peer_manager.nim b/logos_delivery/waku/node/peer_manager/peer_manager.nim index 31c6133ab..5ffc185b0 100644 --- a/logos_delivery/waku/node/peer_manager/peer_manager.nim +++ b/logos_delivery/waku/node/peer_manager/peer_manager.nim @@ -223,7 +223,10 @@ proc loadFromStorage(pm: PeerManager) {.gcsafe.} = pm.switch.peerStore[ConnectionBook][peerId] = NotConnected # Reset connectedness state pm.switch.peerStore[DisconnectBook][peerId] = remotePeerInfo.disconnectTime - pm.switch.peerStore[SourceBook][peerId] = remotePeerInfo.origin + # Mark as Cache so it is distinguishable from live-discovered peers. If the + # same peer is later rediscovered, addPeer overrides this with the live + # origin, and reconnect backoff no longer applies to it. + pm.switch.peerStore[SourceBook][peerId] = Cache if remotePeerInfo.enr.isSome(): pm.switch.peerStore[ENRBook][peerId] = remotePeerInfo.enr.get() @@ -695,21 +698,28 @@ proc reconnectPeers*( info "Reconnecting peers", proto = proto - # Proto is not persisted, we need to iterate over all peers. - for peerInfo in pm.switch.peerStore.peers(protocolMatcher(proto)): - # Check that the peer can be connected - if peerInfo.connectedness == CannotConnect: - error "Not reconnecting to unreachable or non-existing peer", - peerId = peerInfo.peerId - continue + # Only reconnect peers that come from persistent storage (Cache). Freshly + # discovered peers must not be delayed by the reconnect backoff: they are + # connected right away by the relay connectivity loop. Rediscovered peers get + # their origin overridden by addPeer, so they naturally drop out of this set. + let peersToReconnect = pm.switch.peerStore.peers(protocolMatcher(proto)).filterIt( + it.origin == Cache and it.connectedness != CannotConnect + ) - if backoffTime > ZeroDuration: - info "Backing off before reconnect", - peerId = peerInfo.peerId, backoffTime = backoffTime - # We disconnected recently and still need to wait for a backoff period before connecting - await sleepAsync(backoffTime) + if peersToReconnect.len == 0: + return - await pm.connectToNodes(@[peerInfo]) + # We disconnected recently and still need to wait a backoff period before + # reconnecting. The wait is shared across all peers rather than applied once + # per peer, so reconnection can't stall behind a slow/unreachable peer and + # keep the node from taking on freshly discovered ones. + if backoffTime > ZeroDuration: + info "Backing off before reconnect", backoffTime = backoffTime + await sleepAsync(backoffTime) + + # Dial all reconnectable peers in parallel; a single slow dial must not delay + # the rest. + await allFutures(peersToReconnect.mapIt(pm.connectToNodes(@[it]))) proc getNumStreams*(pm: PeerManager, protocol: string): (int, int) = var diff --git a/logos_delivery/waku/waku_core/peers.nim b/logos_delivery/waku/waku_core/peers.nim index 6e23c848f..c28ae5427 100644 --- a/logos_delivery/waku/waku_core/peers.nim +++ b/logos_delivery/waku/waku_core/peers.nim @@ -40,6 +40,7 @@ type PeerExchange Dns Kademlia + Cache # Loaded from persistent peer storage (not a live discovery source) PeerDirection* = enum UnknownDirection diff --git a/tests/test_peer_manager.nim b/tests/test_peer_manager.nim index b364fc8c3..1dc42d4ce 100644 --- a/tests/test_peer_manager.nim +++ b/tests/test_peer_manager.nim @@ -300,6 +300,54 @@ procSuite "Peer Manager": await allFutures(nodes.mapIt(it.stop())) + asyncTest "reconnectPeers only reconnects persisted (Cache) peers": + ## Regression: freshly discovered relay peers were wrongly swept into the + ## reconnect backoff path. reconnectPeers must act only on peers loaded from + ## persistent storage (origin == Cache); discovered peers are left for the + ## connectivity loop to dial without any backoff. + let node = newTestWakuNode(generateSecp256k1Key()) + await node.start() + + let basePeerId = "QmeuZJbXrszW2jdT7GdduSjQskPU3S7vvGWKtKgDfkDvW" + var cachePeerId, discoveredPeerId: PeerId + require cachePeerId.init(basePeerId & "1") + require discoveredPeerId.init(basePeerId & "2") + + # Both peers advertise relay and point at an unreachable address, so a dial + # attempt fails fast and is observable via the failed-connection counter. + let unreachableAddr = MultiAddress.init("/ip4/127.0.0.1/tcp/1").tryGet() + + node.peerManager.addPeer( + RemotePeerInfo.init( + peerId = cachePeerId, addrs = @[unreachableAddr], protocols = @[WakuRelayCodec] + ), + origin = Cache, + ) + node.peerManager.addPeer( + RemotePeerInfo.init( + peerId = discoveredPeerId, + addrs = @[unreachableAddr], + protocols = @[WakuRelayCodec], + ), + origin = Discv5, + ) + + # Default backoff is zero, so the test stays fast; the origin filter is what + # we are validating here. + await node.peerManager.reconnectPeers(WakuRelayCodec) + + let peerStore = node.peerManager.switch.peerStore + check: + # The Cache peer was dialed (and failed against the unreachable address). + peerStore[NumberFailedConnBook][cachePeerId] == 1 + peerStore.connectedness(cachePeerId) == CannotConnect + + # The discovered peer was never touched by reconnectPeers. + peerStore[NumberFailedConnBook][discoveredPeerId] == 0 + peerStore.connectedness(discoveredPeerId) == NotConnected + + await node.stop() + asyncTest "Peer manager can use persistent storage and survive restarts": let database = SqliteDatabase.new(":memory:")[]