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

Remove `detectexternalip` #1092

Open
kilrau opened this issue Jul 11, 2019 · 4 comments

Comments

Projects
3 participants
@kilrau
Copy link
Contributor

commented Jul 11, 2019

this is handled by docker now and code and config option should be removed from xud

@kilrau kilrau added the code quality label Jul 11, 2019

@kilrau kilrau added this to To do in 1.0.0-beta via automation Jul 11, 2019

@michael1011 michael1011 referenced a pull request that will close this issue Jul 11, 2019

Open

refactor: remove detectexternalip #1093

@sangaman

This comment has been minimized.

Copy link
Collaborator

commented Jul 11, 2019

What about for users that aren't installing via docker?

@kilrau

This comment has been minimized.

Copy link
Contributor Author

commented Jul 11, 2019

Good point.

Well, detecting external IP was always only part of the solution since it ignored ports and to check if they were forwarded and reachable. Which they aren't - in most of the home router cases. @erkarl 's docker solution will do both.

Anyhow, let's talk about Tor next Tuesday, which will do away with the whole IP address detection / port forwarding problem and I think should be default in future. That's why I believe IP detection shouldn't be an xud core feature anymore. And ... I didn't expect @michael1011 to be that fast.

(On-top TOR might solve our "full mesh network scalability problem", remind me to mention it!)

@michael1011

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2019

I think that if people are able to setup their router to forward their port, they will also be able to set their external IP in the config. The only case in which this feature would have made sense is when people don't have a static external IP which would result in having to set the IP again after every restart of their router. And that is kind of an edge case.

Anyways. Let's talk about Tor on next Tuesday and leave the PR alone until then.

@sangaman

This comment has been minimized.

Copy link
Collaborator

commented Jul 11, 2019

Yeah, to me it's a working feature that doesn't add much in the way of complexity or maintenance to the code, so I'd be fine with leaving it in even if it's only going to be useful in a handful of cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.