Should ProtocolsHandler get a InboundOpenInfo type?

[twittner]

When libp2p-request-response was introduced, it used a combination of upgrades and channels which was seen as a "natural continuation" of the original approach to negotiate protocols. Timeout handling in the context of this approach proved to be difficult (see #1706) because identifying upgrades and upgrade errors is not always possible for inbound upgrades. It seems that the ProtocolsHandler trait is missing a type InboundOpenInfo which would be available in upgrade callbacks, mirroring the already existing OutboundOpenInfo type. This ticket is about the question if others see the need for such a type or not?

[tomaka]

I know this isn't really documented, but in my mind it would be the other way around: the OutboundOpenInfo would be deprecated.

Since the user is free to pass any "meta-data" inside of the OutboundProtocol (and InboundProtocol) itself, the OutboundOpenInfo is a redundant mechanism.

[twittner]

This only applies to successful upgrades and does not help with upgrade errors, no?

Edit 1: For example timeouts.
Edit 2: Sloppy wording. What I meant was that it does not generally help with inject_{dial, listen}_upgrade_err because of ProtocolsHandlerUpgradeErrs such as Timeout or Timer.

[romanb]

I don't see another or better approach to attach context information to inbound upgrades that is also available in the error case. Adding to that the fact that the multi handler cannot even be implemented correctly without such context (as seen in #1710) it seems rather important. libp2p-request-response is relying on inject_listen_upgrade_error to be called properly, otherwise all inbound request failures are silent. While this doesn't have immediate fatal consequences (i.e. memory leaks or anything like it), it is still very undesirable, not just for Throttled, since, if I remember correctly, paritytech/substrate#6634 even uses the multi handler at the moment. So I'm 👍 for InboundOpenInfo.

[twittner]

Another implication of {In,Out}boundUpgradeExt::map_{in, out}bound and {In, Out}boundUpgradeExt::map_{in, out}bound_err is that they usually create closure types which in turn require boxing in order to be named as associated ProtocolsHandler::{In, Out}boundProtocols.

[romanb]

Closed by #1714.