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

add a new QUIC codepoint for QUIC v1 #298

Merged
merged 2 commits into from
Nov 11, 2022
Merged

add a new QUIC codepoint for QUIC v1 #298

merged 2 commits into from
Nov 11, 2022

Conversation

marten-seemann
Copy link
Contributor

See multiformats/multiaddr#145 for a detailed explanation.

Implementations MAY choose to continue accepting /quic multiaddresses (and interpret them as quic-draft29 or quic-v1 at their discretion).


cc @elenaf9 @thomaseizinger, who are apparently not part of the multiformats org yet.

@@ -119,7 +119,8 @@ garlic32, multiaddr, 0x01bf, draft, I2P b
tls, multiaddr, 0x01c0, draft,
sni, multiaddr, 0x01c1, draft, Server Name Indication RFC 6066 § 3
noise, multiaddr, 0x01c6, draft,
quic, multiaddr, 0x01cc, permanent,
Copy link
Member

@lidel lidel Nov 9, 2022

Choose a reason for hiding this comment

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

Removing quic name will break all implementations that parse text multiaddrs based on libraries generated from this table.

@marten-seemann is the plan to do a hard BREAKING CHANGE across ecosystem (forcing end users or implementations to update bootstrap/peering configs and DNSAddrs manually)?

OR

do we want to have a ~1y deprecation period when both are valid, like we did when we renamed /ipfs/{peerid} to /p2p/{peerid}?
(fysa we've removed the legacy alias entry just recently in #283).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was planning to keep parsing quic as draft-29 for the foreseeable future, but emit quic-draft29 from now on (for the limited amount of time that we still support it).

Is there any way to encode this kind of logic into the multicodec table?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think so. We should do the rename later and for now simply add the quic-v1 codepoint. (I left a similar comment in the multiaddr PR: multiformats/multiaddr#147 (comment))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reverted this change.

Choose a reason for hiding this comment

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

We could add another code-point for quic-draft29 and just deprecate the use of quic altogether. That is semantically equivalent to a rename but can be expressed in the table if we also add a "Status" called "deprecated".

Copy link

@guseggert guseggert Nov 11, 2022

Choose a reason for hiding this comment

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

We should do the rename later and for now simply add the quic-v1 codepoint.

I would advocate for not renaming things in this table anymore, ever, and to make this table append-only. We have an increasing number of critical systems and businesses relying on these values remaining constant. There is enormous value in using the names as human-readable codes, but if we are unable to commit to not breaking names then I think we should just remove them entirely and rely on the description, which implies that it's for human and not machine consumption.

@marten-seemann marten-seemann changed the title change existing QUIC code point to draft-29, add one for v1 add a new QUIC codepoint for QUIC v1 Nov 11, 2022
@marten-seemann marten-seemann merged commit 1f8b8f2 into master Nov 11, 2022
@marten-seemann marten-seemann deleted the quic-v1 branch November 11, 2022 07:36
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