waku-org / js-waku

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

fix: increasing maxInboundStreams for lightpush from 32 to 100 #2021

Closed gabrielmer closed 1 month ago

gabrielmer commented 1 month ago

Problem

Solution for https://github.com/waku-org/nwaku/issues/2478 and https://github.com/waku-org/js-waku/issues/1790

Solution

The root cause seems to be in js-libp2p, in the DEFAULT_MAX_INBOUND_STREAMS const defined here

When running the test, got the following log

  libp2p:mplex:stream:receiver:36 abort with error CodeError: Too many inbound protocol streams for protocol "/vac/waku/filter-push/2.0.0-beta1" - limit 32
    at file:///Users/gabrielmer/status/workspace2/js-waku/node_modules/libp2p/src/upgrader.ts:388:29
    at processTicksAndRejections (node:internal/process/task_queues:95:5) {
  code: 'ERR_TOO_MANY_INBOUND_PROTOCOL_STREAMS',
  props: {}
} +0ms

If we configure a higher limit of input streams in js-libp2p, the bug is fixed

Note

Feel free to recommend cleaner ways to implement the solution. Probably better to have a constant instead of a hardcoded 100, but waiting for the recommendations of the js-waku experts 😁

github-actions[bot] commented 1 month ago

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
Waku node 181.41 KB (+0.02% 🔺) 3.7 s (+0.02% 🔺) 922 ms (+105.82% 🔺) 4.6 s
Waku Simple Light Node 181.43 KB (-0.06% 🔽) 3.7 s (-0.06% 🔽) 661 ms (-4.25% 🔽) 4.3 s
ECIES encryption 23.12 KB (0%) 463 ms (0%) 244 ms (+159.64% 🔺) 707 ms
Symmetric encryption 22.58 KB (0%) 452 ms (0%) 209 ms (+80.54% 🔺) 660 ms
DNS discovery 72.49 KB (0%) 1.5 s (0%) 338 ms (-36.99% 🔽) 1.8 s
Peer Exchange discovery 74.15 KB (0%) 1.5 s (0%) 452 ms (+97.61% 🔺) 2 s
Local Peer Cache Discovery 67.68 KB (0%) 1.4 s (0%) 678 ms (+178.23% 🔺) 2.1 s
Privacy preserving protocols 38.87 KB (0%) 778 ms (0%) 279 ms (-23.41% 🔽) 1.1 s
Waku Filter 111.92 KB (-0.1% 🔽) 2.3 s (-0.1% 🔽) 473 ms (+18.34% 🔺) 2.8 s
Waku LightPush 110.39 KB (0%) 2.3 s (0%) 506 ms (-14.47% 🔽) 2.8 s
History retrieval protocols 110.91 KB (0%) 2.3 s (0%) 368 ms (-23.69% 🔽) 2.6 s
Deterministic Message Hashing 7.29 KB (0%) 146 ms (0%) 73 ms (+85.42% 🔺) 219 ms
weboko commented 1 month ago

Thank you @gabrielmer for root causing the issue!

I think this config should be set here - https://github.com/waku-org/js-waku/blob/e49e7289ae632a1c2f3d11b9dc668ca68749ac7c/packages/sdk/src/utils/libp2p.ts#L69

If you don't mind I can commit to this PR

danisharora099 commented 1 month ago

Thank you @gabrielmer for root causing the issue!

I think this config should be set here -

https://github.com/waku-org/js-waku/blob/e49e7289ae632a1c2f3d11b9dc668ca68749ac7c/packages/sdk/src/utils/libp2p.ts#L69

If you don't mind I can commit to this PR

I wouldn't set it for all the connections by the node -- when we set it as done by @gabrielmer, we are only changing the value for that particular registrar handler/connection

gabrielmer commented 1 month ago

@weboko @danisharora099 thanks so much! Feel free to modify and commit to the PR in whatever way you feel it improves the solution. If there's anything I can help me just let me know anytime :))

danisharora099 commented 1 month ago

@weboko merging now, let me know!