waku-org / js-waku

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

fix: use correct shard index when creating encoder #1885

Closed adklempner closed 4 months ago

adklempner commented 6 months ago

Problem

When library consumer specifies a cluster ID and shard index when creating an encoder, it means they are using static sharding and explicitly specifying which shard index (and thus pubsub topic) should be used.

Currently, when an encoder is created this way, it uses the autosharding algorithm to determine the shard index based on content topic, which can lead to unexpected results if it doesn't match the shard index specified by the consumer.

Solution

- Make `shard` property optional in `SingleShardInfo`. - Fix conditional that evaluates to falsy when `shard` is set to `0` (this was preventing static sharding from being used) ## Notes
github-actions[bot] commented 6 months ago

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
Waku node 180.73 KB (-0.07% 🔽) 3.7 s (-0.07% 🔽) 2.7 s (+8.76% 🔺) 6.4 s
Waku Simple Light Node 180.86 KB (-0.07% 🔽) 3.7 s (-0.07% 🔽) 2.1 s (+2.04% 🔺) 5.7 s
ECIES encryption 23.11 KB (-0.02% 🔽) 463 ms (-0.02% 🔽) 538 ms (-16.89% 🔽) 1000 ms
Symmetric encryption 22.59 KB (+0.05% 🔺) 452 ms (+0.05% 🔺) 789 ms (+56.95% 🔺) 1.3 s
DNS discovery 72.42 KB (0%) 1.5 s (0%) 1.8 s (+19.54% 🔺) 3.2 s
Peer Exchange discovery 73.96 KB (0%) 1.5 s (0%) 929 ms (+22.21% 🔺) 2.5 s
Local Peer Cache Discovery 67.64 KB (0%) 1.4 s (0%) 1.5 s (+82.95% 🔺) 2.9 s
Privacy preserving protocols 38.87 KB (0%) 778 ms (0%) 1.1 s (+0.17% 🔺) 1.8 s
Waku Filter 111.25 KB (-0.17% 🔽) 2.3 s (-0.17% 🔽) 1.5 s (-11.53% 🔽) 3.7 s
Waku LightPush 110.23 KB (0%) 2.3 s (0%) 1.3 s (-11.78% 🔽) 3.5 s
History retrieval protocols 110.71 KB (0%) 2.3 s (0%) 2 s (+53.11% 🔺) 4.2 s
Deterministic Message Hashing 4.83 KB (0%) 97 ms (0%) 40 ms (-61.62% 🔽) 137 ms
adklempner commented 6 months ago

@danisharora099 The reason I made the shard field optional is because right now there's no other way for createEncoder to differentiate between static sharding and autosharding. e.g. in the test { clusterId: 0 } makes the function determine shard index using the content topic, but {clusterId: 0, shard: 1} will always use shard index 1, no matter the content topic.