waku-org / js-waku

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

feat: filter.createSubscription accept ShardParams #1967

Closed vpavlin closed 1 month ago

vpavlin commented 2 months ago

support request

Problem

It seems like filter.createSubscription accepts either pubsubTopic or SingleShardInfo - which in turn consists of ClusterId and shard

Would it make more sense to accept ShardingParams to make it more flexible?

Requiring SingleShardInfo forces me to get the shard from contentTopic before calling createSubscription, which seems unnecessary to be done by a user/dev.

But I might be missing the reasoning behind this choice:)

Proposed Solutions

Notes

danisharora099 commented 2 months ago

Looks right to me. Good point. Thanks for flagging this @vpavlin

Looks like the filter API wasn't updated after we introduced autosharding. Moving to priority.

@adklempner are you happy to address this?

weboko commented 2 months ago

I agree with @vpavlin and I think better way would be to simplify as much as possible but also allow to make fine tuning.

For that I see we can do:

Since I am touching this code in https://github.com/waku-org/js-waku/pull/1958 I might be able to address it there.

danisharora099 commented 1 month ago

@weboko issue seems to be in the "done" column but still open -- I guess we can close it?