transports/{tcp,dns,websocket}: Remove Clone imp for *Config (#2682)

This commit removes the `Clone` implementation on `GenTcpConfig` and consequently the `Clone`
implementations on `GenDnsConfig` and `WsConfig`.

When port-reuse is enabled, `GenTcpConfig` tracks the addresses it is listening in a `HashSet`. This
`HashSet` is shared with the `TcpListenStream`s via an `Arc<Mutex<_>>`. Given that `Clone` is
`derive`d on `GenTcpConfig`, cloning a `GenTcpConfig`, results in both instances sharing the same
set of listen addresses. This is not intuitive.

This behavior is for example error prone in the scenario where one wants to speak both plain DNS/TCP and
Websockets. Say a user creates the transport in the following way:

``` Rust
let transport = {
    let tcp = tcp::TcpConfig::new().nodelay(true).port_reuse(true);
    let dns_tcp = dns::DnsConfig::system(tcp).await?;
    let ws_dns_tcp = websocket::WsConfig::new(dns_tcp.clone());
    dns_tcp.or_transport(ws_dns_tcp)
};
```

Both `dns_tcp` and `ws_dns_tcp` share the set of listen addresses, given the `dns_tcp.clone()` to
create the `ws_dns_tcp`. Thus, with port-reuse, a Websocket dial might reuse a DNS/TCP listening
port instead of a Websocket listening port.

With this commit a user is forced to do the below, preventing the above error:

``` Rust
let transport = {
    let dns_tcp = dns::DnsConfig::system(tcp::TcpConfig::new().nodelay(true).port_reuse(true)).await?;
    let ws_dns_tcp = websocket::WsConfig::new(
        dns::DnsConfig::system(tcp::TcpConfig::new().nodelay(true).port_reuse(true)).await?,
    );
    dns_tcp.or_transport(ws_dns_tcp)
};
```

Co-authored-by: Thomas Eizinger <[email protected]>

Author: mxinden

src/lib.rs

@@ -201,9 +201,10 @@ pub async fn development_transport(
     keypair: identity::Keypair,
 ) -> std::io::Result<core::transport::Boxed<(PeerId, core::muxing::StreamMuxerBox)>> {
     let transport = {
-        let tcp = tcp::TcpConfig::new().nodelay(true);
-        let dns_tcp = dns::DnsConfig::system(tcp).await?;
-        let ws_dns_tcp = websocket::WsConfig::new(dns_tcp.clone());
+        let dns_tcp = dns::DnsConfig::system(tcp::TcpConfig::new().nodelay(true)).await?;
+        let ws_dns_tcp = websocket::WsConfig::new(
+            dns::DnsConfig::system(tcp::TcpConfig::new().nodelay(true)).await?,
+        );
         dns_tcp.or_transport(ws_dns_tcp)
     };
 
@@ -258,9 +259,10 @@ pub fn tokio_development_transport(
     keypair: identity::Keypair,
 ) -> std::io::Result<core::transport::Boxed<(PeerId, core::muxing::StreamMuxerBox)>> {
     let transport = {
-        let tcp = tcp::TokioTcpConfig::new().nodelay(true);
-        let dns_tcp = dns::TokioDnsConfig::system(tcp)?;
-        let ws_dns_tcp = websocket::WsConfig::new(dns_tcp.clone());
+        let dns_tcp = dns::TokioDnsConfig::system(tcp::TokioTcpConfig::new().nodelay(true))?;
+        let ws_dns_tcp = websocket::WsConfig::new(dns::TokioDnsConfig::system(
+            tcp::TokioTcpConfig::new().nodelay(true),
+        )?);
         dns_tcp.or_transport(ws_dns_tcp)
     };
 

transports/deflate/tests/test.rs

@@ -54,7 +54,6 @@ async fn run(message1: Vec<u8>) {
     });
 
     let mut listener = transport
-        .clone()
         .listen_on("/ip4/0.0.0.0/tcp/0".parse().expect("multiaddr"))
         .expect("listener");
 

transports/dns/CHANGELOG.md

@@ -2,6 +2,10 @@
 
 - Update to `libp2p-core` `v0.33.0`.
 
+- Remove implementation of `Clone` on `GenDnsConfig`. See [PR 2682].
+
+[PR 2682]: https://github.com/libp2p/rust-libp2p/pull/2682
+
 # 0.32.1
 
 - Update to `trust-dns` `v0.21`. See [PR 2543].

transports/dns/src/lib.rs

@@ -107,7 +107,6 @@ pub type DnsConfig<T> = GenDnsConfig<T, AsyncStdConnection, AsyncStdConnectionPr
 pub type TokioDnsConfig<T> = GenDnsConfig<T, TokioConnection, TokioConnectionProvider>;
 
 /// A `Transport` wrapper for performing DNS lookups when dialing `Multiaddr`esses.
-#[derive(Clone)]
 pub struct GenDnsConfig<T, C, P>
 where
     C: DnsHandle<Error = ResolveError>,
@@ -628,7 +627,7 @@ mod tests {
             }
         }
 
-        async fn run<T, C, P>(transport: GenDnsConfig<T, C, P>)
+        async fn run<T, C, P>(mut transport: GenDnsConfig<T, C, P>)
         where
             C: DnsHandle<Error = ResolveError>,
             P: ConnectionProvider<Conn = C>,
@@ -638,31 +637,27 @@ mod tests {
         {
             // Success due to existing A record for example.com.
             let _ = transport
-                .clone()
                 .dial("/dns4/example.com/tcp/20000".parse().unwrap())
                 .unwrap()
                 .await
                 .unwrap();
 
             // Success due to existing AAAA record for example.com.
             let _ = transport
-                .clone()
                 .dial("/dns6/example.com/tcp/20000".parse().unwrap())
                 .unwrap()
                 .await
                 .unwrap();
 
             // Success due to pass-through, i.e. nothing to resolve.
             let _ = transport
-                .clone()
                 .dial("/ip4/1.2.3.4/tcp/20000".parse().unwrap())
                 .unwrap()
                 .await
                 .unwrap();
 
             // Success due to the DNS TXT records at _dnsaddr.bootstrap.libp2p.io.
             let _ = transport
-                .clone()
                 .dial("/dnsaddr/bootstrap.libp2p.io".parse().unwrap())
                 .unwrap()
                 .await
@@ -672,7 +667,6 @@ mod tests {
             // an entry with suffix `/p2p/QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJN`,
             // i.e. a bootnode with such a peer ID.
             let _ = transport
-                .clone()
                 .dial("/dnsaddr/bootstrap.libp2p.io/p2p/QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJN".parse().unwrap())
                 .unwrap()
                 .await
@@ -681,7 +675,6 @@ mod tests {
             // Failure due to the DNS TXT records at _dnsaddr.libp2p.io not having
             // an entry with a random `p2p` suffix.
             match transport
-                .clone()
                 .dial(
                     format!("/dnsaddr/bootstrap.libp2p.io/p2p/{}", PeerId::random())
                         .parse()
@@ -697,7 +690,6 @@ mod tests {
 
             // Failure due to no records.
             match transport
-                .clone()
                 .dial("/dns4/example.invalid/tcp/20000".parse().unwrap())
                 .unwrap()
                 .await

transports/tcp/CHANGELOG.md

@@ -2,6 +2,10 @@
 
 - Update to `libp2p-core` `v0.33.0`.
 
+- Remove implementation of `Clone` on `GenTcpConfig`. See [PR 2682].
+
+[PR 2682]: https://github.com/libp2p/rust-libp2p/pull/2682
+
 # 0.32.0 [2022-02-22]
 
 - Update to `libp2p-core` `v0.32.0`.

transports/tcp/src/lib.rs

@@ -67,7 +67,7 @@ use std::{
 use provider::{IfEvent, Provider};
 
 /// The configuration for a TCP/IP transport capability for libp2p.
-#[derive(Clone, Debug)]
+#[derive(Debug)]
 pub struct GenTcpConfig<T> {
     /// The type of the I/O provider.
     _impl: std::marker::PhantomData<T>,
@@ -258,7 +258,7 @@ where
     /// let listen_addr2: Multiaddr = "/ip4/127.0.0.1/tcp/9002".parse().unwrap();
     ///
     /// let mut tcp1 = TcpConfig::new().port_reuse(true);
-    /// let mut listener1 = tcp1.clone().listen_on(listen_addr1.clone()).expect("listener");
+    /// let mut listener1 = tcp1.listen_on(listen_addr1.clone()).expect("listener");
     /// match listener1.next().await.expect("event")? {
     ///     ListenerEvent::NewAddress(listen_addr) => {
     ///         println!("Listening on {:?}", listen_addr);
@@ -269,7 +269,7 @@ where
     /// }
     ///
     /// let mut tcp2 = TcpConfig::new().port_reuse(true);
-    /// let mut listener2 = tcp2.clone().listen_on(listen_addr2).expect("listener");
+    /// let mut listener2 = tcp2.listen_on(listen_addr2).expect("listener");
     /// match listener2.next().await.expect("event")? {
     ///     ListenerEvent::NewAddress(listen_addr) => {
     ///         println!("Listening on {:?}", listen_addr);
@@ -952,7 +952,7 @@ mod tests {
         ) {
             let dest_addr = ready_rx.next().await.unwrap();
             let mut tcp = GenTcpConfig::<T>::new().port_reuse(true);
-            let mut listener = tcp.clone().listen_on(addr).unwrap();
+            let mut listener = tcp.listen_on(addr).unwrap();
             match listener.next().await.unwrap().unwrap() {
                 ListenerEvent::NewAddress(_) => {
                     // Check that tcp and listener share the same port reuse SocketAddr
@@ -1018,8 +1018,8 @@ mod tests {
         env_logger::try_init().ok();
 
         async fn listen_twice<T: Provider>(addr: Multiaddr) {
-            let tcp = GenTcpConfig::<T>::new().port_reuse(true);
-            let mut listener1 = tcp.clone().listen_on(addr).unwrap();
+            let mut tcp = GenTcpConfig::<T>::new().port_reuse(true);
+            let mut listener1 = tcp.listen_on(addr).unwrap();
             match listener1.next().await.unwrap().unwrap() {
                 ListenerEvent::NewAddress(addr1) => {
                     // Check that tcp and listener share the same port reuse SocketAddr
@@ -1032,7 +1032,7 @@ mod tests {
                     assert_eq!(port_reuse_tcp, port_reuse_listener1);
 
                     // Listen on the same address a second time.
-                    let mut listener2 = tcp.clone().listen_on(addr1.clone()).unwrap();
+                    let mut listener2 = tcp.listen_on(addr1.clone()).unwrap();
                     match listener2.next().await.unwrap().unwrap() {
                         ListenerEvent::NewAddress(addr2) => {
                             assert_eq!(addr1, addr2);

transports/websocket/CHANGELOG.md

@@ -2,6 +2,10 @@
 
 - Update to `libp2p-core` `v0.33.0`.
 
+- Remove implementation of `Clone` on `WsConfig`. See [PR 2682].
+
+[PR 2682]: https://github.com/libp2p/rust-libp2p/pull/2682
+
 # 0.34.0 [2022-02-22]
 
 - Update to `libp2p-core` `v0.32.0`.

transports/websocket/src/lib.rs

@@ -44,7 +44,7 @@ use std::{
 };
 
 /// A Websocket transport.
-#[derive(Debug, Clone)]
+#[derive(Debug)]
 pub struct WsConfig<T: Transport>
 where
     T: Transport,