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.

Sign up for GitHub

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

Jump to bottom

quic: Add support for quic codepoint , interpreted as QUIC version draft-29 #3133

Closed
Tracked by #2883
elenaf9 opened this issue Nov 17, 2022 · 7 comments
Closed
Tracked by #2883

quic: Add support for quic codepoint , interpreted as QUIC version draft-29 #3133

elenaf9 opened this issue Nov 17, 2022 · 7 comments
Labels
decision-pending Marks issues where a decision is pending before we can move forward. difficulty:easy priority:important The changes needed are critical for libp2p, or are blocking another project

Comments

@elenaf9
Copy link
Contributor

elenaf9 commented Nov 17, 2022 •
edited
Loading

Description

The QUIC transport currently only supports the quic-v1 codepoint in multiaddresses. This codepoint was recently added to represent QUIC version 1 / RFC 9000, whereas the previous codepoint quic should be interpreted as QUIC version draft-29. See multiformats/multiaddr#145.

quinn supports the versions draft-29..34 and version 1. On the server side this means that our users could already advertise each quic-v1 address also as quic address.
For dialing however we have to set one version, right now we use the default v1. We can add support for quic(-draft-29) by checking the multiaddress on dials and set the current version in the quinn_proto::ClientConfig.

Motivation

go-libp2p does not support quic-v1 yet, although there is WIP to add support in the very near future: libp2p/go-libp2p#1841. However, even when support is added we won't be able to communicate via QUIC with any go-libp2p nodes that did not upgrade to the newest version. cc @dignifiedquire.

Requirements

  1. Allow dialing quic multiaddresses
  2. Use QUIC draft-29 version when dialing quic addresses, use version 1 for quic-v1
  3. To be discussed: support quic codepoint in listening addresses

Open questions

go-libp2p is planning a transition period (6 months?) before removing support for quic. @marten-seemann raised a valid point on whether it even makes sense for rust-libp2p to add support, if ideally eventually the quic codepoint dies out because all new nodes run on v1.

I am in favor of at least adding support for dialing quic addresses so that we can connect to old go-libp2p nodes. I don't see any drawbacks in that.
I would propose to not add support for quic listening addresses. However, since in practice we still support draft-29 as servers, users are still free to announce a quic-v1 address also as quic if they need it. They can already do this right now anyway unless we explicitly remove support for draft-29 in our config.
Wdyt @marten-seemann @mxinden?

Are you planning to do it yourself in a pull request?

Yes (if folks agree with this).
@elenaf9 elenaf9 mentioned this issue Nov 17, 2022
Closed
15 tasks
@marten-seemann
Copy link

They can already do this right now anyway unless we explicitly remove support for draft-29 in our config.

I must have missed that in code review, I would've expected that draft-29 is disabled by default.

go-libp2p is planning a transition period (6 months?) before removing support for quic. @marten-seemann raised a valid point on whether it even makes sense for rust-libp2p to add support, if ideally eventually the quic codepoint dies out because all new nodes run on v1.

go-libp2p v0.24.0, to be released next week, will include support for the new code point. This will most likely ship in the December kubo release. The only reason we're still supporting draft-29 is that we don't want to cause a regression in go-libp2p.

@dariusc93
Copy link
Member

I think it make more sense to add support for it for now since a large amount of peers would be go-libp2p nodes, which would already have support for draft-29.

@mxinden
Copy link
Member

mxinden commented Nov 17, 2022

I don't have a strong opinion here. I do see the benefit of it for @dariusc93 (https://github.com/dariusc93/rust-ipfs) and @dignifiedquire (https://github.com/n0-computer/iroh) and potentially @divagant-martian (https://github.com/sigp/lighthouse). That said, they can still connect via TCP.

They can already do this right now anyway unless we explicitly remove support for draft-29 in our config.

I must have missed that in code review, I would've expected that draft-29 is disabled by default.

@marten-seemann are there any technical downsides (e.g. security vulnerabilities) to supporting draft-29?

In case there are no technical downsides and in case the amount of work required is minimal (correct me if I am wrong @elenaf9) I am in favor of adding support for dialing /quic addresses. I am also in favor of adding support for /quic listenaddresses.

(Appreciate moving this discussion to GitHub and the detailed write-up @elenaf9!)

@elenaf9 elenaf9 added priority:important The changes needed are critical for libp2p, or are blocking another project difficulty:easy decision-pending Marks issues where a decision is pending before we can move forward. labels Nov 17, 2022
@dignifiedquire
Copy link
Member

From my side I agree it would be useful to have support for dialing at least, but it is not super critical for iroh, as we can happily continue using tcp for older nodes and I assume the large deployments will update fairly quic(k)ly.

@thomaseizinger
Copy link
Contributor

In case there are no technical downsides and in case the amount of work required is minimal (correct me if I am wrong @elenaf9) I am in favor of adding support for dialing /quic addresses. I am also in favor of adding support for /quic listenaddresses.

I am in the same camp.

This was referenced Nov 21, 2022
Merged
Merged
@elenaf9
Copy link
Contributor Author

elenaf9 commented Nov 23, 2022

@marten-seemann are there any technical downsides (e.g. security vulnerabilities) to supporting draft-29?

Friendly ping @marten-seemann. Unless there are major downsides in supporting draft-29 I'll move forward with #3151, adding opt-in support for draft-29.

dariusc93 added a commit to dariusc93/rust-ipfs that referenced this issue Nov 25, 2022
@dariusc93
feat: Implement Quic-v1 transport
44f7aa1 
Notes:
- This is only compatible with v1 so likelihood of connecting to go-libp2p and go-ipfs/kubo peers via quic will unlikely to work until draft-29 is supported in rust-libp2p (see libp2p/rust-libp2p#3133)
- DCuTR will not work with quic-v1 at this time so unless port mapping/UPNP is used (coming later), we wont be able to utilize quic-v1 with hole punching, but should work fine with relay (see libp2p/rust-libp2p#2883)

Closes #10
@thomaseizinger
Copy link
Contributor

Done in #3151.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
decision-pending Marks issues where a decision is pending before we can move forward. difficulty:easy priority:important The changes needed are critical for libp2p, or are blocking another project
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@dignifiedquire @marten-seemann @dariusc93 @thomaseizinger @mxinden @elenaf9