Allow cancelling IWANT using IDONTWANT #591
Conversation
As specified in the Gossipsub v1.2 spec, we should allow cancelling IWANT by IDONTWANT. That is if IDONTWANT already arrived, we should not process IWANT. However due to the code structure, we can cancel IWANT only in handleIWant.
| @@ -839,6 +839,11 @@ func (gs *GossipSubRouter) handleIWant(p peer.ID, ctl *pb.ControlMessage) []*pb. | |||
| ihave := make(map[string]*pb.Message) | |||
| for _, iwant := range ctl.GetIwant() { | |||
| for _, mid := range iwant.GetMessageIDs() { | |||
| // Check if that peer has sent IDONTWANT before, if so don't send them the message | |||
| if _, ok := gs.unwanted[p][computeChecksum(mid)]; ok { | |||
There was a problem hiding this comment.
You meant computeChecksum? no, it's just one hash function.
|
@nisdas want to chime in? |
|
Is it common for non-mesh peers to send us IDONTWANTs ? The way i see it both control messages target different peers. IDONTWANT - mesh , IWANT - non mesh |
I don't think it's quite useful either, but it's specified in the spec and Lighthouse is already sending IDONTWANT after it sends IWANT. See https://github.com/sigp/lighthouse/blob/stable/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs#L2733 While we don't send IDONTWANT to non-mesh peers ourselves, I think it's useful to handle IDONTWANT for Lighthouse, otherwise their IDONTWANT will be wasteful. So yes, it's common. |
|
@ppopth One thing that is important to confirm is if other implementations correctly ignore this for scoring. As of gossipsub v1.1 , unfulfilled promises lead to the downscoring of peers. So if you advertise an However with |
Yeah, it's specified in the spec that if I don't reply to |
As specified in the Gossipsub v1.2 spec, we should allow cancelling IWANT by IDONTWANT.
That is if IDONTWANT already arrived, we should not process IWANT.
However due to the code structure, we can cancel IWANT only in handleIWant.
https://github.com/libp2p/specs/blob/master/pubsub/gossipsub/gossipsub-v1.2.md#cancelling-iwant