search

helium / erlang-libp2p

An Erlang implementation of libp2p swarms
https://helium.github.io/erlang-libp2p
Apache License 2.0
121 stars 34 forks source link

Try to prevent stungunning relayed peers #432

Closed mawdegroot closed 2 years ago

mawdegroot commented 2 years ago

Filter the list of connected peers to exclude peers that do not have a public address per suggestion of @ci-work in #415

I don't think relayed nodes are the problem except the subset of relayed nodes that is running behind some CGNAT or similar system. The easiest way to avoid the issue is to stungun just those peers of which it is known they have a public ip address.

I have been running this change for almost 24h now and it hasn't yet picked up a relay address. I will deploy this to one more node and have it run over the weekend to limit the amount of luck involved.

Some potential problems I considered:

  1. What if there are only relayed peers connected to this node? It shouldn't matter since it will start a relay if it fails a few times in a row.

Addresses #415

ci-work commented 2 years ago

can I suggest a modification, where has public and does not have relayed, as frequently due to this issue devices have both, e.g. 4.1k validators have a public IP, and 2.5k of them also have a relayed address right now.

mawdegroot commented 2 years ago

I don't think it will matter since if it has a public ip address that will be the prioritized address that you connect to. That it also has a p2p address does not indicate that the peer itself has a broken internet configuration, just that it was hit by the same problem this commit tries to prevent.

I will run a second node with your suggestion and see if it makes any difference regardless 👍

ci-work commented 2 years ago

thanks, and also for the additional insight!

mawdegroot commented 2 years ago

The instance with this check has been running over the weekend without starting up a relay so this fix seems to prevent (at least a large part) of the issue. The second instance that also prevents selecting connected peers with a p2p address (as secondary address) achieved the same results so I propose to keep the change minimal and only check for a public IP.

Removing the draft label as the fix has had the intended result over a longer run.

ci-work commented 2 years ago

confirming no issues in running, appears to resolve or at least help issue.

ci-work commented 2 years ago

confirming still no issues, some weeks later

Vagabond commented 2 years ago

This is actually wrong as libp2p_peer:connected_peers() doesn't return peerbook entries itself, but rather pubkey bins, which are not valid to call libp2p_peer:listen_addrs() on.