IPv6 By Default #6808
Open
IPv6 By Default #6808
+75
−2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jxs
reviewed
Jan 15, 2025
| @@ -224,6 +224,13 @@ pub fn cli_app() -> Command { | |||
| .action(ArgAction::Set) | |||
| .display_order(0) | |||
| ) | |||
| .arg( | |||
| Arg::new("disable-ipv6") | |||
There was a problem hiding this comment.
taking into consideration the description you wrote on the PR, should we then update this PR to make this optional? I.e add instead an option to enable ipv6
| // then we also listen on IPv6. | ||
| // Note that we will only listen on all interfaces if the IPv4 counterpart is also listening on | ||
| // all interfaces, to prevent accidental exposure of ports. | ||
| if maybe_ipv4 == Some(Ipv4Addr::UNSPECIFIED) |
There was a problem hiding this comment.
we could then also re-write this logic to log when user enables ipv6 but doesn't support it
| /// returns true. | ||
| pub fn is_ipv6_supported() -> bool { | ||
| // If IPv6 is supported | ||
| let Ok(std::net::IpAddr::V6(local_ip)) = local_ipv6() else { |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This enables ipv4 and ipv6 by default in Lighthouse.
However, after finishing this PR, I think maybe this isn't a good idea.
Motivation
IPv6 has been around for quite a few years now and most parts of the internet are supporting it. It is strange that a cutting-edge blockchain such as Ethereum doesn't support it natively. I've tested Lighthouse quite a bit and our recent upgrades should have no problem supporting IPv6. The problem is that we need a few more nodes on our DHT to also support IPv6 for some of the discovery mechanisms to function. So we need to bootstrap the network with a few IPv6 nodes and our original plan was to make lighthouse do this by default if the host system supports a globally routable IPV6 address.
What this PR does
If a user has specified a listening address of
0.0.0.0and their node also has a globally routable IPv6 address (i.e their ISP and router is configured for IPv6, then we also listen on::and go to dual stack mode. This also adds a CLI to disable this default behaviour--disable-ipv6.Why I no longer like this approach
0.0.0.0they may not expect us to secretly also open::and expose those ports when they have not specifically set it. I think if users are exposing ports on their computers they need to do it explicitly rather than us trying to force ipv6 on them.--ip-address ::also to enable dual stack.After some thought I think the best approach is just via education rather than code. We should make announcements and encourage users to support dual stack, rather than set it by default.
I'll leave this PR around in case others feel differently.
Also, this should not be merged until: sigp/discv5#275