From f2801fb86a4c9d3b59e52fec80bd3ff9e83b953f Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Mon, 20 Mar 2023 20:44:49 +0100 Subject: [PATCH 01/50] Report changes in supported protocols back to `ConnectionHandler` --- protocols/dcutr/src/handler/direct.rs | 3 +- protocols/dcutr/src/handler/relayed.rs | 2 +- protocols/gossipsub/src/handler.rs | 4 +- protocols/identify/src/handler.rs | 4 +- protocols/kad/src/handler.rs | 4 +- protocols/perf/src/client/handler.rs | 2 +- protocols/perf/src/server/handler.rs | 2 +- protocols/ping/src/handler.rs | 4 +- protocols/relay/src/behaviour/handler.rs | 2 +- protocols/relay/src/priv_client/handler.rs | 2 +- protocols/rendezvous/src/substream_handler.rs | 3 +- protocols/request-response/src/handler.rs | 2 +- swarm/src/behaviour/toggle.rs | 3 + swarm/src/connection.rs | 158 +++++++++++++++++- swarm/src/dummy.rs | 4 +- swarm/src/handler.rs | 8 + swarm/src/handler/either.rs | 6 + swarm/src/handler/multi.rs | 5 + swarm/src/handler/one_shot.rs | 4 +- swarm/src/handler/pending.rs | 3 +- swarm/src/handler/select.rs | 6 + swarm/src/keep_alive.rs | 3 +- 22 files changed, 213 insertions(+), 21 deletions(-) diff --git a/protocols/dcutr/src/handler/direct.rs b/protocols/dcutr/src/handler/direct.rs index aab212483eb..521a351fdf9 100644 --- a/protocols/dcutr/src/handler/direct.rs +++ b/protocols/dcutr/src/handler/direct.rs @@ -92,7 +92,8 @@ impl ConnectionHandler for Handler { | ConnectionEvent::FullyNegotiatedOutbound(_) | ConnectionEvent::DialUpgradeError(_) | ConnectionEvent::ListenUpgradeError(_) - | ConnectionEvent::AddressChange(_) => {} + | ConnectionEvent::AddressChange(_) + | ConnectionEvent::ProtocolsChange(_) => {} } } } diff --git a/protocols/dcutr/src/handler/relayed.rs b/protocols/dcutr/src/handler/relayed.rs index aefaaeec933..a0ec68f7494 100644 --- a/protocols/dcutr/src/handler/relayed.rs +++ b/protocols/dcutr/src/handler/relayed.rs @@ -419,7 +419,7 @@ impl ConnectionHandler for Handler { ConnectionEvent::DialUpgradeError(dial_upgrade_error) => { self.on_dial_upgrade_error(dial_upgrade_error) } - ConnectionEvent::AddressChange(_) => {} + ConnectionEvent::AddressChange(_) | ConnectionEvent::ProtocolsChange(_) => {} } } } diff --git a/protocols/gossipsub/src/handler.rs b/protocols/gossipsub/src/handler.rs index 2c921286553..fa131feecc6 100644 --- a/protocols/gossipsub/src/handler.rs +++ b/protocols/gossipsub/src/handler.rs @@ -576,7 +576,9 @@ impl ConnectionHandler for Handler { warn!("Dial upgrade error {:?}", e); self.upgrade_errors.push_back(e); } - ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) => {} + ConnectionEvent::AddressChange(_) + | ConnectionEvent::ListenUpgradeError(_) + | ConnectionEvent::ProtocolsChange(_) => {} } } } diff --git a/protocols/identify/src/handler.rs b/protocols/identify/src/handler.rs index c0bd9d928eb..653d9d058fc 100644 --- a/protocols/identify/src/handler.rs +++ b/protocols/identify/src/handler.rs @@ -344,7 +344,9 @@ impl ConnectionHandler for Handler { ConnectionEvent::DialUpgradeError(dial_upgrade_error) => { self.on_dial_upgrade_error(dial_upgrade_error) } - ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) => {} + ConnectionEvent::AddressChange(_) + | ConnectionEvent::ListenUpgradeError(_) + | ConnectionEvent::ProtocolsChange(_) => {} } } } diff --git a/protocols/kad/src/handler.rs b/protocols/kad/src/handler.rs index 22c1253dd1e..86e47ac9e27 100644 --- a/protocols/kad/src/handler.rs +++ b/protocols/kad/src/handler.rs @@ -786,7 +786,9 @@ where ConnectionEvent::DialUpgradeError(dial_upgrade_error) => { self.on_dial_upgrade_error(dial_upgrade_error) } - ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) => {} + ConnectionEvent::AddressChange(_) + | ConnectionEvent::ListenUpgradeError(_) + | ConnectionEvent::ProtocolsChange(_) => {} } } } diff --git a/protocols/perf/src/client/handler.rs b/protocols/perf/src/client/handler.rs index f75a43f0a4e..93f71ea48fb 100644 --- a/protocols/perf/src/client/handler.rs +++ b/protocols/perf/src/client/handler.rs @@ -127,7 +127,7 @@ impl ConnectionHandler for Handler { .boxed(), ), - ConnectionEvent::AddressChange(_) => {} + ConnectionEvent::AddressChange(_) | ConnectionEvent::ProtocolsChange(_) => {} ConnectionEvent::DialUpgradeError(DialUpgradeError { info: Command { id, .. }, error, diff --git a/protocols/perf/src/server/handler.rs b/protocols/perf/src/server/handler.rs index 2946b6d4a4c..357f8aa1203 100644 --- a/protocols/perf/src/server/handler.rs +++ b/protocols/perf/src/server/handler.rs @@ -104,7 +104,7 @@ impl ConnectionHandler for Handler { ConnectionEvent::DialUpgradeError(DialUpgradeError { info, .. }) => { void::unreachable(info) } - ConnectionEvent::AddressChange(_) => {} + ConnectionEvent::AddressChange(_) | ConnectionEvent::ProtocolsChange(_) => {} ConnectionEvent::ListenUpgradeError(ListenUpgradeError { info: (), error }) => { match error { ConnectionHandlerUpgrErr::Timeout => {} diff --git a/protocols/ping/src/handler.rs b/protocols/ping/src/handler.rs index 2703b274c77..4c063f1912d 100644 --- a/protocols/ping/src/handler.rs +++ b/protocols/ping/src/handler.rs @@ -411,7 +411,9 @@ impl ConnectionHandler for Handler { ConnectionEvent::DialUpgradeError(dial_upgrade_error) => { self.on_dial_upgrade_error(dial_upgrade_error) } - ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) => {} + ConnectionEvent::AddressChange(_) + | ConnectionEvent::ListenUpgradeError(_) + | ConnectionEvent::ProtocolsChange(_) => {} } } } diff --git a/protocols/relay/src/behaviour/handler.rs b/protocols/relay/src/behaviour/handler.rs index 3acbda0eff5..0608d2cae3d 100644 --- a/protocols/relay/src/behaviour/handler.rs +++ b/protocols/relay/src/behaviour/handler.rs @@ -954,7 +954,7 @@ impl ConnectionHandler for Handler { ConnectionEvent::DialUpgradeError(dial_upgrade_error) => { self.on_dial_upgrade_error(dial_upgrade_error) } - ConnectionEvent::AddressChange(_) => {} + ConnectionEvent::AddressChange(_) | ConnectionEvent::ProtocolsChange(_) => {} } } } diff --git a/protocols/relay/src/priv_client/handler.rs b/protocols/relay/src/priv_client/handler.rs index 3d7bc6c4d1e..c736d219865 100644 --- a/protocols/relay/src/priv_client/handler.rs +++ b/protocols/relay/src/priv_client/handler.rs @@ -613,7 +613,7 @@ impl ConnectionHandler for Handler { ConnectionEvent::DialUpgradeError(dial_upgrade_error) => { self.on_dial_upgrade_error(dial_upgrade_error) } - ConnectionEvent::AddressChange(_) => {} + ConnectionEvent::AddressChange(_) | ConnectionEvent::ProtocolsChange(_) => {} } } } diff --git a/protocols/rendezvous/src/substream_handler.rs b/protocols/rendezvous/src/substream_handler.rs index 16a493ccc3a..57946e7d237 100644 --- a/protocols/rendezvous/src/substream_handler.rs +++ b/protocols/rendezvous/src/substream_handler.rs @@ -396,7 +396,8 @@ where // TODO: Handle upgrade errors properly ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) - | ConnectionEvent::DialUpgradeError(_) => {} + | ConnectionEvent::DialUpgradeError(_) + | ConnectionEvent::ProtocolsChange(_) => {} } } diff --git a/protocols/request-response/src/handler.rs b/protocols/request-response/src/handler.rs index 50cd6adb055..8c8290bd79a 100644 --- a/protocols/request-response/src/handler.rs +++ b/protocols/request-response/src/handler.rs @@ -425,7 +425,7 @@ where ConnectionEvent::ListenUpgradeError(listen_upgrade_error) => { self.on_listen_upgrade_error(listen_upgrade_error) } - ConnectionEvent::AddressChange(_) => {} + ConnectionEvent::AddressChange(_) | ConnectionEvent::ProtocolsChange(_) => {} } } } diff --git a/swarm/src/behaviour/toggle.rs b/swarm/src/behaviour/toggle.rs index bd4678a5e58..e1b5a2a0688 100644 --- a/swarm/src/behaviour/toggle.rs +++ b/swarm/src/behaviour/toggle.rs @@ -368,6 +368,9 @@ where ConnectionEvent::ListenUpgradeError(listen_upgrade_error) => { self.on_listen_upgrade_error(listen_upgrade_error) } + ConnectionEvent::ProtocolsChange(_) => { + todo!() + } } } } diff --git a/swarm/src/connection.rs b/swarm/src/connection.rs index e7e71fdfebc..5f9f68b054d 100644 --- a/swarm/src/connection.rs +++ b/swarm/src/connection.rs @@ -29,9 +29,9 @@ pub use error::{ use crate::handler::{ AddressChange, ConnectionEvent, ConnectionHandler, DialUpgradeError, FullyNegotiatedInbound, - FullyNegotiatedOutbound, ListenUpgradeError, + FullyNegotiatedOutbound, ListenUpgradeError, ProtocolsChange, }; -use crate::upgrade::{InboundUpgradeSend, OutboundUpgradeSend, SendWrapper}; +use crate::upgrade::{InboundUpgradeSend, OutboundUpgradeSend, SendWrapper, UpgradeInfoSend}; use crate::{ConnectionHandlerEvent, ConnectionHandlerUpgrErr, KeepAlive, SubstreamProtocol}; use futures::stream::FuturesUnordered; use futures::FutureExt; @@ -43,7 +43,7 @@ use libp2p_core::multiaddr::Multiaddr; use libp2p_core::muxing::{StreamMuxerBox, StreamMuxerEvent, StreamMuxerExt, SubstreamBox}; use libp2p_core::upgrade::{InboundUpgradeApply, OutboundUpgradeApply}; use libp2p_core::Endpoint; -use libp2p_core::{upgrade, UpgradeError}; +use libp2p_core::{upgrade, ProtocolName as _, UpgradeError}; use libp2p_identity::PeerId; use std::future::Future; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -145,6 +145,8 @@ where requested_substreams: FuturesUnordered< SubstreamRequested, >, + + supported_protocols: Vec, } impl fmt::Debug for Connection @@ -182,6 +184,7 @@ where substream_upgrade_protocol_override, max_negotiating_inbound_streams, requested_substreams: Default::default(), + supported_protocols: vec![], } } @@ -211,6 +214,7 @@ where shutdown, max_negotiating_inbound_streams, substream_upgrade_protocol_override, + supported_protocols, } = self.get_mut(); loop { @@ -355,6 +359,23 @@ where Poll::Ready(substream) => { let protocol = handler.listen_protocol(); + let mut new_protocols = protocol + .upgrade() + .protocol_info() + .filter_map(|i| String::from_utf8(i.protocol_name().to_vec()).ok()) + .collect::>(); + + new_protocols.sort(); + + if supported_protocols != &new_protocols { + handler.on_connection_event(ConnectionEvent::ProtocolsChange( + ProtocolsChange { + protocols: &new_protocols, + }, + )); + *supported_protocols = new_protocols; + } + negotiating_in.push(SubstreamUpgrade::new_inbound(substream, protocol)); continue; // Go back to the top, handler can potentially make progress again. @@ -610,9 +631,10 @@ enum Shutdown { mod tests { use super::*; use crate::keep_alive; + use futures::future; use futures::AsyncRead; use futures::AsyncWrite; - use libp2p_core::upgrade::DeniedUpgrade; + use libp2p_core::upgrade::{DeniedUpgrade, InboundUpgrade, OutboundUpgrade, UpgradeInfo}; use libp2p_core::StreamMuxer; use quickcheck::*; use std::sync::{Arc, Weak}; @@ -673,6 +695,33 @@ mod tests { )) } + #[test] + fn propagates_changes_to_supported_inbound_protocols() { + let mut connection = Connection::new( + StreamMuxerBox::new(DummyStreamMuxer { + counter: Arc::new(()), + }), + ConfigurableProtocolConnectionHandler::default(), + None, + 2, + ); + connection.handler.active_protocols = vec!["/foo"]; + + // DummyStreamMuxer will yield a new stream + let _ = Pin::new(&mut connection) + .poll(&mut Context::from_waker(futures::task::noop_waker_ref())); + assert_eq!(connection.handler.reported_protocols, vec!["/foo"]); + + connection.handler.active_protocols = vec!["/foo", "/bar"]; + connection.negotiating_in.clear(); // Hack to request more substreams from the muxer. + + // DummyStreamMuxer will yield a new stream + let _ = Pin::new(&mut connection) + .poll(&mut Context::from_waker(futures::task::noop_waker_ref())); + + assert_eq!(connection.handler.reported_protocols, vec!["/bar", "/foo"]) + } + struct DummyStreamMuxer { counter: Arc<()>, } @@ -790,6 +839,12 @@ mod tests { } } + #[derive(Default)] + struct ConfigurableProtocolConnectionHandler { + active_protocols: Vec<&'static str>, + reported_protocols: Vec, + } + impl ConnectionHandler for MockConnectionHandler { type InEvent = Void; type OutEvent = Void; @@ -826,7 +881,9 @@ mod tests { ConnectionEvent::DialUpgradeError(DialUpgradeError { error, .. }) => { self.error = Some(error) } - ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) => {} + ConnectionEvent::AddressChange(_) + | ConnectionEvent::ListenUpgradeError(_) + | ConnectionEvent::ProtocolsChange(_) => {} } } @@ -860,6 +917,97 @@ mod tests { Poll::Pending } } + + impl ConnectionHandler for ConfigurableProtocolConnectionHandler { + type InEvent = Void; + type OutEvent = Void; + type Error = Void; + type InboundProtocol = ManyProtocolsUpgrade; + type OutboundProtocol = DeniedUpgrade; + type InboundOpenInfo = (); + type OutboundOpenInfo = (); + + fn listen_protocol( + &self, + ) -> SubstreamProtocol { + SubstreamProtocol::new( + ManyProtocolsUpgrade { + protocols: self.active_protocols.clone(), + }, + (), + ) + } + + fn on_connection_event( + &mut self, + event: ConnectionEvent< + Self::InboundProtocol, + Self::OutboundProtocol, + Self::InboundOpenInfo, + Self::OutboundOpenInfo, + >, + ) { + if let ConnectionEvent::ProtocolsChange(ProtocolsChange { protocols }) = event { + self.reported_protocols = protocols + .to_vec(); + } + } + + fn on_behaviour_event(&mut self, event: Self::InEvent) { + void::unreachable(event) + } + + fn connection_keep_alive(&self) -> KeepAlive { + KeepAlive::Yes + } + + fn poll( + &mut self, + _: &mut Context<'_>, + ) -> Poll< + ConnectionHandlerEvent< + Self::OutboundProtocol, + Self::OutboundOpenInfo, + Self::OutEvent, + Self::Error, + >, + > { + Poll::Pending + } + } + + struct ManyProtocolsUpgrade { + protocols: Vec<&'static str>, + } + + impl UpgradeInfo for ManyProtocolsUpgrade { + type Info = &'static str; + type InfoIter = std::vec::IntoIter; + + fn protocol_info(&self) -> Self::InfoIter { + self.protocols.clone().into_iter() + } + } + + impl InboundUpgrade for ManyProtocolsUpgrade { + type Output = C; + type Error = Void; + type Future = future::Ready>; + + fn upgrade_inbound(self, stream: C, _: Self::Info) -> Self::Future { + future::ready(Ok(stream)) + } + } + + impl OutboundUpgrade for ManyProtocolsUpgrade { + type Output = C; + type Error = Void; + type Future = future::Ready>; + + fn upgrade_outbound(self, stream: C, _: Self::Info) -> Self::Future { + future::ready(Ok(stream)) + } + } } /// The endpoint roles associated with a pending peer-to-peer connection. diff --git a/swarm/src/dummy.rs b/swarm/src/dummy.rs index 4497540a42b..bd1666da3ac 100644 --- a/swarm/src/dummy.rs +++ b/swarm/src/dummy.rs @@ -139,7 +139,9 @@ impl crate::handler::ConnectionHandler for ConnectionHandler { unreachable!("Denied upgrade does not support any protocols") } }, - ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) => {} + ConnectionEvent::AddressChange(_) + | ConnectionEvent::ListenUpgradeError(_) + | ConnectionEvent::ProtocolsChange(_) => {} } } } diff --git a/swarm/src/handler.rs b/swarm/src/handler.rs index 4093ecaee32..7ae92fa9d10 100644 --- a/swarm/src/handler.rs +++ b/swarm/src/handler.rs @@ -210,6 +210,8 @@ pub enum ConnectionEvent<'a, IP: InboundUpgradeSend, OP: OutboundUpgradeSend, IO DialUpgradeError(DialUpgradeError), /// Informs the handler that upgrading an inbound substream to the given protocol has failed. ListenUpgradeError(ListenUpgradeError), + /// The [`ConnectionHandler`] now supports a different set of protocols. + ProtocolsChange(ProtocolsChange<'a>), } /// [`ConnectionEvent`] variant that informs the handler about @@ -239,6 +241,12 @@ pub struct AddressChange<'a> { pub new_address: &'a Multiaddr, } +/// [`ConnectionEvent`] variant that informs the handler about a change in the address of the remote. +#[derive(Clone, Copy)] +pub struct ProtocolsChange<'a> { + pub protocols: &'a [String], +} + /// [`ConnectionEvent`] variant that informs the handler /// that upgrading an outbound substream to the given protocol has failed. pub struct DialUpgradeError { diff --git a/swarm/src/handler/either.rs b/swarm/src/handler/either.rs index 92d82371163..0b8c2f7f14e 100644 --- a/swarm/src/handler/either.rs +++ b/swarm/src/handler/either.rs @@ -322,6 +322,12 @@ where handler.on_connection_event(ConnectionEvent::AddressChange(address_change)) } }, + ConnectionEvent::ProtocolsChange(supported_protocols) => match self { + Either::Left(handler) => handler + .on_connection_event(ConnectionEvent::ProtocolsChange(supported_protocols)), + Either::Right(handler) => handler + .on_connection_event(ConnectionEvent::ProtocolsChange(supported_protocols)), + }, } } } diff --git a/swarm/src/handler/multi.rs b/swarm/src/handler/multi.rs index 146a2a96895..8cd6e4c916a 100644 --- a/swarm/src/handler/multi.rs +++ b/swarm/src/handler/multi.rs @@ -318,6 +318,11 @@ where ConnectionEvent::ListenUpgradeError(listen_upgrade_error) => { self.on_listen_upgrade_error(listen_upgrade_error) } + ConnectionEvent::ProtocolsChange(supported_protocols) => { + for h in self.handlers.values_mut() { + h.on_connection_event(ConnectionEvent::ProtocolsChange(supported_protocols)); + } + } } } diff --git a/swarm/src/handler/one_shot.rs b/swarm/src/handler/one_shot.rs index 29ba45ab678..bc46f05d4b5 100644 --- a/swarm/src/handler/one_shot.rs +++ b/swarm/src/handler/one_shot.rs @@ -217,7 +217,9 @@ where self.keep_alive = KeepAlive::No; } } - ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) => {} + ConnectionEvent::AddressChange(_) + | ConnectionEvent::ListenUpgradeError(_) + | ConnectionEvent::ProtocolsChange(_) => {} } } } diff --git a/swarm/src/handler/pending.rs b/swarm/src/handler/pending.rs index a39e498c3f2..843988c8fd3 100644 --- a/swarm/src/handler/pending.rs +++ b/swarm/src/handler/pending.rs @@ -99,7 +99,8 @@ impl ConnectionHandler for PendingConnectionHandler { } ConnectionEvent::AddressChange(_) | ConnectionEvent::DialUpgradeError(_) - | ConnectionEvent::ListenUpgradeError(_) => {} + | ConnectionEvent::ListenUpgradeError(_) + | ConnectionEvent::ProtocolsChange(_) => {} } } } diff --git a/swarm/src/handler/select.rs b/swarm/src/handler/select.rs index edb9a9154b1..c584b236d40 100644 --- a/swarm/src/handler/select.rs +++ b/swarm/src/handler/select.rs @@ -477,6 +477,12 @@ where ConnectionEvent::ListenUpgradeError(listen_upgrade_error) => { self.on_listen_upgrade_error(listen_upgrade_error) } + ConnectionEvent::ProtocolsChange(supported_protocols) => { + self.proto1 + .on_connection_event(ConnectionEvent::ProtocolsChange(supported_protocols)); + self.proto2 + .on_connection_event(ConnectionEvent::ProtocolsChange(supported_protocols)); + } } } } diff --git a/swarm/src/keep_alive.rs b/swarm/src/keep_alive.rs index c22a926afe4..366f2614f71 100644 --- a/swarm/src/keep_alive.rs +++ b/swarm/src/keep_alive.rs @@ -136,7 +136,8 @@ impl crate::handler::ConnectionHandler for ConnectionHandler { }) => void::unreachable(protocol), ConnectionEvent::DialUpgradeError(_) | ConnectionEvent::ListenUpgradeError(_) - | ConnectionEvent::AddressChange(_) => {} + | ConnectionEvent::AddressChange(_) + | ConnectionEvent::ProtocolsChange(_) => {} } } } From 9a28cd25fe96a2e2bc00bf88b513630a3a86626c Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Wed, 22 Mar 2023 22:38:34 +0100 Subject: [PATCH 02/50] Allow reporting of supported protocols by remote --- protocols/dcutr/src/handler/direct.rs | 3 +- protocols/dcutr/src/handler/relayed.rs | 4 ++- protocols/gossipsub/src/handler.rs | 3 +- protocols/identify/src/handler.rs | 3 +- protocols/kad/src/handler.rs | 21 ++++++++++++- protocols/perf/src/client/handler.rs | 3 +- protocols/perf/src/server/handler.rs | 4 ++- protocols/ping/src/handler.rs | 3 +- protocols/relay/src/behaviour/handler.rs | 4 ++- protocols/relay/src/priv_client/handler.rs | 4 ++- protocols/rendezvous/src/substream_handler.rs | 3 +- protocols/request-response/src/handler.rs | 4 ++- swarm/src/behaviour/toggle.rs | 5 ++- swarm/src/connection.rs | 18 ++++++++--- swarm/src/dummy.rs | 3 +- swarm/src/handler.rs | 31 ++++++++++++++++--- swarm/src/handler/either.rs | 20 +++++++++--- swarm/src/handler/map_out.rs | 3 ++ swarm/src/handler/multi.rs | 13 ++++++-- swarm/src/handler/one_shot.rs | 3 +- swarm/src/handler/pending.rs | 3 +- swarm/src/handler/select.rs | 26 ++++++++++++++-- swarm/src/keep_alive.rs | 3 +- 23 files changed, 151 insertions(+), 36 deletions(-) diff --git a/protocols/dcutr/src/handler/direct.rs b/protocols/dcutr/src/handler/direct.rs index 521a351fdf9..2cdb3f5a3fa 100644 --- a/protocols/dcutr/src/handler/direct.rs +++ b/protocols/dcutr/src/handler/direct.rs @@ -93,7 +93,8 @@ impl ConnectionHandler for Handler { | ConnectionEvent::DialUpgradeError(_) | ConnectionEvent::ListenUpgradeError(_) | ConnectionEvent::AddressChange(_) - | ConnectionEvent::ProtocolsChange(_) => {} + | ConnectionEvent::LocalProtocolsChange(_) + | ConnectionEvent::RemoteProtocolsChange(_) => {} } } } diff --git a/protocols/dcutr/src/handler/relayed.rs b/protocols/dcutr/src/handler/relayed.rs index a0ec68f7494..df84ee004de 100644 --- a/protocols/dcutr/src/handler/relayed.rs +++ b/protocols/dcutr/src/handler/relayed.rs @@ -419,7 +419,9 @@ impl ConnectionHandler for Handler { ConnectionEvent::DialUpgradeError(dial_upgrade_error) => { self.on_dial_upgrade_error(dial_upgrade_error) } - ConnectionEvent::AddressChange(_) | ConnectionEvent::ProtocolsChange(_) => {} + ConnectionEvent::AddressChange(_) + | ConnectionEvent::LocalProtocolsChange(_) + | ConnectionEvent::RemoteProtocolsChange(_) => {} } } } diff --git a/protocols/gossipsub/src/handler.rs b/protocols/gossipsub/src/handler.rs index fa131feecc6..f56e0072230 100644 --- a/protocols/gossipsub/src/handler.rs +++ b/protocols/gossipsub/src/handler.rs @@ -578,7 +578,8 @@ impl ConnectionHandler for Handler { } ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) - | ConnectionEvent::ProtocolsChange(_) => {} + | ConnectionEvent::LocalProtocolsChange(_) + | ConnectionEvent::RemoteProtocolsChange(_) => {} } } } diff --git a/protocols/identify/src/handler.rs b/protocols/identify/src/handler.rs index 653d9d058fc..3c39652c04c 100644 --- a/protocols/identify/src/handler.rs +++ b/protocols/identify/src/handler.rs @@ -346,7 +346,8 @@ impl ConnectionHandler for Handler { } ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) - | ConnectionEvent::ProtocolsChange(_) => {} + | ConnectionEvent::LocalProtocolsChange(_) => {} + ConnectionEvent::RemoteProtocolsChange(_) => {} } } } diff --git a/protocols/kad/src/handler.rs b/protocols/kad/src/handler.rs index 86e47ac9e27..e7979271a87 100644 --- a/protocols/kad/src/handler.rs +++ b/protocols/kad/src/handler.rs @@ -788,7 +788,26 @@ where } ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) - | ConnectionEvent::ProtocolsChange(_) => {} + | ConnectionEvent::LocalProtocolsChange(_) => {} + ConnectionEvent::RemoteProtocolsChange(ProtocolsChange { protocols }) => { + // TODO: We should cache this / it will get simpler with #2831. + let kademlia_protocols = self + .config + .protocol_config + .protocol_names() + .iter() + .filter_map(|b| String::from_utf8(b.to_vec()).ok()) + .collect::>(); + + let remote_supports_our_kademlia_protocols = + kademlia_protocols.iter().all(|p| protocols.contains(p)); + + if remote_supports_our_kademlia_protocols { + self.protocol_status = ProtocolStatus::Confirmed; + } else { + self.protocol_status = ProtocolStatus::NotSupported; + } + } } } } diff --git a/protocols/perf/src/client/handler.rs b/protocols/perf/src/client/handler.rs index 93f71ea48fb..96da11c2e24 100644 --- a/protocols/perf/src/client/handler.rs +++ b/protocols/perf/src/client/handler.rs @@ -127,7 +127,7 @@ impl ConnectionHandler for Handler { .boxed(), ), - ConnectionEvent::AddressChange(_) | ConnectionEvent::ProtocolsChange(_) => {} + ConnectionEvent::AddressChange(_) | ConnectionEvent::LocalProtocolsChange(_) => {} ConnectionEvent::DialUpgradeError(DialUpgradeError { info: Command { id, .. }, error, @@ -147,6 +147,7 @@ impl ConnectionHandler for Handler { }, } } + ConnectionEvent::RemoteProtocolsChange(_) => {} } } diff --git a/protocols/perf/src/server/handler.rs b/protocols/perf/src/server/handler.rs index 357f8aa1203..ffcf62ceff0 100644 --- a/protocols/perf/src/server/handler.rs +++ b/protocols/perf/src/server/handler.rs @@ -104,7 +104,9 @@ impl ConnectionHandler for Handler { ConnectionEvent::DialUpgradeError(DialUpgradeError { info, .. }) => { void::unreachable(info) } - ConnectionEvent::AddressChange(_) | ConnectionEvent::ProtocolsChange(_) => {} + ConnectionEvent::AddressChange(_) + | ConnectionEvent::LocalProtocolsChange(_) + | ConnectionEvent::RemoteProtocolsChange(_) => {} ConnectionEvent::ListenUpgradeError(ListenUpgradeError { info: (), error }) => { match error { ConnectionHandlerUpgrErr::Timeout => {} diff --git a/protocols/ping/src/handler.rs b/protocols/ping/src/handler.rs index 4c063f1912d..7085bb36b68 100644 --- a/protocols/ping/src/handler.rs +++ b/protocols/ping/src/handler.rs @@ -413,7 +413,8 @@ impl ConnectionHandler for Handler { } ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) - | ConnectionEvent::ProtocolsChange(_) => {} + | ConnectionEvent::LocalProtocolsChange(_) + | ConnectionEvent::RemoteProtocolsChange(_) => {} } } } diff --git a/protocols/relay/src/behaviour/handler.rs b/protocols/relay/src/behaviour/handler.rs index 0608d2cae3d..bc9ee109ec4 100644 --- a/protocols/relay/src/behaviour/handler.rs +++ b/protocols/relay/src/behaviour/handler.rs @@ -954,7 +954,9 @@ impl ConnectionHandler for Handler { ConnectionEvent::DialUpgradeError(dial_upgrade_error) => { self.on_dial_upgrade_error(dial_upgrade_error) } - ConnectionEvent::AddressChange(_) | ConnectionEvent::ProtocolsChange(_) => {} + ConnectionEvent::AddressChange(_) + | ConnectionEvent::LocalProtocolsChange(_) + | ConnectionEvent::RemoteProtocolsChange(_) => {} } } } diff --git a/protocols/relay/src/priv_client/handler.rs b/protocols/relay/src/priv_client/handler.rs index c736d219865..389e38733be 100644 --- a/protocols/relay/src/priv_client/handler.rs +++ b/protocols/relay/src/priv_client/handler.rs @@ -613,7 +613,9 @@ impl ConnectionHandler for Handler { ConnectionEvent::DialUpgradeError(dial_upgrade_error) => { self.on_dial_upgrade_error(dial_upgrade_error) } - ConnectionEvent::AddressChange(_) | ConnectionEvent::ProtocolsChange(_) => {} + ConnectionEvent::AddressChange(_) + | ConnectionEvent::LocalProtocolsChange(_) + | ConnectionEvent::RemoteProtocolsChange(_) => {} } } } diff --git a/protocols/rendezvous/src/substream_handler.rs b/protocols/rendezvous/src/substream_handler.rs index 57946e7d237..f954ac685b2 100644 --- a/protocols/rendezvous/src/substream_handler.rs +++ b/protocols/rendezvous/src/substream_handler.rs @@ -397,7 +397,8 @@ where ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) | ConnectionEvent::DialUpgradeError(_) - | ConnectionEvent::ProtocolsChange(_) => {} + | ConnectionEvent::LocalProtocolsChange(_) + | ConnectionEvent::RemoteProtocolsChange(_) => {} } } diff --git a/protocols/request-response/src/handler.rs b/protocols/request-response/src/handler.rs index 8c8290bd79a..4ba4f437f05 100644 --- a/protocols/request-response/src/handler.rs +++ b/protocols/request-response/src/handler.rs @@ -425,7 +425,9 @@ where ConnectionEvent::ListenUpgradeError(listen_upgrade_error) => { self.on_listen_upgrade_error(listen_upgrade_error) } - ConnectionEvent::AddressChange(_) | ConnectionEvent::ProtocolsChange(_) => {} + ConnectionEvent::AddressChange(_) + | ConnectionEvent::LocalProtocolsChange(_) + | ConnectionEvent::RemoteProtocolsChange(_) => {} } } } diff --git a/swarm/src/behaviour/toggle.rs b/swarm/src/behaviour/toggle.rs index e1b5a2a0688..b6533411250 100644 --- a/swarm/src/behaviour/toggle.rs +++ b/swarm/src/behaviour/toggle.rs @@ -368,7 +368,10 @@ where ConnectionEvent::ListenUpgradeError(listen_upgrade_error) => { self.on_listen_upgrade_error(listen_upgrade_error) } - ConnectionEvent::ProtocolsChange(_) => { + ConnectionEvent::LocalProtocolsChange(_) => { + todo!() + } + ConnectionEvent::RemoteProtocolsChange(_) => { todo!() } } diff --git a/swarm/src/connection.rs b/swarm/src/connection.rs index 5f9f68b054d..089768f46ea 100644 --- a/swarm/src/connection.rs +++ b/swarm/src/connection.rs @@ -248,6 +248,14 @@ where Poll::Ready(ConnectionHandlerEvent::Close(err)) => { return Poll::Ready(Err(ConnectionError::Handler(err))); } + Poll::Ready(ConnectionHandlerEvent::ReportRemoteProtocols { protocols }) => { + handler.on_connection_event(ConnectionEvent::RemoteProtocolsChange( + ProtocolsChange { + protocols: &protocols, + }, + )); + continue; + } } // In case the [`ConnectionHandler`] can not make any more progress, poll the negotiating outbound streams. @@ -368,7 +376,7 @@ where new_protocols.sort(); if supported_protocols != &new_protocols { - handler.on_connection_event(ConnectionEvent::ProtocolsChange( + handler.on_connection_event(ConnectionEvent::LocalProtocolsChange( ProtocolsChange { protocols: &new_protocols, }, @@ -883,7 +891,8 @@ mod tests { } ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) - | ConnectionEvent::ProtocolsChange(_) => {} + | ConnectionEvent::LocalProtocolsChange(_) + | ConnectionEvent::RemoteProtocolsChange(_) => {} } } @@ -947,9 +956,8 @@ mod tests { Self::OutboundOpenInfo, >, ) { - if let ConnectionEvent::ProtocolsChange(ProtocolsChange { protocols }) = event { - self.reported_protocols = protocols - .to_vec(); + if let ConnectionEvent::LocalProtocolsChange(ProtocolsChange { protocols }) = event { + self.reported_protocols = protocols.to_vec(); } } diff --git a/swarm/src/dummy.rs b/swarm/src/dummy.rs index bd1666da3ac..f476a7d5013 100644 --- a/swarm/src/dummy.rs +++ b/swarm/src/dummy.rs @@ -141,7 +141,8 @@ impl crate::handler::ConnectionHandler for ConnectionHandler { }, ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) - | ConnectionEvent::ProtocolsChange(_) => {} + | ConnectionEvent::LocalProtocolsChange(_) + | ConnectionEvent::RemoteProtocolsChange(_) => {} } } } diff --git a/swarm/src/handler.rs b/swarm/src/handler.rs index 7ae92fa9d10..0dcc819a69a 100644 --- a/swarm/src/handler.rs +++ b/swarm/src/handler.rs @@ -210,8 +210,10 @@ pub enum ConnectionEvent<'a, IP: InboundUpgradeSend, OP: OutboundUpgradeSend, IO DialUpgradeError(DialUpgradeError), /// Informs the handler that upgrading an inbound substream to the given protocol has failed. ListenUpgradeError(ListenUpgradeError), - /// The [`ConnectionHandler`] now supports a different set of protocols. - ProtocolsChange(ProtocolsChange<'a>), + /// The local [`ConnectionHandler`] now supports a different set of protocols. + LocalProtocolsChange(ProtocolsChange<'a>), + /// The remote [`ConnectionHandler`] now supports a different set of protocols. + RemoteProtocolsChange(ProtocolsChange<'a>), } /// [`ConnectionEvent`] variant that informs the handler about @@ -241,7 +243,7 @@ pub struct AddressChange<'a> { pub new_address: &'a Multiaddr, } -/// [`ConnectionEvent`] variant that informs the handler about a change in the address of the remote. +/// [`ConnectionEvent`] variant that informs the handler about a change in the protocols supported on the connection. #[derive(Clone, Copy)] pub struct ProtocolsChange<'a> { pub protocols: &'a [String], @@ -338,7 +340,7 @@ impl SubstreamProtocol { } /// Event produced by a handler. -#[derive(Debug, Copy, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub enum ConnectionHandlerEvent { /// Request a new outbound substream to be opened with the remote. OutboundSubstreamRequest { @@ -356,6 +358,15 @@ pub enum ConnectionHandlerEvent }, + /// Other event. Custom(TCustom), } @@ -381,6 +392,9 @@ impl } ConnectionHandlerEvent::Custom(val) => ConnectionHandlerEvent::Custom(val), ConnectionHandlerEvent::Close(val) => ConnectionHandlerEvent::Close(val), + ConnectionHandlerEvent::ReportRemoteProtocols { protocols } => { + ConnectionHandlerEvent::ReportRemoteProtocols { protocols } + } } } @@ -401,6 +415,9 @@ impl } ConnectionHandlerEvent::Custom(val) => ConnectionHandlerEvent::Custom(val), ConnectionHandlerEvent::Close(val) => ConnectionHandlerEvent::Close(val), + ConnectionHandlerEvent::ReportRemoteProtocols { protocols } => { + ConnectionHandlerEvent::ReportRemoteProtocols { protocols } + } } } @@ -418,6 +435,9 @@ impl } ConnectionHandlerEvent::Custom(val) => ConnectionHandlerEvent::Custom(map(val)), ConnectionHandlerEvent::Close(val) => ConnectionHandlerEvent::Close(val), + ConnectionHandlerEvent::ReportRemoteProtocols { protocols } => { + ConnectionHandlerEvent::ReportRemoteProtocols { protocols } + } } } @@ -435,6 +455,9 @@ impl } ConnectionHandlerEvent::Custom(val) => ConnectionHandlerEvent::Custom(val), ConnectionHandlerEvent::Close(val) => ConnectionHandlerEvent::Close(map(val)), + ConnectionHandlerEvent::ReportRemoteProtocols { protocols } => { + ConnectionHandlerEvent::ReportRemoteProtocols { protocols } + } } } } diff --git a/swarm/src/handler/either.rs b/swarm/src/handler/either.rs index 0b8c2f7f14e..50c16628e19 100644 --- a/swarm/src/handler/either.rs +++ b/swarm/src/handler/either.rs @@ -322,11 +322,21 @@ where handler.on_connection_event(ConnectionEvent::AddressChange(address_change)) } }, - ConnectionEvent::ProtocolsChange(supported_protocols) => match self { - Either::Left(handler) => handler - .on_connection_event(ConnectionEvent::ProtocolsChange(supported_protocols)), - Either::Right(handler) => handler - .on_connection_event(ConnectionEvent::ProtocolsChange(supported_protocols)), + ConnectionEvent::LocalProtocolsChange(supported_protocols) => match self { + Either::Left(handler) => handler.on_connection_event( + ConnectionEvent::LocalProtocolsChange(supported_protocols), + ), + Either::Right(handler) => handler.on_connection_event( + ConnectionEvent::LocalProtocolsChange(supported_protocols), + ), + }, + ConnectionEvent::RemoteProtocolsChange(supported_protocols) => match self { + Either::Left(handler) => handler.on_connection_event( + ConnectionEvent::RemoteProtocolsChange(supported_protocols), + ), + Either::Right(handler) => handler.on_connection_event( + ConnectionEvent::RemoteProtocolsChange(supported_protocols), + ), }, } } diff --git a/swarm/src/handler/map_out.rs b/swarm/src/handler/map_out.rs index 773df2b6681..3c3f3827dfa 100644 --- a/swarm/src/handler/map_out.rs +++ b/swarm/src/handler/map_out.rs @@ -81,6 +81,9 @@ where ConnectionHandlerEvent::OutboundSubstreamRequest { protocol } => { ConnectionHandlerEvent::OutboundSubstreamRequest { protocol } } + ConnectionHandlerEvent::ReportRemoteProtocols { protocols } => { + ConnectionHandlerEvent::ReportRemoteProtocols { protocols } + } }) } diff --git a/swarm/src/handler/multi.rs b/swarm/src/handler/multi.rs index 8cd6e4c916a..16b11977f6d 100644 --- a/swarm/src/handler/multi.rs +++ b/swarm/src/handler/multi.rs @@ -318,9 +318,18 @@ where ConnectionEvent::ListenUpgradeError(listen_upgrade_error) => { self.on_listen_upgrade_error(listen_upgrade_error) } - ConnectionEvent::ProtocolsChange(supported_protocols) => { + ConnectionEvent::LocalProtocolsChange(supported_protocols) => { for h in self.handlers.values_mut() { - h.on_connection_event(ConnectionEvent::ProtocolsChange(supported_protocols)); + h.on_connection_event(ConnectionEvent::LocalProtocolsChange( + supported_protocols, + )); + } + } + ConnectionEvent::RemoteProtocolsChange(supported_protocols) => { + for h in self.handlers.values_mut() { + h.on_connection_event(ConnectionEvent::RemoteProtocolsChange( + supported_protocols, + )); } } } diff --git a/swarm/src/handler/one_shot.rs b/swarm/src/handler/one_shot.rs index bc46f05d4b5..01e20e85a87 100644 --- a/swarm/src/handler/one_shot.rs +++ b/swarm/src/handler/one_shot.rs @@ -219,7 +219,8 @@ where } ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) - | ConnectionEvent::ProtocolsChange(_) => {} + | ConnectionEvent::LocalProtocolsChange(_) + | ConnectionEvent::RemoteProtocolsChange(_) => {} } } } diff --git a/swarm/src/handler/pending.rs b/swarm/src/handler/pending.rs index 843988c8fd3..7cf8b9209fa 100644 --- a/swarm/src/handler/pending.rs +++ b/swarm/src/handler/pending.rs @@ -100,7 +100,8 @@ impl ConnectionHandler for PendingConnectionHandler { ConnectionEvent::AddressChange(_) | ConnectionEvent::DialUpgradeError(_) | ConnectionEvent::ListenUpgradeError(_) - | ConnectionEvent::ProtocolsChange(_) => {} + | ConnectionEvent::LocalProtocolsChange(_) + | ConnectionEvent::RemoteProtocolsChange(_) => {} } } } diff --git a/swarm/src/handler/select.rs b/swarm/src/handler/select.rs index c584b236d40..2389bc933d6 100644 --- a/swarm/src/handler/select.rs +++ b/swarm/src/handler/select.rs @@ -400,6 +400,9 @@ where .map_info(Either::Left), }); } + Poll::Ready(ConnectionHandlerEvent::ReportRemoteProtocols { protocols }) => { + return Poll::Ready(ConnectionHandlerEvent::ReportRemoteProtocols { protocols }); + } Poll::Pending => (), }; @@ -417,6 +420,9 @@ where .map_info(Either::Right), }); } + Poll::Ready(ConnectionHandlerEvent::ReportRemoteProtocols { protocols }) => { + return Poll::Ready(ConnectionHandlerEvent::ReportRemoteProtocols { protocols }); + } Poll::Pending => (), }; @@ -477,11 +483,25 @@ where ConnectionEvent::ListenUpgradeError(listen_upgrade_error) => { self.on_listen_upgrade_error(listen_upgrade_error) } - ConnectionEvent::ProtocolsChange(supported_protocols) => { + ConnectionEvent::LocalProtocolsChange(supported_protocols) => { + self.proto1 + .on_connection_event(ConnectionEvent::LocalProtocolsChange( + supported_protocols, + )); + self.proto2 + .on_connection_event(ConnectionEvent::LocalProtocolsChange( + supported_protocols, + )); + } + ConnectionEvent::RemoteProtocolsChange(supported_protocols) => { self.proto1 - .on_connection_event(ConnectionEvent::ProtocolsChange(supported_protocols)); + .on_connection_event(ConnectionEvent::RemoteProtocolsChange( + supported_protocols, + )); self.proto2 - .on_connection_event(ConnectionEvent::ProtocolsChange(supported_protocols)); + .on_connection_event(ConnectionEvent::RemoteProtocolsChange( + supported_protocols, + )); } } } diff --git a/swarm/src/keep_alive.rs b/swarm/src/keep_alive.rs index 366f2614f71..aa4da2db826 100644 --- a/swarm/src/keep_alive.rs +++ b/swarm/src/keep_alive.rs @@ -137,7 +137,8 @@ impl crate::handler::ConnectionHandler for ConnectionHandler { ConnectionEvent::DialUpgradeError(_) | ConnectionEvent::ListenUpgradeError(_) | ConnectionEvent::AddressChange(_) - | ConnectionEvent::ProtocolsChange(_) => {} + | ConnectionEvent::LocalProtocolsChange(_) + | ConnectionEvent::RemoteProtocolsChange(_) => {} } } } From e536deaded5235755422cc09990795e5d528e86d Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 24 Mar 2023 15:41:16 +0100 Subject: [PATCH 03/50] Consume supported protocols in identify --- protocols/identify/src/behaviour.rs | 13 +------------ protocols/identify/src/handler.rs | 17 +++++++++-------- 2 files changed, 10 insertions(+), 20 deletions(-) diff --git a/protocols/identify/src/behaviour.rs b/protocols/identify/src/behaviour.rs index 8ba50e2db4b..c5cbc12e8c6 100644 --- a/protocols/identify/src/behaviour.rs +++ b/protocols/identify/src/behaviour.rs @@ -323,7 +323,7 @@ impl NetworkBehaviour for Behaviour { fn poll( &mut self, _cx: &mut Context<'_>, - params: &mut impl PollParameters, + _: &mut impl PollParameters, ) -> Poll>> { if let Some(event) = self.events.pop_front() { return Poll::Ready(event); @@ -344,7 +344,6 @@ impl NetworkBehaviour for Behaviour { .chain(self.external_addresses.iter()) .cloned() .collect(), - supported_protocols: supported_protocols(params), protocol: Protocol::Push, }, }), @@ -361,7 +360,6 @@ impl NetworkBehaviour for Behaviour { .chain(self.external_addresses.iter()) .cloned() .collect(), - supported_protocols: supported_protocols(params), protocol: Protocol::Identify(connection_id), }, }), @@ -488,15 +486,6 @@ pub enum Event { }, } -fn supported_protocols(params: &impl PollParameters) -> Vec { - // The protocol names can be bytes, but the identify protocol except UTF-8 strings. - // There's not much we can do to solve this conflict except strip non-UTF-8 characters. - params - .supported_protocols() - .map(|p| String::from_utf8_lossy(&p).to_string()) - .collect() -} - /// If there is a given peer_id in the multiaddr, make sure it is the same as /// the given peer_id. If there is no peer_id for the peer in the mutiaddr, this returns true. fn multiaddr_matches_peer_id(addr: &Multiaddr, peer_id: &PeerId) -> bool { diff --git a/protocols/identify/src/handler.rs b/protocols/identify/src/handler.rs index 3c39652c04c..45092b06e5a 100644 --- a/protocols/identify/src/handler.rs +++ b/protocols/identify/src/handler.rs @@ -32,6 +32,7 @@ use libp2p_identity::PeerId; use libp2p_identity::PublicKey; use libp2p_swarm::handler::{ ConnectionEvent, DialUpgradeError, FullyNegotiatedInbound, FullyNegotiatedOutbound, + ProtocolsChange, }; use libp2p_swarm::{ ConnectionHandler, ConnectionHandlerEvent, ConnectionHandlerUpgrErr, KeepAlive, @@ -83,6 +84,8 @@ pub struct Handler { /// Address observed by or for the remote. observed_addr: Multiaddr, + + local_supported_protocols: Vec, } /// An event from `Behaviour` with the information requested by the `Handler`. @@ -91,9 +94,6 @@ pub struct InEvent { /// The addresses that the peer is listening on. pub listen_addrs: Vec, - /// The list of protocols supported by the peer, e.g. `/ipfs/ping/1.0.0`. - pub supported_protocols: Vec, - /// The protocol w.r.t. the information requested. pub protocol: Protocol, } @@ -138,6 +138,7 @@ impl Handler { protocol_version, agent_version, observed_addr, + local_supported_protocols: vec![], } } @@ -238,7 +239,6 @@ impl ConnectionHandler for Handler { &mut self, InEvent { listen_addrs, - supported_protocols, protocol, }: Self::InEvent, ) { @@ -247,7 +247,7 @@ impl ConnectionHandler for Handler { protocol_version: self.protocol_version.clone(), agent_version: self.agent_version.clone(), listen_addrs, - protocols: supported_protocols, + protocols: self.local_supported_protocols.clone(), observed_addr: self.observed_addr.clone(), }; @@ -344,9 +344,10 @@ impl ConnectionHandler for Handler { ConnectionEvent::DialUpgradeError(dial_upgrade_error) => { self.on_dial_upgrade_error(dial_upgrade_error) } - ConnectionEvent::AddressChange(_) - | ConnectionEvent::ListenUpgradeError(_) - | ConnectionEvent::LocalProtocolsChange(_) => {} + ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) => {} + ConnectionEvent::LocalProtocolsChange(ProtocolsChange { protocols }) => { + self.local_supported_protocols = protocols.to_vec(); + } ConnectionEvent::RemoteProtocolsChange(_) => {} } } From 7699a1e2edf66c7064078cecdde8d7c8df4be502 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 24 Mar 2023 15:50:26 +0100 Subject: [PATCH 04/50] Report a remote's protocols to other handlers --- protocols/identify/src/handler.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/protocols/identify/src/handler.rs b/protocols/identify/src/handler.rs index 45092b06e5a..bae3a35b8fa 100644 --- a/protocols/identify/src/handler.rs +++ b/protocols/identify/src/handler.rs @@ -187,6 +187,10 @@ impl Handler { ) { match output { future::Either::Left(remote_info) => { + self.events + .push(ConnectionHandlerEvent::ReportRemoteProtocols { + protocols: remote_info.protocols.clone(), + }); self.events .push(ConnectionHandlerEvent::Custom(Event::Identified( remote_info, @@ -307,6 +311,10 @@ impl ConnectionHandler for Handler { self.inbound_identify_push.take(); if let Ok(info) = res { + self.events + .push(ConnectionHandlerEvent::ReportRemoteProtocols { + protocols: info.protocols.clone(), + }); return Poll::Ready(ConnectionHandlerEvent::Custom(Event::Identified(info))); } } From 450fc1ec69dd45ff525f19c68d9b7fb59d45f03e Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 24 Mar 2023 16:46:40 +0100 Subject: [PATCH 05/50] Add test for kademlia client mode --- Cargo.lock | 3 ++ protocols/kad/Cargo.toml | 4 ++ protocols/kad/src/behaviour.rs | 22 ++++++++++- protocols/kad/src/handler.rs | 20 +--------- protocols/kad/src/lib.rs | 2 +- protocols/kad/tests/client_mode.rs | 59 ++++++++++++++++++++++++++++++ 6 files changed, 88 insertions(+), 22 deletions(-) create mode 100644 protocols/kad/tests/client_mode.rs diff --git a/Cargo.lock b/Cargo.lock index cef78ac2a4d..ae797e51433 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2464,6 +2464,7 @@ name = "libp2p-kad" version = "0.43.1" dependencies = [ "arrayvec", + "async-std", "asynchronous-codec", "bytes", "either", @@ -2473,9 +2474,11 @@ dependencies = [ "futures-timer", "instant", "libp2p-core", + "libp2p-identify", "libp2p-identity", "libp2p-noise", "libp2p-swarm", + "libp2p-swarm-test", "libp2p-yamux", "log", "quick-protobuf", diff --git a/protocols/kad/Cargo.toml b/protocols/kad/Cargo.toml index aa76253fe88..1b58ba2a0a7 100644 --- a/protocols/kad/Cargo.toml +++ b/protocols/kad/Cargo.toml @@ -34,9 +34,13 @@ serde = { version = "1.0", optional = true, features = ["derive"] } thiserror = "1" [dev-dependencies] +async-std = { version = "1.12.0", features = ["attributes"] } env_logger = "0.10.0" futures-timer = "3.0" +libp2p-identify = { path = "../identify" } libp2p-noise = { path = "../../transports/noise" } +libp2p-swarm = { path = "../../swarm", features = ["macros"] } +libp2p-swarm-test = { path = "../../swarm-test" } libp2p-yamux = { path = "../../muxers/yamux" } quickcheck = { package = "quickcheck-ext", path = "../../misc/quickcheck-ext" } diff --git a/protocols/kad/src/behaviour.rs b/protocols/kad/src/behaviour.rs index 8d1ee716973..42ce824a5b5 100644 --- a/protocols/kad/src/behaviour.rs +++ b/protocols/kad/src/behaviour.rs @@ -116,6 +116,7 @@ pub struct Kademlia { /// The record storage. store: TStore, + mode: Mode, } /// The configurable strategies for the insertion of peers @@ -182,6 +183,7 @@ pub struct KademliaConfig { connection_idle_timeout: Duration, kbucket_inserts: KademliaBucketInserts, caching: KademliaCaching, + mode: Mode, } impl Default for KademliaConfig { @@ -199,6 +201,7 @@ impl Default for KademliaConfig { connection_idle_timeout: Duration::from_secs(10), kbucket_inserts: KademliaBucketInserts::OnConnected, caching: KademliaCaching::Enabled { max_peers: 1 }, + mode: Mode::Server, } } } @@ -399,6 +402,14 @@ impl KademliaConfig { self.caching = c; self } + + /// Sets the mode. + /// + /// TODO: More docs. + pub fn set_mode(&mut self, m: Mode) -> &mut Self { + self.mode = m; + self + } } impl Kademlia @@ -453,6 +464,7 @@ where connection_idle_timeout: config.connection_idle_timeout, external_addresses: Default::default(), local_peer_id: id, + mode: config.mode, } } @@ -1976,7 +1988,7 @@ where Ok(KademliaHandler::new( KademliaHandlerConfig { protocol_config: self.protocol_config.clone(), - allow_listening: true, + allow_listening: self.mode == Mode::Server, idle_timeout: self.connection_idle_timeout, }, ConnectedPoint::Listener { @@ -1997,7 +2009,7 @@ where Ok(KademliaHandler::new( KademliaHandlerConfig { protocol_config: self.protocol_config.clone(), - allow_listening: true, + allow_listening: self.mode == Mode::Server, idle_timeout: self.connection_idle_timeout, }, ConnectedPoint::Dialer { @@ -3190,3 +3202,9 @@ pub enum RoutingUpdate { /// peer ID). Failed, } + +#[derive(Debug, Clone, Copy, PartialEq)] +pub enum Mode { + Client, + Server, +} diff --git a/protocols/kad/src/handler.rs b/protocols/kad/src/handler.rs index e7979271a87..20b45dab4a7 100644 --- a/protocols/kad/src/handler.rs +++ b/protocols/kad/src/handler.rs @@ -789,25 +789,7 @@ where ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) | ConnectionEvent::LocalProtocolsChange(_) => {} - ConnectionEvent::RemoteProtocolsChange(ProtocolsChange { protocols }) => { - // TODO: We should cache this / it will get simpler with #2831. - let kademlia_protocols = self - .config - .protocol_config - .protocol_names() - .iter() - .filter_map(|b| String::from_utf8(b.to_vec()).ok()) - .collect::>(); - - let remote_supports_our_kademlia_protocols = - kademlia_protocols.iter().all(|p| protocols.contains(p)); - - if remote_supports_our_kademlia_protocols { - self.protocol_status = ProtocolStatus::Confirmed; - } else { - self.protocol_status = ProtocolStatus::NotSupported; - } - } + ConnectionEvent::RemoteProtocolsChange(_) => {} } } } diff --git a/protocols/kad/src/lib.rs b/protocols/kad/src/lib.rs index 3e9dcecc0e4..1480609ccd0 100644 --- a/protocols/kad/src/lib.rs +++ b/protocols/kad/src/lib.rs @@ -61,7 +61,7 @@ pub use behaviour::{ AddProviderContext, AddProviderError, AddProviderOk, AddProviderPhase, AddProviderResult, BootstrapError, BootstrapOk, BootstrapResult, GetClosestPeersError, GetClosestPeersOk, GetClosestPeersResult, GetProvidersError, GetProvidersOk, GetProvidersResult, GetRecordError, - GetRecordOk, GetRecordResult, InboundRequest, NoKnownPeers, PeerRecord, PutRecordContext, + GetRecordOk, GetRecordResult, InboundRequest, Mode, NoKnownPeers, PeerRecord, PutRecordContext, PutRecordError, PutRecordOk, PutRecordPhase, PutRecordResult, QueryInfo, QueryMut, QueryRef, QueryResult, QueryStats, }; diff --git a/protocols/kad/tests/client_mode.rs b/protocols/kad/tests/client_mode.rs new file mode 100644 index 00000000000..9c4efc1199f --- /dev/null +++ b/protocols/kad/tests/client_mode.rs @@ -0,0 +1,59 @@ +use libp2p_identify as identify; +use libp2p_identity as identity; +use libp2p_kad::store::MemoryStore; +use libp2p_kad::{Kademlia, KademliaConfig, KademliaEvent, Mode}; +use libp2p_swarm::Swarm; +use libp2p_swarm_test::SwarmExt; + +#[async_std::test] +async fn connection_to_node_in_client_mode_does_not_update_routing_table() { + let mut client = Swarm::new_ephemeral(MyBehaviour::client); + let mut server = Swarm::new_ephemeral(MyBehaviour::server); + + server.listen().await; + client.connect(&mut server).await; + + let server_peer_id = *server.local_peer_id(); + + match libp2p_swarm_test::drive(&mut client, &mut server).await { + ( + [MyBehaviourEvent::Identify(_), MyBehaviourEvent::Identify(_), MyBehaviourEvent::Kad(KademliaEvent::RoutingUpdated { peer, .. })], + [MyBehaviourEvent::Identify(_), MyBehaviourEvent::Identify(_)], + ) => { + assert_eq!(peer, server_peer_id) + } + other => panic!("Unexpected events: {other:?}"), + } +} + +#[derive(libp2p_swarm::NetworkBehaviour)] +#[behaviour(prelude = "libp2p_swarm::derive_prelude")] +struct MyBehaviour { + identify: identify::Behaviour, + kad: Kademlia, +} + +impl MyBehaviour { + fn client(k: identity::Keypair) -> Self { + let mut config = KademliaConfig::default(); + config.set_mode(Mode::Client); + + Self::with_config(k, config) + } + + fn server(k: identity::Keypair) -> Self { + Self::with_config(k, KademliaConfig::default()) + } + + fn with_config(k: identity::Keypair, config: KademliaConfig) -> MyBehaviour { + let local_peer_id = k.public().to_peer_id(); + + Self { + identify: identify::Behaviour::new(identify::Config::new( + "/test/1.0.0".to_owned(), + k.public(), + )), + kad: Kademlia::with_config(local_peer_id, MemoryStore::new(local_peer_id), config), + } + } +} From a972b9ccaf246881febc3c1940fcd25e26845b27 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 24 Mar 2023 16:09:29 +0100 Subject: [PATCH 06/50] Implement kademlia client-mode --- protocols/kad/src/behaviour.rs | 8 +++++++ protocols/kad/src/handler.rs | 41 +++++++++++++++++++++++++--------- 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/protocols/kad/src/behaviour.rs b/protocols/kad/src/behaviour.rs index 42ce824a5b5..ad2b30b7a55 100644 --- a/protocols/kad/src/behaviour.rs +++ b/protocols/kad/src/behaviour.rs @@ -2074,6 +2074,14 @@ where self.connection_updated(source, address, NodeStatus::Connected); } + KademliaHandlerEvent::ProtocolNotSupported { endpoint } => { + let address = match endpoint { + ConnectedPoint::Dialer { address, .. } => Some(address), + ConnectedPoint::Listener { .. } => None, + }; + self.connection_updated(source, address, NodeStatus::Disconnected); + } + KademliaHandlerEvent::FindNodeReq { key, request_id } => { let closer_peers = self.find_closest(&kbucket::Key::new(key), &source); diff --git a/protocols/kad/src/handler.rs b/protocols/kad/src/handler.rs index 20b45dab4a7..5f660088531 100644 --- a/protocols/kad/src/handler.rs +++ b/protocols/kad/src/handler.rs @@ -31,6 +31,7 @@ use libp2p_core::{upgrade, ConnectedPoint}; use libp2p_identity::PeerId; use libp2p_swarm::handler::{ ConnectionEvent, DialUpgradeError, FullyNegotiatedInbound, FullyNegotiatedOutbound, + ProtocolsChange, }; use libp2p_swarm::{ ConnectionHandler, ConnectionHandlerEvent, ConnectionHandlerUpgrErr, KeepAlive, @@ -92,10 +93,12 @@ pub struct KademliaHandler { enum ProtocolStatus { /// It is as yet unknown whether the remote supports the /// configured protocol name. - Unconfirmed, + Unknown, /// The configured protocol name has been confirmed by the remote /// but has not yet been reported to the `Kademlia` behaviour. Confirmed, + /// The configured protocol name(s) are not or no longer supported by the remote. + NotSupported, /// The configured protocol has been confirmed by the remote /// and the confirmation reported to the `Kademlia` behaviour. Reported, @@ -226,13 +229,11 @@ impl InboundSubstreamState { #[derive(Debug)] pub enum KademliaHandlerEvent { /// The configured protocol name has been confirmed by the peer through - /// a successfully negotiated substream. - /// - /// This event is only emitted once by a handler upon the first - /// successfully negotiated inbound or outbound substream and - /// indicates that the connected peer participates in the Kademlia - /// overlay network identified by the configured protocol name. + /// a successfully negotiated substream or by learning the supported protocols of the remote. ProtocolConfirmed { endpoint: ConnectedPoint }, + /// The configured protocol name(s) are not or no longer supported by the peer on the provided + /// connection and it should be removed from the routing table. + ProtocolNotSupported { endpoint: ConnectedPoint }, /// Request for the list of nodes whose IDs are the closest to `key`. The number of nodes /// returned is not specified, but should be around 20. @@ -501,7 +502,7 @@ where num_requested_outbound_streams: 0, requested_streams: Default::default(), keep_alive, - protocol_status: ProtocolStatus::Unconfirmed, + protocol_status: ProtocolStatus::Unknown, } } @@ -520,7 +521,7 @@ where protocol, msg, user_data, )); self.num_requested_outbound_streams -= 1; - if let ProtocolStatus::Unconfirmed = self.protocol_status { + if let ProtocolStatus::Unknown = self.protocol_status { // Upon the first successfully negotiated substream, we know that the // remote is configured with the same protocol name and we want // the behaviour to add this peer to the routing table, if possible. @@ -542,7 +543,7 @@ where future::Either::Right(p) => void::unreachable(p), }; - if let ProtocolStatus::Unconfirmed = self.protocol_status { + if let ProtocolStatus::Unknown = self.protocol_status { // Upon the first successfully negotiated substream, we know that the // remote is configured with the same protocol name and we want // the behaviour to add this peer to the routing table, if possible. @@ -789,7 +790,25 @@ where ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) | ConnectionEvent::LocalProtocolsChange(_) => {} - ConnectionEvent::RemoteProtocolsChange(_) => {} + ConnectionEvent::RemoteProtocolsChange(ProtocolsChange { protocols }) => { + // TODO: We should cache this / it will get simpler with #2831. + let kademlia_protocols = self + .config + .protocol_config + .protocol_names() + .iter() + .filter_map(|b| String::from_utf8(b.to_vec()).ok()) + .collect::>(); + + let remote_supports_our_kademlia_protocols = + kademlia_protocols.iter().all(|p| protocols.contains(p)); + + if remote_supports_our_kademlia_protocols { + self.protocol_status = ProtocolStatus::Confirmed; + } else { + self.protocol_status = ProtocolStatus::NotSupported; + } + } } } } From f9cf33f99279680a9598391c3c8896376bcc1386 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 24 Mar 2023 16:55:35 +0100 Subject: [PATCH 07/50] Add tests for two servers connecting --- protocols/kad/tests/client_mode.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/protocols/kad/tests/client_mode.rs b/protocols/kad/tests/client_mode.rs index 9c4efc1199f..c2ddd9aa555 100644 --- a/protocols/kad/tests/client_mode.rs +++ b/protocols/kad/tests/client_mode.rs @@ -26,6 +26,31 @@ async fn connection_to_node_in_client_mode_does_not_update_routing_table() { } } +#[async_std::test] +async fn two_servers_add_each_other_to_routing_table() { + let mut server1 = Swarm::new_ephemeral(MyBehaviour::server); + let mut server2 = Swarm::new_ephemeral(MyBehaviour::server); + + server1.listen().await; + server2.listen().await; + + server1.connect(&mut server2).await; + + let server1_peer_id = *server1.local_peer_id(); + let server2_peer_id = *server2.local_peer_id(); + + match libp2p_swarm_test::drive(&mut server1, &mut server2).await { + ( + [MyBehaviourEvent::Identify(_), MyBehaviourEvent::Identify(_), MyBehaviourEvent::Kad(KademliaEvent::RoutingUpdated { peer: peer1, .. })], + [MyBehaviourEvent::Identify(_), MyBehaviourEvent::Identify(_), MyBehaviourEvent::Kad(KademliaEvent::UnroutablePeer { peer: peer2, .. })], // Unroutable because server2 did not dial. + ) => { + assert_eq!(peer1, server2_peer_id); + assert_eq!(peer2, server1_peer_id); + } + other => panic!("Unexpected events: {other:?}"), + } +} + #[derive(libp2p_swarm::NetworkBehaviour)] #[behaviour(prelude = "libp2p_swarm::derive_prelude")] struct MyBehaviour { From deb30f3b5d96d4a7fed420dd2eb0c57baf4bc179 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 13 Apr 2023 19:56:15 +0200 Subject: [PATCH 08/50] Report additions and removals of protocols instead --- protocols/identify/src/handler.rs | 61 +++++++++++++++++-------- protocols/kad/src/handler.rs | 50 ++++++++++++-------- swarm/src/connection.rs | 76 ++++++++++++++++++++++--------- swarm/src/handler.rs | 70 ++++++++++++++++++++-------- swarm/src/handler/map_out.rs | 4 +- swarm/src/handler/multi.rs | 4 +- swarm/src/handler/select.rs | 12 ++--- 7 files changed, 190 insertions(+), 87 deletions(-) diff --git a/protocols/identify/src/handler.rs b/protocols/identify/src/handler.rs index bae3a35b8fa..880930243a3 100644 --- a/protocols/identify/src/handler.rs +++ b/protocols/identify/src/handler.rs @@ -30,17 +30,14 @@ use libp2p_core::upgrade::SelectUpgrade; use libp2p_core::Multiaddr; use libp2p_identity::PeerId; use libp2p_identity::PublicKey; -use libp2p_swarm::handler::{ - ConnectionEvent, DialUpgradeError, FullyNegotiatedInbound, FullyNegotiatedOutbound, - ProtocolsChange, -}; +use libp2p_swarm::handler::{ConnectionEvent, DialUpgradeError, FullyNegotiatedInbound, FullyNegotiatedOutbound, ProtocolsChange, ProtocolSupport}; use libp2p_swarm::{ ConnectionHandler, ConnectionHandlerEvent, ConnectionHandlerUpgrErr, KeepAlive, NegotiatedSubstream, SubstreamProtocol, }; use log::warn; use smallvec::SmallVec; -use std::collections::VecDeque; +use std::collections::{HashSet, VecDeque}; use std::{io, pin::Pin, task::Context, task::Poll, time::Duration}; /// Protocol handler for sending and receiving identification requests. @@ -85,7 +82,8 @@ pub struct Handler { /// Address observed by or for the remote. observed_addr: Multiaddr, - local_supported_protocols: Vec, + local_supported_protocols: HashSet, + remote_supported_protocols: HashSet, } /// An event from `Behaviour` with the information requested by the `Handler`. @@ -138,7 +136,8 @@ impl Handler { protocol_version, agent_version, observed_addr, - local_supported_protocols: vec![], + local_supported_protocols: HashSet::new(), + remote_supported_protocols: HashSet::new(), } } @@ -187,10 +186,30 @@ impl Handler { ) { match output { future::Either::Left(remote_info) => { - self.events - .push(ConnectionHandlerEvent::ReportRemoteProtocols { - protocols: remote_info.protocols.clone(), - }); + let new_remote_protocols = HashSet::from_iter(remote_info.protocols.clone()); + + let remote_added_protocols = new_remote_protocols + .difference(&self.remote_supported_protocols) + .cloned() + .collect::>(); + let remote_removed_protocols = self + .remote_supported_protocols + .difference(&new_remote_protocols) + .cloned() + .collect::>(); + + if !remote_added_protocols.is_empty() { + self.events + .push(ConnectionHandlerEvent::ReportRemoteProtocols(ProtocolSupport::Added(remote_added_protocols))); + } + + if !remote_removed_protocols.is_empty() { + self.events + .push(ConnectionHandlerEvent::ReportRemoteProtocols(ProtocolSupport::Removed(remote_removed_protocols))); + } + + self.remote_supported_protocols = new_remote_protocols; + self.events .push(ConnectionHandlerEvent::Custom(Event::Identified( remote_info, @@ -251,7 +270,7 @@ impl ConnectionHandler for Handler { protocol_version: self.protocol_version.clone(), agent_version: self.agent_version.clone(), listen_addrs, - protocols: self.local_supported_protocols.clone(), + protocols: Vec::from_iter(self.local_supported_protocols.clone()), observed_addr: self.observed_addr.clone(), }; @@ -311,10 +330,11 @@ impl ConnectionHandler for Handler { self.inbound_identify_push.take(); if let Ok(info) = res { - self.events - .push(ConnectionHandlerEvent::ReportRemoteProtocols { - protocols: info.protocols.clone(), - }); + // TODO: report new protocols + // self.events + // .push(ConnectionHandlerEvent::ReportRemoteProtocols { + // protocols: info.protocols.clone(), + // }); return Poll::Ready(ConnectionHandlerEvent::Custom(Event::Identified(info))); } } @@ -353,8 +373,13 @@ impl ConnectionHandler for Handler { self.on_dial_upgrade_error(dial_upgrade_error) } ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) => {} - ConnectionEvent::LocalProtocolsChange(ProtocolsChange { protocols }) => { - self.local_supported_protocols = protocols.to_vec(); + ConnectionEvent::LocalProtocolsChange(ProtocolsChange::Added(added)) => { + self.local_supported_protocols.extend(added.cloned()); + } + ConnectionEvent::LocalProtocolsChange(ProtocolsChange::Removed(removed)) => { + for p in removed { + self.local_supported_protocols.remove(p); + } } ConnectionEvent::RemoteProtocolsChange(_) => {} } diff --git a/protocols/kad/src/handler.rs b/protocols/kad/src/handler.rs index 5f660088531..73f0a0670c0 100644 --- a/protocols/kad/src/handler.rs +++ b/protocols/kad/src/handler.rs @@ -38,7 +38,7 @@ use libp2p_swarm::{ NegotiatedSubstream, SubstreamProtocol, }; use log::trace; -use std::collections::VecDeque; +use std::collections::{HashSet, VecDeque}; use std::task::Waker; use std::{ error, fmt, io, marker::PhantomData, pin::Pin, task::Context, task::Poll, time::Duration, @@ -86,6 +86,8 @@ pub struct KademliaHandler { /// The current state of protocol confirmation. protocol_status: ProtocolStatus, + + remote_supported_protocols: HashSet, } /// The states of protocol confirmation that a connection @@ -503,6 +505,7 @@ where requested_streams: Default::default(), keep_alive, protocol_status: ProtocolStatus::Unknown, + remote_supported_protocols: Default::default(), } } @@ -790,26 +793,37 @@ where ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) | ConnectionEvent::LocalProtocolsChange(_) => {} - ConnectionEvent::RemoteProtocolsChange(ProtocolsChange { protocols }) => { - // TODO: We should cache this / it will get simpler with #2831. - let kademlia_protocols = self - .config - .protocol_config - .protocol_names() - .iter() - .filter_map(|b| String::from_utf8(b.to_vec()).ok()) - .collect::>(); - - let remote_supports_our_kademlia_protocols = - kademlia_protocols.iter().all(|p| protocols.contains(p)); - - if remote_supports_our_kademlia_protocols { - self.protocol_status = ProtocolStatus::Confirmed; - } else { - self.protocol_status = ProtocolStatus::NotSupported; + ConnectionEvent::RemoteProtocolsChange(ProtocolsChange::Added(added)) => { + for p in added { + self.remote_supported_protocols.insert(p.to_owned()); + } + } + ConnectionEvent::RemoteProtocolsChange(ProtocolsChange::Removed(removed)) => { + for p in removed { + self.remote_supported_protocols.remove(p); } } } + + // TODO: We should cache this / it will get simpler with #2831. + let our_kademlia_protocols = self + .config + .protocol_config + .protocol_names() + .iter() + .filter_map(|b| String::from_utf8(b.to_vec()).ok()) + .collect::>(); + + let remote_supports_our_kademlia_protocols = self + .remote_supported_protocols + .iter() + .any(|p| our_kademlia_protocols.contains(p)); + + if remote_supports_our_kademlia_protocols { + self.protocol_status = ProtocolStatus::Confirmed; + } else { + self.protocol_status = ProtocolStatus::NotSupported; + } } } diff --git a/swarm/src/connection.rs b/swarm/src/connection.rs index 089768f46ea..9eef892456f 100644 --- a/swarm/src/connection.rs +++ b/swarm/src/connection.rs @@ -29,7 +29,8 @@ pub use error::{ use crate::handler::{ AddressChange, ConnectionEvent, ConnectionHandler, DialUpgradeError, FullyNegotiatedInbound, - FullyNegotiatedOutbound, ListenUpgradeError, ProtocolsChange, + FullyNegotiatedOutbound, ListenUpgradeError, ProtocolSupport, ProtocolsAdded, ProtocolsChange, + ProtocolsRemoved, }; use crate::upgrade::{InboundUpgradeSend, OutboundUpgradeSend, SendWrapper, UpgradeInfoSend}; use crate::{ConnectionHandlerEvent, ConnectionHandlerUpgrErr, KeepAlive, SubstreamProtocol}; @@ -45,6 +46,7 @@ use libp2p_core::upgrade::{InboundUpgradeApply, OutboundUpgradeApply}; use libp2p_core::Endpoint; use libp2p_core::{upgrade, ProtocolName as _, UpgradeError}; use libp2p_identity::PeerId; +use std::collections::HashSet; use std::future::Future; use std::sync::atomic::{AtomicUsize, Ordering}; use std::task::Waker; @@ -146,7 +148,7 @@ where SubstreamRequested, >, - supported_protocols: Vec, + supported_protocols: HashSet, } impl fmt::Debug for Connection @@ -184,7 +186,7 @@ where substream_upgrade_protocol_override, max_negotiating_inbound_streams, requested_substreams: Default::default(), - supported_protocols: vec![], + supported_protocols: HashSet::new(), } } @@ -248,11 +250,23 @@ where Poll::Ready(ConnectionHandlerEvent::Close(err)) => { return Poll::Ready(Err(ConnectionError::Handler(err))); } - Poll::Ready(ConnectionHandlerEvent::ReportRemoteProtocols { protocols }) => { + Poll::Ready(ConnectionHandlerEvent::ReportRemoteProtocols( + ProtocolSupport::Added(protocols), + )) => { handler.on_connection_event(ConnectionEvent::RemoteProtocolsChange( - ProtocolsChange { - protocols: &protocols, - }, + ProtocolsChange::Added(ProtocolsAdded { + protocols: protocols.difference(&HashSet::new()).peekable(), // This is a bit of a hack to use the same type internally in `ProtocolsAdded`. + }), + )); + continue; + } + Poll::Ready(ConnectionHandlerEvent::ReportRemoteProtocols( + ProtocolSupport::Removed(protocols), + )) => { + handler.on_connection_event(ConnectionEvent::RemoteProtocolsChange( + ProtocolsChange::Removed(ProtocolsRemoved { + protocols: protocols.difference(&HashSet::new()).peekable(), // This is a bit of a hack to use the same type internally in `ProtocolsRemoved`. + }), )); continue; } @@ -367,21 +381,33 @@ where Poll::Ready(substream) => { let protocol = handler.listen_protocol(); - let mut new_protocols = protocol + let new_protocols = protocol .upgrade() .protocol_info() .filter_map(|i| String::from_utf8(i.protocol_name().to_vec()).ok()) - .collect::>(); - - new_protocols.sort(); - - if supported_protocols != &new_protocols { - handler.on_connection_event(ConnectionEvent::LocalProtocolsChange( - ProtocolsChange { - protocols: &new_protocols, - }, - )); - *supported_protocols = new_protocols; + .collect::>(); + + if &new_protocols != supported_protocols { + let mut added_protocols = + new_protocols.difference(supported_protocols).peekable(); + let mut removed_protocols = + supported_protocols.difference(&new_protocols).peekable(); + + if added_protocols.peek().is_some() { + handler.on_connection_event(ConnectionEvent::LocalProtocolsChange( + ProtocolsChange::Removed(ProtocolsRemoved { + protocols: added_protocols, + }), + )); + } + + if removed_protocols.peek().is_some() { + handler.on_connection_event(ConnectionEvent::LocalProtocolsChange( + ProtocolsChange::Removed(ProtocolsRemoved { + protocols: removed_protocols, + }), + )); + } } negotiating_in.push(SubstreamUpgrade::new_inbound(substream, protocol)); @@ -956,8 +982,16 @@ mod tests { Self::OutboundOpenInfo, >, ) { - if let ConnectionEvent::LocalProtocolsChange(ProtocolsChange { protocols }) = event { - self.reported_protocols = protocols.to_vec(); + match event { + ConnectionEvent::LocalProtocolsChange(ProtocolsChange::Added(added)) => { + self.reported_protocols.extend(added.cloned()); + } + ConnectionEvent::LocalProtocolsChange(ProtocolsChange::Removed(removed)) => { + for protocol in removed { + self.reported_protocols.retain(|p| p != protocol); + } + } + _ => {} } } diff --git a/swarm/src/handler.rs b/swarm/src/handler.rs index 0dcc819a69a..c2bec3de114 100644 --- a/swarm/src/handler.rs +++ b/swarm/src/handler.rs @@ -51,7 +51,11 @@ pub use crate::upgrade::{InboundUpgradeSend, OutboundUpgradeSend, SendWrapper, U use instant::Instant; use libp2p_core::{upgrade::UpgradeError, ConnectedPoint, Multiaddr}; use libp2p_identity::PeerId; +use std::collections::hash_set::{Difference}; use std::{cmp::Ordering, error, fmt, task::Context, task::Poll, time::Duration}; +use std::collections::hash_map::RandomState; +use std::collections::HashSet; +use std::iter::Peekable; pub use map_in::MapInEvent; pub use map_out::MapOutEvent; @@ -244,9 +248,34 @@ pub struct AddressChange<'a> { } /// [`ConnectionEvent`] variant that informs the handler about a change in the protocols supported on the connection. -#[derive(Clone, Copy)] -pub struct ProtocolsChange<'a> { - pub protocols: &'a [String], +#[derive(Clone)] +pub enum ProtocolsChange<'a> { + Added(ProtocolsAdded<'a>), + Removed(ProtocolsRemoved<'a>), +} + +#[derive(Clone)] +pub struct ProtocolsAdded<'a> { + pub(crate) protocols: Peekable>, +} + +#[derive(Clone)] +pub struct ProtocolsRemoved<'a> { + pub(crate) protocols: Peekable>, +} + +impl<'a> Iterator for ProtocolsAdded<'a> { + type Item = &'a String; + fn next(&mut self) -> Option { + self.protocols.next() + } +} + +impl<'a> Iterator for ProtocolsRemoved<'a> { + type Item = &'a String; + fn next(&mut self) -> Option { + self.protocols.next() + } } /// [`ConnectionEvent`] variant that informs the handler @@ -357,20 +386,21 @@ pub enum ConnectionHandlerEvent }, + /// We learned something about the protocols supported by the remote. + ReportRemoteProtocols(ProtocolSupport), /// Other event. Custom(TCustom), } +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ProtocolSupport { + /// The remote now supports these protocols. + Added(HashSet), + /// The remote no longer supports these protocols. + Removed(HashSet), +} + /// Event produced by a handler. impl ConnectionHandlerEvent @@ -392,8 +422,8 @@ impl } ConnectionHandlerEvent::Custom(val) => ConnectionHandlerEvent::Custom(val), ConnectionHandlerEvent::Close(val) => ConnectionHandlerEvent::Close(val), - ConnectionHandlerEvent::ReportRemoteProtocols { protocols } => { - ConnectionHandlerEvent::ReportRemoteProtocols { protocols } + ConnectionHandlerEvent::ReportRemoteProtocols(support) => { + ConnectionHandlerEvent::ReportRemoteProtocols(support) } } } @@ -415,8 +445,8 @@ impl } ConnectionHandlerEvent::Custom(val) => ConnectionHandlerEvent::Custom(val), ConnectionHandlerEvent::Close(val) => ConnectionHandlerEvent::Close(val), - ConnectionHandlerEvent::ReportRemoteProtocols { protocols } => { - ConnectionHandlerEvent::ReportRemoteProtocols { protocols } + ConnectionHandlerEvent::ReportRemoteProtocols(support) => { + ConnectionHandlerEvent::ReportRemoteProtocols(support) } } } @@ -435,8 +465,8 @@ impl } ConnectionHandlerEvent::Custom(val) => ConnectionHandlerEvent::Custom(map(val)), ConnectionHandlerEvent::Close(val) => ConnectionHandlerEvent::Close(val), - ConnectionHandlerEvent::ReportRemoteProtocols { protocols } => { - ConnectionHandlerEvent::ReportRemoteProtocols { protocols } + ConnectionHandlerEvent::ReportRemoteProtocols(support) => { + ConnectionHandlerEvent::ReportRemoteProtocols(support) } } } @@ -455,8 +485,8 @@ impl } ConnectionHandlerEvent::Custom(val) => ConnectionHandlerEvent::Custom(val), ConnectionHandlerEvent::Close(val) => ConnectionHandlerEvent::Close(map(val)), - ConnectionHandlerEvent::ReportRemoteProtocols { protocols } => { - ConnectionHandlerEvent::ReportRemoteProtocols { protocols } + ConnectionHandlerEvent::ReportRemoteProtocols(support) => { + ConnectionHandlerEvent::ReportRemoteProtocols(support) } } } diff --git a/swarm/src/handler/map_out.rs b/swarm/src/handler/map_out.rs index 3c3f3827dfa..349aa553764 100644 --- a/swarm/src/handler/map_out.rs +++ b/swarm/src/handler/map_out.rs @@ -81,8 +81,8 @@ where ConnectionHandlerEvent::OutboundSubstreamRequest { protocol } => { ConnectionHandlerEvent::OutboundSubstreamRequest { protocol } } - ConnectionHandlerEvent::ReportRemoteProtocols { protocols } => { - ConnectionHandlerEvent::ReportRemoteProtocols { protocols } + ConnectionHandlerEvent::ReportRemoteProtocols(support) => { + ConnectionHandlerEvent::ReportRemoteProtocols(support) } }) } diff --git a/swarm/src/handler/multi.rs b/swarm/src/handler/multi.rs index 16b11977f6d..ed8a434834c 100644 --- a/swarm/src/handler/multi.rs +++ b/swarm/src/handler/multi.rs @@ -321,14 +321,14 @@ where ConnectionEvent::LocalProtocolsChange(supported_protocols) => { for h in self.handlers.values_mut() { h.on_connection_event(ConnectionEvent::LocalProtocolsChange( - supported_protocols, + supported_protocols.clone(), )); } } ConnectionEvent::RemoteProtocolsChange(supported_protocols) => { for h in self.handlers.values_mut() { h.on_connection_event(ConnectionEvent::RemoteProtocolsChange( - supported_protocols, + supported_protocols.clone(), )); } } diff --git a/swarm/src/handler/select.rs b/swarm/src/handler/select.rs index 2389bc933d6..3faea293734 100644 --- a/swarm/src/handler/select.rs +++ b/swarm/src/handler/select.rs @@ -400,8 +400,8 @@ where .map_info(Either::Left), }); } - Poll::Ready(ConnectionHandlerEvent::ReportRemoteProtocols { protocols }) => { - return Poll::Ready(ConnectionHandlerEvent::ReportRemoteProtocols { protocols }); + Poll::Ready(ConnectionHandlerEvent::ReportRemoteProtocols(support)) => { + return Poll::Ready(ConnectionHandlerEvent::ReportRemoteProtocols(support)); } Poll::Pending => (), }; @@ -420,8 +420,8 @@ where .map_info(Either::Right), }); } - Poll::Ready(ConnectionHandlerEvent::ReportRemoteProtocols { protocols }) => { - return Poll::Ready(ConnectionHandlerEvent::ReportRemoteProtocols { protocols }); + Poll::Ready(ConnectionHandlerEvent::ReportRemoteProtocols(support)) => { + return Poll::Ready(ConnectionHandlerEvent::ReportRemoteProtocols(support)); } Poll::Pending => (), }; @@ -486,7 +486,7 @@ where ConnectionEvent::LocalProtocolsChange(supported_protocols) => { self.proto1 .on_connection_event(ConnectionEvent::LocalProtocolsChange( - supported_protocols, + supported_protocols.clone(), )); self.proto2 .on_connection_event(ConnectionEvent::LocalProtocolsChange( @@ -496,7 +496,7 @@ where ConnectionEvent::RemoteProtocolsChange(supported_protocols) => { self.proto1 .on_connection_event(ConnectionEvent::RemoteProtocolsChange( - supported_protocols, + supported_protocols.clone(), )); self.proto2 .on_connection_event(ConnectionEvent::RemoteProtocolsChange( From 3c1bf5f4a5dc5913f2ebc8668141acdbbc44cb5a Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 13 Apr 2023 20:03:28 +0200 Subject: [PATCH 09/50] Report listen protocols on startup to connection --- protocols/identify/src/handler.rs | 13 ++++++++++--- swarm/src/connection.rs | 15 +++++++++++++++ swarm/src/handler.rs | 4 ++-- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/protocols/identify/src/handler.rs b/protocols/identify/src/handler.rs index 880930243a3..e9d587dc56b 100644 --- a/protocols/identify/src/handler.rs +++ b/protocols/identify/src/handler.rs @@ -30,7 +30,10 @@ use libp2p_core::upgrade::SelectUpgrade; use libp2p_core::Multiaddr; use libp2p_identity::PeerId; use libp2p_identity::PublicKey; -use libp2p_swarm::handler::{ConnectionEvent, DialUpgradeError, FullyNegotiatedInbound, FullyNegotiatedOutbound, ProtocolsChange, ProtocolSupport}; +use libp2p_swarm::handler::{ + ConnectionEvent, DialUpgradeError, FullyNegotiatedInbound, FullyNegotiatedOutbound, + ProtocolSupport, ProtocolsChange, +}; use libp2p_swarm::{ ConnectionHandler, ConnectionHandlerEvent, ConnectionHandlerUpgrErr, KeepAlive, NegotiatedSubstream, SubstreamProtocol, @@ -200,12 +203,16 @@ impl Handler { if !remote_added_protocols.is_empty() { self.events - .push(ConnectionHandlerEvent::ReportRemoteProtocols(ProtocolSupport::Added(remote_added_protocols))); + .push(ConnectionHandlerEvent::ReportRemoteProtocols( + ProtocolSupport::Added(remote_added_protocols), + )); } if !remote_removed_protocols.is_empty() { self.events - .push(ConnectionHandlerEvent::ReportRemoteProtocols(ProtocolSupport::Removed(remote_removed_protocols))); + .push(ConnectionHandlerEvent::ReportRemoteProtocols( + ProtocolSupport::Removed(remote_removed_protocols), + )); } self.remote_supported_protocols = new_remote_protocols; diff --git a/swarm/src/connection.rs b/swarm/src/connection.rs index 9eef892456f..8f68fb4a84b 100644 --- a/swarm/src/connection.rs +++ b/swarm/src/connection.rs @@ -219,6 +219,21 @@ where supported_protocols, } = self.get_mut(); + let protocol = handler.listen_protocol(); + + let new_protocols = protocol + .upgrade() + .protocol_info() + .filter_map(|i| String::from_utf8(i.protocol_name().to_vec()).ok()) + .collect::>(); + + handler.on_connection_event(ConnectionEvent::LocalProtocolsChange( + ProtocolsChange::Added(ProtocolsAdded { + protocols: new_protocols.difference(&HashSet::new()).peekable(), + }), + )); + *supported_protocols = new_protocols; + loop { match requested_substreams.poll_next_unpin(cx) { Poll::Ready(Some(Ok(()))) => continue, diff --git a/swarm/src/handler.rs b/swarm/src/handler.rs index c2bec3de114..84d186f0786 100644 --- a/swarm/src/handler.rs +++ b/swarm/src/handler.rs @@ -51,11 +51,11 @@ pub use crate::upgrade::{InboundUpgradeSend, OutboundUpgradeSend, SendWrapper, U use instant::Instant; use libp2p_core::{upgrade::UpgradeError, ConnectedPoint, Multiaddr}; use libp2p_identity::PeerId; -use std::collections::hash_set::{Difference}; -use std::{cmp::Ordering, error, fmt, task::Context, task::Poll, time::Duration}; use std::collections::hash_map::RandomState; +use std::collections::hash_set::Difference; use std::collections::HashSet; use std::iter::Peekable; +use std::{cmp::Ordering, error, fmt, task::Context, task::Poll, time::Duration}; pub use map_in::MapInEvent; pub use map_out::MapOutEvent; From 9e707297b1aefa0cd9b9a7cc5eb779bb73db95cf Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 13 Apr 2023 20:26:48 +0200 Subject: [PATCH 10/50] Introduce `SupportedProtocols` type --- protocols/identify/src/handler.rs | 21 +++---- swarm/src/connection.rs | 70 +++++++++++++++------ swarm/src/connection/supported_protocols.rs | 26 ++++++++ swarm/src/lib.rs | 2 +- 4 files changed, 85 insertions(+), 34 deletions(-) create mode 100644 swarm/src/connection/supported_protocols.rs diff --git a/protocols/identify/src/handler.rs b/protocols/identify/src/handler.rs index e9d587dc56b..4904dfb07e4 100644 --- a/protocols/identify/src/handler.rs +++ b/protocols/identify/src/handler.rs @@ -32,11 +32,11 @@ use libp2p_identity::PeerId; use libp2p_identity::PublicKey; use libp2p_swarm::handler::{ ConnectionEvent, DialUpgradeError, FullyNegotiatedInbound, FullyNegotiatedOutbound, - ProtocolSupport, ProtocolsChange, + ProtocolSupport, }; use libp2p_swarm::{ ConnectionHandler, ConnectionHandlerEvent, ConnectionHandlerUpgrErr, KeepAlive, - NegotiatedSubstream, SubstreamProtocol, + NegotiatedSubstream, SubstreamProtocol, SupportedProtocols, }; use log::warn; use smallvec::SmallVec; @@ -85,7 +85,7 @@ pub struct Handler { /// Address observed by or for the remote. observed_addr: Multiaddr, - local_supported_protocols: HashSet, + local_supported_protocols: SupportedProtocols, remote_supported_protocols: HashSet, } @@ -139,8 +139,8 @@ impl Handler { protocol_version, agent_version, observed_addr, - local_supported_protocols: HashSet::new(), - remote_supported_protocols: HashSet::new(), + local_supported_protocols: SupportedProtocols::default(), + remote_supported_protocols: HashSet::default(), } } @@ -277,7 +277,7 @@ impl ConnectionHandler for Handler { protocol_version: self.protocol_version.clone(), agent_version: self.agent_version.clone(), listen_addrs, - protocols: Vec::from_iter(self.local_supported_protocols.clone()), + protocols: Vec::from_iter(self.local_supported_protocols.iter().cloned()), observed_addr: self.observed_addr.clone(), }; @@ -380,13 +380,8 @@ impl ConnectionHandler for Handler { self.on_dial_upgrade_error(dial_upgrade_error) } ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) => {} - ConnectionEvent::LocalProtocolsChange(ProtocolsChange::Added(added)) => { - self.local_supported_protocols.extend(added.cloned()); - } - ConnectionEvent::LocalProtocolsChange(ProtocolsChange::Removed(removed)) => { - for p in removed { - self.local_supported_protocols.remove(p); - } + ConnectionEvent::LocalProtocolsChange(change) => { + self.local_supported_protocols.on_protocols_change(change); } ConnectionEvent::RemoteProtocolsChange(_) => {} } diff --git a/swarm/src/connection.rs b/swarm/src/connection.rs index 8f68fb4a84b..7bc66225258 100644 --- a/swarm/src/connection.rs +++ b/swarm/src/connection.rs @@ -21,11 +21,13 @@ mod error; pub(crate) mod pool; +mod supported_protocols; pub use error::{ ConnectionError, PendingConnectionError, PendingInboundConnectionError, PendingOutboundConnectionError, }; +pub use supported_protocols::SupportedProtocols; use crate::handler::{ AddressChange, ConnectionEvent, ConnectionHandler, DialUpgradeError, FullyNegotiatedInbound, @@ -408,15 +410,15 @@ where let mut removed_protocols = supported_protocols.difference(&new_protocols).peekable(); - if added_protocols.peek().is_some() { + if dbg!(added_protocols.peek()).is_some() { handler.on_connection_event(ConnectionEvent::LocalProtocolsChange( - ProtocolsChange::Removed(ProtocolsRemoved { + ProtocolsChange::Added(ProtocolsAdded { protocols: added_protocols, }), )); } - if removed_protocols.peek().is_some() { + if dbg!(removed_protocols.peek()).is_some() { handler.on_connection_event(ConnectionEvent::LocalProtocolsChange( ProtocolsChange::Removed(ProtocolsRemoved { protocols: removed_protocols, @@ -425,6 +427,8 @@ where } } + *supported_protocols = new_protocols; + negotiating_in.push(SubstreamUpgrade::new_inbound(substream, protocol)); continue; // Go back to the top, handler can potentially make progress again. @@ -679,6 +683,7 @@ enum Shutdown { #[cfg(test)] mod tests { use super::*; + use crate::connection::supported_protocols::SupportedProtocols; use crate::keep_alive; use futures::future; use futures::AsyncRead; @@ -754,21 +759,54 @@ mod tests { None, 2, ); - connection.handler.active_protocols = vec!["/foo"]; + connection.handler.active_protocols = HashSet::from(["/foo"]); + + // DummyStreamMuxer will yield a new stream + let _ = Pin::new(&mut connection) + .poll(&mut Context::from_waker(futures::task::noop_waker_ref())); + assert_eq!( + connection + .handler + .supported_local_protocols + .iter() + .cloned() + .collect::>(), + HashSet::from(["/foo".to_owned()]) + ); + + connection.handler.active_protocols = HashSet::from(["/foo", "/bar"]); + connection.negotiating_in.clear(); // Hack to request more substreams from the muxer. // DummyStreamMuxer will yield a new stream let _ = Pin::new(&mut connection) .poll(&mut Context::from_waker(futures::task::noop_waker_ref())); - assert_eq!(connection.handler.reported_protocols, vec!["/foo"]); - connection.handler.active_protocols = vec!["/foo", "/bar"]; + assert_eq!( + connection + .handler + .supported_local_protocols + .iter() + .cloned() + .collect::>(), + HashSet::from(["/bar".to_owned(), "/foo".to_owned()]) + ); + + connection.handler.active_protocols = HashSet::from(["/bar"]); connection.negotiating_in.clear(); // Hack to request more substreams from the muxer. // DummyStreamMuxer will yield a new stream let _ = Pin::new(&mut connection) .poll(&mut Context::from_waker(futures::task::noop_waker_ref())); - assert_eq!(connection.handler.reported_protocols, vec!["/bar", "/foo"]) + assert_eq!( + connection + .handler + .supported_local_protocols + .iter() + .cloned() + .collect::>(), + HashSet::from(["/bar".to_owned()]) + ); } struct DummyStreamMuxer { @@ -890,8 +928,8 @@ mod tests { #[derive(Default)] struct ConfigurableProtocolConnectionHandler { - active_protocols: Vec<&'static str>, - reported_protocols: Vec, + active_protocols: HashSet<&'static str>, + supported_local_protocols: SupportedProtocols, } impl ConnectionHandler for MockConnectionHandler { @@ -982,7 +1020,7 @@ mod tests { ) -> SubstreamProtocol { SubstreamProtocol::new( ManyProtocolsUpgrade { - protocols: self.active_protocols.clone(), + protocols: Vec::from_iter(self.active_protocols.clone()), }, (), ) @@ -997,16 +1035,8 @@ mod tests { Self::OutboundOpenInfo, >, ) { - match event { - ConnectionEvent::LocalProtocolsChange(ProtocolsChange::Added(added)) => { - self.reported_protocols.extend(added.cloned()); - } - ConnectionEvent::LocalProtocolsChange(ProtocolsChange::Removed(removed)) => { - for protocol in removed { - self.reported_protocols.retain(|p| p != protocol); - } - } - _ => {} + if let ConnectionEvent::LocalProtocolsChange(change) = event { + self.supported_local_protocols.on_protocols_change(change); } } diff --git a/swarm/src/connection/supported_protocols.rs b/swarm/src/connection/supported_protocols.rs new file mode 100644 index 00000000000..d607c432d6e --- /dev/null +++ b/swarm/src/connection/supported_protocols.rs @@ -0,0 +1,26 @@ +use crate::handler::ProtocolsChange; +use std::collections::HashSet; + +#[derive(Default, Clone, Debug)] +pub struct SupportedProtocols { + protocols: HashSet, +} + +impl SupportedProtocols { + pub fn on_protocols_change(&mut self, change: ProtocolsChange) { + match change { + ProtocolsChange::Added(added) => { + self.protocols.extend(added.cloned()); + } + ProtocolsChange::Removed(removed) => { + for p in removed { + self.protocols.remove(p); + } + } + } + } + + pub fn iter(&self) -> impl Iterator { + self.protocols.iter() + } +} diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index 20a73b6c350..523a20d1ece 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -118,7 +118,7 @@ pub use behaviour::{ }; #[allow(deprecated)] pub use connection::pool::{ConnectionCounters, ConnectionLimits}; -pub use connection::{ConnectionError, ConnectionId}; +pub use connection::{ConnectionError, ConnectionId, SupportedProtocols}; pub use executor::Executor; #[allow(deprecated)] pub use handler::IntoConnectionHandler; From f1328c529c3641736e50b959cb228b71ce4a34d0 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Wed, 19 Apr 2023 19:46:01 +0200 Subject: [PATCH 11/50] Deduplicate code and propagate supported protocols only once --- swarm/src/connection.rs | 50 ++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/swarm/src/connection.rs b/swarm/src/connection.rs index 7bc66225258..a1249a71e3b 100644 --- a/swarm/src/connection.rs +++ b/swarm/src/connection.rs @@ -175,10 +175,18 @@ where /// and connection handler. pub fn new( muxer: StreamMuxerBox, - handler: THandler, + mut handler: THandler, substream_upgrade_protocol_override: Option, max_negotiating_inbound_streams: usize, ) -> Self { + let initial_protocols = gather_supported_protocols(&handler); + + handler.on_connection_event(ConnectionEvent::LocalProtocolsChange( + ProtocolsChange::Added(ProtocolsAdded { + protocols: initial_protocols.difference(&HashSet::new()).peekable(), + }), + )); + Connection { muxing: muxer, handler, @@ -188,7 +196,7 @@ where substream_upgrade_protocol_override, max_negotiating_inbound_streams, requested_substreams: Default::default(), - supported_protocols: HashSet::new(), + supported_protocols: initial_protocols, } } @@ -221,21 +229,6 @@ where supported_protocols, } = self.get_mut(); - let protocol = handler.listen_protocol(); - - let new_protocols = protocol - .upgrade() - .protocol_info() - .filter_map(|i| String::from_utf8(i.protocol_name().to_vec()).ok()) - .collect::>(); - - handler.on_connection_event(ConnectionEvent::LocalProtocolsChange( - ProtocolsChange::Added(ProtocolsAdded { - protocols: new_protocols.difference(&HashSet::new()).peekable(), - }), - )); - *supported_protocols = new_protocols; - loop { match requested_substreams.poll_next_unpin(cx) { Poll::Ready(Some(Ok(()))) => continue, @@ -396,13 +389,7 @@ where match muxing.poll_inbound_unpin(cx)? { Poll::Pending => {} Poll::Ready(substream) => { - let protocol = handler.listen_protocol(); - - let new_protocols = protocol - .upgrade() - .protocol_info() - .filter_map(|i| String::from_utf8(i.protocol_name().to_vec()).ok()) - .collect::>(); + let new_protocols = gather_supported_protocols(handler); if &new_protocols != supported_protocols { let mut added_protocols = @@ -410,7 +397,7 @@ where let mut removed_protocols = supported_protocols.difference(&new_protocols).peekable(); - if dbg!(added_protocols.peek()).is_some() { + if added_protocols.peek().is_some() { handler.on_connection_event(ConnectionEvent::LocalProtocolsChange( ProtocolsChange::Added(ProtocolsAdded { protocols: added_protocols, @@ -418,7 +405,7 @@ where )); } - if dbg!(removed_protocols.peek()).is_some() { + if removed_protocols.peek().is_some() { handler.on_connection_event(ConnectionEvent::LocalProtocolsChange( ProtocolsChange::Removed(ProtocolsRemoved { protocols: removed_protocols, @@ -429,6 +416,8 @@ where *supported_protocols = new_protocols; + let protocol = handler.listen_protocol(); + negotiating_in.push(SubstreamUpgrade::new_inbound(substream, protocol)); continue; // Go back to the top, handler can potentially make progress again. @@ -441,6 +430,15 @@ where } } +fn gather_supported_protocols(handler: &impl ConnectionHandler) -> HashSet { + handler + .listen_protocol() + .upgrade() + .protocol_info() + .filter_map(|i| String::from_utf8(i.protocol_name().to_vec()).ok()) + .collect() +} + /// Borrowed information about an incoming connection currently being negotiated. #[derive(Debug, Copy, Clone)] pub struct IncomingInfo<'a> { From a4fbcda7426c460bc5db5dea6405795c072a7e1f Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Wed, 19 Apr 2023 20:01:03 +0200 Subject: [PATCH 12/50] Extend docs --- swarm/src/handler.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/swarm/src/handler.rs b/swarm/src/handler.rs index 84d186f0786..1f4b562dcb7 100644 --- a/swarm/src/handler.rs +++ b/swarm/src/handler.rs @@ -395,7 +395,7 @@ pub enum ConnectionHandlerEvent), /// The remote no longer supports these protocols. Removed(HashSet), From 572ed906bea582e574f08175137ad08629e6ff85 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Wed, 19 Apr 2023 22:14:32 +0200 Subject: [PATCH 13/50] Fix compile errors --- protocols/gossipsub/src/handler.rs | 6 +++--- swarm/src/behaviour.rs | 3 +++ swarm/src/handler.rs | 4 ++++ 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/protocols/gossipsub/src/handler.rs b/protocols/gossipsub/src/handler.rs index 5f32346f0b9..928989fa028 100644 --- a/protocols/gossipsub/src/handler.rs +++ b/protocols/gossipsub/src/handler.rs @@ -559,9 +559,9 @@ impl ConnectionHandler for Handler { log::debug!("Protocol negotiation failed: {e}") } ConnectionEvent::AddressChange(_) - | ConnectionEvent::ListenUpgradeError(_) - | ConnectionEvent::LocalProtocolsChange(_) - | ConnectionEvent::RemoteProtocolsChange(_) => {} + | ConnectionEvent::ListenUpgradeError(_) + | ConnectionEvent::LocalProtocolsChange(_) + | ConnectionEvent::RemoteProtocolsChange(_) => {} } } Handler::Disabled(_) => {} diff --git a/swarm/src/behaviour.rs b/swarm/src/behaviour.rs index 9fd014bdc47..92604f8fb07 100644 --- a/swarm/src/behaviour.rs +++ b/swarm/src/behaviour.rs @@ -294,6 +294,9 @@ pub trait PollParameters { /// The iterator's elements are the ASCII names as reported on the wire. /// /// Note that the list is computed once at initialization and never refreshed. + #[deprecated( + note = "Use `libp2p_swarm::SupportedProtocols` in your `ConnectionHandler` instead." + )] fn supported_protocols(&self) -> Self::SupportedProtocolsIter; /// Returns the list of the addresses we're listening on. diff --git a/swarm/src/handler.rs b/swarm/src/handler.rs index 40fc49673c4..3cb52d56d37 100644 --- a/swarm/src/handler.rs +++ b/swarm/src/handler.rs @@ -231,6 +231,8 @@ impl<'a, IP: InboundUpgradeSend, OP: OutboundUpgradeSend, IOI, OOI> } ConnectionEvent::FullyNegotiatedInbound(_) | ConnectionEvent::AddressChange(_) + | ConnectionEvent::LocalProtocolsChange(_) + | ConnectionEvent::RemoteProtocolsChange(_) | ConnectionEvent::ListenUpgradeError(_) => false, } } @@ -247,6 +249,8 @@ impl<'a, IP: InboundUpgradeSend, OP: OutboundUpgradeSend, IOI, OOI> ConnectionEvent::FullyNegotiatedOutbound(_) | ConnectionEvent::ListenUpgradeError(_) | ConnectionEvent::AddressChange(_) + | ConnectionEvent::LocalProtocolsChange(_) + | ConnectionEvent::RemoteProtocolsChange(_) | ConnectionEvent::DialUpgradeError(_) => false, } } From 586394b4dea5e549f35ad5b0bd8b7a22defbd254 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Wed, 26 Apr 2023 20:22:28 +0200 Subject: [PATCH 14/50] Report changes to listen/external address set --- Cargo.lock | 1 + swarm/Cargo.toml | 1 + swarm/src/behaviour/external_addresses.rs | 52 ++++++++++++++++++-- swarm/src/behaviour/listen_addresses.rs | 58 +++++++++++++++++++++-- 4 files changed, 105 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1ce59e1e8a4..9c88d592fc7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2866,6 +2866,7 @@ dependencies = [ "libp2p-swarm-test", "libp2p-yamux", "log", + "once_cell", "quickcheck-ext", "rand 0.8.5", "smallvec", diff --git a/swarm/Cargo.toml b/swarm/Cargo.toml index 820fabc3aac..fe1fd93abe5 100644 --- a/swarm/Cargo.toml +++ b/swarm/Cargo.toml @@ -51,6 +51,7 @@ libp2p-swarm-test = { path = "../swarm-test" } libp2p-yamux = { path = "../muxers/yamux" } quickcheck = { package = "quickcheck-ext", path = "../misc/quickcheck-ext" } void = "1" +once_cell = "1.17.1" [[test]] name = "swarm_derive" diff --git a/swarm/src/behaviour/external_addresses.rs b/swarm/src/behaviour/external_addresses.rs index 2090d4b3481..6f1d523ef37 100644 --- a/swarm/src/behaviour/external_addresses.rs +++ b/swarm/src/behaviour/external_addresses.rs @@ -32,21 +32,67 @@ impl ExternalAddresses { } /// Feed a [`FromSwarm`] event to this struct. + /// + /// Returns whether the event changed our set of external addresses. #[allow(deprecated)] - pub fn on_swarm_event(&mut self, event: &FromSwarm) + pub fn on_swarm_event(&mut self, event: &FromSwarm) -> bool where THandler: IntoConnectionHandler, { match event { FromSwarm::NewExternalAddr(NewExternalAddr { addr, .. }) => { if self.addresses.len() < self.limit { - self.addresses.insert((*addr).clone()); + return self.addresses.insert((*addr).clone()); } } FromSwarm::ExpiredExternalAddr(ExpiredExternalAddr { addr, .. }) => { - self.addresses.remove(addr); + return self.addresses.remove(addr) } _ => {} } + + false + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::dummy; + use libp2p_core::multiaddr::Protocol; + use once_cell::sync::Lazy; + + #[test] + fn new_external_addr_returns_correct_changed_value() { + let mut addresses = ExternalAddresses::default(); + + let changed = addresses.on_swarm_event(&new_external_addr()); + assert!(changed); + + let changed = addresses.on_swarm_event(&new_external_addr()); + assert!(!changed) + } + + #[test] + fn expired_external_addr_returns_correct_changed_value() { + let mut addresses = ExternalAddresses::default(); + addresses.on_swarm_event(&new_external_addr()); + + let changed = addresses.on_swarm_event(&expired_external_addr()); + assert!(changed); + + let changed = addresses.on_swarm_event(&expired_external_addr()); + assert!(!changed) + } + + fn new_external_addr() -> FromSwarm<'static, dummy::ConnectionHandler> { + FromSwarm::NewExternalAddr(NewExternalAddr { addr: &MEMORY_ADDR }) + } + + fn expired_external_addr() -> FromSwarm<'static, dummy::ConnectionHandler> { + FromSwarm::ExpiredExternalAddr(ExpiredExternalAddr { addr: &MEMORY_ADDR }) } + + static MEMORY_ADDR: Lazy = + Lazy::new(|| Multiaddr::empty().with(Protocol::Memory(1000))); } diff --git a/swarm/src/behaviour/listen_addresses.rs b/swarm/src/behaviour/listen_addresses.rs index 07bd003bc8d..f2b61582850 100644 --- a/swarm/src/behaviour/listen_addresses.rs +++ b/swarm/src/behaviour/listen_addresses.rs @@ -17,19 +17,69 @@ impl ListenAddresses { } /// Feed a [`FromSwarm`] event to this struct. + /// + /// Returns whether the event changed our set of listen addresses. #[allow(deprecated)] - pub fn on_swarm_event(&mut self, event: &FromSwarm) + pub fn on_swarm_event(&mut self, event: &FromSwarm) -> bool where THandler: IntoConnectionHandler, { match event { FromSwarm::NewListenAddr(NewListenAddr { addr, .. }) => { - self.addresses.insert((*addr).clone()); + self.addresses.insert((*addr).clone()) } FromSwarm::ExpiredListenAddr(ExpiredListenAddr { addr, .. }) => { - self.addresses.remove(addr); + self.addresses.remove(addr) } - _ => {} + _ => false, } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::dummy; + use libp2p_core::multiaddr::Protocol; + use once_cell::sync::Lazy; + + #[test] + fn new_listen_addr_returns_correct_changed_value() { + let mut addresses = ListenAddresses::default(); + + let changed = addresses.on_swarm_event(&new_listen_addr()); + assert!(changed); + + let changed = addresses.on_swarm_event(&new_listen_addr()); + assert!(!changed) + } + + #[test] + fn expired_listen_addr_returns_correct_changed_value() { + let mut addresses = ListenAddresses::default(); + addresses.on_swarm_event(&new_listen_addr()); + + let changed = addresses.on_swarm_event(&expired_listen_addr()); + assert!(changed); + + let changed = addresses.on_swarm_event(&expired_listen_addr()); + assert!(!changed) + } + + fn new_listen_addr() -> FromSwarm<'static, dummy::ConnectionHandler> { + FromSwarm::NewListenAddr(NewListenAddr { + listener_id: Default::default(), + addr: &MEMORY_ADDR, + }) + } + + fn expired_listen_addr() -> FromSwarm<'static, dummy::ConnectionHandler> { + FromSwarm::ExpiredListenAddr(ExpiredListenAddr { + listener_id: Default::default(), + addr: &MEMORY_ADDR, + }) + } + + static MEMORY_ADDR: Lazy = + Lazy::new(|| Multiaddr::empty().with(Protocol::Memory(1000))); +} From b329a09422cf1ff85a994fe4467e23b98a2b425e Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Wed, 26 Apr 2023 21:02:48 +0200 Subject: [PATCH 15/50] Simplify identify protocol --- protocols/identify/src/behaviour.rs | 155 ++++++++++------------------ protocols/identify/src/handler.rs | 92 +++++++---------- protocols/identify/src/protocol.rs | 8 -- 3 files changed, 87 insertions(+), 168 deletions(-) diff --git a/protocols/identify/src/behaviour.rs b/protocols/identify/src/behaviour.rs index c5cbc12e8c6..7a6b6713b74 100644 --- a/protocols/identify/src/behaviour.rs +++ b/protocols/identify/src/behaviour.rs @@ -19,7 +19,7 @@ // DEALINGS IN THE SOFTWARE. use crate::handler::{self, Handler, InEvent}; -use crate::protocol::{Info, Protocol, UpgradeError}; +use crate::protocol::{Info, UpgradeError}; use libp2p_core::{multiaddr, ConnectedPoint, Endpoint, Multiaddr}; use libp2p_identity::PeerId; use libp2p_identity::PublicKey; @@ -50,10 +50,6 @@ pub struct Behaviour { config: Config, /// For each peer we're connected to, the observed address to send back to it. connected: HashMap>, - /// Pending requests to be fulfilled, either `Handler` requests for `Behaviour` info - /// to address identification requests, or push requests to peers - /// with current information about the local peer. - requests: Vec, /// Pending events to be emitted when polled. events: VecDeque>, /// The addresses of all peers that we have discovered. @@ -63,15 +59,6 @@ pub struct Behaviour { external_addresses: ExternalAddresses, } -/// A `Behaviour` request to be fulfilled, either `Handler` requests for `Behaviour` info -/// to address identification requests, or push requests to peers -/// with current information about the local peer. -#[derive(Debug, PartialEq, Eq)] -struct Request { - peer_id: PeerId, - protocol: Protocol, -} - /// Configuration for the [`identify::Behaviour`](Behaviour). #[non_exhaustive] #[derive(Debug, Clone)] @@ -179,7 +166,6 @@ impl Behaviour { Self { config, connected: HashMap::new(), - requests: Vec::new(), events: VecDeque::new(), discovered_peers, listen_addresses: Default::default(), @@ -193,17 +179,14 @@ impl Behaviour { I: IntoIterator, { for p in peers { - let request = Request { + self.events.push_back(ToSwarm::Dial { + opts: DialOpts::peer_id(p).build(), + }); + self.events.push_back(ToSwarm::NotifyHandler { peer_id: p, - protocol: Protocol::Push, - }; - if !self.requests.contains(&request) { - self.requests.push(request); - - self.events.push_back(ToSwarm::Dial { - opts: DialOpts::peer_id(p).build(), - }); - } + handler: NotifyHandler::Any, + event: InEvent::Push, + }); } } @@ -233,6 +216,14 @@ impl Behaviour { } } } + + fn all_addresses(&self) -> HashSet { + self.listen_addresses + .iter() + .chain(self.external_addresses.iter()) + .cloned() + .collect() + } } impl NetworkBehaviour for Behaviour { @@ -254,6 +245,7 @@ impl NetworkBehaviour for Behaviour { self.config.protocol_version.clone(), self.config.agent_version.clone(), remote_addr.clone(), + self.all_addresses(), )) } @@ -272,13 +264,14 @@ impl NetworkBehaviour for Behaviour { self.config.protocol_version.clone(), self.config.agent_version.clone(), addr.clone(), // TODO: This is weird? That is the public address we dialed, shouldn't need to tell the other party? + self.all_addresses(), )) } fn on_connection_handler_event( &mut self, peer_id: PeerId, - connection_id: ConnectionId, + _: ConnectionId, event: THandlerOutEvent, ) { match event { @@ -307,12 +300,6 @@ impl NetworkBehaviour for Behaviour { self.events .push_back(ToSwarm::GenerateEvent(Event::Pushed { peer_id })); } - handler::Event::Identify => { - self.requests.push(Request { - peer_id, - protocol: Protocol::Identify(connection_id), - }); - } handler::Event::IdentificationError(error) => { self.events .push_back(ToSwarm::GenerateEvent(Event::Error { peer_id, error })); @@ -329,42 +316,7 @@ impl NetworkBehaviour for Behaviour { return Poll::Ready(event); } - // Check for pending requests. - match self.requests.pop() { - Some(Request { - peer_id, - protocol: Protocol::Push, - }) => Poll::Ready(ToSwarm::NotifyHandler { - peer_id, - handler: NotifyHandler::Any, - event: InEvent { - listen_addrs: self - .listen_addresses - .iter() - .chain(self.external_addresses.iter()) - .cloned() - .collect(), - protocol: Protocol::Push, - }, - }), - Some(Request { - peer_id, - protocol: Protocol::Identify(connection_id), - }) => Poll::Ready(ToSwarm::NotifyHandler { - peer_id, - handler: NotifyHandler::One(connection_id), - event: InEvent { - listen_addrs: self - .listen_addresses - .iter() - .chain(self.external_addresses.iter()) - .cloned() - .collect(), - protocol: Protocol::Identify(connection_id), - }, - }), - None => Poll::Pending, - } + Poll::Pending } fn handle_pending_outbound_connection( @@ -383,8 +335,35 @@ impl NetworkBehaviour for Behaviour { } fn on_swarm_event(&mut self, event: FromSwarm) { - self.listen_addresses.on_swarm_event(&event); - self.external_addresses.on_swarm_event(&event); + let listen_addr_changed = self.listen_addresses.on_swarm_event(&event); + let external_addr_changed = self.external_addresses.on_swarm_event(&event); + + if listen_addr_changed || external_addr_changed { + // notify all connected handlers about our changed addresses + let change_events = self + .connected + .iter() + .flat_map(|(peer, map)| map.keys().map(|id| (*peer, id))) + .map(|(peer_id, connection_id)| ToSwarm::NotifyHandler { + peer_id, + handler: NotifyHandler::One(*connection_id), + event: InEvent::AddressesChanged(self.all_addresses()), + }) + .collect::>(); + + self.events.extend(change_events) + } + + if listen_addr_changed && self.config.push_listen_addr_updates { + // trigger an identify push for all connected peers + let push_events = self.connected.keys().map(|peer| ToSwarm::NotifyHandler { + peer_id: *peer, + handler: NotifyHandler::Any, + event: InEvent::Push, + }); + + self.events.extend(push_events); + } match event { FromSwarm::ConnectionEstablished(connection_established) => { @@ -398,30 +377,11 @@ impl NetworkBehaviour for Behaviour { }) => { if remaining_established == 0 { self.connected.remove(&peer_id); - self.requests.retain(|request| { - request - != &Request { - peer_id, - protocol: Protocol::Push, - } - }); } else if let Some(addrs) = self.connected.get_mut(&peer_id) { addrs.remove(&connection_id); } } FromSwarm::DialFailure(DialFailure { peer_id, error, .. }) => { - if let Some(peer_id) = peer_id { - if !self.connected.contains_key(&peer_id) { - self.requests.retain(|request| { - request - != &Request { - peer_id, - protocol: Protocol::Push, - } - }); - } - } - if let Some(entry) = peer_id.and_then(|id| self.discovered_peers.get_mut(&id)) { if let DialError::Transport(errors) = error { for (addr, _error) in errors { @@ -430,20 +390,9 @@ impl NetworkBehaviour for Behaviour { } } } - FromSwarm::NewListenAddr(_) | FromSwarm::ExpiredListenAddr(_) => { - if self.config.push_listen_addr_updates { - for p in self.connected.keys() { - let request = Request { - peer_id: *p, - protocol: Protocol::Push, - }; - if !self.requests.contains(&request) { - self.requests.push(request); - } - } - } - } - FromSwarm::AddressChange(_) + FromSwarm::NewListenAddr(_) + | FromSwarm::ExpiredListenAddr(_) + | FromSwarm::AddressChange(_) | FromSwarm::ListenFailure(_) | FromSwarm::NewListener(_) | FromSwarm::ListenerError(_) diff --git a/protocols/identify/src/handler.rs b/protocols/identify/src/handler.rs index 4904dfb07e4..0b1ac1d65ea 100644 --- a/protocols/identify/src/handler.rs +++ b/protocols/identify/src/handler.rs @@ -18,9 +18,7 @@ // FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // DEALINGS IN THE SOFTWARE. -use crate::protocol::{ - self, Identify, InboundPush, Info, OutboundPush, Protocol, Push, UpgradeError, -}; +use crate::protocol::{Identify, InboundPush, Info, OutboundPush, Push, UpgradeError}; use either::Either; use futures::future::BoxFuture; use futures::prelude::*; @@ -36,11 +34,11 @@ use libp2p_swarm::handler::{ }; use libp2p_swarm::{ ConnectionHandler, ConnectionHandlerEvent, ConnectionHandlerUpgrErr, KeepAlive, - NegotiatedSubstream, SubstreamProtocol, SupportedProtocols, + SubstreamProtocol, SupportedProtocols, }; use log::warn; use smallvec::SmallVec; -use std::collections::{HashSet, VecDeque}; +use std::collections::HashSet; use std::{io, pin::Pin, task::Context, task::Poll, time::Duration}; /// Protocol handler for sending and receiving identification requests. @@ -56,9 +54,6 @@ pub struct Handler { [ConnectionHandlerEvent>, (), Event, io::Error>; 4], >, - /// Streams awaiting `BehaviourInfo` to then send identify requests. - reply_streams: VecDeque, - /// Pending identification replies, awaiting being sent. pending_replies: FuturesUnordered>>, @@ -87,16 +82,14 @@ pub struct Handler { local_supported_protocols: SupportedProtocols, remote_supported_protocols: HashSet, + external_addresses: HashSet, } /// An event from `Behaviour` with the information requested by the `Handler`. #[derive(Debug)] -pub struct InEvent { - /// The addresses that the peer is listening on. - pub listen_addrs: Vec, - - /// The protocol w.r.t. the information requested. - pub protocol: Protocol, +pub enum InEvent { + AddressesChanged(HashSet), + Push, } /// Event produced by the `Handler`. @@ -109,8 +102,6 @@ pub enum Event { Identification(PeerId), /// We actively pushed our identification information to the remote. IdentificationPushed, - /// We received a request for identification. - Identify, /// Failed to identify the remote, or to reply to an identification request. IdentificationError(ConnectionHandlerUpgrErr), } @@ -125,12 +116,12 @@ impl Handler { protocol_version: String, agent_version: String, observed_addr: Multiaddr, + external_addresses: HashSet, ) -> Self { Self { remote_peer_id, inbound_identify_push: Default::default(), events: SmallVec::new(), - reply_streams: VecDeque::new(), pending_replies: FuturesUnordered::new(), trigger_next_identify: Delay::new(initial_delay), keep_alive: KeepAlive::Yes, @@ -141,6 +132,7 @@ impl Handler { observed_addr, local_supported_protocols: SupportedProtocols::default(), remote_supported_protocols: HashSet::default(), + external_addresses: external_addresses, } } @@ -155,16 +147,14 @@ impl Handler { ) { match output { future::Either::Left(substream) => { - self.events - .push(ConnectionHandlerEvent::Custom(Event::Identify)); - if !self.reply_streams.is_empty() { - warn!( - "New inbound identify request from {} while a previous one \ - is still pending. Queueing the new one.", - self.remote_peer_id, - ); - } - self.reply_streams.push_back(substream); + let peer_id = self.remote_peer_id; + let info = self.build_info(); + + self.pending_replies.push(Box::pin(async move { + crate::protocol::send(substream, info).await?; + + Ok(peer_id) + })); } future::Either::Right(fut) => { if self.inbound_identify_push.replace(fut).is_some() { @@ -250,6 +240,17 @@ impl Handler { self.keep_alive = KeepAlive::No; self.trigger_next_identify.reset(self.interval); } + + fn build_info(&mut self) -> Info { + Info { + public_key: self.public_key.clone(), + protocol_version: self.protocol_version.clone(), + agent_version: self.agent_version.clone(), + listen_addrs: Vec::from_iter(self.external_addresses.iter().cloned()), + protocols: Vec::from_iter(self.local_supported_protocols.iter().cloned()), + observed_addr: self.observed_addr.clone(), + } + } } impl ConnectionHandler for Handler { @@ -265,41 +266,18 @@ impl ConnectionHandler for Handler { SubstreamProtocol::new(SelectUpgrade::new(Identify, Push::inbound()), ()) } - fn on_behaviour_event( - &mut self, - InEvent { - listen_addrs, - protocol, - }: Self::InEvent, - ) { - let info = Info { - public_key: self.public_key.clone(), - protocol_version: self.protocol_version.clone(), - agent_version: self.agent_version.clone(), - listen_addrs, - protocols: Vec::from_iter(self.local_supported_protocols.iter().cloned()), - observed_addr: self.observed_addr.clone(), - }; - - match protocol { - Protocol::Push => { + fn on_behaviour_event(&mut self, event: Self::InEvent) { + match dbg!(event) { + InEvent::AddressesChanged(addresses) => { + self.external_addresses = addresses; + } + InEvent::Push => { + let info = self.build_info(); self.events .push(ConnectionHandlerEvent::OutboundSubstreamRequest { protocol: SubstreamProtocol::new(Either::Right(Push::outbound(info)), ()), }); } - Protocol::Identify(_) => { - let substream = self - .reply_streams - .pop_front() - .expect("A BehaviourInfo reply should have a matching substream."); - let peer = self.remote_peer_id; - let fut = Box::pin(async move { - protocol::send(substream, info).await?; - Ok(peer) - }); - self.pending_replies.push(fut); - } } } diff --git a/protocols/identify/src/protocol.rs b/protocols/identify/src/protocol.rs index 1a10b591278..160cfbda5aa 100644 --- a/protocols/identify/src/protocol.rs +++ b/protocols/identify/src/protocol.rs @@ -28,7 +28,6 @@ use libp2p_core::{ }; use libp2p_identity as identity; use libp2p_identity::PublicKey; -use libp2p_swarm::ConnectionId; use log::{debug, trace}; use std::convert::TryFrom; use std::{io, iter, pin::Pin}; @@ -41,13 +40,6 @@ pub const PROTOCOL_NAME: &[u8; 14] = b"/ipfs/id/1.0.0"; pub const PUSH_PROTOCOL_NAME: &[u8; 19] = b"/ipfs/id/push/1.0.0"; -/// The type of the Substream protocol. -#[derive(Debug, PartialEq, Eq)] -pub enum Protocol { - Identify(ConnectionId), - Push, -} - /// Substream upgrade protocol for `/ipfs/id/1.0.0`. #[derive(Debug, Clone)] pub struct Identify; From a63af89ee941f1ef743e080609b9f7dde1eeb44f Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Wed, 26 Apr 2023 21:07:28 +0200 Subject: [PATCH 16/50] Check for changes in inbound protocols on every poll --- swarm/src/connection.rs | 64 ++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 36 deletions(-) diff --git a/swarm/src/connection.rs b/swarm/src/connection.rs index a1249a71e3b..397d1c196ec 100644 --- a/swarm/src/connection.rs +++ b/swarm/src/connection.rs @@ -389,33 +389,6 @@ where match muxing.poll_inbound_unpin(cx)? { Poll::Pending => {} Poll::Ready(substream) => { - let new_protocols = gather_supported_protocols(handler); - - if &new_protocols != supported_protocols { - let mut added_protocols = - new_protocols.difference(supported_protocols).peekable(); - let mut removed_protocols = - supported_protocols.difference(&new_protocols).peekable(); - - if added_protocols.peek().is_some() { - handler.on_connection_event(ConnectionEvent::LocalProtocolsChange( - ProtocolsChange::Added(ProtocolsAdded { - protocols: added_protocols, - }), - )); - } - - if removed_protocols.peek().is_some() { - handler.on_connection_event(ConnectionEvent::LocalProtocolsChange( - ProtocolsChange::Removed(ProtocolsRemoved { - protocols: removed_protocols, - }), - )); - } - } - - *supported_protocols = new_protocols; - let protocol = handler.listen_protocol(); negotiating_in.push(SubstreamUpgrade::new_inbound(substream, protocol)); @@ -425,6 +398,32 @@ where } } + let new_protocols = gather_supported_protocols(handler); + + if &new_protocols != supported_protocols { + let mut added_protocols = new_protocols.difference(supported_protocols).peekable(); + let mut removed_protocols = + supported_protocols.difference(&new_protocols).peekable(); + + if added_protocols.peek().is_some() { + handler.on_connection_event(ConnectionEvent::LocalProtocolsChange( + ProtocolsChange::Added(ProtocolsAdded { + protocols: added_protocols, + }), + )); + } + + if removed_protocols.peek().is_some() { + handler.on_connection_event(ConnectionEvent::LocalProtocolsChange( + ProtocolsChange::Removed(ProtocolsRemoved { + protocols: removed_protocols, + }), + )); + } + } + + *supported_protocols = new_protocols; + return Poll::Pending; // Nothing can make progress, return `Pending`. } } @@ -750,16 +749,13 @@ mod tests { #[test] fn propagates_changes_to_supported_inbound_protocols() { let mut connection = Connection::new( - StreamMuxerBox::new(DummyStreamMuxer { - counter: Arc::new(()), - }), + StreamMuxerBox::new(PendingStreamMuxer), ConfigurableProtocolConnectionHandler::default(), None, - 2, + 0, ); connection.handler.active_protocols = HashSet::from(["/foo"]); - // DummyStreamMuxer will yield a new stream let _ = Pin::new(&mut connection) .poll(&mut Context::from_waker(futures::task::noop_waker_ref())); assert_eq!( @@ -773,9 +769,7 @@ mod tests { ); connection.handler.active_protocols = HashSet::from(["/foo", "/bar"]); - connection.negotiating_in.clear(); // Hack to request more substreams from the muxer. - // DummyStreamMuxer will yield a new stream let _ = Pin::new(&mut connection) .poll(&mut Context::from_waker(futures::task::noop_waker_ref())); @@ -790,9 +784,7 @@ mod tests { ); connection.handler.active_protocols = HashSet::from(["/bar"]); - connection.negotiating_in.clear(); // Hack to request more substreams from the muxer. - // DummyStreamMuxer will yield a new stream let _ = Pin::new(&mut connection) .poll(&mut Context::from_waker(futures::task::noop_waker_ref())); From 72510dd1185eb9b372ac8a82d793e7be7fabe228 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Wed, 26 Apr 2023 21:09:00 +0200 Subject: [PATCH 17/50] Fix clippy lints --- protocols/identify/src/handler.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/protocols/identify/src/handler.rs b/protocols/identify/src/handler.rs index 0b1ac1d65ea..5a8d9c3bd79 100644 --- a/protocols/identify/src/handler.rs +++ b/protocols/identify/src/handler.rs @@ -108,6 +108,7 @@ pub enum Event { impl Handler { /// Creates a new `Handler`. + #[allow(clippy::too_many_arguments)] pub fn new( initial_delay: Duration, interval: Duration, @@ -132,7 +133,7 @@ impl Handler { observed_addr, local_supported_protocols: SupportedProtocols::default(), remote_supported_protocols: HashSet::default(), - external_addresses: external_addresses, + external_addresses, } } From 8ffafddb7a9fb7a2cf56d67f075e60c887159c28 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Wed, 26 Apr 2023 21:31:05 +0200 Subject: [PATCH 18/50] Report changes in `SupportedProtocols` --- swarm/Cargo.toml | 2 +- swarm/src/connection.rs | 8 +-- swarm/src/connection/supported_protocols.rs | 66 ++++++++++++++++++++- swarm/src/handler.rs | 19 ++++++ 4 files changed, 85 insertions(+), 10 deletions(-) diff --git a/swarm/Cargo.toml b/swarm/Cargo.toml index fe1fd93abe5..5e3a5e1e413 100644 --- a/swarm/Cargo.toml +++ b/swarm/Cargo.toml @@ -25,6 +25,7 @@ smallvec = "1.6.1" void = "1" wasm-bindgen-futures = { version = "0.4.34", optional = true } getrandom = { version = "0.2.9", features = ["js"], optional = true } # Explicit dependency to be used in `wasm-bindgen` feature +once_cell = "1.17.1" [target.'cfg(not(any(target_os = "emscripten", target_os = "wasi", target_os = "unknown")))'.dependencies] async-std = { version = "1.6.2", optional = true } @@ -51,7 +52,6 @@ libp2p-swarm-test = { path = "../swarm-test" } libp2p-yamux = { path = "../muxers/yamux" } quickcheck = { package = "quickcheck-ext", path = "../misc/quickcheck-ext" } void = "1" -once_cell = "1.17.1" [[test]] name = "swarm_derive" diff --git a/swarm/src/connection.rs b/swarm/src/connection.rs index 397d1c196ec..53f8c196fdf 100644 --- a/swarm/src/connection.rs +++ b/swarm/src/connection.rs @@ -264,9 +264,7 @@ where ProtocolSupport::Added(protocols), )) => { handler.on_connection_event(ConnectionEvent::RemoteProtocolsChange( - ProtocolsChange::Added(ProtocolsAdded { - protocols: protocols.difference(&HashSet::new()).peekable(), // This is a bit of a hack to use the same type internally in `ProtocolsAdded`. - }), + ProtocolsChange::Added(ProtocolsAdded::from_set(&protocols)), )); continue; } @@ -274,9 +272,7 @@ where ProtocolSupport::Removed(protocols), )) => { handler.on_connection_event(ConnectionEvent::RemoteProtocolsChange( - ProtocolsChange::Removed(ProtocolsRemoved { - protocols: protocols.difference(&HashSet::new()).peekable(), // This is a bit of a hack to use the same type internally in `ProtocolsRemoved`. - }), + ProtocolsChange::Removed(ProtocolsRemoved::from_set(&protocols)), )); continue; } diff --git a/swarm/src/connection/supported_protocols.rs b/swarm/src/connection/supported_protocols.rs index d607c432d6e..7da964dc9f8 100644 --- a/swarm/src/connection/supported_protocols.rs +++ b/swarm/src/connection/supported_protocols.rs @@ -7,15 +7,25 @@ pub struct SupportedProtocols { } impl SupportedProtocols { - pub fn on_protocols_change(&mut self, change: ProtocolsChange) { + pub fn on_protocols_change(&mut self, change: ProtocolsChange) -> bool { match change { ProtocolsChange::Added(added) => { - self.protocols.extend(added.cloned()); + let mut changed = false; + + for p in added { + changed |= self.protocols.insert(p.clone()); + } + + changed } ProtocolsChange::Removed(removed) => { + let mut changed = false; + for p in removed { - self.protocols.remove(p); + changed |= self.protocols.remove(p); } + + changed } } } @@ -24,3 +34,53 @@ impl SupportedProtocols { self.protocols.iter() } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::handler::{ProtocolsAdded, ProtocolsRemoved}; + use once_cell::sync::Lazy; + + #[test] + fn protocols_change_added_returns_correct_changed_value() { + let mut protocols = SupportedProtocols::default(); + + let changed = protocols.on_protocols_change(add_foo()); + assert!(changed); + + let changed = protocols.on_protocols_change(add_foo()); + assert!(!changed); + + let changed = protocols.on_protocols_change(add_foo_bar()); + assert!(changed); + } + + #[test] + fn protocols_change_removed_returns_correct_changed_value() { + let mut protocols = SupportedProtocols::default(); + + let changed = protocols.on_protocols_change(remove_foo()); + assert!(!changed); + + protocols.on_protocols_change(add_foo()); + + let changed = protocols.on_protocols_change(remove_foo()); + assert!(changed); + } + + fn add_foo() -> ProtocolsChange<'static> { + ProtocolsChange::Added(ProtocolsAdded::from_set(&FOO_PROTOCOLS)) + } + + fn add_foo_bar() -> ProtocolsChange<'static> { + ProtocolsChange::Added(ProtocolsAdded::from_set(&FOO_BAR_PROTOCOLS)) + } + + fn remove_foo() -> ProtocolsChange<'static> { + ProtocolsChange::Removed(ProtocolsRemoved::from_set(&FOO_PROTOCOLS)) + } + + static FOO_PROTOCOLS: Lazy> = Lazy::new(|| HashSet::from(["foo".to_string()])); + static FOO_BAR_PROTOCOLS: Lazy> = + Lazy::new(|| HashSet::from(["foo".to_string(), "bar".to_string()])); +} diff --git a/swarm/src/handler.rs b/swarm/src/handler.rs index 3cb52d56d37..af9013c95aa 100644 --- a/swarm/src/handler.rs +++ b/swarm/src/handler.rs @@ -51,6 +51,7 @@ pub use crate::upgrade::{InboundUpgradeSend, OutboundUpgradeSend, SendWrapper, U use instant::Instant; use libp2p_core::{upgrade::UpgradeError, ConnectedPoint, Multiaddr}; use libp2p_identity::PeerId; +use once_cell::sync::Lazy; use std::collections::hash_map::RandomState; use std::collections::hash_set::Difference; use std::collections::HashSet; @@ -295,11 +296,27 @@ pub struct ProtocolsAdded<'a> { pub(crate) protocols: Peekable>, } +impl<'a> ProtocolsAdded<'a> { + pub(crate) fn from_set(protocols: &'a HashSet) -> Self { + ProtocolsAdded { + protocols: protocols.difference(&EMPTY_HASHSET).peekable(), + } + } +} + #[derive(Clone)] pub struct ProtocolsRemoved<'a> { pub(crate) protocols: Peekable>, } +impl<'a> ProtocolsRemoved<'a> { + pub(crate) fn from_set(protocols: &'a HashSet) -> Self { + ProtocolsRemoved { + protocols: protocols.difference(&EMPTY_HASHSET).peekable(), + } + } +} + impl<'a> Iterator for ProtocolsAdded<'a> { type Item = &'a String; fn next(&mut self) -> Option { @@ -668,3 +685,5 @@ impl Ord for KeepAlive { } } } + +static EMPTY_HASHSET: Lazy> = Lazy::new(HashSet::new); From ea1a08736d5b8489d92217ae30e1e561377445b4 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Wed, 26 Apr 2023 21:35:06 +0200 Subject: [PATCH 19/50] Only report changes to handler if there actually was a change --- swarm/src/connection.rs | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/swarm/src/connection.rs b/swarm/src/connection.rs index 53f8c196fdf..054e28524be 100644 --- a/swarm/src/connection.rs +++ b/swarm/src/connection.rs @@ -150,7 +150,8 @@ where SubstreamRequested, >, - supported_protocols: HashSet, + local_supported_protocols: HashSet, + remote_supported_protocols: SupportedProtocols, } impl fmt::Debug for Connection @@ -196,7 +197,8 @@ where substream_upgrade_protocol_override, max_negotiating_inbound_streams, requested_substreams: Default::default(), - supported_protocols: initial_protocols, + local_supported_protocols: initial_protocols, + remote_supported_protocols: SupportedProtocols::default(), } } @@ -226,7 +228,7 @@ where shutdown, max_negotiating_inbound_streams, substream_upgrade_protocol_override, - supported_protocols, + local_supported_protocols: supported_protocols, } = self.get_mut(); loop { @@ -263,17 +265,31 @@ where Poll::Ready(ConnectionHandlerEvent::ReportRemoteProtocols( ProtocolSupport::Added(protocols), )) => { - handler.on_connection_event(ConnectionEvent::RemoteProtocolsChange( - ProtocolsChange::Added(ProtocolsAdded::from_set(&protocols)), - )); + let change = ProtocolsChange::Added(ProtocolsAdded::from_set(&protocols)); + + if self + .remote_supported_protocols + .on_protocols_change(change.clone()) + { + handler.on_connection_event(ConnectionEvent::RemoteProtocolsChange(change)); + // TODO: Should we optimise this to be the _actual_ change? + } + continue; } Poll::Ready(ConnectionHandlerEvent::ReportRemoteProtocols( ProtocolSupport::Removed(protocols), )) => { - handler.on_connection_event(ConnectionEvent::RemoteProtocolsChange( - ProtocolsChange::Removed(ProtocolsRemoved::from_set(&protocols)), - )); + let change = ProtocolsChange::Removed(ProtocolsRemoved::from_set(&protocols)); + + if self + .remote_supported_protocols + .on_protocols_change(change.clone()) + { + handler.on_connection_event(ConnectionEvent::RemoteProtocolsChange(change)); + // TODO: Should we optimise this to be the _actual_ change? + } + continue; } } From 27bc50748d138eed61ed64fe20eed3770f6811f1 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 27 Apr 2023 15:04:08 +0200 Subject: [PATCH 20/50] Do not implicitly dial peers upon push --- protocols/identify/src/behaviour.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/protocols/identify/src/behaviour.rs b/protocols/identify/src/behaviour.rs index 7a6b6713b74..d25ed2537f3 100644 --- a/protocols/identify/src/behaviour.rs +++ b/protocols/identify/src/behaviour.rs @@ -179,9 +179,11 @@ impl Behaviour { I: IntoIterator, { for p in peers { - self.events.push_back(ToSwarm::Dial { - opts: DialOpts::peer_id(p).build(), - }); + if !self.connected.contains_key(&p) { + log::debug!("Not pushing to {p} because we are not connected"); + continue; + } + self.events.push_back(ToSwarm::NotifyHandler { peer_id: p, handler: NotifyHandler::Any, From 74dd94a2fd388483430821f393353ee676bf87a6 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 27 Apr 2023 15:08:43 +0200 Subject: [PATCH 21/50] Remove unused import --- protocols/identify/src/behaviour.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/protocols/identify/src/behaviour.rs b/protocols/identify/src/behaviour.rs index d25ed2537f3..4fab772390c 100644 --- a/protocols/identify/src/behaviour.rs +++ b/protocols/identify/src/behaviour.rs @@ -25,9 +25,8 @@ use libp2p_identity::PeerId; use libp2p_identity::PublicKey; use libp2p_swarm::behaviour::{ConnectionClosed, ConnectionEstablished, DialFailure, FromSwarm}; use libp2p_swarm::{ - dial_opts::DialOpts, AddressScore, ConnectionDenied, ConnectionHandlerUpgrErr, DialError, - ExternalAddresses, ListenAddresses, NetworkBehaviour, NotifyHandler, PollParameters, - THandlerInEvent, ToSwarm, + AddressScore, ConnectionDenied, ConnectionHandlerUpgrErr, DialError, ExternalAddresses, + ListenAddresses, NetworkBehaviour, NotifyHandler, PollParameters, THandlerInEvent, ToSwarm, }; use libp2p_swarm::{ConnectionId, THandler, THandlerOutEvent}; use lru::LruCache; From 628b519919d4f8fc2f8243751e7a046d9676d5fa Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 27 Apr 2023 15:08:50 +0200 Subject: [PATCH 22/50] Fix compile error --- swarm/src/connection.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/swarm/src/connection.rs b/swarm/src/connection.rs index a4055076c3a..1f90b6319a0 100644 --- a/swarm/src/connection.rs +++ b/swarm/src/connection.rs @@ -229,6 +229,7 @@ where max_negotiating_inbound_streams, substream_upgrade_protocol_override, local_supported_protocols: supported_protocols, + remote_supported_protocols, } = self.get_mut(); loop { @@ -267,10 +268,7 @@ where )) => { let change = ProtocolsChange::Added(ProtocolsAdded::from_set(&protocols)); - if self - .remote_supported_protocols - .on_protocols_change(change.clone()) - { + if remote_supported_protocols.on_protocols_change(change.clone()) { handler.on_connection_event(ConnectionEvent::RemoteProtocolsChange(change)); // TODO: Should we optimise this to be the _actual_ change? } @@ -282,10 +280,7 @@ where )) => { let change = ProtocolsChange::Removed(ProtocolsRemoved::from_set(&protocols)); - if self - .remote_supported_protocols - .on_protocols_change(change.clone()) - { + if remote_supported_protocols.on_protocols_change(change.clone()) { handler.on_connection_event(ConnectionEvent::RemoteProtocolsChange(change)); // TODO: Should we optimise this to be the _actual_ change? } From c7b5011691c639a7a12b44ec9ff7fdb64ce49ba8 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 27 Apr 2023 15:12:45 +0200 Subject: [PATCH 23/50] Remove dbg! --- protocols/identify/src/handler.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocols/identify/src/handler.rs b/protocols/identify/src/handler.rs index 5a8d9c3bd79..3b8460022d7 100644 --- a/protocols/identify/src/handler.rs +++ b/protocols/identify/src/handler.rs @@ -268,7 +268,7 @@ impl ConnectionHandler for Handler { } fn on_behaviour_event(&mut self, event: Self::InEvent) { - match dbg!(event) { + match event { InEvent::AddressesChanged(addresses) => { self.external_addresses = addresses; } From a02ca55dfe59d2e0535c79f13eb063d2f3e2aa9d Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 27 Apr 2023 23:17:41 +1000 Subject: [PATCH 24/50] Update swarm/src/handler.rs Co-authored-by: Max Inden --- swarm/src/handler.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/swarm/src/handler.rs b/swarm/src/handler.rs index af9013c95aa..ae05424024e 100644 --- a/swarm/src/handler.rs +++ b/swarm/src/handler.rs @@ -215,7 +215,7 @@ pub enum ConnectionEvent<'a, IP: InboundUpgradeSend, OP: OutboundUpgradeSend, IO DialUpgradeError(DialUpgradeError), /// Informs the handler that upgrading an inbound substream to the given protocol has failed. ListenUpgradeError(ListenUpgradeError), - /// The local [`ConnectionHandler`] now supports a different set of protocols. + /// The local [`ConnectionHandler`] added or removed support for one or more protocols. LocalProtocolsChange(ProtocolsChange<'a>), /// The remote [`ConnectionHandler`] now supports a different set of protocols. RemoteProtocolsChange(ProtocolsChange<'a>), From f3e5e71b788a9fe629778c2653de89077831397d Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 2 May 2023 14:40:44 +0100 Subject: [PATCH 25/50] Combine match arms where possible --- protocols/identify/src/handler.rs | 5 +++-- protocols/perf/src/client/handler.rs | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/protocols/identify/src/handler.rs b/protocols/identify/src/handler.rs index 3b8460022d7..95f1576d461 100644 --- a/protocols/identify/src/handler.rs +++ b/protocols/identify/src/handler.rs @@ -358,11 +358,12 @@ impl ConnectionHandler for Handler { ConnectionEvent::DialUpgradeError(dial_upgrade_error) => { self.on_dial_upgrade_error(dial_upgrade_error) } - ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) => {} + ConnectionEvent::AddressChange(_) + | ConnectionEvent::ListenUpgradeError(_) + | ConnectionEvent::RemoteProtocolsChange(_) => {} ConnectionEvent::LocalProtocolsChange(change) => { self.local_supported_protocols.on_protocols_change(change); } - ConnectionEvent::RemoteProtocolsChange(_) => {} } } } diff --git a/protocols/perf/src/client/handler.rs b/protocols/perf/src/client/handler.rs index 96da11c2e24..33bc3fcd888 100644 --- a/protocols/perf/src/client/handler.rs +++ b/protocols/perf/src/client/handler.rs @@ -127,7 +127,9 @@ impl ConnectionHandler for Handler { .boxed(), ), - ConnectionEvent::AddressChange(_) | ConnectionEvent::LocalProtocolsChange(_) => {} + ConnectionEvent::AddressChange(_) + | ConnectionEvent::LocalProtocolsChange(_) + | ConnectionEvent::RemoteProtocolsChange(_) => {} ConnectionEvent::DialUpgradeError(DialUpgradeError { info: Command { id, .. }, error, @@ -147,7 +149,6 @@ impl ConnectionHandler for Handler { }, } } - ConnectionEvent::RemoteProtocolsChange(_) => {} } } From b7fa7effa07c86b9b20b0eb37a8df9dd231fcaa9 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 2 May 2023 14:46:25 +0100 Subject: [PATCH 26/50] Add comment explaining static hashset --- swarm/src/handler.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/swarm/src/handler.rs b/swarm/src/handler.rs index ae05424024e..344eca3c246 100644 --- a/swarm/src/handler.rs +++ b/swarm/src/handler.rs @@ -686,4 +686,7 @@ impl Ord for KeepAlive { } } +/// A statically declared, empty [`HashSet`] allows us to work around borrow-checker rules for +/// [`ProtocolsAdded::from_set`] and [`ProtocolsRemoved::from_set`]. Those have lifetime-constraints +/// which don't work unless we have a [`HashSet`] with a `'static' lifetime. static EMPTY_HASHSET: Lazy> = Lazy::new(HashSet::new); From 2bd9d73fd0d1a41451ebe5961eb8be44966073b7 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 2 May 2023 14:48:36 +0100 Subject: [PATCH 27/50] Add docs --- swarm/src/handler.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/swarm/src/handler.rs b/swarm/src/handler.rs index 344eca3c246..28d2038ca7d 100644 --- a/swarm/src/handler.rs +++ b/swarm/src/handler.rs @@ -291,6 +291,7 @@ pub enum ProtocolsChange<'a> { Removed(ProtocolsRemoved<'a>), } +/// An [`Iterator`] over all protocols that have been added. #[derive(Clone)] pub struct ProtocolsAdded<'a> { pub(crate) protocols: Peekable>, @@ -304,6 +305,7 @@ impl<'a> ProtocolsAdded<'a> { } } +/// An [`Iterator`] over all protocols that have been removed. #[derive(Clone)] pub struct ProtocolsRemoved<'a> { pub(crate) protocols: Peekable>, From e90c40de79eb249a2e43177eb14ed36ac2ad9cc0 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 2 May 2023 15:00:06 +0100 Subject: [PATCH 28/50] Update supported protocols for push messages --- protocols/identify/src/handler.rs | 66 +++++++++++++++---------------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/protocols/identify/src/handler.rs b/protocols/identify/src/handler.rs index 95f1576d461..6208e6a923b 100644 --- a/protocols/identify/src/handler.rs +++ b/protocols/identify/src/handler.rs @@ -180,38 +180,12 @@ impl Handler { ) { match output { future::Either::Left(remote_info) => { - let new_remote_protocols = HashSet::from_iter(remote_info.protocols.clone()); - - let remote_added_protocols = new_remote_protocols - .difference(&self.remote_supported_protocols) - .cloned() - .collect::>(); - let remote_removed_protocols = self - .remote_supported_protocols - .difference(&new_remote_protocols) - .cloned() - .collect::>(); - - if !remote_added_protocols.is_empty() { - self.events - .push(ConnectionHandlerEvent::ReportRemoteProtocols( - ProtocolSupport::Added(remote_added_protocols), - )); - } - - if !remote_removed_protocols.is_empty() { - self.events - .push(ConnectionHandlerEvent::ReportRemoteProtocols( - ProtocolSupport::Removed(remote_removed_protocols), - )); - } - - self.remote_supported_protocols = new_remote_protocols; - + self.update_supported_protocols_for_remote(&remote_info); self.events .push(ConnectionHandlerEvent::Custom(Event::Identified( remote_info, ))); + self.keep_alive = KeepAlive::No; } future::Either::Right(()) => self @@ -252,6 +226,36 @@ impl Handler { observed_addr: self.observed_addr.clone(), } } + + fn update_supported_protocols_for_remote(&mut self, remote_info: &Info) { + let new_remote_protocols = HashSet::from_iter(remote_info.protocols.clone()); + + let remote_added_protocols = new_remote_protocols + .difference(&self.remote_supported_protocols) + .cloned() + .collect::>(); + let remote_removed_protocols = self + .remote_supported_protocols + .difference(&new_remote_protocols) + .cloned() + .collect::>(); + + if !remote_added_protocols.is_empty() { + self.events + .push(ConnectionHandlerEvent::ReportRemoteProtocols( + ProtocolSupport::Added(remote_added_protocols), + )); + } + + if !remote_removed_protocols.is_empty() { + self.events + .push(ConnectionHandlerEvent::ReportRemoteProtocols( + ProtocolSupport::Removed(remote_removed_protocols), + )); + } + + self.remote_supported_protocols = new_remote_protocols; + } } impl ConnectionHandler for Handler { @@ -316,11 +320,7 @@ impl ConnectionHandler for Handler { self.inbound_identify_push.take(); if let Ok(info) = res { - // TODO: report new protocols - // self.events - // .push(ConnectionHandlerEvent::ReportRemoteProtocols { - // protocols: info.protocols.clone(), - // }); + self.update_supported_protocols_for_remote(&info); return Poll::Ready(ConnectionHandlerEvent::Custom(Event::Identified(info))); } } From 84979e45e50ebbbff8f2e87995603f2b8cbfb59b Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 2 May 2023 17:57:52 +0200 Subject: [PATCH 29/50] Use `pop` to avoid panicking branch in `remove` --- protocols/identify/src/handler.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/protocols/identify/src/handler.rs b/protocols/identify/src/handler.rs index 6208e6a923b..83bc88460b0 100644 --- a/protocols/identify/src/handler.rs +++ b/protocols/identify/src/handler.rs @@ -296,8 +296,8 @@ impl ConnectionHandler for Handler { ) -> Poll< ConnectionHandlerEvent, > { - if !self.events.is_empty() { - return Poll::Ready(self.events.remove(0)); + if let Some(event) = self.events.pop() { + return Poll::Ready(event); } // Poll the future that fires when we need to identify the node again. From dbfc7e7082047ed0c5e09885446aea74fa8ddc6e Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 2 May 2023 17:58:24 +0200 Subject: [PATCH 30/50] Use `poll_unpin` --- protocols/identify/src/handler.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/protocols/identify/src/handler.rs b/protocols/identify/src/handler.rs index 83bc88460b0..16362f75898 100644 --- a/protocols/identify/src/handler.rs +++ b/protocols/identify/src/handler.rs @@ -39,7 +39,7 @@ use libp2p_swarm::{ use log::warn; use smallvec::SmallVec; use std::collections::HashSet; -use std::{io, pin::Pin, task::Context, task::Poll, time::Duration}; +use std::{io, task::Context, task::Poll, time::Duration}; /// Protocol handler for sending and receiving identification requests. /// @@ -301,7 +301,7 @@ impl ConnectionHandler for Handler { } // Poll the future that fires when we need to identify the node again. - match Future::poll(Pin::new(&mut self.trigger_next_identify), cx) { + match self.trigger_next_identify.poll_unpin(cx) { Poll::Pending => {} Poll::Ready(()) => { self.trigger_next_identify.reset(self.interval); From f2d2c886a06d86004990c8efccc9545705cb49d8 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 2 May 2023 17:58:48 +0200 Subject: [PATCH 31/50] Use `if let` for consistency --- protocols/identify/src/handler.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/protocols/identify/src/handler.rs b/protocols/identify/src/handler.rs index 16362f75898..77633abf059 100644 --- a/protocols/identify/src/handler.rs +++ b/protocols/identify/src/handler.rs @@ -301,15 +301,12 @@ impl ConnectionHandler for Handler { } // Poll the future that fires when we need to identify the node again. - match self.trigger_next_identify.poll_unpin(cx) { - Poll::Pending => {} - Poll::Ready(()) => { - self.trigger_next_identify.reset(self.interval); - let ev = ConnectionHandlerEvent::OutboundSubstreamRequest { - protocol: SubstreamProtocol::new(Either::Left(Identify), ()), - }; - return Poll::Ready(ev); - } + if let Poll::Ready(()) = self.trigger_next_identify.poll_unpin(cx) { + self.trigger_next_identify.reset(self.interval); + let ev = ConnectionHandlerEvent::OutboundSubstreamRequest { + protocol: SubstreamProtocol::new(Either::Left(Identify), ()), + }; + return Poll::Ready(ev); } if let Some(Poll::Ready(res)) = self From b41aeb83348eeeaeee0b34f4352c400e5a2fb3b9 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 2 May 2023 18:01:58 +0200 Subject: [PATCH 32/50] Rewrite to `if let` for consistency --- protocols/identify/src/handler.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/protocols/identify/src/handler.rs b/protocols/identify/src/handler.rs index 77633abf059..966339cc612 100644 --- a/protocols/identify/src/handler.rs +++ b/protocols/identify/src/handler.rs @@ -323,17 +323,17 @@ impl ConnectionHandler for Handler { } // Check for pending replies to send. - match self.pending_replies.poll_next_unpin(cx) { - Poll::Ready(Some(Ok(peer_id))) => Poll::Ready(ConnectionHandlerEvent::Custom( - Event::Identification(peer_id), - )), - Poll::Ready(Some(Err(err))) => Poll::Ready(ConnectionHandlerEvent::Custom( + if let Poll::Ready(Some(result)) = self.pending_replies.poll_next_unpin(cx) { + let event = result.map(Event::Identification).unwrap_or_else(|err| { Event::IdentificationError(ConnectionHandlerUpgrErr::Upgrade( - libp2p_core::upgrade::UpgradeError::Apply(err), - )), - )), - Poll::Ready(None) | Poll::Pending => Poll::Pending, + libp2p_core::UpgradeError::Apply(err), + )) + }); + + return Poll::Ready(ConnectionHandlerEvent::Custom(event)); } + + Poll::Pending } fn on_connection_event( From bcd872b002fc9ce25a6ca671039db704b6cf3313 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 2 May 2023 18:03:26 +0200 Subject: [PATCH 33/50] Fill in todo in toggle --- swarm/src/behaviour/toggle.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/swarm/src/behaviour/toggle.rs b/swarm/src/behaviour/toggle.rs index b6533411250..4dcbae1609a 100644 --- a/swarm/src/behaviour/toggle.rs +++ b/swarm/src/behaviour/toggle.rs @@ -368,11 +368,15 @@ where ConnectionEvent::ListenUpgradeError(listen_upgrade_error) => { self.on_listen_upgrade_error(listen_upgrade_error) } - ConnectionEvent::LocalProtocolsChange(_) => { - todo!() + ConnectionEvent::LocalProtocolsChange(change) => { + if let Some(inner) = self.inner.as_mut() { + inner.on_connection_event(ConnectionEvent::LocalProtocolsChange(change)); + } } - ConnectionEvent::RemoteProtocolsChange(_) => { - todo!() + ConnectionEvent::RemoteProtocolsChange(change) => { + if let Some(inner) = self.inner.as_mut() { + inner.on_connection_event(ConnectionEvent::RemoteProtocolsChange(change)); + } } } } From 82642b82e42104b050b6ac5259f27ca5c51c7d60 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 May 2023 10:41:51 +0200 Subject: [PATCH 34/50] Restore `libp2p-kad` --- Cargo.lock | 3 -- protocols/kad/Cargo.toml | 4 -- protocols/kad/src/behaviour.rs | 30 +---------- protocols/kad/src/handler_priv.rs | 53 +++++-------------- protocols/kad/src/lib.rs | 2 +- protocols/kad/tests/client_mode.rs | 84 ------------------------------ 6 files changed, 16 insertions(+), 160 deletions(-) delete mode 100644 protocols/kad/tests/client_mode.rs diff --git a/Cargo.lock b/Cargo.lock index 69edcc13f4b..04ea1bd39e8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2580,7 +2580,6 @@ name = "libp2p-kad" version = "0.44.0" dependencies = [ "arrayvec", - "async-std", "asynchronous-codec", "bytes", "either", @@ -2590,11 +2589,9 @@ dependencies = [ "futures-timer", "instant", "libp2p-core", - "libp2p-identify", "libp2p-identity", "libp2p-noise", "libp2p-swarm", - "libp2p-swarm-test", "libp2p-yamux", "log", "quick-protobuf", diff --git a/protocols/kad/Cargo.toml b/protocols/kad/Cargo.toml index defeeb53fe1..561d6e4c424 100644 --- a/protocols/kad/Cargo.toml +++ b/protocols/kad/Cargo.toml @@ -34,13 +34,9 @@ serde = { version = "1.0", optional = true, features = ["derive"] } thiserror = "1" [dev-dependencies] -async-std = { version = "1.12.0", features = ["attributes"] } env_logger = "0.10.0" futures-timer = "3.0" -libp2p-identify = { path = "../identify" } libp2p-noise = { workspace = true } -libp2p-swarm = { path = "../../swarm", features = ["macros"] } -libp2p-swarm-test = { path = "../../swarm-test" } libp2p-yamux = { workspace = true } quickcheck = { workspace = true } diff --git a/protocols/kad/src/behaviour.rs b/protocols/kad/src/behaviour.rs index 4876cf45a3e..ed161d7a6ce 100644 --- a/protocols/kad/src/behaviour.rs +++ b/protocols/kad/src/behaviour.rs @@ -116,7 +116,6 @@ pub struct Kademlia { /// The record storage. store: TStore, - mode: Mode, } /// The configurable strategies for the insertion of peers @@ -183,7 +182,6 @@ pub struct KademliaConfig { connection_idle_timeout: Duration, kbucket_inserts: KademliaBucketInserts, caching: KademliaCaching, - mode: Mode, } impl Default for KademliaConfig { @@ -201,7 +199,6 @@ impl Default for KademliaConfig { connection_idle_timeout: Duration::from_secs(10), kbucket_inserts: KademliaBucketInserts::OnConnected, caching: KademliaCaching::Enabled { max_peers: 1 }, - mode: Mode::Server, } } } @@ -402,14 +399,6 @@ impl KademliaConfig { self.caching = c; self } - - /// Sets the mode. - /// - /// TODO: More docs. - pub fn set_mode(&mut self, m: Mode) -> &mut Self { - self.mode = m; - self - } } impl Kademlia @@ -464,7 +453,6 @@ where connection_idle_timeout: config.connection_idle_timeout, external_addresses: Default::default(), local_peer_id: id, - mode: config.mode, } } @@ -1990,7 +1978,7 @@ where Ok(KademliaHandler::new( KademliaHandlerConfig { protocol_config: self.protocol_config.clone(), - allow_listening: self.mode == Mode::Server, + allow_listening: true, idle_timeout: self.connection_idle_timeout, }, ConnectedPoint::Listener { @@ -2011,7 +1999,7 @@ where Ok(KademliaHandler::new( KademliaHandlerConfig { protocol_config: self.protocol_config.clone(), - allow_listening: self.mode == Mode::Server, + allow_listening: true, idle_timeout: self.connection_idle_timeout, }, ConnectedPoint::Dialer { @@ -2076,14 +2064,6 @@ where self.connection_updated(source, address, NodeStatus::Connected); } - KademliaHandlerEvent::ProtocolNotSupported { endpoint } => { - let address = match endpoint { - ConnectedPoint::Dialer { address, .. } => Some(address), - ConnectedPoint::Listener { .. } => None, - }; - self.connection_updated(source, address, NodeStatus::Disconnected); - } - KademliaHandlerEvent::FindNodeReq { key, request_id } => { let closer_peers = self.find_closest(&kbucket_priv::Key::new(key), &source); @@ -3212,9 +3192,3 @@ pub enum RoutingUpdate { /// peer ID). Failed, } - -#[derive(Debug, Clone, Copy, PartialEq)] -pub enum Mode { - Client, - Server, -} diff --git a/protocols/kad/src/handler_priv.rs b/protocols/kad/src/handler_priv.rs index a3aced0b235..7a8d5e44515 100644 --- a/protocols/kad/src/handler_priv.rs +++ b/protocols/kad/src/handler_priv.rs @@ -31,14 +31,13 @@ use libp2p_core::{upgrade, ConnectedPoint}; use libp2p_identity::PeerId; use libp2p_swarm::handler::{ ConnectionEvent, DialUpgradeError, FullyNegotiatedInbound, FullyNegotiatedOutbound, - ProtocolsChange, }; use libp2p_swarm::{ ConnectionHandler, ConnectionHandlerEvent, ConnectionHandlerUpgrErr, KeepAlive, - NegotiatedSubstream, StreamProtocol, SubstreamProtocol, + NegotiatedSubstream, SubstreamProtocol, }; use log::trace; -use std::collections::{HashSet, VecDeque}; +use std::collections::VecDeque; use std::task::Waker; use std::{ error, fmt, io, marker::PhantomData, pin::Pin, task::Context, task::Poll, time::Duration, @@ -85,8 +84,6 @@ pub struct KademliaHandler { /// The current state of protocol confirmation. protocol_status: ProtocolStatus, - - remote_supported_protocols: HashSet, } /// The states of protocol confirmation that a connection @@ -94,12 +91,10 @@ pub struct KademliaHandler { enum ProtocolStatus { /// It is as yet unknown whether the remote supports the /// configured protocol name. - Unknown, + Unconfirmed, /// The configured protocol name has been confirmed by the remote /// but has not yet been reported to the `Kademlia` behaviour. Confirmed, - /// The configured protocol name(s) are not or no longer supported by the remote. - NotSupported, /// The configured protocol has been confirmed by the remote /// and the confirmation reported to the `Kademlia` behaviour. Reported, @@ -230,11 +225,13 @@ impl InboundSubstreamState { #[derive(Debug)] pub enum KademliaHandlerEvent { /// The configured protocol name has been confirmed by the peer through - /// a successfully negotiated substream or by learning the supported protocols of the remote. + /// a successfully negotiated substream. + /// + /// This event is only emitted once by a handler upon the first + /// successfully negotiated inbound or outbound substream and + /// indicates that the connected peer participates in the Kademlia + /// overlay network identified by the configured protocol name. ProtocolConfirmed { endpoint: ConnectedPoint }, - /// The configured protocol name(s) are not or no longer supported by the peer on the provided - /// connection and it should be removed from the routing table. - ProtocolNotSupported { endpoint: ConnectedPoint }, /// Request for the list of nodes whose IDs are the closest to `key`. The number of nodes /// returned is not specified, but should be around 20. @@ -503,8 +500,7 @@ where num_requested_outbound_streams: 0, pending_messages: Default::default(), keep_alive, - protocol_status: ProtocolStatus::Unknown, - remote_supported_protocols: Default::default(), + protocol_status: ProtocolStatus::Unconfirmed, } } @@ -526,7 +522,7 @@ where self.num_requested_outbound_streams -= 1; - if let ProtocolStatus::Unknown = self.protocol_status { + if let ProtocolStatus::Unconfirmed = self.protocol_status { // Upon the first successfully negotiated substream, we know that the // remote is configured with the same protocol name and we want // the behaviour to add this peer to the routing table, if possible. @@ -548,7 +544,7 @@ where future::Either::Right(p) => void::unreachable(p), }; - if let ProtocolStatus::Unknown = self.protocol_status { + if let ProtocolStatus::Unconfirmed = self.protocol_status { // Upon the first successfully negotiated substream, we know that the // remote is configured with the same protocol name and we want // the behaviour to add this peer to the routing table, if possible. @@ -781,30 +777,7 @@ where ConnectionEvent::DialUpgradeError(dial_upgrade_error) => { self.on_dial_upgrade_error(dial_upgrade_error) } - ConnectionEvent::AddressChange(_) - | ConnectionEvent::ListenUpgradeError(_) - | ConnectionEvent::LocalProtocolsChange(_) => {} - ConnectionEvent::RemoteProtocolsChange(ProtocolsChange::Added(added)) => { - for p in added { - self.remote_supported_protocols.insert(p.to_owned()); - } - } - ConnectionEvent::RemoteProtocolsChange(ProtocolsChange::Removed(removed)) => { - for p in removed { - self.remote_supported_protocols.remove(p); - } - } - } - - let remote_supports_our_kademlia_protocols = self - .remote_supported_protocols - .iter() - .any(|p| self.config.protocol_config.protocol_names().contains(p)); - - if remote_supports_our_kademlia_protocols { - self.protocol_status = ProtocolStatus::Confirmed; - } else { - self.protocol_status = ProtocolStatus::NotSupported; + ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) | ConnectionEvent::LocalProtocolsChange(_) | ConnectionEvent::RemoteProtocolsChange(_) => {} } } } diff --git a/protocols/kad/src/lib.rs b/protocols/kad/src/lib.rs index 085c1f13c68..c3a705900d8 100644 --- a/protocols/kad/src/lib.rs +++ b/protocols/kad/src/lib.rs @@ -89,7 +89,7 @@ pub use behaviour::{ AddProviderContext, AddProviderError, AddProviderOk, AddProviderPhase, AddProviderResult, BootstrapError, BootstrapOk, BootstrapResult, GetClosestPeersError, GetClosestPeersOk, GetClosestPeersResult, GetProvidersError, GetProvidersOk, GetProvidersResult, GetRecordError, - GetRecordOk, GetRecordResult, InboundRequest, Mode, NoKnownPeers, PeerRecord, PutRecordContext, + GetRecordOk, GetRecordResult, InboundRequest, NoKnownPeers, PeerRecord, PutRecordContext, PutRecordError, PutRecordOk, PutRecordPhase, PutRecordResult, QueryInfo, QueryMut, QueryRef, QueryResult, QueryStats, RoutingUpdate, }; diff --git a/protocols/kad/tests/client_mode.rs b/protocols/kad/tests/client_mode.rs deleted file mode 100644 index c2ddd9aa555..00000000000 --- a/protocols/kad/tests/client_mode.rs +++ /dev/null @@ -1,84 +0,0 @@ -use libp2p_identify as identify; -use libp2p_identity as identity; -use libp2p_kad::store::MemoryStore; -use libp2p_kad::{Kademlia, KademliaConfig, KademliaEvent, Mode}; -use libp2p_swarm::Swarm; -use libp2p_swarm_test::SwarmExt; - -#[async_std::test] -async fn connection_to_node_in_client_mode_does_not_update_routing_table() { - let mut client = Swarm::new_ephemeral(MyBehaviour::client); - let mut server = Swarm::new_ephemeral(MyBehaviour::server); - - server.listen().await; - client.connect(&mut server).await; - - let server_peer_id = *server.local_peer_id(); - - match libp2p_swarm_test::drive(&mut client, &mut server).await { - ( - [MyBehaviourEvent::Identify(_), MyBehaviourEvent::Identify(_), MyBehaviourEvent::Kad(KademliaEvent::RoutingUpdated { peer, .. })], - [MyBehaviourEvent::Identify(_), MyBehaviourEvent::Identify(_)], - ) => { - assert_eq!(peer, server_peer_id) - } - other => panic!("Unexpected events: {other:?}"), - } -} - -#[async_std::test] -async fn two_servers_add_each_other_to_routing_table() { - let mut server1 = Swarm::new_ephemeral(MyBehaviour::server); - let mut server2 = Swarm::new_ephemeral(MyBehaviour::server); - - server1.listen().await; - server2.listen().await; - - server1.connect(&mut server2).await; - - let server1_peer_id = *server1.local_peer_id(); - let server2_peer_id = *server2.local_peer_id(); - - match libp2p_swarm_test::drive(&mut server1, &mut server2).await { - ( - [MyBehaviourEvent::Identify(_), MyBehaviourEvent::Identify(_), MyBehaviourEvent::Kad(KademliaEvent::RoutingUpdated { peer: peer1, .. })], - [MyBehaviourEvent::Identify(_), MyBehaviourEvent::Identify(_), MyBehaviourEvent::Kad(KademliaEvent::UnroutablePeer { peer: peer2, .. })], // Unroutable because server2 did not dial. - ) => { - assert_eq!(peer1, server2_peer_id); - assert_eq!(peer2, server1_peer_id); - } - other => panic!("Unexpected events: {other:?}"), - } -} - -#[derive(libp2p_swarm::NetworkBehaviour)] -#[behaviour(prelude = "libp2p_swarm::derive_prelude")] -struct MyBehaviour { - identify: identify::Behaviour, - kad: Kademlia, -} - -impl MyBehaviour { - fn client(k: identity::Keypair) -> Self { - let mut config = KademliaConfig::default(); - config.set_mode(Mode::Client); - - Self::with_config(k, config) - } - - fn server(k: identity::Keypair) -> Self { - Self::with_config(k, KademliaConfig::default()) - } - - fn with_config(k: identity::Keypair, config: KademliaConfig) -> MyBehaviour { - let local_peer_id = k.public().to_peer_id(); - - Self { - identify: identify::Behaviour::new(identify::Config::new( - "/test/1.0.0".to_owned(), - k.public(), - )), - kad: Kademlia::with_config(local_peer_id, MemoryStore::new(local_peer_id), config), - } - } -} From eb6648901b76c4c8d73b3fa586f7439a5ba3afc8 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 May 2023 10:44:46 +0200 Subject: [PATCH 35/50] Fix formatting --- protocols/kad/src/handler_priv.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/protocols/kad/src/handler_priv.rs b/protocols/kad/src/handler_priv.rs index 7a8d5e44515..d8ec0e36595 100644 --- a/protocols/kad/src/handler_priv.rs +++ b/protocols/kad/src/handler_priv.rs @@ -777,7 +777,10 @@ where ConnectionEvent::DialUpgradeError(dial_upgrade_error) => { self.on_dial_upgrade_error(dial_upgrade_error) } - ConnectionEvent::AddressChange(_) | ConnectionEvent::ListenUpgradeError(_) | ConnectionEvent::LocalProtocolsChange(_) | ConnectionEvent::RemoteProtocolsChange(_) => {} + ConnectionEvent::AddressChange(_) + | ConnectionEvent::ListenUpgradeError(_) + | ConnectionEvent::LocalProtocolsChange(_) + | ConnectionEvent::RemoteProtocolsChange(_) => {} } } } From 8c47bd60cc5ef624b61391092e1458b69fe7fdcf Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 May 2023 18:03:48 +0200 Subject: [PATCH 36/50] Fix unit tests --- swarm/src/connection/supported_protocols.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/swarm/src/connection/supported_protocols.rs b/swarm/src/connection/supported_protocols.rs index 58edb93626d..0575046bb44 100644 --- a/swarm/src/connection/supported_protocols.rs +++ b/swarm/src/connection/supported_protocols.rs @@ -82,7 +82,7 @@ mod tests { } static FOO_PROTOCOLS: Lazy> = - Lazy::new(|| HashSet::from([StreamProtocol::new("foo")])); + Lazy::new(|| HashSet::from([StreamProtocol::new("/foo")])); static FOO_BAR_PROTOCOLS: Lazy> = - Lazy::new(|| HashSet::from([StreamProtocol::new("foo"), StreamProtocol::new("bar")])); + Lazy::new(|| HashSet::from([StreamProtocol::new("/foo"), StreamProtocol::new("/bar")])); } From bf9421edadfaab37a189713ae4dcd60657ce77be Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 May 2023 18:26:58 +0200 Subject: [PATCH 37/50] Change test to assert actual events received --- swarm/src/connection.rs | 53 ++++++++++++++++++++++------------------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/swarm/src/connection.rs b/swarm/src/connection.rs index 1f370f11b59..2c070aee900 100644 --- a/swarm/src/connection.rs +++ b/swarm/src/connection.rs @@ -689,7 +689,6 @@ enum Shutdown { #[cfg(test)] mod tests { use super::*; - use crate::connection::supported_protocols::SupportedProtocols; use crate::keep_alive; use futures::future; use futures::AsyncRead; @@ -768,14 +767,10 @@ mod tests { let _ = Pin::new(&mut connection) .poll(&mut Context::from_waker(futures::task::noop_waker_ref())); assert_eq!( - connection - .handler - .supported_local_protocols - .iter() - .cloned() - .collect::>(), - HashSet::from([StreamProtocol::new("/foo")]) + connection.handler.added, + vec![vec![StreamProtocol::new("/foo")]] ); + assert!(connection.handler.removed.is_empty()); connection.handler.active_protocols = HashSet::from([StreamProtocol::new("/foo"), StreamProtocol::new("/bar")]); @@ -784,14 +779,13 @@ mod tests { .poll(&mut Context::from_waker(futures::task::noop_waker_ref())); assert_eq!( - connection - .handler - .supported_local_protocols - .iter() - .cloned() - .collect::>(), - HashSet::from([StreamProtocol::new("/bar"), StreamProtocol::new("/foo")]) + connection.handler.added, + vec![ + vec![StreamProtocol::new("/foo")], + vec![StreamProtocol::new("/bar")] + ] ); + assert!(connection.handler.removed.is_empty()); connection.handler.active_protocols = HashSet::from([StreamProtocol::new("/bar")]); @@ -799,13 +793,15 @@ mod tests { .poll(&mut Context::from_waker(futures::task::noop_waker_ref())); assert_eq!( - connection - .handler - .supported_local_protocols - .iter() - .cloned() - .collect::>(), - HashSet::from([StreamProtocol::new("/bar")]) + connection.handler.added, + vec![ + vec![StreamProtocol::new("/foo")], + vec![StreamProtocol::new("/bar")] + ] + ); + assert_eq!( + connection.handler.removed, + vec![vec![StreamProtocol::new("/foo")]] ); } @@ -929,7 +925,8 @@ mod tests { #[derive(Default)] struct ConfigurableProtocolConnectionHandler { active_protocols: HashSet, - supported_local_protocols: SupportedProtocols, + added: Vec>, + removed: Vec>, } impl ConnectionHandler for MockConnectionHandler { @@ -1035,8 +1032,14 @@ mod tests { Self::OutboundOpenInfo, >, ) { - if let ConnectionEvent::LocalProtocolsChange(change) = event { - self.supported_local_protocols.on_protocols_change(change); + match event { + ConnectionEvent::LocalProtocolsChange(ProtocolsChange::Added(added)) => { + self.added.push(added.cloned().collect()) + } + ConnectionEvent::LocalProtocolsChange(ProtocolsChange::Removed(removed)) => { + self.removed.push(removed.cloned().collect()) + } + _ => {} } } From a82343a4c5d60948e0ddbe881623b8952055bdbe Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 May 2023 18:29:38 +0200 Subject: [PATCH 38/50] Don't report empty set of protocols to handler --- swarm/src/connection.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/swarm/src/connection.rs b/swarm/src/connection.rs index 2c070aee900..b0d3776e28c 100644 --- a/swarm/src/connection.rs +++ b/swarm/src/connection.rs @@ -184,11 +184,13 @@ where ) -> Self { let initial_protocols = gather_supported_protocols(&handler); - handler.on_connection_event(ConnectionEvent::LocalProtocolsChange( - ProtocolsChange::Added(ProtocolsAdded { - protocols: initial_protocols.difference(&HashSet::new()).peekable(), - }), - )); + if !initial_protocols.is_empty() { + handler.on_connection_event(ConnectionEvent::LocalProtocolsChange( + ProtocolsChange::Added(ProtocolsAdded { + protocols: initial_protocols.difference(&HashSet::new()).peekable(), + }), + )); + } Connection { muxing: muxer, From 19cd9b90d52952d67033148618fafceabd5fa4a5 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 May 2023 18:30:53 +0200 Subject: [PATCH 39/50] Use constructor --- swarm/src/connection.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/swarm/src/connection.rs b/swarm/src/connection.rs index b0d3776e28c..7df6483b565 100644 --- a/swarm/src/connection.rs +++ b/swarm/src/connection.rs @@ -186,9 +186,7 @@ where if !initial_protocols.is_empty() { handler.on_connection_event(ConnectionEvent::LocalProtocolsChange( - ProtocolsChange::Added(ProtocolsAdded { - protocols: initial_protocols.difference(&HashSet::new()).peekable(), - }), + ProtocolsChange::Added(ProtocolsAdded::from_set(&initial_protocols)), )); } From 6d3e9ee3b20f8758655b79ed4e8a11c5bef65def Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 May 2023 18:41:08 +0200 Subject: [PATCH 40/50] Introduce test helper --- swarm/src/connection.rs | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/swarm/src/connection.rs b/swarm/src/connection.rs index 7df6483b565..03b7d84b254 100644 --- a/swarm/src/connection.rs +++ b/swarm/src/connection.rs @@ -436,6 +436,13 @@ where return Poll::Pending; // Nothing can make progress, return `Pending`. } } + + #[cfg(test)] + fn poll_noop_waker( + &mut self, + ) -> Poll, ConnectionError>> { + Pin::new(self).poll(&mut Context::from_waker(futures::task::noop_waker_ref())) + } } fn gather_supported_protocols(handler: &impl ConnectionHandler) -> HashSet { @@ -715,8 +722,7 @@ mod tests { max_negotiating_inbound_streams, ); - let result = Pin::new(&mut connection) - .poll(&mut Context::from_waker(futures::task::noop_waker_ref())); + let result = connection.poll_noop_waker(); assert!(result.is_pending()); assert_eq!( @@ -740,13 +746,11 @@ mod tests { ); connection.handler.open_new_outbound(); - let _ = Pin::new(&mut connection) - .poll(&mut Context::from_waker(futures::task::noop_waker_ref())); + let _ = connection.poll_noop_waker(); std::thread::sleep(upgrade_timeout + Duration::from_secs(1)); - let _ = Pin::new(&mut connection) - .poll(&mut Context::from_waker(futures::task::noop_waker_ref())); + let _ = connection.poll_noop_waker(); assert!(matches!( connection.handler.error.unwrap(), @@ -764,8 +768,7 @@ mod tests { ); connection.handler.active_protocols = HashSet::from([StreamProtocol::new("/foo")]); - let _ = Pin::new(&mut connection) - .poll(&mut Context::from_waker(futures::task::noop_waker_ref())); + let _ = connection.poll_noop_waker(); assert_eq!( connection.handler.added, vec![vec![StreamProtocol::new("/foo")]] @@ -775,8 +778,7 @@ mod tests { connection.handler.active_protocols = HashSet::from([StreamProtocol::new("/foo"), StreamProtocol::new("/bar")]); - let _ = Pin::new(&mut connection) - .poll(&mut Context::from_waker(futures::task::noop_waker_ref())); + let _ = connection.poll_noop_waker(); assert_eq!( connection.handler.added, @@ -789,8 +791,7 @@ mod tests { connection.handler.active_protocols = HashSet::from([StreamProtocol::new("/bar")]); - let _ = Pin::new(&mut connection) - .poll(&mut Context::from_waker(futures::task::noop_waker_ref())); + let _ = connection.poll_noop_waker(); assert_eq!( connection.handler.added, From df93a4e5c6a3006d08447e1fa8ad774bb1f4dbab Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 May 2023 18:45:42 +0200 Subject: [PATCH 41/50] Make tests less noisy --- swarm/src/connection.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/swarm/src/connection.rs b/swarm/src/connection.rs index 03b7d84b254..754d43eacb4 100644 --- a/swarm/src/connection.rs +++ b/swarm/src/connection.rs @@ -766,18 +766,17 @@ mod tests { None, 0, ); - connection.handler.active_protocols = HashSet::from([StreamProtocol::new("/foo")]); + connection.handler.listen_on(&["/foo"]); let _ = connection.poll_noop_waker(); + assert_eq!( connection.handler.added, vec![vec![StreamProtocol::new("/foo")]] ); assert!(connection.handler.removed.is_empty()); - connection.handler.active_protocols = - HashSet::from([StreamProtocol::new("/foo"), StreamProtocol::new("/bar")]); - + connection.handler.listen_on(&["/foo", "/bar"]); let _ = connection.poll_noop_waker(); assert_eq!( @@ -789,8 +788,7 @@ mod tests { ); assert!(connection.handler.removed.is_empty()); - connection.handler.active_protocols = HashSet::from([StreamProtocol::new("/bar")]); - + connection.handler.listen_on(&["/bar"]); let _ = connection.poll_noop_waker(); assert_eq!( @@ -930,6 +928,12 @@ mod tests { removed: Vec>, } + impl ConfigurableProtocolConnectionHandler { + fn listen_on(&mut self, protocols: &[&'static str]) { + self.active_protocols = protocols.iter().copied().map(StreamProtocol::new).collect(); + } + } + impl ConnectionHandler for MockConnectionHandler { type InEvent = Void; type OutEvent = Void; From c50bcfd6bbac1655e80b036fd77aefa82b4c0ba7 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 May 2023 18:50:43 +0200 Subject: [PATCH 42/50] Further simplify test and add comments --- swarm/src/connection.rs | 40 ++++++++++++++++------------------------ 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/swarm/src/connection.rs b/swarm/src/connection.rs index 754d43eacb4..b6cb755201a 100644 --- a/swarm/src/connection.rs +++ b/swarm/src/connection.rs @@ -767,41 +767,33 @@ mod tests { 0, ); + // First, start listening on a single protocol. connection.handler.listen_on(&["/foo"]); let _ = connection.poll_noop_waker(); - assert_eq!( - connection.handler.added, - vec![vec![StreamProtocol::new("/foo")]] - ); - assert!(connection.handler.removed.is_empty()); + assert_eq!(connection.handler.local_added, vec![vec!["/foo"]]); + assert!(connection.handler.local_removed.is_empty()); + // Second, listen on two protocols. connection.handler.listen_on(&["/foo", "/bar"]); let _ = connection.poll_noop_waker(); assert_eq!( - connection.handler.added, - vec![ - vec![StreamProtocol::new("/foo")], - vec![StreamProtocol::new("/bar")] - ] + connection.handler.local_added, + vec![vec!["/foo"], vec!["/bar"]], + "expect to only receive an event for the newly added protocols" ); - assert!(connection.handler.removed.is_empty()); + assert!(connection.handler.local_removed.is_empty()); + // Third, stop listening on the first protocol. connection.handler.listen_on(&["/bar"]); let _ = connection.poll_noop_waker(); assert_eq!( - connection.handler.added, - vec![ - vec![StreamProtocol::new("/foo")], - vec![StreamProtocol::new("/bar")] - ] - ); - assert_eq!( - connection.handler.removed, - vec![vec![StreamProtocol::new("/foo")]] + connection.handler.local_added, + vec![vec!["/foo"], vec!["/bar"]] ); + assert_eq!(connection.handler.local_removed, vec![vec!["/foo"]]); } struct DummyStreamMuxer { @@ -924,8 +916,8 @@ mod tests { #[derive(Default)] struct ConfigurableProtocolConnectionHandler { active_protocols: HashSet, - added: Vec>, - removed: Vec>, + local_added: Vec>, + local_removed: Vec>, } impl ConfigurableProtocolConnectionHandler { @@ -1039,10 +1031,10 @@ mod tests { ) { match event { ConnectionEvent::LocalProtocolsChange(ProtocolsChange::Added(added)) => { - self.added.push(added.cloned().collect()) + self.local_added.push(added.cloned().collect()) } ConnectionEvent::LocalProtocolsChange(ProtocolsChange::Removed(removed)) => { - self.removed.push(removed.cloned().collect()) + self.local_removed.push(removed.cloned().collect()) } _ => {} } From a799798a6812e5bf803d72fbb60c33a5440f7f40 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 May 2023 19:00:40 +0200 Subject: [PATCH 43/50] Add failing test --- swarm/src/connection.rs | 81 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/swarm/src/connection.rs b/swarm/src/connection.rs index b6cb755201a..dfbaabf9773 100644 --- a/swarm/src/connection.rs +++ b/swarm/src/connection.rs @@ -796,6 +796,56 @@ mod tests { assert_eq!(connection.handler.local_removed, vec![vec!["/foo"]]); } + #[test] + fn only_propagtes_actual_changes_to_remote_protocols_to_handler() { + let mut connection = Connection::new( + StreamMuxerBox::new(PendingStreamMuxer), + ConfigurableProtocolConnectionHandler::default(), + None, + 0, + ); + + // First, remote supports a single protocol. + connection.handler.remote_adds_support_for(&["/foo"]); + let _ = connection.poll_noop_waker(); + + assert_eq!(connection.handler.remote_added, vec![vec!["/foo"]]); + assert!(connection.handler.remote_removed.is_empty()); + + // Second, it adds a protocol but also still includes the first one. + connection + .handler + .remote_adds_support_for(&["/foo", "/bar"]); + let _ = connection.poll_noop_waker(); + + assert_eq!( + connection.handler.remote_added, + vec![vec!["/foo"], vec!["/bar"]], + "expect to only receive an event for the newly added protocol" + ); + assert!(connection.handler.remote_removed.is_empty()); + + // Third, stop listening on a protocol it never advertised (we can't control what handlers do so this needs to be handled gracefully). + connection.handler.remote_removes_support_for(&["/baz"]); + let _ = connection.poll_noop_waker(); + + assert_eq!( + connection.handler.remote_added, + vec![vec!["/foo"], vec!["/bar"]] + ); + assert!(&connection.handler.remote_removed.is_empty()); + + // Fourth, stop listening on a protocol that was previously supported + connection.handler.remote_removes_support_for(&["/bar"]); + let _ = connection.poll_noop_waker(); + + assert_eq!( + connection.handler.remote_added, + vec![vec!["/foo"], vec!["/bar"]] + ); + assert_eq!(connection.handler.remote_removed, vec![vec!["/bar"]]); + } + struct DummyStreamMuxer { counter: Arc<()>, } @@ -915,15 +965,36 @@ mod tests { #[derive(Default)] struct ConfigurableProtocolConnectionHandler { + events: Vec>, active_protocols: HashSet, local_added: Vec>, local_removed: Vec>, + remote_added: Vec>, + remote_removed: Vec>, } impl ConfigurableProtocolConnectionHandler { fn listen_on(&mut self, protocols: &[&'static str]) { self.active_protocols = protocols.iter().copied().map(StreamProtocol::new).collect(); } + + fn remote_adds_support_for(&mut self, protocols: &[&'static str]) { + self.events + .push(ConnectionHandlerEvent::ReportRemoteProtocols( + ProtocolSupport::Added( + protocols.iter().copied().map(StreamProtocol::new).collect(), + ), + )); + } + + fn remote_removes_support_for(&mut self, protocols: &[&'static str]) { + self.events + .push(ConnectionHandlerEvent::ReportRemoteProtocols( + ProtocolSupport::Removed( + protocols.iter().copied().map(StreamProtocol::new).collect(), + ), + )); + } } impl ConnectionHandler for MockConnectionHandler { @@ -1036,6 +1107,12 @@ mod tests { ConnectionEvent::LocalProtocolsChange(ProtocolsChange::Removed(removed)) => { self.local_removed.push(removed.cloned().collect()) } + ConnectionEvent::RemoteProtocolsChange(ProtocolsChange::Added(added)) => { + self.remote_added.push(added.cloned().collect()) + } + ConnectionEvent::RemoteProtocolsChange(ProtocolsChange::Removed(removed)) => { + self.remote_removed.push(removed.cloned().collect()) + } _ => {} } } @@ -1059,6 +1136,10 @@ mod tests { Self::Error, >, > { + if let Some(event) = self.events.pop() { + return Poll::Ready(event); + } + Poll::Pending } } From 0260ad17e82a304c7621d4d9306ddeb73ac676fc Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 May 2023 19:10:39 +0200 Subject: [PATCH 44/50] Make test pass, i.e. only report actual changes back to the handler --- swarm/src/connection.rs | 36 +++++++++++++++++++++++++----------- swarm/src/handler.rs | 11 ++++++++--- 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/swarm/src/connection.rs b/swarm/src/connection.rs index dfbaabf9773..c7840498ff6 100644 --- a/swarm/src/connection.rs +++ b/swarm/src/connection.rs @@ -38,6 +38,7 @@ use crate::upgrade::{InboundUpgradeSend, OutboundUpgradeSend, SendWrapper, Upgra use crate::{ ConnectionHandlerEvent, ConnectionHandlerUpgrErr, KeepAlive, StreamProtocol, SubstreamProtocol, }; +use either::Either; use futures::stream::FuturesUnordered; use futures::FutureExt; use futures::StreamExt; @@ -153,7 +154,7 @@ where >, local_supported_protocols: HashSet, - remote_supported_protocols: SupportedProtocols, + remote_supported_protocols: HashSet, } impl fmt::Debug for Connection @@ -200,7 +201,7 @@ where max_negotiating_inbound_streams, requested_substreams: Default::default(), local_supported_protocols: initial_protocols, - remote_supported_protocols: SupportedProtocols::default(), + remote_supported_protocols: Default::default(), } } @@ -268,11 +269,17 @@ where Poll::Ready(ConnectionHandlerEvent::ReportRemoteProtocols( ProtocolSupport::Added(protocols), )) => { - let change = ProtocolsChange::Added(ProtocolsAdded::from_set(&protocols)); + let mut actually_added_protocols = + protocols.difference(remote_supported_protocols).peekable(); + + if actually_added_protocols.peek().is_some() { + handler.on_connection_event(ConnectionEvent::RemoteProtocolsChange( + ProtocolsChange::Added(ProtocolsAdded { + protocols: actually_added_protocols, + }), + )); - if remote_supported_protocols.on_protocols_change(change.clone()) { - handler.on_connection_event(ConnectionEvent::RemoteProtocolsChange(change)); - // TODO: Should we optimise this to be the _actual_ change? + remote_supported_protocols.extend(protocols); } continue; @@ -280,11 +287,18 @@ where Poll::Ready(ConnectionHandlerEvent::ReportRemoteProtocols( ProtocolSupport::Removed(protocols), )) => { - let change = ProtocolsChange::Removed(ProtocolsRemoved::from_set(&protocols)); + let mut actually_removed_protocols = remote_supported_protocols + .intersection(&protocols) + .peekable(); + + if actually_removed_protocols.peek().is_some() { + handler.on_connection_event(ConnectionEvent::RemoteProtocolsChange( + ProtocolsChange::Removed(ProtocolsRemoved { + protocols: Either::Right(actually_removed_protocols), + }), + )); - if remote_supported_protocols.on_protocols_change(change.clone()) { - handler.on_connection_event(ConnectionEvent::RemoteProtocolsChange(change)); - // TODO: Should we optimise this to be the _actual_ change? + remote_supported_protocols.retain(|p| !protocols.contains(p)); } continue; @@ -425,7 +439,7 @@ where if removed_protocols.peek().is_some() { handler.on_connection_event(ConnectionEvent::LocalProtocolsChange( ProtocolsChange::Removed(ProtocolsRemoved { - protocols: removed_protocols, + protocols: Either::Left(removed_protocols), }), )); } diff --git a/swarm/src/handler.rs b/swarm/src/handler.rs index 3092ac750e0..bca19f6a2ae 100644 --- a/swarm/src/handler.rs +++ b/swarm/src/handler.rs @@ -48,12 +48,13 @@ mod select; pub use crate::upgrade::{InboundUpgradeSend, OutboundUpgradeSend, SendWrapper, UpgradeInfoSend}; +use ::either::Either; use instant::Instant; use libp2p_core::{upgrade::UpgradeError, ConnectedPoint, Multiaddr}; use libp2p_identity::PeerId; use once_cell::sync::Lazy; use std::collections::hash_map::RandomState; -use std::collections::hash_set::Difference; +use std::collections::hash_set::{Difference, Intersection}; use std::collections::HashSet; use std::iter::Peekable; use std::{cmp::Ordering, error, fmt, task::Context, task::Poll, time::Duration}; @@ -309,13 +310,17 @@ impl<'a> ProtocolsAdded<'a> { /// An [`Iterator`] over all protocols that have been removed. #[derive(Clone)] pub struct ProtocolsRemoved<'a> { - pub(crate) protocols: Peekable>, + pub(crate) protocols: Either< + Peekable>, + Peekable>, + >, } impl<'a> ProtocolsRemoved<'a> { + #[cfg(test)] pub(crate) fn from_set(protocols: &'a HashSet) -> Self { ProtocolsRemoved { - protocols: protocols.difference(&EMPTY_HASHSET).peekable(), + protocols: Either::Left(protocols.difference(&EMPTY_HASHSET).peekable()), } } } From bf99654e3d54aabc22ce8f012511c086ade6df87 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 May 2023 19:30:31 +0200 Subject: [PATCH 45/50] Extract helpers to make fields crate-private and add docs --- swarm/src/connection.rs | 57 ++++++++++----------------------- swarm/src/handler.rs | 70 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 84 insertions(+), 43 deletions(-) diff --git a/swarm/src/connection.rs b/swarm/src/connection.rs index c7840498ff6..4efbe33e5bb 100644 --- a/swarm/src/connection.rs +++ b/swarm/src/connection.rs @@ -32,13 +32,11 @@ pub use supported_protocols::SupportedProtocols; use crate::handler::{ AddressChange, ConnectionEvent, ConnectionHandler, DialUpgradeError, FullyNegotiatedInbound, FullyNegotiatedOutbound, ListenUpgradeError, ProtocolSupport, ProtocolsAdded, ProtocolsChange, - ProtocolsRemoved, }; use crate::upgrade::{InboundUpgradeSend, OutboundUpgradeSend, SendWrapper, UpgradeInfoSend}; use crate::{ ConnectionHandlerEvent, ConnectionHandlerUpgrErr, KeepAlive, StreamProtocol, SubstreamProtocol, }; -use either::Either; use futures::stream::FuturesUnordered; use futures::FutureExt; use futures::StreamExt; @@ -269,16 +267,10 @@ where Poll::Ready(ConnectionHandlerEvent::ReportRemoteProtocols( ProtocolSupport::Added(protocols), )) => { - let mut actually_added_protocols = - protocols.difference(remote_supported_protocols).peekable(); - - if actually_added_protocols.peek().is_some() { - handler.on_connection_event(ConnectionEvent::RemoteProtocolsChange( - ProtocolsChange::Added(ProtocolsAdded { - protocols: actually_added_protocols, - }), - )); - + if let Some(added) = + ProtocolsChange::add(remote_supported_protocols, &protocols) + { + handler.on_connection_event(ConnectionEvent::RemoteProtocolsChange(added)); remote_supported_protocols.extend(protocols); } @@ -287,17 +279,11 @@ where Poll::Ready(ConnectionHandlerEvent::ReportRemoteProtocols( ProtocolSupport::Removed(protocols), )) => { - let mut actually_removed_protocols = remote_supported_protocols - .intersection(&protocols) - .peekable(); - - if actually_removed_protocols.peek().is_some() { - handler.on_connection_event(ConnectionEvent::RemoteProtocolsChange( - ProtocolsChange::Removed(ProtocolsRemoved { - protocols: Either::Right(actually_removed_protocols), - }), - )); - + if let Some(removed) = + ProtocolsChange::remove(remote_supported_protocols, &protocols) + { + handler + .on_connection_event(ConnectionEvent::RemoteProtocolsChange(removed)); remote_supported_protocols.retain(|p| !protocols.contains(p)); } @@ -423,26 +409,15 @@ where let new_protocols = gather_supported_protocols(handler); - if &new_protocols != supported_protocols { - let mut added_protocols = new_protocols.difference(supported_protocols).peekable(); - let mut removed_protocols = - supported_protocols.difference(&new_protocols).peekable(); + let (added, removed) = + ProtocolsChange::from_full_sets(supported_protocols, &new_protocols); - if added_protocols.peek().is_some() { - handler.on_connection_event(ConnectionEvent::LocalProtocolsChange( - ProtocolsChange::Added(ProtocolsAdded { - protocols: added_protocols, - }), - )); - } + if let Some(added) = added { + handler.on_connection_event(ConnectionEvent::LocalProtocolsChange(added)); + } - if removed_protocols.peek().is_some() { - handler.on_connection_event(ConnectionEvent::LocalProtocolsChange( - ProtocolsChange::Removed(ProtocolsRemoved { - protocols: Either::Left(removed_protocols), - }), - )); - } + if let Some(removed) = removed { + handler.on_connection_event(ConnectionEvent::LocalProtocolsChange(removed)); } *supported_protocols = new_protocols; diff --git a/swarm/src/handler.rs b/swarm/src/handler.rs index bca19f6a2ae..e31558e301c 100644 --- a/swarm/src/handler.rs +++ b/swarm/src/handler.rs @@ -293,10 +293,76 @@ pub enum ProtocolsChange<'a> { Removed(ProtocolsRemoved<'a>), } +impl<'a> ProtocolsChange<'a> { + /// Compute the [`ProtocolsChange`] that results from adding `to_add` to `existing_protocols`. + /// + /// Returns `None` if the change is a no-op, i.e. `to_add` is a subset of `existing_protocols`. + pub(crate) fn add( + existing_protocols: &'a HashSet, + to_add: &'a HashSet, + ) -> Option { + let mut actually_added_protocols = to_add.difference(existing_protocols).peekable(); + + if actually_added_protocols.peek().is_none() { + return None; + } + + Some(ProtocolsChange::Added(ProtocolsAdded { + protocols: actually_added_protocols, + })) + } + + /// Compute the [`ProtocolsChange`] that results from removing `to_remove` from `existing_protocols`. + /// + /// Returns `None` if the change is a no-op, i.e. none of the protocols in `to_remove` are in `existing_protocols`. + pub(crate) fn remove( + existing_protocols: &'a HashSet, + to_remove: &'a HashSet, + ) -> Option { + let mut actually_removed_protocols = existing_protocols.intersection(&to_remove).peekable(); + + if actually_removed_protocols.peek().is_none() { + return None; + } + + Some(ProtocolsChange::Removed(ProtocolsRemoved { + protocols: Either::Right(actually_removed_protocols), + })) + } + + /// Compute the [`ProtocolsChange`]s required to go from `existing_protocols` to `new_protocols`. + pub(crate) fn from_full_sets( + existing_protocols: &'a HashSet, + new_protocols: &'a HashSet, + ) -> (Option, Option) { + if existing_protocols == new_protocols { + return (None, None); + } + + let mut added_protocols = new_protocols.difference(existing_protocols).peekable(); + let mut removed_protocols = existing_protocols.difference(&new_protocols).peekable(); + + let added = added_protocols + .peek() + .is_some() + .then_some(ProtocolsChange::Added(ProtocolsAdded { + protocols: added_protocols, + })); + let removed = removed_protocols + .peek() + .is_some() + .then_some(ProtocolsChange::Removed(ProtocolsRemoved { + protocols: Either::Left(removed_protocols), + })); + + (added, removed) + } +} + /// An [`Iterator`] over all protocols that have been added. #[derive(Clone)] pub struct ProtocolsAdded<'a> { - pub(crate) protocols: Peekable>, + protocols: Peekable>, } impl<'a> ProtocolsAdded<'a> { @@ -310,7 +376,7 @@ impl<'a> ProtocolsAdded<'a> { /// An [`Iterator`] over all protocols that have been removed. #[derive(Clone)] pub struct ProtocolsRemoved<'a> { - pub(crate) protocols: Either< + protocols: Either< Peekable>, Peekable>, >, From 46f4e9679b17388177cd5bed2b9940ecef71ce4a Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 May 2023 19:39:23 +0200 Subject: [PATCH 46/50] Return `SmallVec` from `from_full_sets` which allows iteration --- swarm/src/connection.rs | 11 ++--------- swarm/src/handler.rs | 24 +++++++++++++----------- 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/swarm/src/connection.rs b/swarm/src/connection.rs index 4efbe33e5bb..d247e8bae00 100644 --- a/swarm/src/connection.rs +++ b/swarm/src/connection.rs @@ -409,15 +409,8 @@ where let new_protocols = gather_supported_protocols(handler); - let (added, removed) = - ProtocolsChange::from_full_sets(supported_protocols, &new_protocols); - - if let Some(added) = added { - handler.on_connection_event(ConnectionEvent::LocalProtocolsChange(added)); - } - - if let Some(removed) = removed { - handler.on_connection_event(ConnectionEvent::LocalProtocolsChange(removed)); + for change in ProtocolsChange::from_full_sets(supported_protocols, &new_protocols) { + handler.on_connection_event(ConnectionEvent::LocalProtocolsChange(change)); } *supported_protocols = new_protocols; diff --git a/swarm/src/handler.rs b/swarm/src/handler.rs index e31558e301c..566d1582e6b 100644 --- a/swarm/src/handler.rs +++ b/swarm/src/handler.rs @@ -53,6 +53,7 @@ use instant::Instant; use libp2p_core::{upgrade::UpgradeError, ConnectedPoint, Multiaddr}; use libp2p_identity::PeerId; use once_cell::sync::Lazy; +use smallvec::SmallVec; use std::collections::hash_map::RandomState; use std::collections::hash_set::{Difference, Intersection}; use std::collections::HashSet; @@ -334,28 +335,29 @@ impl<'a> ProtocolsChange<'a> { pub(crate) fn from_full_sets( existing_protocols: &'a HashSet, new_protocols: &'a HashSet, - ) -> (Option, Option) { + ) -> SmallVec<[Self; 2]> { if existing_protocols == new_protocols { - return (None, None); + return SmallVec::new(); } + let mut changes = SmallVec::new(); + let mut added_protocols = new_protocols.difference(existing_protocols).peekable(); let mut removed_protocols = existing_protocols.difference(&new_protocols).peekable(); - let added = added_protocols - .peek() - .is_some() - .then_some(ProtocolsChange::Added(ProtocolsAdded { + if added_protocols.peek().is_some() { + changes.push(ProtocolsChange::Added(ProtocolsAdded { protocols: added_protocols, })); - let removed = removed_protocols - .peek() - .is_some() - .then_some(ProtocolsChange::Removed(ProtocolsRemoved { + } + + if removed_protocols.peek().is_some() { + changes.push(ProtocolsChange::Removed(ProtocolsRemoved { protocols: Either::Left(removed_protocols), })); + } - (added, removed) + changes } } From ae7fc939da0be63a6cad9ffdcb38d96db0427cdf Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 May 2023 19:57:44 +0200 Subject: [PATCH 47/50] Fix clippy warnings --- swarm/src/handler.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/swarm/src/handler.rs b/swarm/src/handler.rs index 566d1582e6b..fbf90edc67d 100644 --- a/swarm/src/handler.rs +++ b/swarm/src/handler.rs @@ -304,9 +304,7 @@ impl<'a> ProtocolsChange<'a> { ) -> Option { let mut actually_added_protocols = to_add.difference(existing_protocols).peekable(); - if actually_added_protocols.peek().is_none() { - return None; - } + actually_added_protocols.peek()?; Some(ProtocolsChange::Added(ProtocolsAdded { protocols: actually_added_protocols, @@ -320,11 +318,9 @@ impl<'a> ProtocolsChange<'a> { existing_protocols: &'a HashSet, to_remove: &'a HashSet, ) -> Option { - let mut actually_removed_protocols = existing_protocols.intersection(&to_remove).peekable(); + let mut actually_removed_protocols = existing_protocols.intersection(to_remove).peekable(); - if actually_removed_protocols.peek().is_none() { - return None; - } + actually_removed_protocols.peek()?; Some(ProtocolsChange::Removed(ProtocolsRemoved { protocols: Either::Right(actually_removed_protocols), @@ -343,7 +339,7 @@ impl<'a> ProtocolsChange<'a> { let mut changes = SmallVec::new(); let mut added_protocols = new_protocols.difference(existing_protocols).peekable(); - let mut removed_protocols = existing_protocols.difference(&new_protocols).peekable(); + let mut removed_protocols = existing_protocols.difference(new_protocols).peekable(); if added_protocols.peek().is_some() { changes.push(ProtocolsChange::Added(ProtocolsAdded { From fe9a6e306ea497cdde054154ea7831894215c70b Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 May 2023 19:58:57 +0200 Subject: [PATCH 48/50] Fix rustdoc --- swarm/src/handler.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/swarm/src/handler.rs b/swarm/src/handler.rs index fbf90edc67d..023c3a60586 100644 --- a/swarm/src/handler.rs +++ b/swarm/src/handler.rs @@ -759,6 +759,5 @@ impl Ord for KeepAlive { } /// A statically declared, empty [`HashSet`] allows us to work around borrow-checker rules for -/// [`ProtocolsAdded::from_set`] and [`ProtocolsRemoved::from_set`]. Those have lifetime-constraints -/// which don't work unless we have a [`HashSet`] with a `'static' lifetime. +/// [`ProtocolsAdded::from_set`]. The lifetimes don't work unless we have a [`HashSet`] with a `'static' lifetime. static EMPTY_HASHSET: Lazy> = Lazy::new(HashSet::new); From 75681c170251b6e814ab646e0100e93a7cf5d607 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Mon, 8 May 2023 10:23:15 +0200 Subject: [PATCH 49/50] Add changelog entry --- swarm/CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/swarm/CHANGELOG.md b/swarm/CHANGELOG.md index 64de61ff521..90270a54025 100644 --- a/swarm/CHANGELOG.md +++ b/swarm/CHANGELOG.md @@ -18,12 +18,21 @@ This variant was never constructed and thus dead code. See [PR 3605]. +- Allow `ConnectionHandler`s to report and learn about the supported protocols on a connection. + The newly introduced API elements are: + - `ConnectionHandlerEvent::ReportRemoteProtocols` + - `ConnectionEvent::LocalProtocolsChange` + - `ConnectionEvent::RemoteProtocolsChange` + + See [PR 3651]. + [PR 3605]: https://github.com/libp2p/rust-libp2p/pull/3605 [PR 3746]: https://github.com/libp2p/rust-libp2p/pull/3746 [PR 3715]: https://github.com/libp2p/rust-libp2p/pull/3715 [PR 3746]: https://github.com/libp2p/rust-libp2p/pull/3746 [PR 3865]: https://github.com/libp2p/rust-libp2p/pull/3865 [PR 3886]: https://github.com/libp2p/rust-libp2p/pull/3886 +[PR 3651]: https://github.com/libp2p/rust-libp2p/pull/3651 ## 0.42.2 From 88362e5df598b455a3cf85316c25045a974657da Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Mon, 8 May 2023 10:43:02 +0200 Subject: [PATCH 50/50] Sort imports --- swarm/src/handler.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/swarm/src/handler.rs b/swarm/src/handler.rs index 44c2a440bdc..0c3416fe1e7 100644 --- a/swarm/src/handler.rs +++ b/swarm/src/handler.rs @@ -47,7 +47,13 @@ mod pending; mod select; pub use crate::upgrade::{InboundUpgradeSend, OutboundUpgradeSend, SendWrapper, UpgradeInfoSend}; +pub use map_in::MapInEvent; +pub use map_out::MapOutEvent; +pub use one_shot::{OneShotHandler, OneShotHandlerConfig}; +pub use pending::PendingConnectionHandler; +pub use select::ConnectionHandlerSelect; +use crate::StreamProtocol; use ::either::Either; use instant::Instant; use libp2p_core::{upgrade::UpgradeError, Multiaddr}; @@ -59,13 +65,6 @@ use std::collections::HashSet; use std::iter::Peekable; use std::{cmp::Ordering, error, fmt, task::Context, task::Poll, time::Duration}; -use crate::StreamProtocol; -pub use map_in::MapInEvent; -pub use map_out::MapOutEvent; -pub use one_shot::{OneShotHandler, OneShotHandlerConfig}; -pub use pending::PendingConnectionHandler; -pub use select::ConnectionHandlerSelect; - /// A handler for a set of protocols used on a connection with a remote. /// /// This trait should be implemented for a type that maintains the state for