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

Improve announced addresses #769

Closed
vasco-santos opened this issue Oct 2, 2020 · 8 comments
Closed

Improve announced addresses #769

vasco-santos opened this issue Oct 2, 2020 · 8 comments

Comments

@vasco-santos
Copy link
Member

vasco-santos commented Oct 2, 2020

This issue aims to find the best solution for announced addresses.

Libp2p is not controlling in any way the announced addresses. This means that users will need to properly configure them, in order to not announce not diable addresses. Moreover, we do not have any multiaddr confidence yet.

In this moment, libp2p announces its addresses as: Union between all listen addresses and provided announce addresses, filtering out provided noAnnounce addresses.

Listen addresses filtering

One of the common issues it to announce local network addresses. While uses cases that leverage these addresses exist, they are not the most common and they are easy to predict (MDNS discovery, or manual dials / bootstrap).

🏁 The best approach is defaulting to not announcing local network addresses unless explicitly added as announce addresses. This would result in: Union between all non-local listen addresses and provided announce addresses, filtering out provided noAnnounce addresses. This will result in identify not announcing local addresses that might get propagated from other peers.

Manual dials and bootstrap nodes rely on previously provided addresses. If a node needs to connect to other nodes in the local network, these will be added to the AddressBook anyway and there will be no issue. For MDNS, we can also add the local addresses in the listen addresses for the query responses.

Dial improvements

As there will be older nodes on the network, we should also try to improve the dialer to avoid using undiable addresses provided from older nodes.

While we do not have multiaddr confidence, we can also slightly improve the dialer. There are two angles we can look. The dial request can sort the known multiaddrs and try the locals in the end. But, if enough tokens it might try locals in a first batch. We can probably do the dial in 2 steps, first try out all non local addresses and then the local ones.

🏁If a combination of non local and local addresses exist, the probability that the local addr work is not high. This way, I think we can probably take the 2 steps dial.

Migration

This change should not have big consequences in working systems as they need to be relying on non local addresses. Anyway, people experimenting with local projects or local setups will probably break as a result of this change.

This should be well documented in the next migration guide.

@vasco-santos
Copy link
Member Author

@jacobheun let me know your thoughts on this.

🏁 are the points where we need a decision

@jacobheun
Copy link
Contributor

The best approach is defaulting to not announcing local network addresses unless explicitly added as announce addresses.

With this approach, how would you announce your local network addresses? Right now announce addresses are stringified multiaddrs, how would a user generate this for configuration?

I'd really just prefer to be able to pass a sorter/filter function for this, because it makes it much easier to provide fine grained control of the announced addresses without risking some configuration / environment values from adding "surprise" values.

If a combination of non local and local addresses exist, the probability that the local addr work is not high. This way, I think we can probably take the 2 steps dial.

I'm not sure I get the 2 step approach. It sounds like you're proposing two waves of dials, public and then private as a fallback? If so, I think this is unnecessary. We should prioritize addresses by public first then private, and we should also really expose a filter for people to be able to custom sort addresses.

Additionally, we just need to add multiaddr confidence and denylists, as they'll solve these problems and give us more functionality in general. This would solve problems like websockets attempting to connect to a local ip:port combo when there is no local server there.

re: migration, the only thing that should really "break" here is that we shouldn't continue to dial addresses that fail, we need to add the dial backoff back in (this was missed during the async/await transition).

@jacobheun
Copy link
Contributor

Also, @vasco-santos have you looked at the way go-libp2p composes the announced addresses? It would be helpful to document that here so we can clarify any differences.

@vasco-santos
Copy link
Member Author

With this approach, how would you announce your local network addresses? Right now announce addresses are stringified multiaddrs, how would a user generate this for configuration?

In the same way they currently can use the noAnnounce. The main issue is leveraging automatic ports.

I'd really just prefer to be able to pass a sorter/filter function for this, because it makes it much easier to provide fine grained control of the announced addresses without risking some configuration / environment values from adding "surprise" values.

I think you are right! It seems a better approach to have a custom sort and a custom filter. We can default for not announcing private addresses, but users can overwrite the function.

I'm not sure I get the 2 step approach. It sounds like you're proposing two waves of dials, public and then private as a fallback?

Yes. The main goal here was to avoid using dial tokens if not needed. But I guess we can postpone thinking about this type of optimization.

Also, @vasco-santos have you looked at the way go-libp2p composes the announced addresses? It would be helpful to document that here so we can clarify any differences.

No, but I will have a look and update this

@vasco-santos
Copy link
Member Author

go-libp2p has a similar concept for custom announcing addresses via the config with a provided function. However, they default to return all the addresses. Just in a test for autoRelay they custom it to not use local addresses.

So, taking into account the above discussion, what I think we should do is:

  • announceAddressFilter - function to filter addresses to announce (Default: filter out local addresses)
  • dialAddressFilter - function to filter addresses to dial (Default: all addresses)
  • dialAddressSorter - function to sort available addresses to dial (Default: public addresses first)

Regarding dialAddressFilter, I would like to filter out local addresses but this will have a big impact, specially for new users that are trying out libp2p and will need to custom it, just for it to work with 2 local peers. What do you think on default to: filter out local addresses if public available? While the sort function will dial public first, if we have multiple tokens in the dialer available, the local address might also be used.

I am expecting the following options:

{
  addresses: {
    announceFilter: () => []
  },
  dialer: {
    addressFilter: () => [],
    addrerrSorter: () => []
  }
}

@jacobheun
Copy link
Contributor

For dialing I think implementing the ConnectionGater, libp2p/go-libp2p#872, is going to be the best approach here, we can create a default Gater similar to libp2p/go-libp2p#1005. We'll need to figure out how addr sorting would work but this handles filtering easily. I don't remember how Go is prioritizing the dials right now.

For announcing, I think the filter function makes sense.

@greenSnot
Copy link
Contributor

To be more universal, we may not need to distinguish local address or public address because it's impossible to detect how many NAT layers we have (imaging the Internet is just a local net of the Comosnet).

I am working on DynamicRelay, which combines addresses announcement, hole punching and autoRelay.
Here is my solution.

  1. every peer should enable AutoRelay if possible. (relay.hop = false)
  2. joining p2p network via bootstrap node
  3. (periodically) randomly connect to a relay and get address relative to it. Relative address is local address or public address.
    2.1. randomly connect to another relay and ask it to use the relative address to dial back.
    2.2. if success, close all relayed connections, advertise relay, announce relative address, set relay.hop = true, until no new dial-in peers in n hours. else set relay.hop = false.

achingbrain added a commit that referenced this issue Jan 25, 2022
Port of https://github.com/libp2p/go-libp2p-core/blob/master/connmgr/gater.go

Adds a new configuration key `connectionGater` which allows denying the dialing of certain peers, individual multiaddrs and the creation of connections at certain points in the connection flow.

Fixes: #175
Refs: #744
Refs: #769

Co-authored-by: mzdws <[email protected]>
@maschad
Copy link
Member

maschad commented Sep 28, 2023

Closing as completed

@maschad maschad closed this as completed Sep 28, 2023
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

No branches or pull requests

4 participants