From 440461b24b9e66542b34d26a0b908c17f6549d05 Mon Sep 17 00:00:00 2001
From: Tanguy <tanguy@status.im>
Date: Tue, 11 Jul 2023 12:17:50 +0200
Subject: [PATCH] GS: improve handleIHave (#922)

---
 libp2p/protocols/pubsub/gossipsub/behavior.nim | 18 ++++++------------
 tests/pubsub/testgossipinternal.nim            |  3 +++
 2 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/libp2p/protocols/pubsub/gossipsub/behavior.nim b/libp2p/protocols/pubsub/gossipsub/behavior.nim
index 651ccf8d4..d13fac68b 100644
--- a/libp2p/protocols/pubsub/gossipsub/behavior.nim
+++ b/libp2p/protocols/pubsub/gossipsub/behavior.nim
@@ -245,24 +245,18 @@ proc handleIHave*(g: GossipSub,
   elif peer.iHaveBudget <= 0:
     trace "ihave: ignoring out of budget peer", peer, score = peer.score
   else:
-    # TODO review deduplicate algorithm
-    # * https://github.com/nim-lang/Nim/blob/5f46474555ee93306cce55342e81130c1da79a42/lib/pure/collections/sequtils.nim#L184
-    # * it's probably not efficient and might give preference to the first dupe
-    let deIhaves = ihaves.deduplicate()
-    for ihave in deIhaves:
+    for ihave in ihaves:
       trace "peer sent ihave",
         peer, topic = ihave.topicId, msgs = ihave.messageIds
-      if ihave.topicId in g.mesh:
-        # also avoid duplicates here!
-        let deIhavesMsgs = ihave.messageIds.deduplicate()
-        for msgId in deIhavesMsgs:
+      if ihave.topicId in g.topics:
+        for msgId in ihave.messageIds:
           if not g.hasSeen(msgId):
-            if peer.iHaveBudget > 0:
+            if peer.iHaveBudget <= 0:
+              break
+            elif msgId notin res.messageIds:
               res.messageIds.add(msgId)
               dec peer.iHaveBudget
               trace "requested message via ihave", messageID=msgId
-            else:
-              break
     # shuffling res.messageIDs before sending it out to increase the likelihood
     # of getting an answer if the peer truncates the list due to internal size restrictions.
     g.rng.shuffle(res.messageIds)
diff --git a/tests/pubsub/testgossipinternal.nim b/tests/pubsub/testgossipinternal.nim
index 2e79e2355..e574ce611 100644
--- a/tests/pubsub/testgossipinternal.nim
+++ b/tests/pubsub/testgossipinternal.nim
@@ -658,11 +658,14 @@ suite "GossipSub internal":
 
     proc handler(peer: PubSubPeer, msg: RPCMsg) {.async.} =
       check false
+    proc handler2(topic: string, data: seq[byte]) {.async.} = discard
 
     let topic = "foobar"
     var conns = newSeq[Connection]()
     gossipSub.gossipsub[topic] = initHashSet[PubSubPeer]()
     gossipSub.mesh[topic] = initHashSet[PubSubPeer]()
+    gossipSub.subscribe(topic, handler2)
+
     for i in 0..<30:
       let conn = TestBufferStream.new(noop)
       conns &= conn