When should I wrap the InboundProtocol in ReadyUpgrade?

[pieterdd]

I have the impression that there are two common ways to implement ConnectionHandler::listen_protocol.

On the one hand, here's a snippet from libp2p-perf. Its InboundProtocol type is set to PerfProtocolConfig, which implements InboundUpgrade.

fn listen_protocol(&self) -> SubstreamProtocol<Self::InboundProtocol, Self::InboundOpenInfo> {
    SubstreamProtocol::new(PerfProtocolConfig {}, ())
}

On the other hand, here's a snippet from rust-libp2p's ConnectionHandler for ping. Its InboundProtocol type is set to ReadyUpgrade<StreamProtocol>, and StreamProtocol does not implement InboundUpgrade.

fn listen_protocol(&self) -> SubstreamProtocol<ReadyUpgrade<StreamProtocol>, ()> {
    SubstreamProtocol::new(ReadyUpgrade::new(PROTOCOL_NAME), ())
}

What are the implications of choosing one approach over the other?

[thomaseizinger]

There is ongoing work to deprecate and eventually remove the former, see #2863.

Hopefully, a ConnectionHandler will in the future just return a list of StreamProtocols that it wants to listen on.

In regards to the implications:

The former gives you a bit more type-safety because the connection task will run the upgrade future for you and only give you back the result. The downside is that it is quite verbose and hard to understand because there are many moving pieces:

• InboundUpgrade
• OutboundUpgrade
• DialUpgradeError
• ListenUpgradeError
• UpgradeInfo

Another downside is that there is less flexibility in what you can do with the stream. You have to process it fully within the upgrade traits.

This is where the 2nd approach comes in. By just passing through the stream to the handler, you can do whatever you want with it.

My vision is to offer many, well-composable pieces that make stream handling itself easy and reduce the API surface of ConnectionHandler.

Here is some draft work in that direction: mxinden/asynchronous-codec#5

[pieterdd]

That clears things up. Thanks Thomas!