Merge ef28c5f18e0325e1f2b7e7cbde7d619e1343115f into a62ab1e7b13ee5dcd52708952b910b61345396fc

tests/test_peer_store_extended.nim

@@ -1,7 +1,7 @@
 {.used.}
 
 import
-  std/[sequtils, times],
+  std/[sequtils, times, sets],
   chronos,
   libp2p/crypto/crypto,
   libp2p/peerid,
@@ -187,6 +187,25 @@ suite "Extended nim-libp2p Peer Store":
       p3.numberFailedConn == 3
       p3.lastFailedConn == Moment.init(1003, Second)
 
+  test "peers() randomizes returned ordering across calls":
+    let firstOrder = peerStore.peers().mapIt(it.peerId)
+    var differentOrder: seq[PeerId] = @[]
+    var seed = int64(times.epochTime() * 1000)
+
+    for _ in 0 ..< 10:
+      while int64(times.epochTime() * 1000) == seed:
+        ## enforce certain delay to ensure different seed for randomization across calls to peers()
+        discard
+      seed = int64(times.epochTime() * 1000)
+      let nextOrder = peerStore.peers().mapIt(it.peerId)
+      if nextOrder != firstOrder:
+        differentOrder = nextOrder
+        break
+
+    check differentOrder.len == firstOrder.len
+    check differentOrder != firstOrder
+    check differentOrder.toHashSet() == firstOrder.toHashSet()
+
   test "peers() returns all StoredInfo matching a specific protocol":
     # When
     let storePeers = peerStore.peers("/vac/waku/store/2.0.0")

waku/node/peer_manager/waku_peer_store.nim

@@ -1,7 +1,7 @@
 {.push raises: [].}
 
 import
-  std/[tables, sequtils, sets, options, strutils],
+  std/[tables, sequtils, sets, options, strutils, random, times],
   chronos,
   chronicles,
   eth/p2p/discoveryv5/enr,
@@ -43,6 +43,11 @@ type
   # Keeps track of peer shards
   ShardBook* = ref object of PeerBook[seq[uint16]]
 
+proc randomizePeers(peers: var seq[RemotePeerInfo]) =
+  let time = int64(times.epochTime() * 1000) and 0x7fff_ffff
+  var rand = initRand(time)
+  shuffle(rand, peers)
+
 proc getPeer*(peerStore: PeerStore, peerId: PeerId): RemotePeerInfo =
   let addresses =
     if peerStore[LastSeenBook][peerId].isSome():
@@ -90,7 +95,9 @@ proc peers*(peerStore: PeerStore): seq[RemotePeerInfo] =
     )
     .toHashSet()
 
-  return allKeys.mapIt(peerStore.getPeer(it))
+  var peers = allKeys.mapIt(peerStore.getPeer(it))
+  randomizePeers(peers)
+  return peers
 
 proc addPeer*(peerStore: PeerStore, peer: RemotePeerInfo, origin = UnknownOrigin) =
   ## Storing MixPubKey even if peer is already present as this info might be new
@@ -200,33 +207,33 @@ proc getWakuProtos*(peerStore: PeerStore): seq[string] =
 proc getPeersByDirection*(
     peerStore: PeerStore, direction: PeerDirection
 ): seq[RemotePeerInfo] =
-  return peerStore.peers.filterIt(it.direction == direction)
+  return peerStore.peers().filterIt(it.direction == direction)
 
 proc getDisconnectedPeers*(peerStore: PeerStore): seq[RemotePeerInfo] =
-  return peerStore.peers.filterIt(it.connectedness != Connected)
+  return peerStore.peers().filterIt(it.connectedness != Connected)
 
 proc getConnectedPeers*(peerStore: PeerStore): seq[RemotePeerInfo] =
-  return peerStore.peers.filterIt(it.connectedness == Connected)
+  return peerStore.peers().filterIt(it.connectedness == Connected)
 
 proc getPeersByProtocol*(peerStore: PeerStore, proto: string): seq[RemotePeerInfo] =
-  return peerStore.peers.filterIt(it.protocols.contains(proto))
+  return peerStore.peers().filterIt(it.protocols.contains(proto))
 
 proc getReachablePeers*(peerStore: PeerStore): seq[RemotePeerInfo] =
-  return peerStore.peers.filterIt(it.connectedness != CannotConnect)
+  return peerStore.peers().filterIt(it.connectedness != CannotConnect)
 
 proc getPeersByShard*(
     peerStore: PeerStore, cluster, shard: uint16
 ): seq[RemotePeerInfo] =
-  return peerStore.peers.filterIt(
-    (it.enr.isSome() and it.enr.get().containsShard(cluster, shard)) or
-      it.shards.contains(shard)
-  )
+  return peerStore.peers().filterIt(
+      (it.enr.isSome() and it.enr.get().containsShard(cluster, shard)) or
+        it.shards.contains(shard)
+    )
 
 proc getPeersByCapability*(
     peerStore: PeerStore, cap: Capabilities
 ): seq[RemotePeerInfo] =
   return
-    peerStore.peers.filterIt(it.enr.isSome() and it.enr.get().supportsCapability(cap))
+    peerStore.peers().filterIt(it.enr.isSome() and it.enr.get().supportsCapability(cap))
 
 template forEnrPeers*(
     peerStore: PeerStore,