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

refactor: move stream upgrades to libp2p-swarm #4789

Closed
wants to merge 2 commits into from

Conversation

thomaseizinger
Copy link
Contributor

Description

In #4307, we introduced dedicated traits for connection upgrades. This was to separate the concept of stream and connection upgrades. The next logic step is to move all existing stream upgrades to libp2p-swarm. Utilities like ReadyUpgrade or DeniedUpgrade are only ever used by ConnectionHandler implementations. Thus, there is no need for them to reside in libp2p-core.

On the surface, this looks like a really big change but this is only because these types are used in a lot of places. Moving them to libp2p-swarm allows us to remove the UpgradeInfoSend, InboundUpgradeSend and OutboundUpgradeSend types because we simply apply the Send bounds directly to the traits themselves.

Within libp2p-swarm, we also know that these traits are only ever called with a Stream as a socket which allows us to remove the C type parameter from them.

At the same time, we can remove the P type parameter from types like ReadyUpgrade by always specifying it as a StreamProtocol.

Related: #2863.
Resolves: #4697.

Notes & open questions

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

@thomaseizinger
Copy link
Contributor Author

I am setting this to draft for now. I don't want to block the release on it. Whilst I think this particular step is going in the right direction, I want to experiment with a few more APIs before making such an impactful change.

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.

core: move {In,Out}boundUpgrade to libp2p-swarm
1 participant