hanvleIWant ignores IDONTWANT, so we send messages go-libp2p and rust-libp2p would skip
Nobody has claimed this yet.
- Dominant language
- Kotlin
- Stars
- 366
- Forks
- 85
- Avg merge
- 2d 39m
- Merged PRs (30d)
- 6
Description
Problem
handleIWant never consults the per-peer IDONTWANT set. If a peer sends us IDONTWANT for a message id and then we get an IWANT for that same id, we serve the full message anyway. go-libp2p and rust-libp2p both skip it.
We honor IDONTWANT everywhere else. peerDoesNotWantMessage (GossipRouter.kt:805) is applied on both broadcast paths, at GossipRouter.kt:526 for inbound forwarding and GossipRouter.kt:550 for local publish. The IWANT response path is the one place it is missing.
This is not a vulnerability. An attacker gains nothing by telling us not to send a message and then asking for it. The cost falls on honest peers, and since we only emit IDONTWANT for messages at or above iDontWantMinMessageSizeThreshold (16 KiB by default), every message affected is a large one.
Root cause
GossipRouter.kt:380-387:
private fun handleIWant(msg: Rpc.ControlIWant, peer: PeerHandler) {
val peerScore = score.score(peer.peerId)
if (peerScore < scoreParams.gossipThreshold) return
msg.messageIDsList
.mapNotNull { mCache.getMessageForPeer(peer.peerId, it.toWBytes()) }
.filter { it.sentCount < params.gossipRetransmission }
.forEach { submitPublishMessageSilently(peer, it.msg) }
}
Score gate, cache lookup, retransmission cap, send. No IDONTWANT check.
Cross-implementation comparison
go-libp2p-pubsub, gossipsub.go:1023, first check inside the id loop:
// Check if that peer has sent IDONTWANT before, if so don't send them the message
if _, ok := gs.unwanted[p][computeChecksum(mid)]; ok {
continue
}
rust-libp2p, behaviour.rs handle_iwant, checked after the retransmission cap:
if let Some(peer) = self.connected_peers.get_mut(peer_id)
&& peer.dont_send.contains_key(&id) {
tracing::debug!(%peer_id, message_id=%id, "Peer already sent IDONTWANT for this message");
continue;
}
We are the only one of the three that skips this check.
Spec
gossipsub v1.2 scopes the MUST to relaying to the mesh, which we already satisfy:
When later relaying the
messageIdmessage to the mesh the peers found indont_send_message_idsMUST be skipped.
The IWANT path is covered separately, under "Cancelling IWANT", at SHOULD/MAY strength:
If a node requested a message via
IWANTand then occasionally receives the message from other peer it MAY try to cancel itsIWANTrequests with the correspondingIDONTWANTmessage. It may work in cases when a peer delays/queuesIWANTrequests and theIWANTrequest SHOULD be removed from the queue if not processed yet
So this is not a spec violation. It is the spec's stated use case for IDONTWANT cancelling an IWANT, and we cannot honor it at all today because the set is never read on that path.
Impact
Reachability: Any mesh peer, no special capability. Reached on the normal path, by a peer that requests a message via IWANT, receives it from someone faster, and cancels with IDONTWANT.
Cost: One redundant transmission of a >=16 KiB message per (peer, message id), up to gossipRetransmission (3) times, inside the iDontWantTTL window (3s). The receiver discards it as a duplicate.
Where it bites: Exactly the case IDONTWANT exists to fix. Large messages, blob sidecars in particular, where the whole point of the extension is to avoid paying for a transmission the peer has already said it does not need.
Suggested fix
Add the filter to the existing chain in handleIWant:
msg.messageIDsList
.map { it.toWBytes() }
.filterNot { peerDoesNotWantMessage(peer, it) }
.mapNotNull { mCache.getMessageForPeer(peer.peerId, it) }
.filter { it.sentCount < params.gossipRetransmission }
.forEach { submitPublishMessageSilently(peer, it.msg) }
Filtering before getMessageForPeer also stops a skipped id from burning a retransmission slot. That matches go, where the IDONTWANT check runs before GetForPeer. rust checks after. go's order is the better one: a message we deliberately did not send should not count against the peer's retransmission budget.
No protocol.supportsIDontWant() guard is needed. peerIDontWant is only populated in handleIDontWant, which already returns early for peers that do not support the extension (GossipRouter.kt:390).
Test to add: send IDONTWANT for an id, then IWANT for the same id, and assert nothing is sent and the retransmission counter is untouched.
Related
- #522, #519, #526, from the same cross-implementation review pass.
- Found while comparing IWANT response-side controls across the three implementations.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in GossipRouter.kt at handleIWant and read peerDoesNotWantMessage, then inspect the existing IDONTWANT handling and broadcast paths. Add coverage for sending IDONTWANT followed by IWANT for the same id; done means no message is sent and the retransmission counter is unchanged.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- kotlin
- Domain
- distributed-systems, networking
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 88/100