`p2p-interface.md`: Add `quic` ENR entry.
This ENR entry seems already used by Lighthouse BN. Exemple found on the Holesky network:
enr:-MS4QEyTlybz9KMHKN7pXHOvlD8Q1muUXN7mbeUCPjSyKOEYScKDpmkaxxuE8DOC-YlCIqweCQpEw8uLG4s4z0huDAEUh2F0dG5ldHOIAAAAAAAGAACEZXRoMpBprg6ZBQFwAP__________gmlkgnY0gmlwhIjzqrKEcXVpY4IjKYlzZWNwMjU2azGhA-tFvPZaDfibme3y3o9xderqssFhY1Pnjw6AwqwreQz7iHN5bmNuZXRzAIN0Y3CCIyiDdWRwgiMo
cc @michaelsproul
libp2p has several quic implementations of varying quality - which one is this? if anything, we should be using the standards-compliant one, ie quic-v1: https://docs.libp2p.io/concepts/transports/quic/#distinguishing-multiple-quic-versions-in-libp2p
but this has not yet gone through a standardisation / testing process on the ethereum side (ie afaik this is an LH experiment) - I'm not sure adding an entry here is the right end to start that process
As far as I know, LH uses quic-v1 (multiaddr looking like /ip4/192.0.2.0/udp/65432/quic-v1/).
I agree a quic-v1 entry would be more idiomatic than quic.
To add a little bit of context to this present pull request, I'm implementing the QUIC (V1) support in Prysm: https://github.com/prysmaticlabs/prysm/pull/13786
With this linked PR, Prysm BN is able to connect to other Prysm BN and LH BN with both QUIC (V1) and TCP. QUIC (V1) is favored if both are available. As far as I know, only LH, and with the linked PR, Prysm, do support QUIC (V1).
The linked PR also adds the quic ENR entry in Prysm BN, as LH does. Because this entry is not currently standardised, I think it is a good time to start such a process.
Yep. I added this field in lighthouse.
It was originally a test amongst all Lighthouse peers because other clients didn't support quic. We wanted to test to see how well the transport works.
We didn't bother standardising it, as it is likely involved in a larger conversation around which kind of quic and who wants to support it etc as @arnetheduck has suggested.
However if others want to test along with us, I have no objections to adding it to the specs. I'm personally fine with the quic key, provided we also standardise the exact version we are using. Also happy to change to quic-v1 if others desire.
It was originally a test amongst all Lighthouse peers because other clients didn't support quic. We wanted to test to see how well the transport works.
Nice! When this PR is merged and the --enable-quic feature flag is used, then Prysm BN is able to other peers via QUIC (as long as the quic entry is in the peer's ENR and the peer does support quic-v1 protocol.)
This PR has been successfully tested with multiple inbound/outbound (LH) peers.
About the ENR entry itself, I would vote for quic-v1.
Pros:
The currently used TCP multiaddr has this shape: /ip4/94.16.114.102/tcp/13000/p2p/16Uiu2HAkv4gec8oWzbk89naQ7dymAM7QYDvy6ZnzuC7XXChULiQt
- The substring
/tcp/is included into the multiaddr. tcpentry is used in the ENR.
Using quic-v1 ENR entry would achieve the same result:
The currently used QUIC multiaddr has this shape:
/ip4/94.16.114.102/udp/13000/quic-v1/p2p/16Uiu2HAkv4gec8oWzbk89naQ7dymAM7QYDvy6ZnzuC7XXChULiQt
- The substring
/quic-v1/is included into the multiaddr. - Using
quic-v1entry in the ENR instead ofquicwill be more consistent.
Also, using the quic-v1 entry indicates that RFC 9000 is used and not draft-29. (See Distinguishing multiple QUIC versions in libp2p
Cons:
- Using
quic-v1ENR entry instead ofquicadds some extra work for LH.
Sure. I'm happy to switch to the quic-v1 ENR field. Logic seems reasonable to me.
I guess up until the next hard fork we will also support the quic field and then deprecate it after the next fork.
For reference @jxs linked me some research done last year into the different implementations and ultimately why we went with Quinn. Might be useful for others:
https://github.com/libp2p/rust-libp2p/issues/2883#issuecomment-1547189526
It was originally a test amongst all Lighthouse peers because other clients didn't support quic. We wanted to test to see how well the transport works.
Nice! When this PR is merged and the
--enable-quicfeature flag is used, then Prysm BN is able to other peers via QUIC (as long as thequicentry is in the peer's ENR and the peer does supportquic-v1protocol.)This PR has been successfully tested with multiple inbound/outbound (LH) peers.
About the ENR entry itself, I would vote for
quic-v1.Pros: The currently used TCP multiaddr has this shape:
/ip4/94.16.114.102/tcp/13000/p2p/16Uiu2HAkv4gec8oWzbk89naQ7dymAM7QYDvy6ZnzuC7XXChULiQt
- The substring
/tcp/is included into the multiaddr.tcpentry is used in the ENR.Using
quic-v1ENR entry would achieve the same result: The currently used QUIC multiaddr has this shape:/ip4/94.16.114.102/udp/13000/quic-v1/p2p/16Uiu2HAkv4gec8oWzbk89naQ7dymAM7QYDvy6ZnzuC7XXChULiQt
- The substring
/quic-v1/is included into the multiaddr.- Using
quic-v1entry in the ENR instead ofquicwill be more consistent.Also, using the
quic-v1entry indicates thatRFC 9000is used and notdraft-29. (See Distinguishing multiple QUIC versions in libp2pCons:
- Using
quic-v1ENR entry instead ofquicadds some extra work for LH.
there's also the issue with the ipv6 fields, what could be the ipv6 quic ENR field?
Both quic6-v1 or quic-v1-6 seem odd, OTOH tcp6 already doesn't have equivalence with its multiaddress counterpart
RE the field name, one thing to keep in mind is that ENRs are limited by the spec to 300 bytes when encoded. So making longer field names isn't free. It eats into space that may be used by future fields.
Using quic-v1 instead of quic uses an additional 1% of the total available size.
As far as mapping the field name to the multiaddr prefix, I don't think this needs to be preserved. As mentioned above, we already don't have equivalence there. And keep in mind that the multiaddr string prefixes are just human-readable representations of a varint code. We have different needs here (field names are serialized directly, limited size) so we likely shouldn't be choosing field names based on entirely on human-readability.
My opinion would be to use a smaller field name, all else being the same. The meaning of the field will be handled by standards, this PR or an EIP, so ambiguity shouldn't be a problem for conforming implementations.
RE the field name, one thing to keep in mind is that ENRs are limited by the spec to 300 bytes when encoded. So making longer field names isn't free. It eats into space that may be used by future fields.
Not related to this topic, but EIP-7594 introduces a new custody_subnet_count ENR field, which is the longest we have so far:
According to your comment, it may be interesting to rename it.
Let's stick with quic . We can reduce it to qui if needed.
I agree with quic. I think it's short enough already. I also agree that we don't need to make it consistent with libp2p multiaddr.
Another point I want to make is that I think Discv5 and ENR are considered independent from libp2p so I think it makes sense that we don't need to make them consistent.
@hwwhww @jtraglia I think this is ready to be merged.