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

Upgrade flow refactoring #807

Merged
merged 30 commits into from
Mar 8, 2023
Merged

Upgrade flow refactoring #807

merged 30 commits into from
Mar 8, 2023

Conversation

Menduist
Copy link
Contributor

This PR simplifies a lot the upgrade flow

The main thing is that upgradeIncoming and upgradeOutgoing now return the Muxer, that the switch / dialer will then pass to the connManager to be stored.
Previously, this relied on the upgrade manager storing the connection itself at the end of the process

Other things:

  • connManager now only store the muxer (instead of muxer & connection). Connection is available in muxer.connection
  • MultiStreamselect is now usable without holding the instance (since it's stateless anyway)
  • Identify is now handled by the PeerStore instead of the upgrademanager
  • Removed the "upgrade" future from the Connection which is no longer necessary
  • Unified outgoing & incoming upgrade flows
  • Simplified MuxerProvider

Sorry, big PR, but one thing lead to another

@codecov
Copy link

codecov bot commented Nov 24, 2022

Codecov Report

Merging #807 (c9cc2bc) into unstable (c1a3bd8) will decrease coverage by 0.12%.
The diff coverage is 96.47%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##           unstable     #807      +/-   ##
============================================
- Coverage     83.88%   83.77%   -0.12%     
============================================
  Files            86       86              
  Lines         15096    14969     -127     
============================================
- Hits          12663    12540     -123     
+ Misses         2433     2429       -4     
Impacted Files Coverage Δ
libp2p/protocols/secure/secure.nim 71.15% <ø> (ø)
libp2p/muxers/muxer.nim 65.00% <57.14%> (-22.81%) ⬇️
libp2p/multistream.nim 95.34% <94.28%> (+0.54%) ⬆️
libp2p/dialer.nim 93.08% <97.05%> (+1.45%) ⬆️
libp2p/connmanager.nim 90.31% <97.56%> (+0.03%) ⬆️
libp2p/builders.nim 95.07% <100.00%> (-0.68%) ⬇️
libp2p/muxers/yamux/yamux.nim 88.39% <100.00%> (-1.07%) ⬇️
libp2p/peerstore.nim 97.89% <100.00%> (-2.11%) ⬇️
libp2p/protocols/connectivity/autonat/client.nim 99.66% <100.00%> (ø)
libp2p/protocols/connectivity/autonat/service.nim 95.65% <100.00%> (ø)
... and 25 more

@lchenut lchenut mentioned this pull request Jan 3, 2023
@Menduist Menduist marked this pull request as ready for review January 27, 2023 10:03
Co-authored-by: diegomrsantos <diego@status.im>
# Conflicts:
#	libp2p/connmanager.nim
#	libp2p/protocols/connectivity/autonat/service.nim
#	tests/testconnmngr.nim
libp2p/dialer.nim Outdated Show resolved Hide resolved
@Menduist
Copy link
Contributor Author

Menduist commented Mar 3, 2023

This PR is now stable on nimbus, anything else to change?

@Menduist Menduist merged commit 8d5ea43 into unstable Mar 8, 2023
@Menduist Menduist deleted the upgraderefacto branch March 8, 2023 11:30
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.

4 participants