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: inconsistency in contentTopic generation #3165

Open
vpavlin opened this issue Oct 31, 2024 · 2 comments
Open

bug: inconsistency in contentTopic generation #3165

vpavlin opened this issue Oct 31, 2024 · 2 comments
Labels
bug Something isn't working effort/days Estimated to be completed in a few days, less than a week

Comments

@vpavlin
Copy link
Member

vpavlin commented Oct 31, 2024

Problem

When using js-waku and go-waku as clients and nwaku as a service node, We (I and @richard-ramos) noticed inconsistent behaviour with the genration in contentTopic

The flow is:

  1. Publish using lightPush from js-waku on contentTopic: /0/qaku/1/persist/json
  2. Subscribe to filter using go-waku on contentTopic: /qaku/1/persist/json
  3. Observe that messages are not pushed to the fitler node

After a lot of looking we realized the reason is the missing generation number which is optional (https://github.com/waku-org/specs/blob/b38f248b4fb56a442f609c605b9ab65471b5afed/standards/core/relay-sharding.md?plain=1#L195), but IMO the default should be added by the service node if not provided

Impact

Medium - it can be workarounded by using (or not using) the generation number across app implementations

To reproduce

As mentioned above

Expected behavior

Service node (nwaku) adds default generation number (0) if it is not included in the contentTopic

Screenshots/logs

If applicable, add screenshots or logs to help explain your problem.

nwaku version/commit hash

js-waku 0.0.29
go-waku v0.8.1-0.20240921011719-821481fec446
nwaku - waku.test and waku.sandbox fleets

CC @SionoiS @Ivansete-status

@vpavlin vpavlin added the bug Something isn't working label Oct 31, 2024
@SionoiS
Copy link
Contributor

SionoiS commented Oct 31, 2024

🤔 If we define /0/qaku/1/persist/json as equivalent to /qaku/1/persist/json why would the nwaku node add the 0, it's redundant.

It's true that the message should be relayed thought! On the other hand, the content topic generation should not affect the routing.

Can you check that all nodes use the same pubsub topic? This is weird...

@SionoiS
Copy link
Contributor

SionoiS commented Oct 31, 2024

Could it be that no impl. treat /0/qaku/1/persist/json as equivalent to /qaku/1/persist/json ?

I can't find the code that would do that for nwaku and also we use content topic as string in a lot of places so were F.

Who thought using string everywhere would be fine. Type systems exist for a reason... 😠

@Ivansete-status Ivansete-status moved this to To Do in Waku Nov 26, 2024
@Ivansete-status Ivansete-status added the effort/days Estimated to be completed in a few days, less than a week label Nov 26, 2024
@Ivansete-status Ivansete-status changed the title bug: bug: inconsistency in contentTopic generation Nov 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 effort/days Estimated to be completed in a few days, less than a week
Projects
Status: To Do
Development

No branches or pull requests

3 participants