Skip to content
This repository has been archived by the owner on Sep 9, 2022. It is now read-only.

feat: remove relay discovery and unspecified relay dialing #101

Merged
merged 2 commits into from
Apr 1, 2020

Conversation

Stebalien
Copy link
Member

This was a misfeature that looks great in demos but isn't useful (and wastes bandwidth and could potentially cause other DoS issues) in practice.

First, random relays we discover are unlikely to support active relaying. This means that picking a random relay is usually going to fail because the random relay won't already be connected to the target.

Second, there are two use-cases for relaying:

  1. Connecting to undialable nodes.
  2. Relaying all outbound traffic.

For the first case, we only want to use the relays specified by the target peers. For the second case, we'd need to modify either libp2p's dialer logic (in the swarm) to prefix all addresses with the specified relay.

The current logic covers neither use-case so I'm removing it.

This was a misfeature that looks great in demos but isn't useful in practice.

First, random relays we discover are unlikely to support _active_ relaying. This
means that picking a random relay is usually going to fail because the random
relay won't already be connected to the target.

Second, there are two use-cases for relaying:

1. Connecting to undialable nodes.
2. Relaying all outbound traffic.

For the first case, we only want to use the relays specified by the target
peers. For the second case, we'd need to modify either libp2p's dialer logic (in
the swarm) to prefix all addresses with the specified relay.

The logic _here_ covers neither use-case.
Introduce functional options. This should make it easier for us to add
additional options in the future (e.g., known relays, relay policies, etc.).
Copy link
Contributor

@vyzo vyzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fine with removing the unspecific relay dialing (you are right, it is a misfeature), but I would like to keep relay discovery and possibly couple it with events.

@Stebalien
Copy link
Member Author

Why?

@vyzo
Copy link
Contributor

vyzo commented Mar 31, 2020

It is useful for driving relay discovery (say for autorelay); couple that with a relay filters, and we can have a very simple mechanism for setting up relay forwarding.

@Stebalien
Copy link
Member Author

We don't use this for AutoRelay, we don't even expose the relays we find. AutoRelay now discovers its own relays via identify events.

Copy link
Contributor

@vyzo vyzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, fine, kill it.

@Stebalien Stebalien merged commit 9d90e68 into master Apr 1, 2020
@Stebalien Stebalien deleted the feat/remove-discovery branch April 1, 2020 08:20
@Stebalien
Copy link
Member Author

(sorry, I just like deleting code)
KILL KILL KILL!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants