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

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`.
libp2p · Apr 6, 2022 · 2ad905f · 2ad905f
1 parent 7a11478
commit 2ad905f
Show file tree

Hide file tree

Showing 82 changed files with 751 additions and 449 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -42,6 +42,27 @@
 
 # `libp2p` facade crate
 
+# 0.45.0 [unreleased]
+
+- Update individual crates.
+    - Update to [`libp2p-plaintext` `v0.33.0`](transports/plaintext/CHANGELOG.md).
+    - Update to [`libp2p-noise` `v0.36.0`](transports/noise/CHANGELOG.md).
+    - Update to [`libp2p-wasm-ext` `v0.33.0`](transports/wasm-ext/CHANGELOG.md).
+    - Update to [`libp2p-yamux` `v0.37.0`](muxers/yamux/CHANGELOG.md).
+    - Update to [`libp2p-mplex` `v0.33.0`](muxers/mplex/CHANGELOG.md).
+    - Update to [`libp2p-dcutr` `v0.3.0`](protocols/dcutr/CHANGELOG.md).
+    - Update to [`libp2p-rendezvous` `v0.6.0`](protocols/rendezvous/CHANGELOG.md).
+    - Update to [`libp2p-ping` `v0.36.0`](protocols/ping/CHANGELOG.md).
+    - Update to [`libp2p-identify` `v0.36.0`](protocols/identify/CHANGELOG.md).
+    - Update to [`libp2p-floodsub` `v0.36.0`](protocols/floodsub/CHANGELOG.md).
+    - Update to [`libp2p-relay` `v0.9.0`](protocols/relay/CHANGELOG.md).
+    - Update to [`libp2p-metrics` `v0.6.0`](misc/metrics/CHANGELOG.md).
+    - Update to [`libp2p-kad` `v0.37.0`](protocols/kad/CHANGELOG.md).
+    - Update to [`libp2p-autonat` `v0.4.0`](protocols/autonat/CHANGELOG.md).
+    - Update to [`libp2p-request-response` `v0.18.0`](protocols/request-response/CHANGELOG.md).
+    - Update to [`libp2p-swarm` `v0.36.0`](swarm/CHANGELOG.md).
+    - Update to [`libp2p-core` `v0.33.0`](core/CHANGELOG.md).
+
 ## 0.44.0
 
 - Update individual crates.

diff --git a/Cargo.toml b/Cargo.toml
@@ -3,7 +3,7 @@ name = "libp2p"
 edition = "2021"
 rust-version = "1.56.1"
 description = "Peer-to-peer networking library"
-version = "0.44.0"
+version = "0.45.0"
 authors = ["Parity Technologies <admin@parity.io>"]
 license = "MIT"
 repository = "https://github.com/libp2p/rust-libp2p"
@@ -77,41 +77,41 @@ getrandom = "0.2.3" # Explicit dependency to be used in `wasm-bindgen` feature
 instant = "0.1.11" # Explicit dependency to be used in `wasm-bindgen` feature
 lazy_static = "1.2"
 
-libp2p-autonat = { version = "0.3.0", path = "protocols/autonat", optional = true }
-libp2p-core = { version = "0.32.1", path = "core",  default-features = false }
-libp2p-dcutr = { version = "0.2.0", path = "protocols/dcutr",  optional = true }
-libp2p-floodsub = { version = "0.35.0", path = "protocols/floodsub", optional = true }
-libp2p-identify = { version = "0.35.0", path = "protocols/identify", optional = true }
-libp2p-kad = { version = "0.36.0", path = "protocols/kad", optional = true }
-libp2p-metrics = { version = "0.5.0", path = "misc/metrics", optional = true }
-libp2p-mplex = { version = "0.32.0", path = "muxers/mplex", optional = true }
-libp2p-noise = { version = "0.35.0", path = "transports/noise", optional = true }
-libp2p-ping = { version = "0.35.0", path = "protocols/ping", optional = true }
-libp2p-plaintext = { version = "0.32.0", path = "transports/plaintext", optional = true }
+libp2p-autonat = { version = "0.4.0", path = "protocols/autonat", optional = true }
+libp2p-core = { version = "0.33.0", path = "core",  default-features = false }
+libp2p-dcutr = { version = "0.3.0", path = "protocols/dcutr",  optional = true }
+libp2p-floodsub = { version = "0.36.0", path = "protocols/floodsub", optional = true }
+libp2p-identify = { version = "0.36.0", path = "protocols/identify", optional = true }
+libp2p-kad = { version = "0.37.0", path = "protocols/kad", optional = true }
+libp2p-metrics = { version = "0.6.0", path = "misc/metrics", optional = true }
+libp2p-mplex = { version = "0.33.0", path = "muxers/mplex", optional = true }
+libp2p-noise = { version = "0.36.0", path = "transports/noise", optional = true }
+libp2p-ping = { version = "0.36.0", path = "protocols/ping", optional = true }
+libp2p-plaintext = { version = "0.33.0", path = "transports/plaintext", optional = true }
 libp2p-pnet = { version = "0.22.0", path = "transports/pnet", optional = true }
-libp2p-relay = { version = "0.8.0", path = "protocols/relay", optional = true }
-libp2p-rendezvous = { version = "0.5.0", path = "protocols/rendezvous", optional = true }
-libp2p-request-response = { version = "0.17.0", path = "protocols/request-response", optional = true }
-libp2p-swarm = { version = "0.35.0", path = "swarm" }
+libp2p-relay = { version = "0.9.0", path = "protocols/relay", optional = true }
+libp2p-rendezvous = { version = "0.6.0", path = "protocols/rendezvous", optional = true }
+libp2p-request-response = { version = "0.18.0", path = "protocols/request-response", optional = true }
+libp2p-swarm = { version = "0.36.0", path = "swarm" }
 libp2p-swarm-derive = { version = "0.27.0", path = "swarm-derive" }
 libp2p-uds = { version = "0.32.0", path = "transports/uds", optional = true }
-libp2p-wasm-ext = { version = "0.32.0", path = "transports/wasm-ext", default-features = false, optional = true }
-libp2p-yamux = { version = "0.36.0", path = "muxers/yamux", optional = true }
+libp2p-wasm-ext = { version = "0.33.0", path = "transports/wasm-ext", default-features = false, optional = true }
+libp2p-yamux = { version = "0.37.0", path = "muxers/yamux", optional = true }
 multiaddr = { version = "0.14.0" }
 parking_lot = "0.12.0"
 pin-project = "1.0.0"
 rand = "0.7.3" # Explicit dependency to be used in `wasm-bindgen` feature
 smallvec = "1.6.1"
 
 [target.'cfg(not(any(target_os = "emscripten", target_os = "wasi", target_os = "unknown")))'.dependencies]
-libp2p-deflate = { version = "0.32.0", path = "transports/deflate", optional = true }
-libp2p-dns = { version = "0.32.1", path = "transports/dns", optional = true, default-features = false }
-libp2p-mdns = { version = "0.36.0", path = "protocols/mdns", optional = true }
-libp2p-tcp = { version = "0.32.0", path = "transports/tcp", default-features = false, optional = true }
-libp2p-websocket = { version = "0.34.0", path = "transports/websocket", optional = true }
+libp2p-deflate = { version = "0.33.0", path = "transports/deflate", optional = true }
+libp2p-dns = { version = "0.33.0", path = "transports/dns", optional = true, default-features = false }
+libp2p-mdns = { version = "0.37.0", path = "protocols/mdns", optional = true }
+libp2p-tcp = { version = "0.33.0", path = "transports/tcp", default-features = false, optional = true }
+libp2p-websocket = { version = "0.35.0", path = "transports/websocket", optional = true }
 
 [target.'cfg(not(target_os = "unknown"))'.dependencies]
-libp2p-gossipsub = { version = "0.37.0", path = "protocols/gossipsub", optional = true }
+libp2p-gossipsub = { version = "0.38.0", path = "protocols/gossipsub", optional = true }
 
 [dev-dependencies]
 async-std = { version = "1.6.2", features = ["attributes"] }

diff --git a/core/CHANGELOG.md b/core/CHANGELOG.md
@@ -1,3 +1,9 @@
+# 0.33.0 [unreleased]
+
+- Have methods on `Transport` take `&mut self` instead of `self`. See [PR 2529].
+
+[PR 2529]: https://github.com/libp2p/rust-libp2p/pull/2529
+
 # 0.32.1
 
 - Add `PeerId::try_from_multiaddr` to extract a `PeerId` from a `Multiaddr` that ends in `/p2p/<peer-id>`.

diff --git a/core/Cargo.toml b/core/Cargo.toml
@@ -3,7 +3,7 @@ name = "libp2p-core"
 edition = "2021"
 rust-version = "1.56.1"
 description = "Core traits and structs of libp2p"
-version = "0.32.1"
+version = "0.33.0"
 authors = ["Parity Technologies <admin@parity.io>"]
 license = "MIT"
 repository = "https://github.com/libp2p/rust-libp2p"

diff --git a/core/src/either.rs b/core/src/either.rs
@@ -497,7 +497,10 @@ where
     type ListenerUpgrade = EitherFuture<A::ListenerUpgrade, B::ListenerUpgrade>;
     type Dial = EitherFuture<A::Dial, B::Dial>;
 
-    fn listen_on(self, addr: Multiaddr) -> Result<Self::Listener, TransportError<Self::Error>> {
+    fn listen_on(
+        &mut self,
+        addr: Multiaddr,
+    ) -> Result<Self::Listener, TransportError<Self::Error>> {
         use TransportError::*;
         match self {
             EitherTransport::Left(a) => match a.listen_on(addr) {
@@ -513,7 +516,7 @@ where
         }
     }
 
-    fn dial(self, addr: Multiaddr) -> Result<Self::Dial, TransportError<Self::Error>> {
+    fn dial(&mut self, addr: Multiaddr) -> Result<Self::Dial, TransportError<Self::Error>> {
         use TransportError::*;
         match self {
             EitherTransport::Left(a) => match a.dial(addr) {
@@ -529,7 +532,10 @@ where
         }
     }
 
-    fn dial_as_listener(self, addr: Multiaddr) -> Result<Self::Dial, TransportError<Self::Error>>
+    fn dial_as_listener(
+        &mut self,
+        addr: Multiaddr,
+    ) -> Result<Self::Dial, TransportError<Self::Error>>
     where
         Self: Sized,
     {

diff --git a/core/src/transport.rs b/core/src/transport.rs
@@ -70,10 +70,12 @@ pub use self::upgrade::Upgrade;
 /// by a [`Transport`] through an upgrade mechanism that is initiated via
 /// [`upgrade`](Transport::upgrade).
 ///
-/// > **Note**: The methods of this trait use `self` and not `&self` or `&mut self`. In other
-/// >           words, listening or dialing consumes the transport object. This has been designed
-/// >           so that you would implement this trait on `&Foo` or `&mut Foo` instead of directly
-/// >           on `Foo`.
+/// Note for implementors: Futures returned by [`Transport::dial`] should only
+/// do work once polled for the first time. E.g. in the case of TCP, connecting
+/// to the remote should not happen immediately on [`Transport::dial`] but only
+/// once the returned [`Future`] is polled. The caller of [`Transport::dial`]
+/// may call the method multiple times with a set of addresses, racing a subset
+/// of the returned dials to success concurrently.
 pub trait Transport {
     /// The result of a connection setup process, including protocol upgrades.
     ///
@@ -118,15 +120,15 @@ pub trait Transport {
     ///
     /// Returning an error from the stream is considered fatal. The listener can also report
     /// non-fatal errors by producing a [`ListenerEvent::Error`].
-    fn listen_on(self, addr: Multiaddr) -> Result<Self::Listener, TransportError<Self::Error>>
+    fn listen_on(&mut self, addr: Multiaddr) -> Result<Self::Listener, TransportError<Self::Error>>
     where
         Self: Sized;
 
     /// Dials the given [`Multiaddr`], returning a future for a pending outbound connection.
     ///
     /// If [`TransportError::MultiaddrNotSupported`] is returned, it may be desirable to
     /// try an alternative [`Transport`], if available.
-    fn dial(self, addr: Multiaddr) -> Result<Self::Dial, TransportError<Self::Error>>
+    fn dial(&mut self, addr: Multiaddr) -> Result<Self::Dial, TransportError<Self::Error>>
     where
         Self: Sized;
 
@@ -135,7 +137,10 @@ pub trait Transport {
     /// This option is needed for NAT and firewall hole punching.
     ///
     /// See [`ConnectedPoint::Dialer`](crate::connection::ConnectedPoint::Dialer) for related option.
-    fn dial_as_listener(self, addr: Multiaddr) -> Result<Self::Dial, TransportError<Self::Error>>
+    fn dial_as_listener(
+        &mut self,
+        addr: Multiaddr,
+    ) -> Result<Self::Dial, TransportError<Self::Error>>
     where
         Self: Sized;
 
@@ -147,7 +152,7 @@ pub trait Transport {
     /// Boxes the transport, including custom transport errors.
     fn boxed(self) -> boxed::Boxed<Self::Output>
     where
-        Self: Transport + Sized + Clone + Send + Sync + 'static,
+        Self: Transport + Sized + Send + Sync + 'static,
         Self::Dial: Send + 'static,
         Self::Listener: Send + 'static,
         Self::ListenerUpgrade: Send + 'static,
@@ -160,7 +165,7 @@ pub trait Transport {
     fn map<F, O>(self, f: F) -> map::Map<Self, F>
     where
         Self: Sized,
-        F: FnOnce(Self::Output, ConnectedPoint) -> O + Clone,
+        F: FnOnce(Self::Output, ConnectedPoint) -> O,
     {
         map::Map::new(self, f)
     }
@@ -169,7 +174,7 @@ pub trait Transport {
     fn map_err<F, E>(self, f: F) -> map_err::MapErr<Self, F>
     where
         Self: Sized,
-        F: FnOnce(Self::Error) -> E + Clone,
+        F: FnOnce(Self::Error) -> E,
     {
         map_err::MapErr::new(self, f)
     }
@@ -198,7 +203,7 @@ pub trait Transport {
     fn and_then<C, F, O>(self, f: C) -> and_then::AndThen<Self, C>
     where
         Self: Sized,
-        C: FnOnce(Self::Output, ConnectedPoint) -> F + Clone,
+        C: FnOnce(Self::Output, ConnectedPoint) -> F,
         F: TryFuture<Ok = O>,
         <F as TryFuture>::Error: Error + 'static,
     {

diff --git a/core/src/transport/and_then.rs b/core/src/transport/and_then.rs
@@ -53,7 +53,10 @@ where
     type ListenerUpgrade = AndThenFuture<T::ListenerUpgrade, C, F>;
     type Dial = AndThenFuture<T::Dial, C, F>;
 
-    fn listen_on(self, addr: Multiaddr) -> Result<Self::Listener, TransportError<Self::Error>> {
+    fn listen_on(
+        &mut self,
+        addr: Multiaddr,
+    ) -> Result<Self::Listener, TransportError<Self::Error>> {
         let listener = self
             .transport
             .listen_on(addr)
@@ -64,20 +67,20 @@ where
         // `stream` can only produce an `Err` if `listening_stream` produces an `Err`.
         let stream = AndThenStream {
             stream: listener,
-            fun: self.fun,
+            fun: self.fun.clone(),
         };
         Ok(stream)
     }
 
-    fn dial(self, addr: Multiaddr) -> Result<Self::Dial, TransportError<Self::Error>> {
+    fn dial(&mut self, addr: Multiaddr) -> Result<Self::Dial, TransportError<Self::Error>> {
         let dialed_fut = self
             .transport
             .dial(addr.clone())
             .map_err(|err| err.map(EitherError::A))?;
         let future = AndThenFuture {
             inner: Either::Left(Box::pin(dialed_fut)),
             args: Some((
-                self.fun,
+                self.fun.clone(),
                 ConnectedPoint::Dialer {
                     address: addr,
                     role_override: Endpoint::Dialer,
@@ -88,15 +91,18 @@ where
         Ok(future)
     }
 
-    fn dial_as_listener(self, addr: Multiaddr) -> Result<Self::Dial, TransportError<Self::Error>> {
+    fn dial_as_listener(
+        &mut self,
+        addr: Multiaddr,
+    ) -> Result<Self::Dial, TransportError<Self::Error>> {
         let dialed_fut = self
             .transport
             .dial_as_listener(addr.clone())
             .map_err(|err| err.map(EitherError::A))?;
         let future = AndThenFuture {
             inner: Either::Left(Box::pin(dialed_fut)),
             args: Some((
-                self.fun,
+                self.fun.clone(),
                 ConnectedPoint::Dialer {
                     address: addr,
                     role_override: Endpoint::Listener,