fix: don't mount metadata for clusterID 0

[chaitanyaprem]

Description

While debugging some issue with waku simulator, i had noticed that eventhough clusterID is set to 0 we are mounting metadata as it is indicated in the identify protocol.

159 DBG 2024-04-18 17:47:35.862+00:00 identify: decoded message topics="libp2p identify" tid=1 file=identify.nim:180 c onn=16U*oFR2ip:66215cb741a196224444402d pubkey=some(s...671d)) addresses=/ip4/10.1.0.20/tcp/60000 protocols=/ipfs/id/1.0.0,/libp2p/ autonat/1.0.0,/libp2p/circuit/relay/0.2.0/hop,/vac/waku/metadata/1.0.0,/vac/waku/relay/2.0.0,/rendezvous/1.0.0,/ipfs/ping/1.0.0 obs ervable_address=some(/ip4/10.1.0.1/tcp/36914) proto_version=ipfs/0.1.0 agent_version=nwaku signedPeerRecord=None

Changes

• ...
• ...

[github-actions]

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2611-rln-v2-true

Built from 9bae910

[gabrielmer]

LGTM, thank you!

[Ivansete-status]

I think clusterId == 0 could be a valid value. I think the allowed values goes from 0 to 2^16-1, but better @alrevuelta to confirm :)

Maybe we should use the value -1 to consider the "non metadata" case

[chaitanyaprem]

I think clusterId == 0 could be a valid value. I think the allowed values goes from 0 to 2^16-1, but better @alrevuelta to confirm :)

Maybe we should use the value -1 to consider the "non metadata" case

It is a valid value but not for sharding, because clusterID 0 is defined as global in the spec and used for default pubsubTopic.
Hence, metadata should not be mounted with this. This is how it is implemented in go-waku as well.

[SionoiS]

I think the intent is to have the Metadata protocol always mounted so that it can disconnect nodes not on the same cluster.

If not mounted for cluster 0, nodes would connect to any other nodes.

[chaitanyaprem]

I think the intent is to have the Metadata protocol always mounted so that it can disconnect nodes not on the same cluster.

If not mounted for cluster 0, nodes would connect to any other nodes.

Good Point, but wouldn't nodes on any other cluster anyway disconnect connections from cluster 0 as it doesn't support metadata protocol?

[chaitanyaprem]

Maybe we should use the value -1 to consider the "non metadata" case

I think any value less than 0 should result in config error.

[SionoiS]

I think the intent is to have the Metadata protocol always mounted so that it can disconnect nodes not on the same cluster.
If not mounted for cluster 0, nodes would connect to any other nodes.

Good Point, but wouldn't nodes on any other cluster anyway disconnect connections from cluster 0 as it doesn't support metadata protocol?

Maybe it should work this way but ATM in nwaku the disconnection happen when 2 cluster are different, so 2 mounted Metadata is needed.

[chaitanyaprem]

Maybe it should work this way but ATM in nwaku the disconnection happen when 2 cluster are different, so 2 mounted Metadata is needed.

Yeah, realized it wont make a difference. Closing this PR.