Skip to content
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

fix: do not allow dial to large number of multiaddrs #954

Merged

Conversation

vasco-santos
Copy link
Member

@vasco-santos vasco-santos commented Jun 25, 2021

There are peers on the wild that advertise hundreds of multiaddrs, either by bugs with weird UPnP behavior/Port mapping on their router, or even by potential disturbers in the network. These peers make a libp2p node attempt to dial every single multiaddr and likely fail for all the hundreds of discovered multiaddrs for the peer. Moreover, libp2p will attempt over and over again if it has a small number of peers connected.

A dialable peer from js-libp2p will advertise TCP || Websockets || WebRTCStar || WebRTCDirect + IPv4 || IPv6, which means 8 different combinations. However, a peer should be able to advertise multiple different addresses for a single transport and it is typical to have peers advertising public and private addresses.

In this PR, we will start by a quick in solution, until we have multiaddr confidence in place.

Reasonably, a dialable peer should not have more than 25 multiaddrs supported by a js-libp2p node. This PR adds a limit of 25 addresses when creating a dial target (after filtering out incompatible addresses like QUIC). If a peer has more than 25 dialable multiaddrs, the dial will throw without attempting to dial all of them and the peer will be removed from the PeerStore to not be advertised.

Closes #928

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@vasco-santos vasco-santos force-pushed the fix/do-not-allow-dial-to-large-number-of-multiaddrs branch from 9b20e80 to 2d4b6a6 Compare June 25, 2021 11:00
@lidel lidel requested a review from achingbrain June 25, 2021 14:24
@vasco-santos vasco-santos merged commit af723b3 into master Jul 9, 2021
@vasco-santos vasco-santos deleted the fix/do-not-allow-dial-to-large-number-of-multiaddrs branch July 9, 2021 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Strategy when dialling peers with a lot of addresses
1 participant