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

Proposal: Add MPP for libp2p _Protocol Select_ protocol #119

Closed
wants to merge 6 commits into from

Conversation

mxinden
Copy link

@mxinden mxinden commented Jun 23, 2021

Adds a Minimal Project Proposal for Protocol Select, the successor
of the libp2p protocol negotiation protocol multistream-select 1.0.


Status

Ready for review.

Adds a _Minimal Project Proposal_ for _Protocol Select_, the successor
of the libp2p protocol negotiation protocol _multistream-select 1.0_.
@mxinden mxinden changed the title Proposal: Add MPP for libp2p _Protocol Select_ protocol Proposal: Add MPP for libp2p *Protocol Select* protocol Jun 23, 2021
@mxinden mxinden changed the title Proposal: Add MPP for libp2p *Protocol Select* protocol Proposal: Add MPP for libp2p _Protocol Select_ protocol Jun 23, 2021
@jacobheun jacobheun added the Steward Priority Stewards priority project due to enabling us to move faster and/or safer. label Jun 23, 2021
@mxinden mxinden marked this pull request as ready for review June 29, 2021 19:34
@mxinden
Copy link
Author

mxinden commented Jun 29, 2021

This minimal project proposal is ready for review. @BigLep could you help steering this in the right direction?

Copy link
Contributor

@BigLep BigLep left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together @mxinden and @marten-seemann.

Is there anyone else we should flag for this review? @marten-seemann : who have been historical folks that you would want to make sure is aware of this effort? I know we'll also tag them once we have the spec, but for visibility, it's good to flag now as well I believe.

Comment on lines +54 to +55
- **Bandwidth**: [multistream-select 1.0] is not as bandwidth efficient as it
could be. For example negotiating a protocol requires sending the protocol
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this really matter? Do we have a ballpark of how much bandwidth we can save?

Copy link
Author

@mxinden mxinden Jul 16, 2021

Choose a reason for hiding this comment

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

This is debatable and not relevant in all scenarios.

Let's put the bandwidth requirement of a plaintext protocol string in relation to the remaining bytes to be send with the example below:

Say we want to send a Kademlia FindNode request to a remote peer. In order to do that we would need to:

  1. Negotiate the Kademlia protocol on a new stream
  2. Send the FindNode message payload to the remote.

For (1) we will need to send /multistream/1.0.0 (18 bytes) and /ipfs/kad/1.0.0 (15 bytes). For (2) we would need to send the FindNode message with the peer ID of the node to be found (44 bytes).

With Protocol Select we would not have to send /multistream/1.0.0 (18 bytes). In addition, with the above proposed bandwidth optimization, instead of sending /ipfs/kad/1.0.0 (15 bytes), we might be able to reduce the protocol identifier to ~2 bytes.

All that said, Protocol Select will likely not tackle this in the first iteration / roll out version. It will only be designed in a way to enable this optimization in future versions.

Copy link
Author

@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.

Thanks @BigLep for taking a look.

Is there anyone else we should flag for this review?

Tagging @vyzo and @Stebalien, though I am reasonably sure they are already aware of this concrete minimal project proposal.

@BigLep what are next steps here? Would this pull request be merged once accepted, or would we simply change the status of the pull request?

Comment on lines +54 to +55
- **Bandwidth**: [multistream-select 1.0] is not as bandwidth efficient as it
could be. For example negotiating a protocol requires sending the protocol
Copy link
Author

@mxinden mxinden Jul 16, 2021

Choose a reason for hiding this comment

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

This is debatable and not relevant in all scenarios.

Let's put the bandwidth requirement of a plaintext protocol string in relation to the remaining bytes to be send with the example below:

Say we want to send a Kademlia FindNode request to a remote peer. In order to do that we would need to:

  1. Negotiate the Kademlia protocol on a new stream
  2. Send the FindNode message payload to the remote.

For (1) we will need to send /multistream/1.0.0 (18 bytes) and /ipfs/kad/1.0.0 (15 bytes). For (2) we would need to send the FindNode message with the peer ID of the node to be found (44 bytes).

With Protocol Select we would not have to send /multistream/1.0.0 (18 bytes). In addition, with the above proposed bandwidth optimization, instead of sending /ipfs/kad/1.0.0 (15 bytes), we might be able to reduce the protocol identifier to ~2 bytes.

All that said, Protocol Select will likely not tackle this in the first iteration / roll out version. It will only be designed in a way to enable this optimization in future versions.

@marten-seemann
Copy link

I've added some reviewers, mostly people who were involved in the multistream 2.0 discussion. Would love to hear their feedback!

@BigLep
Copy link
Contributor

BigLep commented Jul 16, 2021

@BigLep what are next steps here? Would this pull request be merged once accepted, or would we simply change the status of the pull request?

I'm going to move this to In Progress, given Stewards are working on this by putting together an official spec: libp2p/specs#349

If there are no concerns from others that we're working on specing this, I think reviewers can leave their comments in the spec itself.

@BigLep BigLep marked this pull request as draft July 17, 2021 00:46
@BigLep BigLep marked this pull request as ready for review July 17, 2021 00:46
@yusefnapora
Copy link
Contributor

Big 👍 to this proposal 😄

I'll check out the draft spec and chime in if I have any opinions.

@mxinden mxinden closed this May 1, 2024
@BigLep BigLep removed their assignment Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Steward Priority Stewards priority project due to enabling us to move faster and/or safer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants