Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: dont harcode clusterid for autosharding #1061

Closed
fbarbu15 opened this issue Mar 19, 2024 · 6 comments
Closed

bug: dont harcode clusterid for autosharding #1061

fbarbu15 opened this issue Mar 19, 2024 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@fbarbu15
Copy link

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

@fbarbu15 fbarbu15 added the bug Something isn't working label Mar 19, 2024
@chaitanyaprem chaitanyaprem self-assigned this Mar 20, 2024
@chaitanyaprem
Copy link
Collaborator

chaitanyaprem commented Mar 20, 2024

Started this here #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
Copy link
Author

fbarbu15 commented Mar 20, 2024

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
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Collaborator

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

@chaitanyaprem
Copy link
Collaborator

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

@chaitanyaprem chaitanyaprem closed this as not planned Won't fix, can't repro, duplicate, stale Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

No branches or pull requests

2 participants