Ipv6 #3971

divagant-martian · 2023-02-13T19:45:36Z

Issue Addressed

Add support for ipv6 and dual stack in lighthouse.

Proposed Changes

From an user perspective, now setting an ipv6 address, optionally configuring the ports should feel exactly the same as using an ipv4 address. If listening over both ipv4 and ipv6 then the user needs to:

use the --listen-address two times (ipv4 and ipv6 addresses)
--port6 becomes then required
--discovery-port6 can now be used to additionally configure the ipv6 udp port

Rough list of code changes

Discovery:
- Ipv6 address, tcp port and udp port set in the ENR builder
- Reported addresses now check which tcp port to give to libp2p
LH Network Service:
- Can listen over Ipv6, Ipv4, or both. This uses two sockets. Using mapped addresses is disabled from libp2p and it's the most compatible option.
NetworkGlobals:
- No longer stores udp port since was not used at all. Instead, stores the Ipv4 and Ipv6 TCP ports.
NetworkConfig:
- Update names to make it clear that previous udp and tcp ports in ENR were Ipv4
- Add fields to configure Ipv6 udp and tcp ports in the ENR
- Include advertised enr Ipv6 address.
- Add type to model Listening address that's either Ipv4, Ipv6 or both. A listening address includes the ip, udp port and tcp port.
UPnP:
- Kept only for ipv4
Cli flags:
- --listen-addresses now can take up to two values
- --port will apply to ipv4 or ipv6 if only one listening address is given. If two listening addresses are given it will apply only to Ipv4.
- --port6 New flag required when listening over ipv4 and ipv6 that applies exclusively to Ipv6.
- --discovery-port will now apply to ipv4 and ipv6 if only one listening address is given.
- --discovery-port6 New flag to configure the individual udp port of ipv6 if listening over both ipv4 and ipv6.
- --enr-udp-port Updated docs to specify that it only applies to ipv4. This is an old behaviour.
- --enr-udp6-port Added to configure the enr udp6 field.
- --enr-tcp-port Updated docs to specify that it only applies to ipv4. This is an old behaviour.
- --enr-tcp6-port Added to configure the enr tcp6 field.
- --enr-addresses now can take two values.
Common:
- rename unused_port functions to specify that they are over ipv4.
- add functions to get unused ports over ipv6.
Testing binaries
- Updated code to reflect network config changes and unused_port changes.

Additional Info

TODOs:

set the ip filters with respect to the listening sockets.
use two sockets in discovery.
lcli allow listening over two sockets in generate_bootnodes_enr
add cli flag tests
add at least one smoke flag for ipv6

AgeManning

Had a preliminary review, looks good.

There is a lot more conditional handling of CLI args. It might be worth adding some tests here to try and check some of these: https://github.com/sigp/lighthouse/blob/stable/lighthouse/tests/beacon_node.rs

(I can help out with these tests when this PR is ready for final review)

beacon_node/lighthouse_network/src/config.rs

beacon_node/lighthouse_network/src/discovery/enr.rs

AgeManning · 2023-02-20T03:12:20Z

beacon_node/lighthouse_network/src/discovery/enr.rs

+            .or_else(|| config.listen_addresses.v4().map(|v4_addr| v4_addr.tcp_port));
+        if let Some(tcp_port) = tcp4_port {
+            builder.tcp4(tcp_port);
+        }


🤔 - Is there a case where this can be None? Should we error in this case?

when it's an only ipv6 node, which wouldn't be an error

beacon_node/lighthouse_network/src/discovery/mod.rs

AgeManning · 2023-02-20T03:23:36Z

beacon_node/network/src/nat.rs

-            udp_port: config.discovery_port,
+impl UPnPConfig {
+    pub fn from_config(config: &NetworkConfig) -> Option<Self> {
+        // TODO: need to read more on UPnP over ipv6


Yeah me too. I think the UPnP library is pretty opaque and we just send it IPs and the router tries to open them for us.

for completeness I add it here: the lib we use accepts exclusively ipv4 addresses so this is basically left as work for the future

common/unused_port/src/lib.rs

AgeManning · 2023-02-20T03:35:07Z

lcli/src/generate_bootnode_enr.rs

@@ -25,10 +25,21 @@ pub fn run<T: EthSpec>(matches: &ArgMatches) -> Result<(), String> {
        ));
    }

+    // TODO: allow using dual stack.


Libp2p supports this right?

this is the generate_bootnode_enr so I think this is not relevant here. I'm disallowing dual stack in this cases for now, but to answer the question yes of course

divagant-martian · 2023-03-06T00:12:49Z

closed in favor of #4046

divagant-martian added 21 commits February 9, 2023 11:09

Config field rename

5732e94

wip

72bbb2b

set the enr builder options

292bd7b

formatting

7f1607c

wip

3d3a138

logic for listening over the two ports

fbab32a

report observed addresses with corresponding ports

7375608

cleanup and fix some tests

ec3e78f

update upnp, do not use if only ipv6

8f2ca09

unused ports too

c971483

couple of fixes

6092a5a

add function for parsing the listening addresses

48ad93c

improve user docs

f0b6044

enr match

107f0c6

add enr ipv6 options

eed85b3

handle ipv6 host resolution

4e97a12

lil cleanup

48c384f

set listening address in testing

03ee796

set listening address in testing

bfded07

Merge branch 'unstable' into ipv6

9c63ec5

misc doc and comment updates

ad72859

AgeManning reviewed Feb 20, 2023

View reviewed changes

divagant-martian added 8 commits February 24, 2023 09:22

Merge branch 'unstable' into ipv6

b426640

boot nodes

ba06847

update setting discv5 related vals

b6fccda

fix address used for discovery

2559bca

@AgeManning contributes

f3cb1f1

update tests

6e5aabf

fix tests

92f9d79

fix more stuf

ecda9c3

divagant-martian added 12 commits February 27, 2023 15:49

fix more tests

789a6d8

revert debug tests

c9f0130

clippy

74040f6

more tests to update

1fed858

fix the generate_boonodes_enr command

062292a

fix service tests

1cb5a17

add some of the cli flag tests

60498e0

add/fix tests

5e04250

add/fix tests

a7e1cce

self review stuff and more tests

13d6346

port6 does not appear in the bootnodes

ecc908a

turn off debug tests

368e27f

divagant-martian force-pushed the ipv6 branch from 13e48ac to 368e27f Compare March 3, 2023 05:20

recreate Cargo.lock

2ad7215

divagant-martian closed this Mar 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Ipv6 #3971

Ipv6 #3971

Uh oh!

divagant-martian commented Feb 13, 2023 •

edited

Loading

Uh oh!

AgeManning left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AgeManning Feb 20, 2023

Uh oh!

divagant-martian Feb 27, 2023

Uh oh!

Uh oh!

Uh oh!

AgeManning Feb 20, 2023

Uh oh!

divagant-martian Feb 27, 2023

Uh oh!

Uh oh!

AgeManning Feb 20, 2023

Uh oh!

divagant-martian Feb 27, 2023

Uh oh!

divagant-martian commented Mar 6, 2023

Uh oh!

Uh oh!

Ipv6 #3971

Ipv6 #3971

Uh oh!

Conversation

divagant-martian commented Feb 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue Addressed

Proposed Changes

Rough list of code changes

Additional Info

Uh oh!

AgeManning left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AgeManning Feb 20, 2023

Choose a reason for hiding this comment

Uh oh!

divagant-martian Feb 27, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AgeManning Feb 20, 2023

Choose a reason for hiding this comment

Uh oh!

divagant-martian Feb 27, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AgeManning Feb 20, 2023

Choose a reason for hiding this comment

Uh oh!

divagant-martian Feb 27, 2023

Choose a reason for hiding this comment

Uh oh!

divagant-martian commented Mar 6, 2023

Uh oh!

Uh oh!

divagant-martian commented Feb 13, 2023 •

edited

Loading