Gossipsub Hello RPC not spec compliant

[dennis-tra]

Summary

I'm currently extending Nebula to also track the GossipSub topics that the remote peer is subscribed to. To do that I'm waiting for the remote peer to open a GossipSub stream and send the hello RPC which contains the list of subscribed topics.

I noticed that for Lighthouse peers the list only contained a single entry. Turns out that each topic subscription is sent as an individual messaage:

rust-libp2p/protocols/gossipsub/src/behaviour.rs

Lines 2955 to 2957 in 21d2c46

	for topic_hash in self.mesh.clone().into_keys() {
	self.send_message(peer_id, RpcOut::Subscribe(topic_hash));
	}

This is, I guess, not a big issue when it comes to interoperability but it's not spec compliant:

Whenever a new peer is connected, the gossipsub implementation checks to see if the peer implements floodsub and/or gossipsub, and if so, it sends it a hello packet that announces the topics that it is currently subscribing to.

Expected behavior

I would expect that the initial RPC packet contains all topics that a peer is subscribed to.

Actual behavior

The topics that a peer is subscribed to are split across multiple RPCs.

Relevant log output

Possible Solution

No response

Version

No response

Would you like to work on fixing this bug?

Maybe

cc @cortze

[MarcoPolo]

I think in slightly more concrete terms this "hello packet" is at most a SHOULD. It can't be a MUST because a peer could always subscribe right after connecting and the receiver could not distinguish this case. Since it's a SHOULD an implementation can choose not to do this.

I don't think rust-libp2p needs to make a change here. Maybe Nebula should handle this case instead?

[dennis-tra]

Agreed, it can't be a MUST since a receiver must always handle the case where a peer subscribes right after connecting. That's exactly why I hedged with "not a big issue" in the original report, though really it's not at all an interoperability issue. Implementations work fine together precisely because they already handle that case.

Still, the current behaviour doesn't match the spec. The fix would be minor on either side, clarifying the spec or batching the subscriptions here (I assume this is minor). Since Go, Nim, and JS all already send a single hello packet with all subscriptions, I'd lean towards aligning rust-libp2p with them rather than updating the spec to match the outlier.

As for Nebula: I've already implemented handling for this, so I'm fine. It's a bit less elegant since there's no signal that all subscriptions have been sent, forcing me to rely on a timeout. But that's my problem, nothing to inform any decision here. I just happened to rely on the spec'd behaviour and got bitten.

[MarcoPolo]

Agreed, it can't be a MUST since a receiver must always handle the case where a peer subscribes right after connecting. That's exactly why I hedged with "not a big issue" in the original report, though really it's not at all an interoperability issue. Implementations work fine together precisely because they already handle that case.

👍

Still, the current behaviour doesn't match the spec. The fix would be minor on either side, clarifying the spec or batching the subscriptions here (I assume this is minor).

I think clarifying the spec makes sense. The "hello packet" is only mentioned once in passing. If this is a "SHOULD", then I'd argue the current rust-libp2p is spec compliant even if it doesn't follow this SHOULD.

That said, I'm not opposed to rust-libp2p changing its behavior to align with the "SHOULD" (if that's what we define in the spec). Or, in learning why rust-libp2p does not follow the "SHOULD."

As for Nebula: I've already implemented handling for this, so I'm fine.

Nice :)

[jxs]

Hi Dennis, thanks for opening this, and hi Marco thanks for chiming in!

I think in slightly more concrete terms this "hello packet" is at most a SHOULD. It can't be a MUST because a peer could always subscribe right after connecting and the receiver could not distinguish this case.

Yeah I think we have discussed this before when I asked you on a call or so what was the "hello packet". We don't have that concept in the rust-libp2p, what happened was that on #4914 we decided to send a single message/subscription per Rpc.
I think it's a small change to send all the subscriptions in a single Rpc when first connecting, I can do it

[Shashwat-Nautiyal]

Hii, I went through the above discussion. Since the issue hasn't been closed yet, can i give it a shot?

[jxs]

Hi, yeah you can go ahead, thanks

[0xAysh]

Hi @jxs, since there is no update from @Shashwat-Nautiyal , can I work on this?