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

fix(comms/core): upgrade to yamux 0.13 #6317

Merged
merged 3 commits into from
May 6, 2024

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented May 3, 2024

Description

Updates yamux to 0.13.2 from 0.10.2
Refactor RPC tests to account for internal changes in yamux

Motivation and Context

Includes improvements in yamux since 0.10. Including libp2p/rust-yamux@460baf2

One major difference is that the substream is not opened until the client sends a message. This only affects some tests written in a "weird" way where the outbound stream was notified as an inbound substream and the inbound stream is written to first. In all real usages of substreams, the outbound stream always writes first.

This has been tested on the existing nextnet network for breaking changes and there were no issues detected.

How Has This Been Tested?

Existing tests updated, manually syncing nextnet (non-breaking change test) and wallet

Localnet manual test with 2 base nodes and 1 wallet

What process can a PR reviewer use to test or verify this change?

All applications should work as before

Breaking Changes

  • None
  • Requires data directory on base node to be deleted
  • Requires hard fork
  • Other - Please specify

Copy link

github-actions bot commented May 3, 2024

Test Results (CI)

    3 files    120 suites   37m 17s ⏱️
1 277 tests 1 277 ✅ 0 💤 0 ❌
3 823 runs  3 823 ✅ 0 💤 0 ❌

Results for commit 0a2aaf7.

♻️ This comment has been updated with latest results.

@ghpbot-tari-project ghpbot-tari-project added P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged labels May 3, 2024
Copy link

github-actions bot commented May 3, 2024

Test Results (Integration tests)

 2 files  + 2  11 suites  +11   14m 4s ⏱️ + 14m 4s
33 tests +33  32 ✅ +32  0 💤 ±0  1 ❌ +1 
35 runs  +35  32 ✅ +32  0 💤 ±0  3 ❌ +3 

For more details on these failures, see this check.

Results for commit 0a2aaf7. ± Comparison against base commit 925d29a.

♻️ This comment has been updated with latest results.

SWvheerden
SWvheerden previously approved these changes May 3, 2024
@ghpbot-tari-project ghpbot-tari-project removed the P-reviews_required Process - Requires a review from a lead maintainer to be merged label May 3, 2024
@ghpbot-tari-project ghpbot-tari-project added the P-reviews_required Process - Requires a review from a lead maintainer to be merged label May 3, 2024
@SWvheerden SWvheerden merged commit 1b5e217 into tari-project:development May 6, 2024
14 of 16 checks passed
@sdbondi sdbondi deleted the update-yamux branch May 6, 2024 05:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants