From 5559a76bfcb3610b491a01af44efea2dbe0c866c Mon Sep 17 00:00:00 2001 From: Max Inden Date: Mon, 30 May 2022 17:21:07 +0200 Subject: [PATCH 1/4] transports/{tcp,dns,websocket}: Remove Clone implementation for *Config 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>`. 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) }; ``` --- src/lib.rs | 7 ++++--- transports/dns/CHANGELOG.md | 2 ++ transports/dns/src/lib.rs | 1 - transports/tcp/CHANGELOG.md | 2 ++ transports/tcp/src/lib.rs | 6 +++--- transports/websocket/CHANGELOG.md | 2 ++ transports/websocket/src/lib.rs | 2 +- 7 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 9060d39c517..23c3a4bf778 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -201,9 +201,10 @@ pub async fn development_transport( keypair: identity::Keypair, ) -> std::io::Result> { 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) }; diff --git a/transports/dns/CHANGELOG.md b/transports/dns/CHANGELOG.md index 64ee464b3c9..43c03184328 100644 --- a/transports/dns/CHANGELOG.md +++ b/transports/dns/CHANGELOG.md @@ -2,6 +2,8 @@ - Update to `libp2p-core` `v0.33.0`. +- Remove implementation of `Clone` on `GenDnsConfig`. + # 0.32.1 - Update to `trust-dns` `v0.21`. See [PR 2543]. diff --git a/transports/dns/src/lib.rs b/transports/dns/src/lib.rs index 00d52122278..eb00996751f 100644 --- a/transports/dns/src/lib.rs +++ b/transports/dns/src/lib.rs @@ -107,7 +107,6 @@ pub type DnsConfig = GenDnsConfig = GenDnsConfig; /// A `Transport` wrapper for performing DNS lookups when dialing `Multiaddr`esses. -#[derive(Clone)] pub struct GenDnsConfig where C: DnsHandle, diff --git a/transports/tcp/CHANGELOG.md b/transports/tcp/CHANGELOG.md index 0e44d181e31..09e10a8da90 100644 --- a/transports/tcp/CHANGELOG.md +++ b/transports/tcp/CHANGELOG.md @@ -2,6 +2,8 @@ - Update to `libp2p-core` `v0.33.0`. +- Remove implementation of `Clone` on `GenTcpConfig`. + # 0.32.0 [2022-02-22] - Update to `libp2p-core` `v0.32.0`. diff --git a/transports/tcp/src/lib.rs b/transports/tcp/src/lib.rs index dbb1c563e99..72fe6f791cd 100644 --- a/transports/tcp/src/lib.rs +++ b/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 { /// The type of the I/O provider. _impl: std::marker::PhantomData, @@ -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); diff --git a/transports/websocket/CHANGELOG.md b/transports/websocket/CHANGELOG.md index 9d8a32c861a..0540804a35c 100644 --- a/transports/websocket/CHANGELOG.md +++ b/transports/websocket/CHANGELOG.md @@ -2,6 +2,8 @@ - Update to `libp2p-core` `v0.33.0`. +- Remove implementation of `Clone` on `WsConfig`. + # 0.34.0 [2022-02-22] - Update to `libp2p-core` `v0.32.0`. diff --git a/transports/websocket/src/lib.rs b/transports/websocket/src/lib.rs index a6a1c0971c1..770e559a633 100644 --- a/transports/websocket/src/lib.rs +++ b/transports/websocket/src/lib.rs @@ -44,7 +44,7 @@ use std::{ }; /// A Websocket transport. -#[derive(Debug, Clone)] +#[derive(Debug)] pub struct WsConfig where T: Transport, From a7d69b64a8d4e0d7fc0b8744b5f76f6638494552 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Mon, 30 May 2022 19:26:31 +0200 Subject: [PATCH 2/4] transports/: Reference pull request in changelogs Co-authored-by: Thomas Eizinger --- transports/dns/CHANGELOG.md | 4 +++- transports/tcp/CHANGELOG.md | 4 +++- transports/websocket/CHANGELOG.md | 4 +++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/transports/dns/CHANGELOG.md b/transports/dns/CHANGELOG.md index 43c03184328..6af2f9fc0f9 100644 --- a/transports/dns/CHANGELOG.md +++ b/transports/dns/CHANGELOG.md @@ -2,7 +2,9 @@ - Update to `libp2p-core` `v0.33.0`. -- Remove implementation of `Clone` on `GenDnsConfig`. +- Remove implementation of `Clone` on `GenDnsConfig`. See [PR 2682]. + +[PR 2682]: https://github.com/libp2p/rust-libp2p/pull/2682 # 0.32.1 diff --git a/transports/tcp/CHANGELOG.md b/transports/tcp/CHANGELOG.md index 09e10a8da90..a014ff52824 100644 --- a/transports/tcp/CHANGELOG.md +++ b/transports/tcp/CHANGELOG.md @@ -2,7 +2,9 @@ - Update to `libp2p-core` `v0.33.0`. -- Remove implementation of `Clone` on `GenTcpConfig`. +- 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] diff --git a/transports/websocket/CHANGELOG.md b/transports/websocket/CHANGELOG.md index 0540804a35c..e97ea249095 100644 --- a/transports/websocket/CHANGELOG.md +++ b/transports/websocket/CHANGELOG.md @@ -2,7 +2,9 @@ - Update to `libp2p-core` `v0.33.0`. -- Remove implementation of `Clone` on `WsConfig`. +- 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] From 2ede3d0e012c35155b1730aa7c3cd6a1c19b2afe Mon Sep 17 00:00:00 2001 From: Max Inden Date: Mon, 30 May 2022 19:51:41 +0200 Subject: [PATCH 3/4] transports/: Fix tests --- transports/deflate/tests/test.rs | 1 - transports/dns/src/lib.rs | 9 +-------- transports/tcp/src/lib.rs | 8 ++++---- 3 files changed, 5 insertions(+), 13 deletions(-) diff --git a/transports/deflate/tests/test.rs b/transports/deflate/tests/test.rs index 0bb27ed8d85..743d464c1cf 100644 --- a/transports/deflate/tests/test.rs +++ b/transports/deflate/tests/test.rs @@ -54,7 +54,6 @@ async fn run(message1: Vec) { }); let mut listener = transport - .clone() .listen_on("/ip4/0.0.0.0/tcp/0".parse().expect("multiaddr")) .expect("listener"); diff --git a/transports/dns/src/lib.rs b/transports/dns/src/lib.rs index eb00996751f..45806a7b772 100644 --- a/transports/dns/src/lib.rs +++ b/transports/dns/src/lib.rs @@ -627,7 +627,7 @@ mod tests { } } - async fn run(transport: GenDnsConfig) + async fn run(mut transport: GenDnsConfig) where C: DnsHandle, P: ConnectionProvider, @@ -637,7 +637,6 @@ mod tests { { // Success due to existing A record for example.com. let _ = transport - .clone() .dial("/dns4/example.com/tcp/20000".parse().unwrap()) .unwrap() .await @@ -645,7 +644,6 @@ mod tests { // Success due to existing AAAA record for example.com. let _ = transport - .clone() .dial("/dns6/example.com/tcp/20000".parse().unwrap()) .unwrap() .await @@ -653,7 +651,6 @@ mod tests { // 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 @@ -661,7 +658,6 @@ mod tests { // Success due to the DNS TXT records at _dnsaddr.bootstrap.libp2p.io. let _ = transport - .clone() .dial("/dnsaddr/bootstrap.libp2p.io".parse().unwrap()) .unwrap() .await @@ -671,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 @@ -680,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() @@ -696,7 +690,6 @@ mod tests { // Failure due to no records. match transport - .clone() .dial("/dns4/example.invalid/tcp/20000".parse().unwrap()) .unwrap() .await diff --git a/transports/tcp/src/lib.rs b/transports/tcp/src/lib.rs index 72fe6f791cd..a2650f7d216 100644 --- a/transports/tcp/src/lib.rs +++ b/transports/tcp/src/lib.rs @@ -952,7 +952,7 @@ mod tests { ) { let dest_addr = ready_rx.next().await.unwrap(); let mut tcp = GenTcpConfig::::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(addr: Multiaddr) { - let tcp = GenTcpConfig::::new().port_reuse(true); - let mut listener1 = tcp.clone().listen_on(addr).unwrap(); + let mut tcp = GenTcpConfig::::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); From 3b2c7a09cc6ec6677283f9d1af1fb733a014f491 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Mon, 30 May 2022 21:41:27 +0200 Subject: [PATCH 4/4] src/lib: Fix tokio development transport --- src/lib.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 23c3a4bf778..45174b66af7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -259,9 +259,10 @@ pub fn tokio_development_transport( keypair: identity::Keypair, ) -> std::io::Result> { 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) };