search

vacp2p / nim-libp2p

libp2p implementation in Nim
https://vacp2p.github.io/nim-libp2p/docs/
MIT License
240 stars 52 forks source link

Support gossipsub 1.2.0 #1102

Closed arnetheduck closed 2 weeks ago

arnetheduck commented 1 month ago

https://github.com/libp2p/specs/pull/548 - this has been merged and IDONTWANT is officially part of the spec - the feature is formally only enabled in gossipsub 1.2.0, meaning we must start supporting that version as a protocol in order to receive IDONTWANT messages from other clients (putting the fields in 1.1 was a hack).

diegomrsantos commented 1 month ago

Is it enough to create a new version here https://github.com/vacp2p/nim-libp2p/blob/0911cb20f4a0b880451938988f24cb077cfd8b1b/libp2p/protocols/pubsub/gossipsub/types.nim#L22 and add it here https://github.com/vacp2p/nim-libp2p/blob/0911cb20f4a0b880451938988f24cb077cfd8b1b/libp2p/protocols/pubsub/gossipsub.nim#L197 ?

kaiserd commented 1 month ago

Generally, yes. We should also limit sending IDONTWANT to peers that support v 1.2.0.

Still, would make sense to just up the version number to fix the issue of not receiving IDONTWANT from 1.2 version impls.

Let's also double check our current implementation matches the spec.

kaiserd commented 3 weeks ago

work on this started in #1106

ufarooqstatus commented 3 weeks ago

Gossipsub-v1.2 specification suggest that we MAY cancel outstanding IWANT if we receive the message from some other peer

This specification also suggests that we SHOULD cancel any queued IWANT reply if we receive IDONTWANT from the requesting peer. IMO, we have this available in this draft PR