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

Expands CID size text just a bit #2008

Merged
merged 2 commits into from
Nov 21, 2018
Merged

Expands CID size text just a bit #2008

merged 2 commits into from
Nov 21, 2018

Conversation

janaiyengar
Copy link
Contributor

No description provided.

subsequent Initial packets are routed to the same server.
The final version used for a connection might be different from the version of
the first Initial from the client. To enable consistent routing through the
handshake, a client SHOULD select a Destination Connection ID length long enough
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
handshake, a client SHOULD select a Destination Connection ID length long enough
handshake, a client SHOULD select an initial Destination Connection ID length long enough

Copy link
Contributor

Choose a reason for hiding this comment

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

Note that accepting this suggestion directly on GitHub will break lint -- you'll need to rewrap.

Copy link
Contributor

Choose a reason for hiding this comment

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

I gave that feedback to github on this feature - that you can handle line lengths.

Copy link
Member

@martinthomson martinthomson left a comment

Choose a reason for hiding this comment

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

With @mikkelfj's change, LGTM

Copy link
Contributor

@MikeBishop MikeBishop left a comment

Choose a reason for hiding this comment

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

IIRC, the reason this wasn't stated before is that you shouldn't be routing based on a client-chosen CID to begin with. Implying that you can/do sometimes is encouraging bad behavior.

@MikeBishop MikeBishop added editorial An issue that does not affect the design of the protocol; does not require consensus. -transport labels Nov 19, 2018
@janaiyengar
Copy link
Contributor Author

@MikeBishop: There's an implicit assumption though in the text about this. Are you opposed to this PR?

@MikeBishop
Copy link
Contributor

Fair enough; this PR doesn't introduce that problem, though it magnifies it a bit. I've opened #2026 to resolve that.

@janaiyengar janaiyengar merged commit 046b4f3 into master Nov 21, 2018
@MikeBishop MikeBishop deleted the expand branch February 6, 2019 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-transport editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants