consensus-specs icon indicating copy to clipboard operation
consensus-specs copied to clipboard
verified publisher icon ethereum

ENR structure: Add `tcp6`, `quic6` and `udp6`.

Open nalepae opened this issue 1 year ago • 2 comments

As discussed in ACDC#139.

This PR is draft until https://github.com/ethereum/consensus-specs/pull/3644 is merged.

nalepae avatar Aug 08 '24 15:08 nalepae

we probably should also update https://github.com/ethereum/consensus-specs/blob/f4e3908801ef440771347439e72a6d37080c74b6/specs/phase0/p2p-interface.md?plain=1#L121-L137

to state that clients may support QUIC right?

jxs avatar Aug 08 '24 18:08 jxs

we probably should also update

https://github.com/ethereum/consensus-specs/blob/f4e3908801ef440771347439e72a6d37080c74b6/specs/phase0/p2p-interface.md?plain=1#L121-L137

to state that clients may support QUIC right?

Yep I agree. Should add QUIC support in here

AgeManning avatar Sep 04 '24 21:09 AgeManning

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

jxs avatar Dec 06 '24 16:12 jxs

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 avatar Dec 09 '24 15:12 ppopth

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

ppopth avatar Dec 10 '24 13:12 ppopth

we probably should also update

https://github.com/ethereum/consensus-specs/blob/f4e3908801ef440771347439e72a6d37080c74b6/specs/phase0/p2p-interface.md?plain=1#L121-L137

to state that clients may support QUIC right?

Fixed in 8ab7bc60a5c5f7d709951e1486a9556d471e3ffb.

nalepae avatar Dec 12 '24 09:12 nalepae

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

Ready for review, but added an extra commit: See https://github.com/ethereum/consensus-specs/pull/3874#issuecomment-2538328504

nalepae avatar Dec 12 '24 09:12 nalepae

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.

fjl avatar Dec 18 '24 20:12 fjl

@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.

ppopth avatar Dec 19 '24 06:12 ppopth