GossipSub: IDontWant #934
GossipSub: IDontWant #934
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## unstable #934 +/- ##
============================================
+ Coverage 83.43% 83.46% +0.02%
============================================
Files 91 91
Lines 15117 15144 +27
============================================
+ Hits 12613 12640 +27
Misses 2504 2504
|
|
Confirmed to work properly in sims (~15% BW reduction with: 500 nodes, 100mbps, 100ms ping, 0.5kb messages) |
| @@ -332,11 +333,25 @@ proc validateAndRelay(g: GossipSub, | |||
| g.floodsub.withValue(t, peers): toSendPeers.incl(peers[]) | |||
| g.mesh.withValue(t, peers): toSendPeers.incl(peers[]) | |||
|
|
|||
|
|
|||
| # Don't send it to source peer, or peers that | ||
| # sent it during validation | ||
| toSendPeers.excl(peer) | ||
| toSendPeers.excl(seenPeers) | ||
|
|
||
| if msg.data.len > msgId.len * 10: |
There was a problem hiding this comment.
Can we make this condition more descriptive - like making it a proc or adding a comment with the reasoning?
| @@ -60,6 +60,7 @@ type | |||
|
|
|||
| score*: float64 | |||
| sentIHaves*: Deque[HashSet[MessageId]] | |||
| heDontWants*: Deque[HashSet[MessageId]] | |||
There was a problem hiding this comment.
why not iDontWants or sentIDontWants?
There was a problem hiding this comment.
This is the list of "iDontWant" he sent, so I think "heDontWants" or "receivedDontWants" is more descriptive
There was a problem hiding this comment.
so maybe rcvDontWants.
There was a problem hiding this comment.
just dontWants is enough really
| peer: PubSubPeer, | ||
| iDontWants: seq[ControlIWant]) = | ||
| for dontWant in iDontWants: | ||
| for message in dontWant.messageIds: |
| for dontWant in iDontWants: | ||
| for message in dontWant.messageIds: | ||
| if peer.heDontWants[^1].len > 1000: break | ||
| if message.len > 100: break |
There was a problem hiding this comment.
should it be a continue?
| # IDontWant is only worth it if the message is substantially | ||
| # bigger than the messageId | ||
| if msg.data.len > msgId.len * 10: | ||
| g.broadcast(toSendPeers, RPCMsg(control: some(ControlMessage( |
There was a problem hiding this comment.
we should only broadcast this to peers that haven't themselves sent us an idontwant for the given hash, ie this should be moved below
There was a problem hiding this comment.
That is true in this version of IDontWant, because as soon as we sent the IDontWant, we also send the full message, so it's too late to "choke" us
However, in future versions we might send the IDontWant earlier (before validation), or send the full message later (staggered sending)
So in these future versions, receiving a IDontWant won't mean it's too late to send ours. Sending the IDontWant to everyone is "forward-compatibility" at the cost of a few bytes
Implements libp2p/specs#548