salt idontwant (#1090)

vacp2p · May 2, 2024 · aef44ed · aef44ed
1 parent 02c96fc
commit aef44ed
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 9 deletions.
diff --git a/libp2p/protocols/pubsub/gossipsub.nim b/libp2p/protocols/pubsub/gossipsub.nim
@@ -360,13 +360,13 @@ proc handleControl(g: GossipSub, peer: PubSubPeer, control: ControlMessage) =
 
 proc validateAndRelay(g: GossipSub,
                       msg: Message,
-                      msgId: MessageId, msgIdSalted: SaltedId,
+                      msgId: MessageId, saltedId: SaltedId,
                       peer: PubSubPeer) {.async.} =
   try:
     let validation = await g.validate(msg)
 
     var seenPeers: HashSet[PubSubPeer]
-    discard g.validationSeen.pop(msgIdSalted, seenPeers)
+    discard g.validationSeen.pop(saltedId, seenPeers)
     libp2p_gossipsub_duplicate_during_validation.inc(seenPeers.len.int64)
     libp2p_gossipsub_saved_bytes.inc((msg.data.len * seenPeers.len).int64, labelValues = ["validation_duplicate"])
 
@@ -413,14 +413,13 @@ proc validateAndRelay(g: GossipSub,
 
     for peer in toSendPeers:
       for heDontWant in peer.heDontWants:
-        if msgId in heDontWant:
+        if saltedId in heDontWant:
           seenPeers.incl(peer)
           libp2p_gossipsub_idontwant_saved_messages.inc
           libp2p_gossipsub_saved_bytes.inc(msg.data.len.int64, labelValues = ["idontwant"])
           break
     toSendPeers.excl(seenPeers)
 
-
     # In theory, if topics are the same in all messages, we could batch - we'd
     # also have to be careful to only include validated messages
     g.broadcast(toSendPeers, RPCMsg(messages: @[msg]), isHighPriority = false)

diff --git a/libp2p/protocols/pubsub/gossipsub/behavior.nim b/libp2p/protocols/pubsub/gossipsub/behavior.nim
@@ -269,8 +269,7 @@ proc handleIDontWant*(g: GossipSub,
   for dontWant in iDontWants:
     for messageId in dontWant.messageIDs:
       if peer.heDontWants[^1].len > 1000: break
-      if messageId.len > 100: continue
-      peer.heDontWants[^1].incl(messageId)
+      peer.heDontWants[^1].incl(g.salt(messageId))
 
 proc handleIWant*(g: GossipSub,
                  peer: PubSubPeer,
@@ -639,7 +638,7 @@ proc onHeartbeat(g: GossipSub) {.raises: [].} =
         peer.sentIHaves.addFirst(default(HashSet[MessageId]))
         if peer.sentIHaves.len > g.parameters.historyLength:
           discard peer.sentIHaves.popLast()
-        peer.heDontWants.addFirst(default(HashSet[MessageId]))
+        peer.heDontWants.addFirst(default(HashSet[SaltedId]))
         if peer.heDontWants.len > g.parameters.historyLength:
           discard peer.heDontWants.popLast()
         peer.iHaveBudget = IHavePeerBudget

diff --git a/libp2p/protocols/pubsub/pubsubpeer.nim b/libp2p/protocols/pubsub/pubsubpeer.nim
@@ -80,7 +80,10 @@ type
 
     score*: float64
     sentIHaves*: Deque[HashSet[MessageId]]
-    heDontWants*: Deque[HashSet[MessageId]]
+    heDontWants*: Deque[HashSet[SaltedId]]
+      ## IDONTWANT contains unvalidated message id:s which may be long and/or
+      ## expensive to look up, so we apply the same salting to them as during
+      ## unvalidated message processing
     iHaveBudget*: int
     pingBudget*: int
     maxMessageSize: int
@@ -504,5 +507,5 @@ proc new*(
     maxNumElementsInNonPriorityQueue: maxNumElementsInNonPriorityQueue
   )
   result.sentIHaves.addFirst(default(HashSet[MessageId]))
-  result.heDontWants.addFirst(default(HashSet[MessageId]))
+  result.heDontWants.addFirst(default(HashSet[SaltedId]))
   result.startSendNonPriorityTask()