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

roadmap: rewrite the section about efficient handshakes #461

Merged
merged 2 commits into from
Oct 12, 2022

Conversation

marten-seemann
Copy link
Contributor

Over the last couple of weeks, we've broken the early muxer negotiation into its own project (#446), and found an arguably nicer solution than proposed in the Protocol Select spec (#349). The change to move the security address into the multiaddr is already split out into a separate PR: #354.

With these two changes having been separated from Protocol Select, we save 2 RTTs during a TCP handshake, reducing the total latency to the bare minimum consumed by the components used (TCP: 1 RTT, security handshake: 1 RTT).

Implementing the rest of the Protocol Select spec now appears less pressing. The main additional benefits would be:

  1. Slightly shorter protocol names, since the protocol string would be wrapped in a protobuf instead of being prefixed with /multistream/1.0.0. This saving of ~10 bytes on a new stream is likely negligible in almost all situations.
  2. The ability to propose multiple protocols on a stream at the same time, instead of sequentially. In theory, this saves roundtrips, but in practice, one can just open multiple streams, one for each of the protocols (and then even send application data right away, if desired).
  3. The Protocol Select protobuf is as a basis for future extensions to the protocol, e.g. an extension that abbreviates protocol IDs with an integer, saving another ~10 bytes per stream.

(1) and (2) probably don't justify a high priority for Protocol Select. (3) is a valid point, but it would be preferable if the project was driving by a concrete use case that in itself would justify a high priority, rather than by the wish to create an extensible platform.

@MarcoPolo
Copy link
Contributor

With the caveat that I'm not as familiar with protocol select as others. I agree with these points.

On point 3, couldn't users already define an integer that represents their protocol? Is there anything that says it has to be a human readable string?

Even if not, we could imagine a small change to multistream-select that allows them to pass multicodecs instead of protocol ids with a similar extensions protobuf. This could be /multistream/1.1.0 that gracefully falls back to the old method if it gets a /multistream/1.0.0 response (we'd have to support semver matching in multistream code).

@marten-seemann
Copy link
Contributor Author

On point 3, couldn't users already define an integer that represents their protocol? Is there anything that says it has to be a human readable string?

They could, but without a place to coordinate the small values, this will probably run into interoperability issues rather quickly.

I’m not convinced this is a problem worth solving in the first place. Even if you have a crazily chatty protocol that opens a stream in every single packet you send, the overhead of multistream will still just be 20 bytes (compared to a Protobuf with a protocol name compression extension). For reference, 20 bytes is the overhead of IPv6 vs IPv4, or 1.5% of the packet payload.

Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

Implementing the rest of the Protocol Select spec now appears less pressing.

Agreed that it became less pressing. Agreed with (1), (2) and (3).

The rephrasing of the section looks good to me.

I hope that we will get to Protocol Select one day.

ROADMAP.md Outdated Show resolved Hide resolved
Copy link
Contributor

@julian88110 julian88110 left a comment

Choose a reason for hiding this comment

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

Agreed, with the recent changes that are coming, this is not a priority anymore and actual work should only be committed when there are actual needs rather than speculated needs

ROADMAP.md Show resolved Hide resolved
Copy link
Member

@p-shahi p-shahi left a comment

Choose a reason for hiding this comment

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

minor grammatical changes & a nit

ROADMAP.md Outdated Show resolved Hide resolved
ROADMAP.md Outdated Show resolved Hide resolved
ROADMAP.md Outdated Show resolved Hide resolved
Co-authored-by: Max Inden <[email protected]>
Co-authored-by: Prithvi Shahi <[email protected]>
@marten-seemann marten-seemann merged commit 2e4c260 into master Oct 12, 2022
@marten-seemann marten-seemann deleted the rewrite-efficient-handshake branch October 12, 2022 19:54
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.

5 participants