libp2p / libp2p/rust-libp2p

swarm: Remove `{In,Out}boundUpgrade`

Open
#4,790 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
5.6k
Forks
1.3k
Avg merge
8h 47m
Merged PRs (30d)
19

Description

Whilst working on https://github.com/libp2p/rust-libp2p/pull/4789, I had an epiphany. So far, I was of the opinion that we will remove the UpgradeInfo trait together with the {In,Out}boundUpgrade traits.

Doing this always left a somewhat bad taste because the associated types on UpgradeInfo is what allows the entire SelectUpgrade machinery to work:

https://github.com/libp2p/rust-libp2p/blob/d05d83615963747eabc59c2f4a247b0cec041e6e/core/src/upgrade/select.rs#L43-L68

In other words, building a tree of ConnectionHandlers, getting their protocols and dispatching back to the correct handler only works because of the associated type of UpgradeInfo::Info.

To remove some of the complexity of ConnectionHandlers but still retaining this property, we can simply keep the UpgradeInfo trait and just remove the upgrade step. I made a PoC for this in https://github.com/libp2p/rust-libp2p/pull/4791/commits/310311aed134d747bf7ba31a3bb034d8d0899606. It works actually quite well. I've only adjusted the "ping" protocol here: https://github.com/libp2p/rust-libp2p/blob/310311aed134d747bf7ba31a3bb034d8d0899606/protocols/ping/src/handler.rs#L330-L366

In essence, this now always received a Stream and the protocol is the one that was negotiated. If a user uses ReadyUpgrade, that will always be an instance of StreamProtocol.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with core/src/upgrade/select.rs and the ping handler around protocols/ping/src/handler.rs#L330-L366, then compare the proof of concept in PR #4791. Determine the full scope of removing the upgrade step while retaining UpgradeInfo and protocol dispatch; done when the affected upgrade and handler paths are consistently updated and the behavior is validated.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
networking
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.