{core,swarm}/: Don't require Transport: Clone and take &mut

[mxinden]

Previously libp2p-swarm required a Transport to be Clone. Methods
on Transport, e.g. Transport::dial would take ownership, requiring
e.g. a Clone::clone before calling Transport::dial.

The requirement of Transport to be Clone is no longer needed in
libp2p-swarm. E.g. concurrent dialing can be done without a clone per
dial.

This commit removes the requirement of Clone for Transport in
libp2p-swarm. As a follow-up methods on Transport no longer take
ownership, but instead a mutable reference (&mut self).

On the one hand this simplifies libp2p-swarm, on the other it
simplifies implementations of Transport.

---

I hope this will significantly simplify #2289. Testing it out next.

As a follow-up we could as well remove the concept of listeners, thus polling a Transport instead of a Listener returned by Transport::listen_on. Still need to give this idea more thought.

//CC @kpp

[mxinden]

I am assuming, based on your reaction, that this would simplify https://github.com/wngr/libp2p-webrtc as well, correct @wngr?

[thomaseizinger]

Concept ACK

I like the idea. Also liking the idea of a polling a Transport. Always found it weird that we use a different abstraction here than in the rest of the codebase.

If we end up polling, I think connections created from listen_on and dial should be reported by poll.

[kpp]

Actually I already managed to overcome this restriction. But thanks =)

[wngr]

I am assuming, based on your reaction, that this would simplify https://github.com/wngr/libp2p-webrtc as well, correct @wngr?

At least a little bit, yes. More importantly, it will reduce a cognitive dissonance for me (How many transports are there?).

[mxinden]

This is ready for a review. Any comments?

[thomaseizinger]

LGTM!

Are you planning to expose a poll interface from the Transport later?

[thomaseizinger]

Delete these?

[thomaseizinger]

Changelog entry seems to be missing :)

[mxinden]

Are you planning to expose a poll interface from the Transport later?

Yes. That would be my follow up proposal, namely to remove the concept of Listener and instead emit new incoming connection Futures through Transport::poll.

If we end up polling, I think connections created from listen_on and dial should be reported by poll.

I will have to experiment with this. I am not yet sure how much benefit it would be to emit the dial Future via Transport::poll instead of directly via Transport::dial.

I argue that the change proposed in this pull request is a step forward, whether we do the above follow-up or not.

[thomaseizinger]

Are you planning to expose a poll interface from the Transport later?

Yes. That would be my follow up proposal, namely to remove the concept of Listener and instead emit new incoming connection Futures through Transport::poll.

Nice, looking forward to that :)

If we end up polling, I think connections created from listen_on and dial should be reported by poll.

I will have to experiment with this. I am not yet sure how much benefit it would be to emit the dial Future via Transport::poll instead of directly via Transport::dial.

In my latest experiments, I ended up having to delegate to the same code from dial as for each element of the listener stream.

More generally, I see it as one of the benefits of libp2p (as a result of multiplexing) that the endpoint (dialer/listener) matters less for the application and thus allows for a more generalized handling of all connections.

Cases where you need exactly the connection that the dial future resolves to should be rather rare IMO.

I argue that the change proposed in this pull request is a step forward, whether we do the above follow-up or not.

Yep, on board with that!