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

[swarm] Permit configuration override for the substream upgrade protocol to use. #1858

Merged
merged 4 commits into from
Nov 25, 2020

Conversation

romanb
Copy link
Contributor

@romanb romanb commented Nov 25, 2020

It is desirable to configure the substream upgrade protocol (i.e. multistream-select protocol) to use for substreams in general. Since introducing configuration options for the substream upgrade protocol to all libp2p protocol crates seems to be noisy and likely unnecessary, this PR proposes a configuration option for the Swarm that will apply to all (outbound) substreams. In this way, one can configure the upgrade protocols in a single place, once for the transport and once for the substreams.

@romanb romanb requested a review from tomaka November 25, 2020 12:23
@romanb
Copy link
Contributor Author

romanb commented Nov 25, 2020

This is intended for inclusion in #1857.

Copy link
Member

@tomaka tomaka left a comment

Choose a reason for hiding this comment

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

Before this PR, my opinion was that each protocol should individually decide whether V1 or V1Lazy should be used, considering that the safety guarantees of V1Lazy depend on the details of the higher-level protocol.

However I've now realized that this would be a bad idea if multistream-select 2.0 gets released. It should indeed be the Swarm that decides which version of multistream-select is in use, and not individual protocols.

@romanb
Copy link
Contributor Author

romanb commented Nov 25, 2020

However I've now realized that this would be a bad idea if multistream-select 2.0 gets released. It should indeed be the Swarm that decides which version of multistream-select is in use, and not individual protocols.

Yes, we should probably remove the ability to configure the upgrade protocol on a SubstreamProtocol. The reason it got introduced was to allow very selective use of V1Lazy for specific protocols, but that was before V1Lazy was pretty much interoperable with V1.

@romanb romanb merged commit 83e87f7 into libp2p:master Nov 25, 2020
@romanb romanb deleted the substream-protocol-override branch November 25, 2020 13:26
@mxinden
Copy link
Member

mxinden commented Nov 25, 2020

However I've now realized that this would be a bad idea if multistream-select 2.0 gets released. It should indeed be the Swarm that decides which version of multistream-select is in use, and not individual protocols.

Yes, we should probably remove the ability to configure the upgrade protocol on a SubstreamProtocol. The reason it got introduced was to allow very selective use of V1Lazy for specific protocols, but that was before V1Lazy was pretty much interoperable with V1.

I created #1859 to continue this discussion as well as to keep track of it.

romanb pushed a commit to romanb/rust-libp2p that referenced this pull request Feb 22, 2021
Remove the option for a substream-specific multistream select protocol override.
The override at this granularity is no longer deemed useful, in particular because
it can usually not be configured for existing protocols like `libp2p-kad` and others.
There is a `Swarm`-scoped configuration for this version available since
[1858](libp2p#1858).
romanb added a commit that referenced this pull request Feb 25, 2021
* Remove substream-specific protocol negotiation version.

Remove the option for a substream-specific multistream select protocol override.
The override at this granularity is no longer deemed useful, in particular because
it can usually not be configured for existing protocols like `libp2p-kad` and others.
There is a `Swarm`-scoped configuration for this version available since
[1858](#1858).

* Update protocol crate versions and changelogs.

* Clean up documentation.
santos227 pushed a commit to santos227/rustlib that referenced this pull request Jun 20, 2022
* Remove substream-specific protocol negotiation version.

Remove the option for a substream-specific multistream select protocol override.
The override at this granularity is no longer deemed useful, in particular because
it can usually not be configured for existing protocols like `libp2p-kad` and others.
There is a `Swarm`-scoped configuration for this version available since
[1858](libp2p/rust-libp2p#1858).

* Update protocol crate versions and changelogs.

* Clean up documentation.
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.

3 participants