search

waku-org / js-waku

JavaScript implementation of Waku v2
https://js.waku.org
Apache License 2.0
166 stars 42 forks source link

bug: ApplicationInfo to PubsubTopic doesn't take clusterId into consideration #1902

Closed fbarbu15 closed 1 month ago

fbarbu15 commented 5 months ago

This is a bug report

Problem

shardInfoToPubsubTopics and ensureShardingConfigured doesn't take clusterId into consideration when resolving the PubsubTopic for ApplicationInfo

IMO this needs to work because ApplicationInfo also contains clusterId and I expect the PubsubTopic to match it

Proposed Solutions

clusterId needs to be passed to contentTopicToPubsubTopic in those 2 functions

danisharora099 commented 5 months ago

related to https://github.com/waku-org/nwaku/issues/2502

fbarbu15 commented 5 months ago

This was fixed on nwanku side here

danisharora099 commented 2 months ago

@fbarbu15 I'm not sure I fully understand this issue. clusterId is indeed taken into account with ApplicationInfo. What am I missing? https://github.com/waku-org/js-waku/blob/4db508b962736426f4897995a2b525c82fe0ba37/packages/utils/src/common/sharding.ts#L63-L70

cc @waku-org/js-waku-developers

fbarbu15 commented 2 months ago

Sorry, totally forgot about this issue I've applied this fix when I worked on js-waku sharding tests If it's not correct please feel free to revert/fix it

fbarbu15 commented 1 month ago

Closing according to the above comment