waku-org / js-waku

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

feat!: accept one filed for network configuration #2027

Open vpavlin opened 1 month ago

vpavlin commented 1 month ago
This is a **bug report** ## Problem When `contentTopics` is specified, `options.shardInfo` will get populated here https://github.com/waku-org/js-waku/blob/master/packages/sdk/src/utils/libp2p.ts#L98 If `shardInfo` is specified, `pubsubTopics` get rewritten https://github.com/waku-org/js-waku/blob/master/packages/sdk/src/utils/libp2p.ts#L105 ## Proposed Solutions Either error out if both `shardInfo` and `pubsubTopics` is populated, or do not overwrite `pubsubTopics` if they are explicitly defined
vpavlin commented 1 month ago

WDYT @weboko ?

weboko commented 1 month ago

@vpavlin, what is the case for users to specify both shardInfo and pubsubTopics?

vpavlin commented 1 month ago

No idea:) I think the case is specifying contentTopic and pubsubTopic - maybe even unintentionally without fully understanding what they are doing?

Consider this with assumption js-waku works with TWN:

  1. I specify contentTopic and lean on TWN
  2. I decide to move to my own pubsub for arbitrary reason, so I simply add pubsubTopic assuming that is all I need
  3. Things break:)

The actual case is this change https://github.com/waku-org/js-waku/commit/5b03709dfe683b1cb001fe67c5bd013e664b4d89 broke things that previously worked - i.e. contentTopic and pubsubTopic specified at once.

Regardless of what the use case for having both shardInfo and pubsubTopic js-waku should let users know they should not specify both - i.e. report the error - as I suggested in the description.

weboko commented 1 week ago

I think this is something that is not clear and we need more feedback on it. Overall I think node should be run either in TWN or on custom network, not both.

For action point for this task I think we should refine createNode options as they are confusing.

My approach would be to:

Slightly intervened with https://github.com/waku-org/js-waku/issues/2034