waku-org / go-waku

Go implementation of Waku v2 protocol
https://waku.org/
Other
116 stars 42 forks source link

bug: dont harcode clusterid for autosharding #1061

Closed fbarbu15 closed 5 months ago

fbarbu15 commented 5 months ago

I see that all content topics are resolving to cluster id = 1 because this hardcoding Maybe we can remove it like it was removed on nwaku and js-waku

chaitanyaprem commented 5 months ago

Started this here https://github.com/waku-org/go-waku/pull/1063 thinking it would be simpler change, but it doesn't seem that straight-forward.

We may not take it up if it involves lot of changes to the code and to the API's that would be used by Status.

How important is it that this is needed in go-waku? Because before Status moves to auto-sharding, i think go-waku would be replaced by nwaku in status app. cc @richard-ramos

fbarbu15 commented 5 months ago

From interop testing POV (nwaku -> go-waku) we are testing autosharding on other cluster IDs because ID 1 is used by TWN and enforces the use of RLN and we don't have support yet for RLN in our testing framework. And even when we will support it I think we will tend to not use RLN for non-RLN tests because of rate-limiting and problems with concurrent tests. So if we can't use another cluster ID to test autosharding in go-waku, we will probably skip those tests for nwaku->go-waku runs and only run static-sharding tests

chaitanyaprem commented 5 months ago

So if we can't use another cluster ID to test autosharding in go-waku, we will probably skip those tests for nwaku->go-waku runs and only run static-sharding tests

I think this should be fine, because go-waku is anyways not being used for autosharding. Status is using go-waku with static-sharding only. And IIRC, all other users of go-waku use their own sharding. Maybe you can have interop between js-waku and nwaku for autosharding? cc @richard-ramos @fryorcraken

fbarbu15 commented 5 months ago

I think this should be fine, because go-waku is anyways not being used for autosharding. Status is using go-waku with static-sharding only. And IIRC, all other users of go-waku use their own sharding.

Will do that

Maybe you can have interop between js-waku and nwaku for autosharding?

Yes, we have such tests

chaitanyaprem commented 5 months ago

I mean, there is way to hack around to handle the complications. But don't want to do that unless absolutely necessary.

chaitanyaprem commented 5 months ago

As explained here, this need not be fixed in go-waku.