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

ENR structure: Add tcp6, quic6 and udp6. #3874

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

nalepae
Copy link
Contributor

@nalepae nalepae commented Aug 8, 2024

As discussed in ACDC#139.

This PR is draft until #3644 is merged.

@jxs
Copy link

jxs commented Aug 8, 2024

we probably should also update

### Transport
Even though libp2p is a multi-transport stack (designed to listen on multiple simultaneous transports and endpoints transparently),
we hereby define a profile for basic interoperability.
All implementations MUST support the TCP libp2p transport, and it MUST be enabled for both dialing and listening (i.e. outbound and inbound connections).
The libp2p TCP transport supports listening on IPv4 and IPv6 addresses (and on multiple simultaneously).
Clients must support listening on at least one of IPv4 or IPv6.
Clients that do _not_ have support for listening on IPv4 SHOULD be cognizant of the potential disadvantages in terms of
Internet-wide routability/support. Clients MAY choose to listen only on IPv6, but MUST be capable of dialing both IPv4 and IPv6 addresses.
All listening endpoints must be publicly dialable, and thus not rely on libp2p circuit relay, AutoNAT, or AutoRelay facilities.
(Usage of circuit relay, AutoNAT, or AutoRelay will be specifically re-examined soon.)
Nodes operating behind a NAT, or otherwise undialable by default (e.g. container runtime, firewall, etc.),
MUST have their infrastructure configured to enable inbound traffic on the announced public listening endpoint.

to state that clients may support QUIC right?

@AgeManning
Copy link
Contributor

we probably should also update

### Transport
Even though libp2p is a multi-transport stack (designed to listen on multiple simultaneous transports and endpoints transparently),
we hereby define a profile for basic interoperability.
All implementations MUST support the TCP libp2p transport, and it MUST be enabled for both dialing and listening (i.e. outbound and inbound connections).
The libp2p TCP transport supports listening on IPv4 and IPv6 addresses (and on multiple simultaneously).
Clients must support listening on at least one of IPv4 or IPv6.
Clients that do _not_ have support for listening on IPv4 SHOULD be cognizant of the potential disadvantages in terms of
Internet-wide routability/support. Clients MAY choose to listen only on IPv6, but MUST be capable of dialing both IPv4 and IPv6 addresses.
All listening endpoints must be publicly dialable, and thus not rely on libp2p circuit relay, AutoNAT, or AutoRelay facilities.
(Usage of circuit relay, AutoNAT, or AutoRelay will be specifically re-examined soon.)
Nodes operating behind a NAT, or otherwise undialable by default (e.g. container runtime, firewall, etc.),
MUST have their infrastructure configured to enable inbound traffic on the announced public listening endpoint.

to state that clients may support QUIC right?

Yep I agree. Should add QUIC support in here

Copy link
Member

@ppopth ppopth left a comment

Choose a reason for hiding this comment

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

This makes sense to me. I'm fine with both this PR and #3644

@jxs
Copy link

jxs commented Dec 6, 2024

ping after #3644 got merged, this can now also be merged with the suggested updates above right?

@ppopth
Copy link
Member

ppopth commented Dec 9, 2024

This seems like a breaking change?

Currently if some node sets ip6 and tcp, we assume that we can connect to that node with the specified IP address and TCP port?

But after this PR, specifying only ip6 and tcp doesn't make any sense? the node is supposed to specify ip6 and tcp6 instead.

I think we should ensure that all the clients have changed the logic properly before we merge this PR.

@ppopth
Copy link
Member

ppopth commented Dec 10, 2024

@nalepae Could you undraft the PR if it's ready for review and merge?

@nalepae
Copy link
Contributor Author

nalepae commented Dec 12, 2024

we probably should also update

### Transport
Even though libp2p is a multi-transport stack (designed to listen on multiple simultaneous transports and endpoints transparently),
we hereby define a profile for basic interoperability.
All implementations MUST support the TCP libp2p transport, and it MUST be enabled for both dialing and listening (i.e. outbound and inbound connections).
The libp2p TCP transport supports listening on IPv4 and IPv6 addresses (and on multiple simultaneously).
Clients must support listening on at least one of IPv4 or IPv6.
Clients that do _not_ have support for listening on IPv4 SHOULD be cognizant of the potential disadvantages in terms of
Internet-wide routability/support. Clients MAY choose to listen only on IPv6, but MUST be capable of dialing both IPv4 and IPv6 addresses.
All listening endpoints must be publicly dialable, and thus not rely on libp2p circuit relay, AutoNAT, or AutoRelay facilities.
(Usage of circuit relay, AutoNAT, or AutoRelay will be specifically re-examined soon.)
Nodes operating behind a NAT, or otherwise undialable by default (e.g. container runtime, firewall, etc.),
MUST have their infrastructure configured to enable inbound traffic on the announced public listening endpoint.

to state that clients may support QUIC right?

Fixed in 8ab7bc6.

@nalepae nalepae marked this pull request as ready for review December 12, 2024 09:20
@nalepae
Copy link
Contributor Author

nalepae commented Dec 12, 2024

@nalepae Could you undraft the PR if it's ready for review and merge?

Ready for review, but added an extra commit: See #3874 (comment)

@fjl
Copy link

fjl commented Dec 18, 2024

But after this PR, specifying only ip6 and tcp doesn't make any sense? the node is supposed to specify ip6 and tcp6 instead.

@ppopth if "tcp6" is unspecified, but "tcp" is present, the client should use it. The idea with tcp6 is just to support an alternative port on v6 when it is present.

@ppopth
Copy link
Member

ppopth commented Dec 19, 2024

@ppopth if "tcp6" is unspecified, but "tcp" is present, the client should use it. The idea with tcp6 is just to support an alternative port on v6 when it is present.

alright, got it.

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

Successfully merging this pull request may close these issues.

6 participants