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

Update yamux requirement from 0.8.0 to 0.9.0 #1960

Merged
merged 4 commits into from
Apr 12, 2021

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Feb 17, 2021

Updates the requirements on yamux to permit the latest version.

Changelog

Sourced from yamux's changelog.

0.9.0

  • Force-split larger frames, for better interleaving of reads and writes between different substreams and to avoid single, large writes. By default frames are capped at, and thus split at, 16KiB, which can be adjusted by a new configuration option, if necessary.

  • Send window updates earlier, when half of the window has been consumed, to minimise pauses due to transmission delays, particularly if there is just a single dominant substream.

  • Avoid possible premature stream resets of streams that have been properly closed and already dropped but receive window update or other frames while the remaining buffered frames are still sent out. Incoming frames for unknown streams are now ignored, instead of triggering a stream reset for the remote.

0.8.0

  • Upgrade step 4 of 4. This version always assumes the new semantics and no longer sets the non-standard flag in intial window updates.
  • The configuration option lazy_open is removed. Initial window updates are sent automatically if the receive window is configured to be larger than the default.

0.7.0

Upgrade step 3 of 4. This version sets the non-standard flag, but irrespective of whether it is present or not, always assumes the new additive semantics of the intial window update.

0.6.0

Upgrade step 2 of 4. This version sets the non-standard flag, version 0.5.0 already recognises.

0.5.0

This version begins the upgrade process spawning multiple versions that changes the meaning of the initial window update from "This is the total size of the receive window." to "This is the size of the receive window in addition to the default size." This is necessary for compatibility with other yamux implementations. See issue #92 for details.

As a first step, version 0.5.0 interprets a non-standard flag to imply the new meaning. Future versions will set this flag and eventually the new meaning will always be assumed. Upgrading from the current implemention to the new semantics requires deployment of every intermediate version, each of

... (truncated)

Commits

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

@dependabot dependabot bot added dependencies Pull requests that update a dependency file rust Pull requests that update Rust code labels Feb 17, 2021
@romanb romanb added the on-ice label Feb 17, 2021
@dependabot dependabot bot force-pushed the dependabot/cargo/yamux-0.9.0 branch from 3235dc9 to d05f3d2 Compare February 17, 2021 09:58
@dependabot dependabot bot force-pushed the dependabot/cargo/yamux-0.9.0 branch 6 times, most recently from 86cb453 to 86cd81b Compare March 3, 2021 10:05
@mxinden
Copy link
Member

mxinden commented Mar 8, 2021

Nodes running yamux v0.8.0 might run into issues communicating with nodes running yamux v0.9.0. Roman released yamux v0.8.1 which does not face these issues when communicating with nodes running yamux v0.9.0. See libp2p/rust-yamux#115 for details. In addition, to speed up the upgrade process #1959 sets a requirement for yamux v0.8.1 in the recent libp2p-yamux release.

With the above in mind and given that #1959 was merged and released 20 days ago, is there anyone objecting to merging this pull request updating libp2p-yamux to v0.9.0? Of course, as the above could be considered a breaking change, this would be released with a minor version upgrade in both libp2p-yamux and libp2p (both still below v1.0 thus minor and not major).

//CC @tomaka & @romanb for https://github.com/paritytech/substrate
//CC @AgeManning for https://github.com/sigp/lighthouse
//CC @austinabell for https://github.com/ChainSafe/forest
//CC @thomaseizinger for https://github.com/comit-network
//CC @rklaehn for https://github.com/rs-ipfs/rust-ipfs

@austinabell
Copy link
Contributor

@ec2

(I'm not working on Forest anymore)

@ec2
Copy link

ec2 commented Mar 8, 2021

Forest is already on yamux 0.8.1.

@dependabot dependabot bot force-pushed the dependabot/cargo/yamux-0.9.0 branch 2 times, most recently from e623195 to a8c098c Compare March 11, 2021 15:09
@AgeManning
Copy link
Contributor

We avoid yamux in our connection currently. So this is fine for us :).

@dependabot dependabot bot force-pushed the dependabot/cargo/yamux-0.9.0 branch from a8c098c to 4da2340 Compare March 12, 2021 14:33
@mxinden
Copy link
Member

mxinden commented Mar 13, 2021

We avoid yamux in our connection currently.

@AgeManning out of curiosity may I ask why?

@AgeManning
Copy link
Contributor

We had issues with the go implementation. All our clients support mplex and yamux. The go-client was the only other client that was using yamux and there were connection issues. So we removed it in favour of mplex.

From memory, I think we did track down the incompatibilities and fixed them. I think we then negotiated yamux secondary to mplex.

I just re-checked the code, looks like we prefer yamux again. So this could be an issue for us when connecting to go clients or ourselves (of older versions).

Our current version targets libp2p v0.35.1 and our previous version targets v0.34. I guess we can give users more time to update by releasing newer versions which favour mplex to negotiate which should resolve the lighthouse <-> lighthouse potential conflicts and I'll chat with the go team to see what they are doing.

@dependabot dependabot bot force-pushed the dependabot/cargo/yamux-0.9.0 branch 9 times, most recently from 1c9f6f2 to 50506d1 Compare March 22, 2021 10:25
@dependabot dependabot bot force-pushed the dependabot/cargo/yamux-0.9.0 branch from 50506d1 to 5397c5f Compare March 22, 2021 12:55
@dependabot dependabot bot force-pushed the dependabot/cargo/yamux-0.9.0 branch from 5397c5f to 7c329e2 Compare April 1, 2021 11:59
@dependabot dependabot bot force-pushed the dependabot/cargo/yamux-0.9.0 branch from 7c329e2 to b2f0646 Compare April 1, 2021 12:26
@dependabot dependabot bot force-pushed the dependabot/cargo/yamux-0.9.0 branch from b2f0646 to b778c49 Compare April 1, 2021 12:37
@mxinden
Copy link
Member

mxinden commented Apr 1, 2021

Unless there are any objections, I will merge this pull request tomorrow. Yamux v0.9.0 will then be part of libp2p v0.37.0

@mxinden
Copy link
Member

mxinden commented Apr 2, 2021

I will merge this pull request tomorrow. Yamux v0.9.0 will then be part of libp2p v0.37.0

Easier said than done. yamux v0.9.0 updated to rand v0.8.3. With rand v0.8.0 platform support changed due to its upgrade to getrandom v0.2. With getrandom v0.2 wasm32-unknown-unknown is no longer supported out of the box:

This crate fully supports the wasm32-wasi and wasm32-unknown-emscripten targets. However, the wasm32-unknown-unknown target is not automatically supported since, from the target name alone, we cannot deduce which JavaScript interface is in use (or if JavaScript is available at all).

Instead, if the "js" Cargo feature is enabled, this crate will assume that you are building for an environment containing JavaScript, and will call the appropriate methods. Both web browser (main window and Web Workers) and Node.js environments are supported, invoking the methods described above using the wasm-bindgen toolchain.

This feature has no effect on targets other than wasm32-unknown-unknown.

Note that this is not specific to the upgrade to yamux v0.9.0. We will want to upgrade to rand >=v0.8.0 across rust-libp2p in the future (see #1899).

I haven't worked on our wasm support enough to make an informed decision. Simply adding the js feature does the trick, but seems like a hack, given that not all wasm32-unknown-unknown platforms have a JS environment. I wonder whether we should restrict wasm support on rust-libp2p to wasm32-wasi and wasm32-unknown-emscripten only.

@tomaka @expenses any thoughts here?

@mxinden mxinden merged commit 426a20c into master Apr 12, 2021
@dependabot dependabot bot deleted the dependabot/cargo/yamux-0.9.0 branch April 12, 2021 20:06
elenaf9 pushed a commit to elenaf9/rust-libp2p that referenced this pull request Apr 16, 2021
* Update yamux requirement from 0.8.0 to 0.9.0

Updates the requirements on [yamux](https://github.com/paritytech/yamux) to permit the latest version.
- [Release notes](https://github.com/paritytech/yamux/releases)
- [Changelog](https://github.com/paritytech/yamux/blob/develop/CHANGELOG.md)
- [Commits](https://github.com/paritytech/yamux/commits)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Max Inden <[email protected]>
AgeManning pushed a commit to sigp/rust-libp2p that referenced this pull request May 4, 2021
* Update yamux requirement from 0.8.0 to 0.9.0

Updates the requirements on [yamux](https://github.com/paritytech/yamux) to permit the latest version.
- [Release notes](https://github.com/paritytech/yamux/releases)
- [Changelog](https://github.com/paritytech/yamux/blob/develop/CHANGELOG.md)
- [Commits](https://github.com/paritytech/yamux/commits)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Max Inden <[email protected]>
@AgeManning AgeManning mentioned this pull request Jun 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants