-
Notifications
You must be signed in to change notification settings - Fork 41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: only dial a peer if it has a wss multiaddr #1983
Conversation
dfe7320
to
789a5e1
Compare
size-limit report 📦
|
const peer = await this.libp2p.peerStore.get(peerId); | ||
|
||
if ( | ||
!peer.addresses.some((val) => val.multiaddr.toString().includes("/wss/")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
know it's not for review yet but came here to mention the address can also be of the format /tls/ws/
nice! |
@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? |
closing in favor of #1989 |
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:
!
in title if breaks public API;