core: Remove Transport::{Dial,ListenUpgrade} in favor of using trait objects

[thomaseizinger]

Context

Whilst toying around with #2650 which was motivated by trying to solve the merge conflicts in #3254, I came across our EitherFuture type. This one exists because the trait bounds on Transport::Dial and Transport::ListenUpgrade force the designated type to "transpose" the output. In other words, with Either implementing Transport, the output of Dial is required to be Result<Either<A, B>, ...> instead of Either<Result<A, ..>, Result<B, ..>>. This makes sense but is not compatible with a general implementation of Future on a type like Either. Thus, we need to provide our own, specialized type for this.

Proposal

Should we remove the associated types Dial and ListenUpgrade in favor of always using BoxFuture?

Benefits

• Less code to maintain
• Simplified interface
• Smaller public API of Transport implementations

Drawbacks

• Forces all Transports to be Send
• More allocations as part of creating new network connections

[thomaseizinger]

Drawback discussions

1. Performance:

I've not done any measurements but I'd be very surprised if the allocations matter in the context of network connections. Even if we decide that performance is super critical here, one could argue that for fast transports like QUIC, the impact is smaller because only one allocation will happen. Additionally, libp2p-swarm will box up a dial anyway, thus we effectively don't have any additional allocations I think?

If we are really keen on optimizing the performance here, we could go down a route similar to what is discussed in #2829. If the main Transport interface gets hardcoded to return a BoxFuture<Output = (PeerId, StreamMuxerBox)> on dial then we can provide a Transport implementation that composes something like a SingleStreamTransport trait together with a muxer and a security protocol where all upgrades are composed without allocations and only boxed before they are returned from the Transport::dial interface.

2. Require Send

In libp2p-swarm, we already require everything to be Send. In previous discussions, we already said that supporting standalone usecases of libp2p-core is not a priority and we want all users to go through libp2p-swarm.

[thomaseizinger]

Note that, whilst the initial idea might be very niche and not that important, I think the benefits of simplifying the Transport interface on the whole are worth discussing.

[mxinden]

Based on intuition, I don't think boxing has any performance impact compared to the amount of work required to establish a connection, thus I don't think it needs to be considered in this discussion.

I don't have an opinion on the above (i.e. boxing or not). That said, I agree that this has priority nicetohave.

[Sherlock-Holo]

after the TAIT feature stable, i think we can write async move {} to create a future, use a type alias as the associated type. this can reduce a lot of handwriting future types, also using async block without Box::pin can avoid the heap allocate

so i think we could reserve these associated type but rewrite those handwriting futures with async move {} block

[thomaseizinger]

In my eyes, the associated type is the complexity that needs to go away. "impl trait in trait" would solve that maybe.

However, as argued above, there really is no downside in just removing them. Someone just needs to pick it up and do it :)

(I have a WIP branch I'll push.)

[thomaseizinger]

(I have a WIP branch I'll push.)

It is here: #3436