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

prevent dialing addresses that we're listening on #237

Merged
merged 1 commit into from
Feb 9, 2021

Conversation

marten-seemann
Copy link
Contributor

It's impossible to run two nodes on the same IP:port, so we know for sure that any dial to an address that we're listening on will fail (as the peer IDs won't match).
In practice, this will be most useful for preventing dials to localhost for nodes that are listening on the default port.

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.

hopefully this wont break relay dialing; it shouldn't because it shouldn't show in interface addrs, but mind checking?

@willscott
Copy link
Contributor

can we reuse the filterKnownUndialables logic?

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

We're already doing this on line 449. I think the issue is that we're only filtering IP+TCP and need to adjust line 443 to allow any address starting with an IP addr (not just length 2 ones).

It's impossible to run two nodes on the same IP:port, so we know for
sure that any dial to an address that we're listening on will fail (as
the peer IDs won't match).
In practice, this will be most useful for preventing dials to localhost
for nodes that are listening on the default port.
@marten-seemann
Copy link
Contributor Author

@Stebalien You're right. I removed the check for length 2, as QUIC returns 3 protocols here: IPv{4,6}, UDP, QUIC.

@Stebalien Stebalien merged commit 3feb612 into master Feb 9, 2021
@Stebalien Stebalien mentioned this pull request May 11, 2021
27 tasks
@aschmahmann aschmahmann mentioned this pull request May 14, 2021
71 tasks
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.

4 participants