waku-org / js-waku

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

feat: only dial a peer if it has a wss multiaddr #1983

Closed adklempner closed 2 months ago

adklempner commented 2 months ago

Problem

Connection manager will attempt to dial a peer even if the peer doesn't have a wss multiaddr. This leads to a lot of failing dials with the error "The dial request has no valid addresses"

Solution

When checking if a peer should be dialed, get the peer info from the peer store and skip dialing if none of the multiaddrs use wss ## Notes

Contribution checklist:

github-actions[bot] commented 2 months ago

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
Waku node 180.71 KB (-0.02% 🔽) 3.7 s (-0.02% 🔽) 18.5 s (-3.49% 🔽) 22.1 s
Waku Simple Light Node 180.8 KB (-0.04% 🔽) 3.7 s (-0.04% 🔽) 25.1 s (+25.99% 🔺) 28.7 s
ECIES encryption 23.11 KB (0%) 463 ms (0%) 7.4 s (+47.75% 🔺) 7.9 s
Symmetric encryption 22.59 KB (0%) 452 ms (0%) 5.8 s (+83.76% 🔺) 6.3 s
DNS discovery 72.42 KB (0%) 1.5 s (0%) 19.6 s (+52.32% 🔺) 21 s
Peer Exchange discovery 73.96 KB (0%) 1.5 s (0%) 16 s (+59.86% 🔺) 17.5 s
Local Peer Cache Discovery 67.64 KB (0%) 1.4 s (0%) 12.4 s (+29.85% 🔺) 13.8 s
Privacy preserving protocols 38.87 KB (0%) 778 ms (0%) 8.6 s (+2.73% 🔺) 9.4 s
Waku Filter 111.25 KB (0%) 2.3 s (0%) 18.9 s (+11.62% 🔺) 21.2 s
Waku LightPush 110.23 KB (0%) 2.3 s (0%) 18.7 s (+53.39% 🔺) 20.9 s
History retrieval protocols 110.71 KB (0%) 2.3 s (0%) 14.9 s (+42.13% 🔺) 17.1 s
Deterministic Message Hashing 4.83 KB (0%) 97 ms (0%) 668 ms (+104.17% 🔺) 765 ms
danisharora099 commented 2 months ago

nice!

adklempner commented 2 months ago

@danisharora099 I found an another way to do this by updating libp2p settings: https://github.com/waku-org/js-waku/pull/1989/files

Although this updates it so that the peers without wss don't end up in the peer store at all, whereas the method in this PR still has them in the peer store, it just doesn't attempt to make connections. Do you know if there's a benefit to having peers in the store even if we can't connect to them?

adklempner commented 2 months ago

closing in favor of #1989