From a332591c8dd9317dd616aca4c8206f4b23829d6c Mon Sep 17 00:00:00 2001 From: Max Inden Date: Thu, 12 Aug 2021 16:17:37 +0200 Subject: [PATCH 01/38] core/: Return handler on connection error and closed --- core/src/connection.rs | 6 ++-- core/src/connection/manager.rs | 4 ++- core/src/connection/manager/task.rs | 26 ++++++++++++--- core/src/connection/pool.rs | 52 ++++++++++++++++++----------- core/src/network.rs | 37 ++++++++++---------- core/src/network/event.rs | 17 ++++++++-- 6 files changed, 94 insertions(+), 48 deletions(-) diff --git a/core/src/connection.rs b/core/src/connection.rs index 335e2046c2d..513665a631a 100644 --- a/core/src/connection.rs +++ b/core/src/connection.rs @@ -229,10 +229,12 @@ where self.handler.inject_event(event); } + // TODO: Update comment + // /// Begins an orderly shutdown of the connection, returning a /// `Future` that resolves when connection shutdown is complete. - pub fn close(self) -> Close { - self.muxing.close().0 + pub fn close(self) -> (THandler, Close) { + (self.handler, self.muxing.close().0) } /// Polls the connection for events produced by the associated handler diff --git a/core/src/connection/manager.rs b/core/src/connection/manager.rs index 1d7acb92e69..037364ca159 100644 --- a/core/src/connection/manager.rs +++ b/core/src/connection/manager.rs @@ -192,6 +192,7 @@ pub enum Event<'a, H: IntoConnectionHandler, TE> { /// The error that occurred, if any. If `None`, the connection /// has been actively closed. error: Option>>, + handler: H::Handler, }, /// A connection has been established. @@ -384,7 +385,7 @@ impl Manager { new_endpoint: new, } } - task::Event::Closed { id, error } => { + task::Event::Closed { id, error, handler } => { let id = ConnectionId(id); let task = task.remove(); match task.state { @@ -392,6 +393,7 @@ impl Manager { id, connected, error, + handler, }, TaskState::Pending => unreachable!( "`Event::Closed` implies (2) occurred on that task and thus (3)." diff --git a/core/src/connection/manager/task.rs b/core/src/connection/manager/task.rs index db8fb43adb6..f81c520e6e9 100644 --- a/core/src/connection/manager/task.rs +++ b/core/src/connection/manager/task.rs @@ -71,6 +71,7 @@ pub enum Event { Closed { id: TaskId, error: Option>>, + handler: H::Handler, }, } @@ -177,7 +178,7 @@ where }, /// The connection is closing (active close). - Closing(Close), + Closing { closing_muxer: Close, handler: H::Handler }, /// The task is terminating with a final event for the `Manager`. Terminating(Event), @@ -265,7 +266,11 @@ where // Don't accept any further commands. this.commands.get_mut().close(); // Discard the event, if any, and start a graceful close. - this.state = State::Closing(connection.close()); + let (handler, closing_muxer) = connection.close(); + this.state = State::Closing { + handler, + closing_muxer, + }; continue 'poll; } Poll::Ready(None) => { @@ -324,10 +329,13 @@ where Poll::Ready(Err(error)) => { // Don't accept any further commands. this.commands.get_mut().close(); + // TODO: Good idea if there is already an error? + let (handler, _) = connection.close(); // Terminate the task with the error, dropping the connection. let event = Event::Closed { id, error: Some(error), + handler, }; this.state = State::Terminating(event); } @@ -335,13 +343,17 @@ where } } - State::Closing(mut closing) => { + State::Closing { + handler, + mut closing_muxer, + } => { // Try to gracefully close the connection. - match closing.poll_unpin(cx) { + match closing_muxer.poll_unpin(cx) { Poll::Ready(Ok(())) => { let event = Event::Closed { id: this.id, error: None, + handler, }; this.state = State::Terminating(event); } @@ -349,11 +361,15 @@ where let event = Event::Closed { id: this.id, error: Some(ConnectionError::IO(e)), + handler, }; this.state = State::Terminating(event); } Poll::Pending => { - this.state = State::Closing(closing); + this.state = State::Closing { + handler, + closing_muxer, + }; return Poll::Pending; } } diff --git a/core/src/connection/pool.rs b/core/src/connection/pool.rs index 9925dd526c0..11d030ccaef 100644 --- a/core/src/connection/pool.rs +++ b/core/src/connection/pool.rs @@ -102,6 +102,7 @@ pub enum PoolEvent<'a, THandler: IntoConnectionHandler, TTransErr> { pool: &'a mut Pool, /// The remaining number of established connections to the same peer. num_established: u32, + handler: THandler::Handler, }, /// A connection attempt failed. @@ -114,7 +115,7 @@ pub enum PoolEvent<'a, THandler: IntoConnectionHandler, TTransErr> { error: PendingConnectionError, /// The handler that was supposed to handle the connection, /// if the connection failed before the handler was consumed. - handler: Option, + handler: THandler, /// The (expected) peer of the failed connection. peer: Option, /// A reference to the pool that managed the connection. @@ -554,6 +555,7 @@ impl Pool { num_established, error: None, pool: self, + handler: todo!(), }); } @@ -572,7 +574,7 @@ impl Pool { id, endpoint, error, - handler: Some(handler), + handler: handler, peer, pool: self, }); @@ -582,6 +584,7 @@ impl Pool { id, connected, error, + handler, } => { let num_established = if let Some(conns) = self.established.get_mut(&connected.peer_id) { @@ -601,6 +604,7 @@ impl Pool { error, num_established, pool: self, + handler, }); } manager::Event::ConnectionEstablished { entry } => { @@ -610,30 +614,38 @@ impl Pool { // Check general established connection limit. if let Err(e) = self.counters.check_max_established(&endpoint) { - let connected = entry.remove(); - return Poll::Ready(PoolEvent::PendingConnectionError { - id, - endpoint: connected.endpoint, - error: PendingConnectionError::ConnectionLimit(e), - handler: None, - peer, - pool: self, - }); + // TODO: Good idea? How should we let the user know that the close + // happened due to a conneciton limit? + entry.start_close(); + // let connected = entry.remove(); + // return Poll::Ready(PoolEvent::PendingConnectionError { + // id, + // endpoint: connected.endpoint, + // error: PendingConnectionError::ConnectionLimit(e), + // handler: None, + // peer, + // pool: self, + // }); + continue; } // Check per-peer established connection limit. let current = num_peer_established(&self.established, &entry.connected().peer_id); if let Err(e) = self.counters.check_max_established_per_peer(current) { - let connected = entry.remove(); - return Poll::Ready(PoolEvent::PendingConnectionError { - id, - endpoint: connected.endpoint, - error: PendingConnectionError::ConnectionLimit(e), - handler: None, - peer, - pool: self, - }); + // TODO: Good idea? How should we let the user know that the close + // happened due to a conneciton limit? + entry.start_close(); + // let connected = entry.remove(); + // return Poll::Ready(PoolEvent::PendingConnectionError { + // id, + // endpoint: connected.endpoint, + // error: PendingConnectionError::ConnectionLimit(e), + // handler: None, + // peer, + // pool: self, + // }); + continue; } // Peer ID checks must already have happened. See `add_pending`. diff --git a/core/src/network.rs b/core/src/network.rs index 784c1e01ca7..3342afc55df 100644 --- a/core/src/network.rs +++ b/core/src/network.rs @@ -22,7 +22,7 @@ mod event; pub mod peer; pub use crate::connection::{ConnectionCounters, ConnectionLimits}; -pub use event::{IncomingConnection, NetworkEvent}; +pub use event::{IncomingConnection, NetworkEvent, DialAttemptsRemaining}; pub use peer::Peer; use crate::{ @@ -438,6 +438,7 @@ where log::warn!("Dialing aborted: {:?}", e); } } + // TODO: Include handler in event. event } Poll::Ready(PoolEvent::ConnectionClosed { @@ -445,12 +446,14 @@ where connected, error, num_established, + handler, .. }) => NetworkEvent::ConnectionClosed { id, connected, num_established, error, + handler, }, Poll::Ready(PoolEvent::ConnectionEvent { connection, event }) => { NetworkEvent::ConnectionEvent { connection, event } @@ -563,7 +566,7 @@ fn on_connection_failed<'a, TTrans, THandler>( id: ConnectionId, endpoint: ConnectedPoint, error: PendingConnectionError, - handler: Option, + handler: THandler, ) -> ( Option>, NetworkEvent<'a, TTrans, THandlerInEvent, THandlerOutEvent, THandler>, @@ -592,27 +595,21 @@ where let failed_addr = attempt.current.1.clone(); let (opts, attempts_remaining) = if num_remain > 0 { - if let Some(handler) = handler { - let next_attempt = attempt.remaining.remove(0); - let opts = DialingOpts { - peer: peer_id, - handler, - address: next_attempt, - remaining: attempt.remaining, - }; - (Some(opts), num_remain) - } else { - // The error is "fatal" for the dialing attempt, since - // the handler was already consumed. All potential - // remaining connection attempts are thus void. - (None, 0) - } + let next_attempt = attempt.remaining.remove(0); + let opts = DialingOpts { + peer: peer_id, + handler, + address: next_attempt, + remaining: attempt.remaining, + }; + (Some(opts), DialAttemptsRemaining::Some(num_remain)) } else { - (None, 0) + (None, DialAttemptsRemaining::None(handler)) }; ( opts, + // TODO: This is the place to return the handler. NetworkEvent::DialError { attempts_remaining, peer_id, @@ -625,9 +622,11 @@ where match endpoint { ConnectedPoint::Dialer { address } => ( None, + // TODO: This is the place to return the handler. NetworkEvent::UnknownPeerDialError { multiaddr: address, error, + handler, }, ), ConnectedPoint::Listener { @@ -635,10 +634,12 @@ where send_back_addr, } => ( None, + // TODO: This is the place to return the handler. NetworkEvent::IncomingConnectionError { local_addr, send_back_addr, error, + handler, }, ), } diff --git a/core/src/network/event.rs b/core/src/network/event.rs index 7b4158265d9..68b25f26eca 100644 --- a/core/src/network/event.rs +++ b/core/src/network/event.rs @@ -92,6 +92,7 @@ where send_back_addr: Multiaddr, /// The error that happened. error: PendingConnectionError, + handler: THandler, }, /// A new connection to a peer has been established. @@ -124,12 +125,13 @@ where error: Option::Error>>, /// The remaining number of established connections to the same peer. num_established: u32, + handler: THandler::Handler, }, /// A dialing attempt to an address of a peer failed. DialError { /// The number of remaining dialing attempts. - attempts_remaining: u32, + attempts_remaining: DialAttemptsRemaining, /// Id of the peer we were trying to dial. peer_id: PeerId, @@ -148,6 +150,8 @@ where /// The error that happened. error: PendingConnectionError, + + handler: THandler, }, /// An established connection produced an event. @@ -169,6 +173,12 @@ where }, } +pub enum DialAttemptsRemaining { + // TODO: Make this a NonZeroU32. + Some(u32), + None(THandler), +} + impl fmt::Debug for NetworkEvent<'_, TTrans, TInEvent, TOutEvent, THandler> where @@ -221,6 +231,8 @@ where local_addr, send_back_addr, error, + // TODO: Should this be printed as well? + handler: _, } => f .debug_struct("IncomingConnectionError") .field("local_addr", local_addr) @@ -249,7 +261,8 @@ where error, } => f .debug_struct("DialError") - .field("attempts_remaining", attempts_remaining) + // TODO: Bring back. + // .field("attempts_remaining", attempts_remaining) .field("peer_id", peer_id) .field("multiaddr", multiaddr) .field("error", error) From 46904a6f04a22f933c23ea50e784f8210ee4ec95 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Thu, 12 Aug 2021 16:51:13 +0200 Subject: [PATCH 02/38] swarm/: Inject handler on connection error and closed --- core/src/network/event.rs | 9 +++++ swarm/src/behaviour.rs | 4 +-- swarm/src/lib.rs | 38 +++++++++++++++------ swarm/src/protocols_handler/node_handler.rs | 13 +++++++ swarm/src/toggle.rs | 9 +++-- 5 files changed, 57 insertions(+), 16 deletions(-) diff --git a/core/src/network/event.rs b/core/src/network/event.rs index 68b25f26eca..82f12b133b1 100644 --- a/core/src/network/event.rs +++ b/core/src/network/event.rs @@ -179,6 +179,15 @@ pub enum DialAttemptsRemaining { None(THandler), } +impl From<&DialAttemptsRemaining> for u32 { + fn from(attempts_remaining: &DialAttemptsRemaining) -> Self { + match attempts_remaining { + DialAttemptsRemaining::Some(attempts) => *attempts, + DialAttemptsRemaining::None(_) => 0, + } + } +} + impl fmt::Debug for NetworkEvent<'_, TTrans, TInEvent, TOutEvent, THandler> where diff --git a/swarm/src/behaviour.rs b/swarm/src/behaviour.rs index a21c7a023b8..773432ed0ae 100644 --- a/swarm/src/behaviour.rs +++ b/swarm/src/behaviour.rs @@ -112,7 +112,7 @@ pub trait NetworkBehaviour: Send + 'static { /// A call to this method is always paired with an earlier call to /// `inject_connection_established` with the same peer ID, connection ID and /// endpoint. - fn inject_connection_closed(&mut self, _: &PeerId, _: &ConnectionId, _: &ConnectedPoint) {} + fn inject_connection_closed(&mut self, _: &PeerId, _: &ConnectionId, _: &ConnectedPoint, _: ::Handler) {} /// Informs the behaviour that the [`ConnectedPoint`] of an existing connection has changed. fn inject_address_change( @@ -153,7 +153,7 @@ pub trait NetworkBehaviour: Send + 'static { /// /// The `peer_id` is guaranteed to be in a disconnected state. In other words, /// `inject_connected` has not been called, or `inject_disconnected` has been called since then. - fn inject_dial_failure(&mut self, _peer_id: &PeerId) {} + fn inject_dial_failure(&mut self, _peer_id: &PeerId, _handler: Self::ProtocolsHandler) {} /// Indicates to the behaviour that a new listener was created. fn inject_new_listener(&mut self, _id: ListenerId) {} diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index 0bc718690be..5aa18e9f43d 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -82,8 +82,8 @@ use libp2p_core::{ }, muxing::StreamMuxerBox, network::{ - self, peer::ConnectedPeer, ConnectionLimits, Network, NetworkConfig, NetworkEvent, - NetworkInfo, + self, peer::ConnectedPeer, ConnectionLimits, DialAttemptsRemaining, Network, NetworkConfig, + NetworkEvent, NetworkInfo, }, transport::{self, TransportError}, upgrade::ProtocolName, @@ -342,7 +342,8 @@ where /// Initiates a new dialing attempt to the given peer. pub fn dial(&mut self, peer_id: &PeerId) -> Result<(), DialError> { if self.banned_peers.contains(peer_id) { - self.behaviour.inject_dial_failure(peer_id); + // TODO: Needed? + // self.behaviour.inject_dial_failure(peer_id); return Err(DialError::Banned); } @@ -374,7 +375,8 @@ where peer_id, error ); - self.behaviour.inject_dial_failure(&peer_id); + // TODO: Needed? + // self.behaviour.inject_dial_failure(&peer_id); } result @@ -568,6 +570,7 @@ where connected, error, num_established, + handler, }) => { if let Some(error) = error.as_ref() { log::debug!("Connection {:?} closed: {:?}", connected, error); @@ -576,8 +579,12 @@ where } let peer_id = connected.peer_id; let endpoint = connected.endpoint; - this.behaviour - .inject_connection_closed(&peer_id, &id, &endpoint); + this.behaviour.inject_connection_closed( + &peer_id, + &id, + &endpoint, + handler.into_protocol_handler(), + ); if num_established == 0 { this.behaviour.inject_disconnected(&peer_id); } @@ -668,7 +675,9 @@ where local_addr, send_back_addr, error, + handler: _, }) => { + // TODO: Should handler not be injected into behaviour? log::debug!("Incoming connection failed: {:?}", error); return Poll::Ready(SwarmEvent::IncomingConnectionError { local_addr, @@ -684,17 +693,21 @@ where }) => { log::debug!( "Connection attempt to {:?} via {:?} failed with {:?}. Attempts remaining: {}.", - peer_id, multiaddr, error, attempts_remaining); + // TODO: Can we do better on conversion? + peer_id, multiaddr, error, Into::::into(&attempts_remaining)); this.behaviour .inject_addr_reach_failure(Some(&peer_id), &multiaddr, &error); - if attempts_remaining == 0 { - this.behaviour.inject_dial_failure(&peer_id); + let attempts_remaining_num = (&attempts_remaining).into(); + if let DialAttemptsRemaining::None(handler) = attempts_remaining { + this.behaviour + .inject_dial_failure(&peer_id, handler.into_protocol_handler()); } return Poll::Ready(SwarmEvent::UnreachableAddr { peer_id, address: multiaddr, error, - attempts_remaining, + // TODO: Can we do better? + attempts_remaining: attempts_remaining_num, }); } Poll::Ready(NetworkEvent::UnknownPeerDialError { @@ -766,7 +779,10 @@ where } Poll::Ready(NetworkBehaviourAction::DialPeer { peer_id, condition }) => { if this.banned_peers.contains(&peer_id) { - this.behaviour.inject_dial_failure(&peer_id); + this.behaviour.inject_dial_failure( + &peer_id, + todo!("Have DialPeer contain handler which can then be returned."), + ); } else { let condition_matched = match condition { DialPeerCondition::Disconnected => { diff --git a/swarm/src/protocols_handler/node_handler.rs b/swarm/src/protocols_handler/node_handler.rs index edb383282cd..c12a7e6a689 100644 --- a/swarm/src/protocols_handler/node_handler.rs +++ b/swarm/src/protocols_handler/node_handler.rs @@ -65,6 +65,11 @@ where self.substream_upgrade_protocol_override = version; self } + + // TODO: Rethink this method. + pub(crate) fn into_protocol_handler(self) -> TIntoProtoHandler { + self.handler + } } impl IntoConnectionHandler @@ -130,6 +135,14 @@ where substream_upgrade_protocol_override: Option, } +impl NodeHandlerWrapper { + // TODO: Should this be a From impl? + // TODO: Find better name. + pub fn into_protocol_handler(self) -> TProtoHandler { + self.handler + } +} + struct SubstreamUpgrade { user_data: Option, timeout: Delay, diff --git a/swarm/src/toggle.rs b/swarm/src/toggle.rs index 5a86a4824ed..5fff9b4d98b 100644 --- a/swarm/src/toggle.rs +++ b/swarm/src/toggle.rs @@ -113,9 +113,11 @@ where peer_id: &PeerId, connection: &ConnectionId, endpoint: &ConnectedPoint, + handler: ::Handler, ) { if let Some(inner) = self.inner.as_mut() { - inner.inject_connection_closed(peer_id, connection, endpoint) + // TODO: Unwrap safe here? + inner.inject_connection_closed(peer_id, connection, endpoint, handler.inner.unwrap()) } } @@ -153,9 +155,10 @@ where } } - fn inject_dial_failure(&mut self, peer_id: &PeerId) { + fn inject_dial_failure(&mut self, peer_id: &PeerId, handler: Self::ProtocolsHandler) { if let Some(inner) = self.inner.as_mut() { - inner.inject_dial_failure(peer_id) + // TODO: Unwrap safe here? + inner.inject_dial_failure(peer_id, handler.inner.unwrap()) } } From d756be114d4f848f5cd70a4fddc767eb58019824 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Thu, 12 Aug 2021 17:16:20 +0200 Subject: [PATCH 03/38] swarm/src/behaviour: Provide handler with Dial and DialAddr --- swarm/src/behaviour.rs | 29 +++++++++++++++------------ swarm/src/lib.rs | 45 +++++++++++++++++++++++++++++------------- swarm/src/toggle.rs | 5 +++-- 3 files changed, 50 insertions(+), 29 deletions(-) diff --git a/swarm/src/behaviour.rs b/swarm/src/behaviour.rs index 773432ed0ae..ac99bd3e701 100644 --- a/swarm/src/behaviour.rs +++ b/swarm/src/behaviour.rs @@ -183,7 +183,7 @@ pub trait NetworkBehaviour: Send + 'static { /// This API mimics the API of the `Stream` trait. The method may register the current task in /// order to wake it up at a later point in time. fn poll(&mut self, cx: &mut Context<'_>, params: &mut impl PollParameters) - -> Poll::Handler as ProtocolsHandler>::InEvent, Self::OutEvent>>; + -> Poll::Handler as ProtocolsHandler>::InEvent, Self::OutEvent, Self::ProtocolsHandler>>; } /// Parameters passed to `poll()`, that the `NetworkBehaviour` has access to. @@ -229,7 +229,8 @@ pub trait NetworkBehaviourEventProcess { /// /// [`Swarm`]: super::Swarm #[derive(Debug)] -pub enum NetworkBehaviourAction { +// TODO: Derive TInEvent from THandler. +pub enum NetworkBehaviourAction { /// Instructs the `Swarm` to return an event when it is being polled. GenerateEvent(TOutEvent), @@ -237,6 +238,7 @@ pub enum NetworkBehaviourAction { DialAddress { /// The address to dial. address: Multiaddr, + handler: THandler, }, /// Instructs the swarm to dial a known `PeerId`. @@ -254,6 +256,7 @@ pub enum NetworkBehaviourAction { peer_id: PeerId, /// The condition for initiating a new dialing attempt. condition: DialPeerCondition, + handler: THandler, }, /// Instructs the `Swarm` to send an event to the handler dedicated to a @@ -314,16 +317,16 @@ pub enum NetworkBehaviourAction { }, } -impl NetworkBehaviourAction { +impl NetworkBehaviourAction { /// Map the handler event. - pub fn map_in(self, f: impl FnOnce(TInEvent) -> E) -> NetworkBehaviourAction { + pub fn map_in(self, f: impl FnOnce(TInEvent) -> E) -> NetworkBehaviourAction { match self { NetworkBehaviourAction::GenerateEvent(e) => NetworkBehaviourAction::GenerateEvent(e), - NetworkBehaviourAction::DialAddress { address } => { - NetworkBehaviourAction::DialAddress { address } + NetworkBehaviourAction::DialAddress { address, handler } => { + NetworkBehaviourAction::DialAddress { address, handler } } - NetworkBehaviourAction::DialPeer { peer_id, condition } => { - NetworkBehaviourAction::DialPeer { peer_id, condition } + NetworkBehaviourAction::DialPeer { peer_id, condition, handler } => { + NetworkBehaviourAction::DialPeer { peer_id, condition, handler } } NetworkBehaviourAction::NotifyHandler { peer_id, @@ -348,14 +351,14 @@ impl NetworkBehaviourAction { } /// Map the event the swarm will return. - pub fn map_out(self, f: impl FnOnce(TOutEvent) -> E) -> NetworkBehaviourAction { + pub fn map_out(self, f: impl FnOnce(TOutEvent) -> E) -> NetworkBehaviourAction { match self { NetworkBehaviourAction::GenerateEvent(e) => NetworkBehaviourAction::GenerateEvent(f(e)), - NetworkBehaviourAction::DialAddress { address } => { - NetworkBehaviourAction::DialAddress { address } + NetworkBehaviourAction::DialAddress { address, handler } => { + NetworkBehaviourAction::DialAddress { address, handler } } - NetworkBehaviourAction::DialPeer { peer_id, condition } => { - NetworkBehaviourAction::DialPeer { peer_id, condition } + NetworkBehaviourAction::DialPeer { peer_id, condition, handler } => { + NetworkBehaviourAction::DialPeer { peer_id, condition, handler } } NetworkBehaviourAction::NotifyHandler { peer_id, diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index 5aa18e9f43d..47cac9fbb1d 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -331,9 +331,16 @@ where /// Initiates a new dialing attempt to the given address. pub fn dial_addr(&mut self, addr: Multiaddr) -> Result<(), DialError> { - let handler = self - .behaviour - .new_handler() + let handler = self.behaviour.new_handler(); + self.dial_addr_with_handler(addr, handler) + } + + fn dial_addr_with_handler( + &mut self, + addr: Multiaddr, + handler: ::ProtocolsHandler, + ) -> Result<(), DialError> { + let handler = handler .into_node_handler_builder() .with_substream_upgrade_protocol_override(self.substream_upgrade_protocol_override); Ok(self.network.dial(&addr, handler).map(|_id| ())?) @@ -341,6 +348,15 @@ where /// Initiates a new dialing attempt to the given peer. pub fn dial(&mut self, peer_id: &PeerId) -> Result<(), DialError> { + let handler = self.behaviour.new_handler(); + self.dial_with_handler(peer_id, handler) + } + + fn dial_with_handler( + &mut self, + peer_id: &PeerId, + handler: ::ProtocolsHandler, + ) -> Result<(), DialError> { if self.banned_peers.contains(peer_id) { // TODO: Needed? // self.behaviour.inject_dial_failure(peer_id); @@ -355,9 +371,7 @@ where .filter(|a| !self_listening.contains(a)); let result = if let Some(first) = addrs.next() { - let handler = self - .behaviour - .new_handler() + let handler = handler .into_node_handler_builder() .with_substream_upgrade_protocol_override(self.substream_upgrade_protocol_override); self.network @@ -774,15 +788,16 @@ where Poll::Ready(NetworkBehaviourAction::GenerateEvent(event)) => { return Poll::Ready(SwarmEvent::Behaviour(event)) } - Poll::Ready(NetworkBehaviourAction::DialAddress { address }) => { - let _ = Swarm::dial_addr(&mut *this, address); + Poll::Ready(NetworkBehaviourAction::DialAddress { address, handler }) => { + let _ = Swarm::dial_addr_with_handler(&mut *this, address, handler); } - Poll::Ready(NetworkBehaviourAction::DialPeer { peer_id, condition }) => { + Poll::Ready(NetworkBehaviourAction::DialPeer { + peer_id, + condition, + handler, + }) => { if this.banned_peers.contains(&peer_id) { - this.behaviour.inject_dial_failure( - &peer_id, - todo!("Have DialPeer contain handler which can then be returned."), - ); + this.behaviour.inject_dial_failure(&peer_id, handler); } else { let condition_matched = match condition { DialPeerCondition::Disconnected => { @@ -792,7 +807,7 @@ where DialPeerCondition::Always => true, }; if condition_matched { - if Swarm::dial(this, &peer_id).is_ok() { + if Swarm::dial_with_handler(this, &peer_id, handler).is_ok() { return Poll::Ready(SwarmEvent::Dialing(peer_id)); } } else { @@ -814,6 +829,7 @@ where } } } + // TODO: Return the handler to the behaviour. } } } @@ -1261,6 +1277,7 @@ impl NetworkBehaviour for DummyBehaviour { NetworkBehaviourAction< ::InEvent, Self::OutEvent, + Self::ProtocolsHandler, >, > { Poll::Pending diff --git a/swarm/src/toggle.rs b/swarm/src/toggle.rs index 5fff9b4d98b..6e72f2c6b90 100644 --- a/swarm/src/toggle.rs +++ b/swarm/src/toggle.rs @@ -205,10 +205,11 @@ where } fn poll(&mut self, cx: &mut Context<'_>, params: &mut impl PollParameters) - -> Poll::Handler as ProtocolsHandler>::InEvent, Self::OutEvent>> + -> Poll::Handler as ProtocolsHandler>::InEvent, Self::OutEvent, Self::ProtocolsHandler>> { if let Some(inner) = self.inner.as_mut() { - inner.poll(cx, params) + // inner.poll(cx, params) + todo!("Wrap handler in Dial events with ToggleHandler."); } else { Poll::Pending } From 705842f3565c235aa2b6342142c3ae0b3ca561b9 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Wed, 18 Aug 2021 16:49:26 +0200 Subject: [PATCH 04/38] swarm/src/behaviour: Add default trait para on NetworkBehaviourAction --- core/src/connection.rs | 6 +-- core/src/connection/manager/task.rs | 3 +- swarm/src/behaviour.rs | 64 ++++++++++++++++++++++------- swarm/src/lib.rs | 1 - swarm/src/toggle.rs | 8 ++-- 5 files changed, 58 insertions(+), 24 deletions(-) diff --git a/core/src/connection.rs b/core/src/connection.rs index 513665a631a..9e39ae21807 100644 --- a/core/src/connection.rs +++ b/core/src/connection.rs @@ -229,10 +229,8 @@ where self.handler.inject_event(event); } - // TODO: Update comment - // - /// Begins an orderly shutdown of the connection, returning a - /// `Future` that resolves when connection shutdown is complete. + /// Begins an orderly shutdown of the connection, returning the connection + /// handler and a `Future` that resolves when connection shutdown is complete. pub fn close(self) -> (THandler, Close) { (self.handler, self.muxing.close().0) } diff --git a/core/src/connection/manager/task.rs b/core/src/connection/manager/task.rs index f81c520e6e9..1dc56df81ca 100644 --- a/core/src/connection/manager/task.rs +++ b/core/src/connection/manager/task.rs @@ -329,8 +329,7 @@ where Poll::Ready(Err(error)) => { // Don't accept any further commands. this.commands.get_mut().close(); - // TODO: Good idea if there is already an error? - let (handler, _) = connection.close(); + let (handler, _closing_muxer) = connection.close(); // Terminate the task with the error, dropping the connection. let event = Event::Closed { id, diff --git a/swarm/src/behaviour.rs b/swarm/src/behaviour.rs index ac99bd3e701..3dd02bac308 100644 --- a/swarm/src/behaviour.rs +++ b/swarm/src/behaviour.rs @@ -26,6 +26,10 @@ use libp2p_core::{ }; use std::{error, task::Context, task::Poll}; +/// Custom event that can be received by the [`ProtocolsHandler`]. +type THandlerInEvent = + <::Handler as ProtocolsHandler>::InEvent; + /// A behaviour for the network. Allows customizing the swarm. /// /// This trait has been designed to be composable. Multiple implementations can be combined into @@ -112,7 +116,14 @@ pub trait NetworkBehaviour: Send + 'static { /// A call to this method is always paired with an earlier call to /// `inject_connection_established` with the same peer ID, connection ID and /// endpoint. - fn inject_connection_closed(&mut self, _: &PeerId, _: &ConnectionId, _: &ConnectedPoint, _: ::Handler) {} + fn inject_connection_closed( + &mut self, + _: &PeerId, + _: &ConnectionId, + _: &ConnectedPoint, + _: ::Handler, + ) { + } /// Informs the behaviour that the [`ConnectedPoint`] of an existing connection has changed. fn inject_address_change( @@ -182,8 +193,11 @@ pub trait NetworkBehaviour: Send + 'static { /// /// This API mimics the API of the `Stream` trait. The method may register the current task in /// order to wake it up at a later point in time. - fn poll(&mut self, cx: &mut Context<'_>, params: &mut impl PollParameters) - -> Poll::Handler as ProtocolsHandler>::InEvent, Self::OutEvent, Self::ProtocolsHandler>>; + fn poll( + &mut self, + cx: &mut Context<'_>, + params: &mut impl PollParameters, + ) -> Poll>; } /// Parameters passed to `poll()`, that the `NetworkBehaviour` has access to. @@ -228,9 +242,13 @@ pub trait NetworkBehaviourEventProcess { /// in whose context it is executing. /// /// [`Swarm`]: super::Swarm +// +// Note: `TInEvent` is needed to be able to implement +// [`NetworkBehaviourAction::map_in`], mapping the handler `InEvent` leaving the +// handler itself untouched. #[derive(Debug)] -// TODO: Derive TInEvent from THandler. -pub enum NetworkBehaviourAction { +pub enum NetworkBehaviourAction> +{ /// Instructs the `Swarm` to return an event when it is being polled. GenerateEvent(TOutEvent), @@ -317,17 +335,29 @@ pub enum NetworkBehaviourAction { }, } -impl NetworkBehaviourAction { +impl NetworkBehaviourAction { /// Map the handler event. - pub fn map_in(self, f: impl FnOnce(TInEvent) -> E) -> NetworkBehaviourAction { + pub fn map_in( + self, + f: impl FnOnce(THandlerInEvent) -> New, + ) -> NetworkBehaviourAction + where + Self: , + { match self { NetworkBehaviourAction::GenerateEvent(e) => NetworkBehaviourAction::GenerateEvent(e), NetworkBehaviourAction::DialAddress { address, handler } => { NetworkBehaviourAction::DialAddress { address, handler } } - NetworkBehaviourAction::DialPeer { peer_id, condition, handler } => { - NetworkBehaviourAction::DialPeer { peer_id, condition, handler } - } + NetworkBehaviourAction::DialPeer { + peer_id, + condition, + handler, + } => NetworkBehaviourAction::DialPeer { + peer_id, + condition, + handler, + }, NetworkBehaviourAction::NotifyHandler { peer_id, handler, @@ -351,15 +381,21 @@ impl NetworkBehaviourAction(self, f: impl FnOnce(TOutEvent) -> E) -> NetworkBehaviourAction { + pub fn map_out(self, f: impl FnOnce(TOutEvent) -> E) -> NetworkBehaviourAction { match self { NetworkBehaviourAction::GenerateEvent(e) => NetworkBehaviourAction::GenerateEvent(f(e)), NetworkBehaviourAction::DialAddress { address, handler } => { NetworkBehaviourAction::DialAddress { address, handler } } - NetworkBehaviourAction::DialPeer { peer_id, condition, handler } => { - NetworkBehaviourAction::DialPeer { peer_id, condition, handler } - } + NetworkBehaviourAction::DialPeer { + peer_id, + condition, + handler, + } => NetworkBehaviourAction::DialPeer { + peer_id, + condition, + handler, + }, NetworkBehaviourAction::NotifyHandler { peer_id, handler, diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index 47cac9fbb1d..b93ae56ff4b 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -1275,7 +1275,6 @@ impl NetworkBehaviour for DummyBehaviour { _: &mut impl PollParameters, ) -> Poll< NetworkBehaviourAction< - ::InEvent, Self::OutEvent, Self::ProtocolsHandler, >, diff --git a/swarm/src/toggle.rs b/swarm/src/toggle.rs index 6e72f2c6b90..84cdbfd3163 100644 --- a/swarm/src/toggle.rs +++ b/swarm/src/toggle.rs @@ -204,9 +204,11 @@ where } } - fn poll(&mut self, cx: &mut Context<'_>, params: &mut impl PollParameters) - -> Poll::Handler as ProtocolsHandler>::InEvent, Self::OutEvent, Self::ProtocolsHandler>> - { + fn poll( + &mut self, + cx: &mut Context<'_>, + params: &mut impl PollParameters, + ) -> Poll> { if let Some(inner) = self.inner.as_mut() { // inner.poll(cx, params) todo!("Wrap handler in Dial events with ToggleHandler."); From d744b4afd6b1c3893801e45adef5edea1aa74198 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Wed, 18 Aug 2021 17:13:19 +0200 Subject: [PATCH 05/38] core/src/connection/manager: Fully close a task on disconnect --- core/src/connection/manager.rs | 23 ++++++------ core/src/connection/manager/task.rs | 2 ++ core/src/connection/pool.rs | 54 +++-------------------------- 3 files changed, 18 insertions(+), 61 deletions(-) diff --git a/core/src/connection/manager.rs b/core/src/connection/manager.rs index 037364ca159..a62cfe847d8 100644 --- a/core/src/connection/manager.rs +++ b/core/src/connection/manager.rs @@ -467,7 +467,7 @@ impl<'a, I> EstablishedEntry<'a, I> { } /// Sends a close command to the associated background task, - /// thus initiating a graceful active close of the connection. + /// thus initiating a graceful active close of the connectione /// /// Has no effect if the connection is already closing. /// @@ -496,16 +496,17 @@ impl<'a, I> EstablishedEntry<'a, I> { } } - /// Instantly removes the entry from the manager, dropping - /// the command channel to the background task of the connection, - /// which will thus drop the connection asap without an orderly - /// close or emitting another event. - pub fn remove(self) -> Connected { - match self.task.remove().state { - TaskState::Established(c) => c, - TaskState::Pending => unreachable!("By Entry::new()"), - } - } + // TODO: Needed? + // /// Instantly removes the entry from the manager, dropping + // /// the command channel to the background task of the connection, + // /// which will thus drop the connection asap without an orderly + // /// close or emitting another event. + // pub fn remove(self) -> Connected { + // match self.task.remove().state { + // TaskState::Established(c) => c, + // TaskState::Pending => unreachable!("By Entry::new()"), + // } + // } /// Returns the connection ID. pub fn id(&self) -> ConnectionId { diff --git a/core/src/connection/manager/task.rs b/core/src/connection/manager/task.rs index 1dc56df81ca..35c401e5969 100644 --- a/core/src/connection/manager/task.rs +++ b/core/src/connection/manager/task.rs @@ -223,6 +223,7 @@ where Poll::Pending => {} Poll::Ready(None) => { // The manager has dropped the task; abort. + // TODO: Should we return the handler in this case? return Poll::Ready(()); } Poll::Ready(Some(_)) => { @@ -275,6 +276,7 @@ where } Poll::Ready(None) => { // The manager has dropped the task or disappeared; abort. + // TODO: Should we return the handler in this case? return Poll::Ready(()); } } diff --git a/core/src/connection/pool.rs b/core/src/connection/pool.rs index 11d030ccaef..b1d8050e8dc 100644 --- a/core/src/connection/pool.rs +++ b/core/src/connection/pool.rs @@ -54,12 +54,6 @@ pub struct Pool { /// The pending connections that are currently being negotiated. pending: FnvHashMap)>, - - /// Established connections that have been closed in the context of - /// a [`Pool::disconnect`] in order to emit a `ConnectionClosed` - /// event for each. Every `ConnectionEstablished` event must be - /// paired with (eventually) a `ConnectionClosed`. - disconnected: Vec, } impl fmt::Debug for Pool { @@ -201,7 +195,6 @@ impl Pool { manager: Manager::new(manager_config), established: Default::default(), pending: Default::default(), - disconnected: Vec::new(), } } @@ -421,16 +414,12 @@ impl Pool { pub fn disconnect(&mut self, peer: &PeerId) { if let Some(conns) = self.established.get(peer) { // Count upwards because we push to / pop from the end. See also `Pool::poll`. - let mut num_established = 0; for (&id, endpoint) in conns.iter() { if let Some(manager::Entry::Established(e)) = self.manager.entry(id) { - let connected = e.remove(); - self.disconnected.push(Disconnected { - id, - connected, - num_established, - }); - num_established += 1; + e.start_close(); + // TODO: I removed the disconnected logic, thus depending on start_close to + // eventually trigger a ConnectionClosed event. Make sure that is the case and + // also that the num_established counters are kept consistent. } self.counters.dec_established(endpoint); } @@ -536,29 +525,6 @@ impl Pool { &'a mut self, cx: &mut Context<'_>, ) -> Poll> { - // Drain events resulting from forced disconnections. - // - // Note: The `Disconnected` entries in `self.disconnected` - // are inserted in ascending order of the remaining `num_established` - // connections. Thus we `pop()` them off from the end to emit the - // events in an order that properly counts down `num_established`. - // See also `Pool::disconnect`. - if let Some(Disconnected { - id, - connected, - num_established, - }) = self.disconnected.pop() - { - return Poll::Ready(PoolEvent::ConnectionClosed { - id, - connected, - num_established, - error: None, - pool: self, - handler: todo!(), - }); - } - // Poll the connection `Manager`. loop { let item = match self.manager.poll(cx) { @@ -1108,15 +1074,3 @@ impl ConnectionLimits { self } } - -/// Information about a former established connection to a peer -/// that was dropped via [`Pool::disconnect`]. -struct Disconnected { - /// The unique identifier of the dropped connection. - id: ConnectionId, - /// Information about the dropped connection. - connected: Connected, - /// The remaining number of established connections - /// to the same peer. - num_established: u32, -} From 5c2aef654c56069d0e5b24500ce11a2a02684272 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Wed, 18 Aug 2021 17:31:13 +0200 Subject: [PATCH 06/38] core/: Remove DisconnectedPeer::set_connected and Pool::add This logic seems to be a leftover of https://github.com/libp2p/rust-libp2p/pull/889 and unused today. --- core/src/connection/pool.rs | 31 ------------------------------ core/src/network/peer.rs | 38 ------------------------------------- 2 files changed, 69 deletions(-) diff --git a/core/src/connection/pool.rs b/core/src/connection/pool.rs index b1d8050e8dc..0540533d6f3 100644 --- a/core/src/connection/pool.rs +++ b/core/src/connection/pool.rs @@ -307,37 +307,6 @@ impl Pool { id } - /// Adds an existing established connection to the pool. - /// - /// Returns the assigned connection ID on success. An error is returned - /// if the configured maximum number of established connections for the - /// connected peer has been reached. - pub fn add( - &mut self, - c: Connection, - i: Connected, - ) -> Result - where - THandler: IntoConnectionHandler + Send + 'static, - THandler::Handler: - ConnectionHandler> + Send + 'static, - ::OutboundOpenInfo: Send + 'static, - TTransErr: error::Error + Send + 'static, - TMuxer: StreamMuxer + Send + Sync + 'static, - TMuxer::OutboundSubstream: Send + 'static, - { - self.counters.check_max_established(&i.endpoint)?; - self.counters - .check_max_established_per_peer(self.num_peer_established(&i.peer_id))?; - let id = self.manager.add(c, i.clone()); - self.counters.inc_established(&i.endpoint); - self.established - .entry(i.peer_id) - .or_default() - .insert(id, i.endpoint); - Ok(id) - } - /// Gets an entry representing a connection in the pool. /// /// Returns `None` if the pool has no connection with the given ID. diff --git a/core/src/network/peer.rs b/core/src/network/peer.rs index ca1b9be7502..ea3502c0bb4 100644 --- a/core/src/network/peer.rs +++ b/core/src/network/peer.rs @@ -472,44 +472,6 @@ where pub fn into_peer(self) -> Peer<'a, TTrans, THandler> { Peer::Disconnected(self) } - - /// Moves the peer into a connected state by supplying an existing - /// established connection. - /// - /// No event is generated for this action. - /// - /// # Panics - /// - /// Panics if `connected.peer_id` does not identify the current peer. - pub fn set_connected( - self, - connected: Connected, - connection: Connection, - ) -> Result, ConnectionLimit> - where - THandler: Send + 'static, - TTrans::Error: Send + 'static, - THandler::Handler: ConnectionHandler> + Send, - ::OutboundOpenInfo: Send, - ::Error: error::Error + Send + 'static, - TMuxer: StreamMuxer + Send + Sync + 'static, - TMuxer::OutboundSubstream: Send, - { - if connected.peer_id != self.peer_id { - panic!( - "Invalid peer ID given: {:?}. Expected: {:?}", - connected.peer_id, self.peer_id - ) - } - - self.network - .pool - .add(connection, connected) - .map(move |_id| ConnectedPeer { - network: self.network, - peer_id: self.peer_id, - }) - } } /// The (internal) state of a `DialingAttempt`, tracking the From ce0d278073676ccb5d8a717234832ff54aef8b86 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Wed, 18 Aug 2021 18:03:35 +0200 Subject: [PATCH 07/38] core/src/connection: Report ConnectionLimit through task --- core/src/connection/error.rs | 16 +++++++------- core/src/connection/manager.rs | 6 ++--- core/src/connection/manager/task.rs | 19 +++++++++++----- core/src/connection/pool.rs | 34 +++++------------------------ core/src/network/event.rs | 3 +-- core/src/network/peer.rs | 6 ++--- 6 files changed, 34 insertions(+), 50 deletions(-) diff --git a/core/src/connection/error.rs b/core/src/connection/error.rs index 66da0670c98..8b67ca0c044 100644 --- a/core/src/connection/error.rs +++ b/core/src/connection/error.rs @@ -29,6 +29,10 @@ pub enum ConnectionError { // TODO: Eventually this should also be a custom error? IO(io::Error), + /// The connection was dropped because the connection limit + /// for a peer has been reached. + ConnectionLimit(ConnectionLimit), + /// The connection handler produced an error. Handler(THandlerErr), } @@ -41,6 +45,9 @@ where match self { ConnectionError::IO(err) => write!(f, "Connection error: I/O error: {}", err), ConnectionError::Handler(err) => write!(f, "Connection error: Handler error: {}", err), + ConnectionError::ConnectionLimit(l) => { + write!(f, "Connection error: Connection limit: {}.", l) + } } } } @@ -53,6 +60,7 @@ where match self { ConnectionError::IO(err) => Some(err), ConnectionError::Handler(err) => Some(err), + ConnectionError::ConnectionLimit(..) => None, } } } @@ -67,10 +75,6 @@ pub enum PendingConnectionError { /// match the one that was expected or is otherwise invalid. InvalidPeerId, - /// The connection was dropped because the connection limit - /// for a peer has been reached. - ConnectionLimit(ConnectionLimit), - /// An I/O error occurred on the connection. // TODO: Eventually this should also be a custom error? IO(io::Error), @@ -89,9 +93,6 @@ where PendingConnectionError::InvalidPeerId => { write!(f, "Pending connection: Invalid peer ID.") } - PendingConnectionError::ConnectionLimit(l) => { - write!(f, "Connection error: Connection limit: {}.", l) - } } } } @@ -105,7 +106,6 @@ where PendingConnectionError::IO(err) => Some(err), PendingConnectionError::Transport(err) => Some(err), PendingConnectionError::InvalidPeerId => None, - PendingConnectionError::ConnectionLimit(..) => None, } } } diff --git a/core/src/connection/manager.rs b/core/src/connection/manager.rs index a62cfe847d8..1dae8581f47 100644 --- a/core/src/connection/manager.rs +++ b/core/src/connection/manager.rs @@ -20,7 +20,7 @@ use super::{ handler::{THandlerError, THandlerInEvent, THandlerOutEvent}, - Connected, ConnectedPoint, Connection, ConnectionError, ConnectionHandler, + Connected, ConnectedPoint, Connection, ConnectionError, ConnectionHandler, ConnectionLimit, IntoConnectionHandler, PendingConnectionError, Substream, }; use crate::{muxing::StreamMuxer, Executor}; @@ -473,7 +473,7 @@ impl<'a, I> EstablishedEntry<'a, I> { /// /// When the connection is ultimately closed, [`Event::ConnectionClosed`] /// is emitted by [`Manager::poll`]. - pub fn start_close(mut self) { + pub fn start_close(mut self, error: Option) { // Clone the sender so that we are guaranteed to have // capacity for the close command (every sender gets a slot). match self @@ -481,7 +481,7 @@ impl<'a, I> EstablishedEntry<'a, I> { .get_mut() .sender .clone() - .try_send(task::Command::Close) + .try_send(task::Command::Close(error)) { Ok(()) => {} Err(e) => assert!(e.is_disconnected(), "No capacity for close command."), diff --git a/core/src/connection/manager/task.rs b/core/src/connection/manager/task.rs index 35c401e5969..4c71b899820 100644 --- a/core/src/connection/manager/task.rs +++ b/core/src/connection/manager/task.rs @@ -23,8 +23,8 @@ use crate::{ connection::{ self, handler::{THandlerError, THandlerInEvent, THandlerOutEvent}, - Close, Connected, Connection, ConnectionError, ConnectionHandler, IntoConnectionHandler, - PendingConnectionError, Substream, + Close, Connected, Connection, ConnectionError, ConnectionHandler, ConnectionLimit, + IntoConnectionHandler, PendingConnectionError, Substream, }, muxing::StreamMuxer, Multiaddr, @@ -43,7 +43,7 @@ pub enum Command { NotifyHandler(T), /// Gracefully close the connection (active close) before /// terminating the task. - Close, + Close(Option), } /// Events that a task can emit to its manager. @@ -178,7 +178,11 @@ where }, /// The connection is closing (active close). - Closing { closing_muxer: Close, handler: H::Handler }, + Closing { + closing_muxer: Close, + handler: H::Handler, + error: Option, + }, /// The task is terminating with a final event for the `Manager`. Terminating(Event), @@ -263,7 +267,7 @@ where Poll::Ready(Some(Command::NotifyHandler(event))) => { connection.inject_event(event) } - Poll::Ready(Some(Command::Close)) => { + Poll::Ready(Some(Command::Close(error))) => { // Don't accept any further commands. this.commands.get_mut().close(); // Discard the event, if any, and start a graceful close. @@ -271,6 +275,7 @@ where this.state = State::Closing { handler, closing_muxer, + error, }; continue 'poll; } @@ -346,6 +351,7 @@ where State::Closing { handler, + error, mut closing_muxer, } => { // Try to gracefully close the connection. @@ -353,7 +359,7 @@ where Poll::Ready(Ok(())) => { let event = Event::Closed { id: this.id, - error: None, + error: error.map(|limit| ConnectionError::ConnectionLimit(limit)), handler, }; this.state = State::Terminating(event); @@ -369,6 +375,7 @@ where Poll::Pending => { this.state = State::Closing { handler, + error, closing_muxer, }; return Poll::Pending; diff --git a/core/src/connection/pool.rs b/core/src/connection/pool.rs index 0540533d6f3..f11e1361da7 100644 --- a/core/src/connection/pool.rs +++ b/core/src/connection/pool.rs @@ -20,10 +20,10 @@ use crate::{ connection::{ - self, + handler::{THandlerError, THandlerInEvent, THandlerOutEvent}, manager::{self, Manager, ManagerConfig}, - Connected, Connection, ConnectionError, ConnectionHandler, ConnectionId, ConnectionLimit, + Connected, ConnectionError, ConnectionHandler, ConnectionId, ConnectionLimit, IncomingInfo, IntoConnectionHandler, OutgoingInfo, PendingConnectionError, Substream, }, muxing::StreamMuxer, @@ -385,7 +385,7 @@ impl Pool { // Count upwards because we push to / pop from the end. See also `Pool::poll`. for (&id, endpoint) in conns.iter() { if let Some(manager::Entry::Established(e)) = self.manager.entry(id) { - e.start_close(); + e.start_close(None); // TODO: I removed the disconnected logic, thus depending on start_close to // eventually trigger a ConnectionClosed event. Make sure that is the case and // also that the num_established counters are kept consistent. @@ -549,18 +549,7 @@ impl Pool { // Check general established connection limit. if let Err(e) = self.counters.check_max_established(&endpoint) { - // TODO: Good idea? How should we let the user know that the close - // happened due to a conneciton limit? - entry.start_close(); - // let connected = entry.remove(); - // return Poll::Ready(PoolEvent::PendingConnectionError { - // id, - // endpoint: connected.endpoint, - // error: PendingConnectionError::ConnectionLimit(e), - // handler: None, - // peer, - // pool: self, - // }); + entry.start_close(Some(e)); continue; } @@ -568,18 +557,7 @@ impl Pool { let current = num_peer_established(&self.established, &entry.connected().peer_id); if let Err(e) = self.counters.check_max_established_per_peer(current) { - // TODO: Good idea? How should we let the user know that the close - // happened due to a conneciton limit? - entry.start_close(); - // let connected = entry.remove(); - // return Poll::Ready(PoolEvent::PendingConnectionError { - // id, - // endpoint: connected.endpoint, - // error: PendingConnectionError::ConnectionLimit(e), - // handler: None, - // peer, - // pool: self, - // }); + entry.start_close(Some(e)); continue; } @@ -769,7 +747,7 @@ impl EstablishedConnection<'_, TInEvent> { /// /// Has no effect if the connection is already closing. pub fn start_close(self) { - self.entry.start_close() + self.entry.start_close(None) } } diff --git a/core/src/network/event.rs b/core/src/network/event.rs index 82f12b133b1..a6f840be249 100644 --- a/core/src/network/event.rs +++ b/core/src/network/event.rs @@ -270,8 +270,7 @@ where error, } => f .debug_struct("DialError") - // TODO: Bring back. - // .field("attempts_remaining", attempts_remaining) + .field("attempts_remaining", &Into::::into(attempts_remaining)) .field("peer_id", peer_id) .field("multiaddr", multiaddr) .field("error", error) diff --git a/core/src/network/peer.rs b/core/src/network/peer.rs index ea3502c0bb4..7e904ce2c79 100644 --- a/core/src/network/peer.rs +++ b/core/src/network/peer.rs @@ -21,9 +21,9 @@ use super::{DialError, DialingOpts, Network}; use crate::{ connection::{ - handler::THandlerInEvent, pool::Pool, Connected, ConnectedPoint, Connection, - ConnectionHandler, ConnectionId, ConnectionLimit, EstablishedConnection, - EstablishedConnectionIter, IntoConnectionHandler, PendingConnection, Substream, + handler::THandlerInEvent, pool::Pool, ConnectedPoint, ConnectionHandler, ConnectionId, + ConnectionLimit, EstablishedConnection, EstablishedConnectionIter, IntoConnectionHandler, + PendingConnection, Substream, }, Multiaddr, PeerId, StreamMuxer, Transport, }; From 425e777dc1aff0e3c70af2d468314f3f873e1b15 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Thu, 19 Aug 2021 18:50:16 +0200 Subject: [PATCH 08/38] core/: Emit Event::Failed on aborted pending connection --- core/src/connection/error.rs | 5 +++ core/src/connection/manager.rs | 48 +---------------------------- core/src/connection/manager/task.rs | 33 ++++++++------------ core/src/connection/pool.rs | 3 +- 4 files changed, 19 insertions(+), 70 deletions(-) diff --git a/core/src/connection/error.rs b/core/src/connection/error.rs index 8b67ca0c044..ec4f7ff6e61 100644 --- a/core/src/connection/error.rs +++ b/core/src/connection/error.rs @@ -71,6 +71,9 @@ pub enum PendingConnectionError { /// An error occurred while negotiating the transport protocol(s). Transport(TransportError), + /// Pending connection attempt has been aborted. + Aborted, + /// The peer identity obtained on the connection did not /// match the one that was expected or is otherwise invalid. InvalidPeerId, @@ -87,6 +90,7 @@ where fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { PendingConnectionError::IO(err) => write!(f, "Pending connection: I/O error: {}", err), + PendingConnectionError::Aborted => write!(f, "Pending connection: Aborted."), PendingConnectionError::Transport(err) => { write!(f, "Pending connection: Transport error: {}", err) } @@ -106,6 +110,7 @@ where PendingConnectionError::IO(err) => Some(err), PendingConnectionError::Transport(err) => Some(err), PendingConnectionError::InvalidPeerId => None, + PendingConnectionError::Aborted => None, } } } diff --git a/core/src/connection/manager.rs b/core/src/connection/manager.rs index 1dae8581f47..437e2680c4d 100644 --- a/core/src/connection/manager.rs +++ b/core/src/connection/manager.rs @@ -20,7 +20,7 @@ use super::{ handler::{THandlerError, THandlerInEvent, THandlerOutEvent}, - Connected, ConnectedPoint, Connection, ConnectionError, ConnectionHandler, ConnectionLimit, + Connected, ConnectedPoint, ConnectionError, ConnectionHandler, ConnectionLimit, IntoConnectionHandler, PendingConnectionError, Substream, }; use crate::{muxing::StreamMuxer, Executor}; @@ -277,40 +277,6 @@ impl Manager { ConnectionId(task_id) } - /// Adds an existing connection to the manager. - pub fn add(&mut self, conn: Connection, info: Connected) -> ConnectionId - where - H: IntoConnectionHandler + Send + 'static, - H::Handler: ConnectionHandler> + Send + 'static, - ::OutboundOpenInfo: Send + 'static, - TE: error::Error + Send + 'static, - M: StreamMuxer + Send + Sync + 'static, - M::OutboundSubstream: Send + 'static, - { - let task_id = self.next_task_id; - self.next_task_id.0 += 1; - - let (tx, rx) = mpsc::channel(self.task_command_buffer_size); - self.tasks.insert( - task_id, - TaskInfo { - sender: tx, - state: TaskState::Established(info), - }, - ); - - let task: Pin>>, _, _, _>>> = - Box::pin(Task::established(task_id, self.events_tx.clone(), rx, conn)); - - if let Some(executor) = &mut self.executor { - executor.exec(task); - } else { - self.local_spawns.push(task); - } - - ConnectionId(task_id) - } - /// Gets an entry for a managed connection, if it exists. pub fn entry(&mut self, id: ConnectionId) -> Option>> { if let hash_map::Entry::Occupied(task) = self.tasks.entry(id.0) { @@ -496,18 +462,6 @@ impl<'a, I> EstablishedEntry<'a, I> { } } - // TODO: Needed? - // /// Instantly removes the entry from the manager, dropping - // /// the command channel to the background task of the connection, - // /// which will thus drop the connection asap without an orderly - // /// close or emitting another event. - // pub fn remove(self) -> Connected { - // match self.task.remove().state { - // TaskState::Established(c) => c, - // TaskState::Pending => unreachable!("By Entry::new()"), - // } - // } - /// Returns the connection ID. pub fn id(&self) -> ConnectionId { ConnectionId(*self.task.key()) diff --git a/core/src/connection/manager/task.rs b/core/src/connection/manager/task.rs index 4c71b899820..1d7ca3c515b 100644 --- a/core/src/connection/manager/task.rs +++ b/core/src/connection/manager/task.rs @@ -131,24 +131,6 @@ where }, } } - - /// Create a task for an existing node we are already connected to. - pub fn established( - id: TaskId, - events: mpsc::Sender>, - commands: mpsc::Receiver>>, - connection: Connection, - ) -> Self { - Task { - id, - events, - commands: commands.fuse(), - state: State::Established { - connection, - event: None, - }, - } - } } /// The state associated with the `Task` of a connection. @@ -227,8 +209,16 @@ where Poll::Pending => {} Poll::Ready(None) => { // The manager has dropped the task; abort. - // TODO: Should we return the handler in this case? - return Poll::Ready(()); + // Don't accept any further commands and terminate the + // task with a final event. + this.commands.get_mut().close(); + let event = Event::Failed { + id, + handler, + error: PendingConnectionError::Aborted, + }; + this.state = State::Terminating(event); + continue 'poll; } Poll::Ready(Some(_)) => { panic!("Task received command while the connection is pending.") @@ -280,9 +270,10 @@ where continue 'poll; } Poll::Ready(None) => { + todo!("Safe to assume that this should never happen?"); // The manager has dropped the task or disappeared; abort. // TODO: Should we return the handler in this case? - return Poll::Ready(()); + // return Poll::Ready(()); } } } diff --git a/core/src/connection/pool.rs b/core/src/connection/pool.rs index f11e1361da7..521ac5b381c 100644 --- a/core/src/connection/pool.rs +++ b/core/src/connection/pool.rs @@ -387,8 +387,7 @@ impl Pool { if let Some(manager::Entry::Established(e)) = self.manager.entry(id) { e.start_close(None); // TODO: I removed the disconnected logic, thus depending on start_close to - // eventually trigger a ConnectionClosed event. Make sure that is the case and - // also that the num_established counters are kept consistent. + // eventually trigger a ConnectionClosed event. Make sure that is the case. } self.counters.dec_established(endpoint); } From 5ff6397d0a6e145a15da61deddbf30dc12440ba7 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Thu, 19 Aug 2021 19:01:56 +0200 Subject: [PATCH 09/38] core/tests: Adjust to type changes --- core/tests/connection_limits.rs | 6 +++--- core/tests/network_dial_error.rs | 7 ++++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/core/tests/connection_limits.rs b/core/tests/connection_limits.rs index 65e61c4b3c4..471f054caf3 100644 --- a/core/tests/connection_limits.rs +++ b/core/tests/connection_limits.rs @@ -23,7 +23,7 @@ mod util; use futures::{future::poll_fn, ready}; use libp2p_core::multiaddr::{multiaddr, Multiaddr}; use libp2p_core::{ - connection::PendingConnectionError, + connection::ConnectionError, network::{ConnectionLimits, DialError, NetworkConfig, NetworkEvent}, PeerId, }; @@ -111,8 +111,8 @@ fn max_established_incoming() { network1.accept(connection, TestHandler()).unwrap(); } NetworkEvent::ConnectionEstablished { .. } => {} - NetworkEvent::IncomingConnectionError { - error: PendingConnectionError::ConnectionLimit(err), + NetworkEvent::ConnectionClosed { + error: Some(ConnectionError::ConnectionLimit(err)), .. } => { assert_eq!(err.limit, limit); diff --git a/core/tests/network_dial_error.rs b/core/tests/network_dial_error.rs index 224d7950eac..04aba9fb420 100644 --- a/core/tests/network_dial_error.rs +++ b/core/tests/network_dial_error.rs @@ -65,11 +65,12 @@ fn deny_incoming_connec() { match swarm2.poll(cx) { Poll::Ready(NetworkEvent::DialError { - attempts_remaining: 0, + attempts_remaining, peer_id, multiaddr, error: PendingConnectionError::Transport(_), }) => { + assert_eq!(0u32, (&attempts_remaining).into()); assert_eq!(&peer_id, swarm1.local_peer_id()); assert_eq!( multiaddr, @@ -201,10 +202,10 @@ fn multiple_addresses_err() { .with(Protocol::P2p(target.clone().into())); assert_eq!(multiaddr, expected); if addresses.is_empty() { - assert_eq!(attempts_remaining, 0); + assert_eq!(Into::::into(&attempts_remaining), 0); return Poll::Ready(Ok(())); } else { - assert_eq!(attempts_remaining, addresses.len() as u32); + assert_eq!(Into::::into(&attempts_remaining), addresses.len() as u32); } } Poll::Ready(_) => unreachable!(), From 7d9285ff5695205a4a686d2ba0c1e0abcb528b31 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Thu, 19 Aug 2021 19:08:13 +0200 Subject: [PATCH 10/38] core/CHANGELOG: Add entry for ConnectionLimit change --- core/CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/core/CHANGELOG.md b/core/CHANGELOG.md index 695d1522ea9..40b74e05d9e 100644 --- a/core/CHANGELOG.md +++ b/core/CHANGELOG.md @@ -20,10 +20,16 @@ - Require `ConnectionHandler::{InEvent,OutEvent,Error}` to implement `Debug` (see [PR 2183]). +- Report `ConnectionLimit` error through `ConnectionError` and thus through + `NetworkEvent::ConnectionClosed` instead of previously through + `PendingConnectionError` and thus `NetworkEvent::{IncomingConnectionError, + DialError}` (see [PR 2191]). + [PR 2145]: https://github.com/libp2p/rust-libp2p/pull/2145 [PR 2142]: https://github.com/libp2p/rust-libp2p/pull/2142 [PR 2137]: https://github.com/libp2p/rust-libp2p/pull/2137 [PR 2183]: https://github.com/libp2p/rust-libp2p/pull/2183 +[PR 2191]: https://github.com/libp2p/rust-libp2p/pull/2191 # 0.29.0 [2021-07-12] From 682f6be2bb83268698f0759beb8d31a898cd9abc Mon Sep 17 00:00:00 2001 From: Max Inden Date: Thu, 19 Aug 2021 19:41:10 +0200 Subject: [PATCH 11/38] protocols/*: Update --- protocols/floodsub/src/layer.rs | 17 +++--- protocols/gossipsub/src/behaviour.rs | 63 ++++++---------------- protocols/gossipsub/src/behaviour/tests.rs | 2 +- protocols/gossipsub/src/protocol.rs | 2 +- protocols/identify/src/identify.rs | 9 +--- protocols/kad/src/behaviour.rs | 12 +++-- protocols/mdns/src/behaviour.rs | 2 +- protocols/ping/src/lib.rs | 2 +- protocols/relay/src/behaviour.rs | 13 +++-- protocols/relay/tests/lib.rs | 4 +- protocols/request-response/src/lib.rs | 11 ++-- swarm/src/behaviour.rs | 22 ++++---- 12 files changed, 69 insertions(+), 90 deletions(-) diff --git a/protocols/floodsub/src/layer.rs b/protocols/floodsub/src/layer.rs index eb5a7cb30b2..a17fd5589f7 100644 --- a/protocols/floodsub/src/layer.rs +++ b/protocols/floodsub/src/layer.rs @@ -40,7 +40,12 @@ use std::{collections::VecDeque, iter}; /// Network behaviour that handles the floodsub protocol. pub struct Floodsub { /// Events that need to be yielded to the outside when polling. - events: VecDeque>, + events: VecDeque< + NetworkBehaviourAction< + FloodsubEvent, + OneShotHandler, + >, + >, config: FloodsubConfig, @@ -104,6 +109,7 @@ impl Floodsub { self.events.push_back(NetworkBehaviourAction::DialPeer { peer_id, condition: DialPeerCondition::Disconnected, + handler: self.new_handler(), }); } } @@ -302,9 +308,11 @@ impl NetworkBehaviour for Floodsub { // We can be disconnected by the remote in case of inactivity for example, so we always // try to reconnect. if self.target_peers.contains(id) { + let handler = self.new_handler(); self.events.push_back(NetworkBehaviourAction::DialPeer { peer_id: *id, condition: DialPeerCondition::Disconnected, + handler, }); } } @@ -426,12 +434,7 @@ impl NetworkBehaviour for Floodsub { &mut self, _: &mut Context<'_>, _: &mut impl PollParameters, - ) -> Poll< - NetworkBehaviourAction< - ::InEvent, - Self::OutEvent, - >, - > { + ) -> Poll> { if let Some(event) = self.events.pop_front() { return Poll::Ready(event); } diff --git a/protocols/gossipsub/src/behaviour.rs b/protocols/gossipsub/src/behaviour.rs index 803f79c924b..ac96d82e4ee 100644 --- a/protocols/gossipsub/src/behaviour.rs +++ b/protocols/gossipsub/src/behaviour.rs @@ -41,8 +41,8 @@ use libp2p_core::{ multiaddr::Protocol::Ip6, ConnectedPoint, Multiaddr, PeerId, }; use libp2p_swarm::{ - DialPeerCondition, NetworkBehaviour, NetworkBehaviourAction, NotifyHandler, PollParameters, - ProtocolsHandler, + DialPeerCondition, IntoProtocolsHandler, NetworkBehaviour, NetworkBehaviourAction, + NotifyHandler, PollParameters, }; use crate::backoff::BackoffStorage; @@ -193,7 +193,7 @@ impl From for PublishConfig { } type GossipsubNetworkBehaviourAction = - NetworkBehaviourAction, GossipsubEvent>; + NetworkBehaviourAction>; /// Network behaviour that handles the gossipsub protocol. /// @@ -425,8 +425,8 @@ where impl Gossipsub where - D: DataTransform, - F: TopicSubscriptionFilter, + D: DataTransform + Send + 'static, + F: TopicSubscriptionFilter + Send + 'static, { /// Lists the hashes of the topics we are currently subscribed to. pub fn topics(&self) -> impl Iterator { @@ -1043,9 +1043,11 @@ where if !self.peer_topics.contains_key(peer_id) { // Connect to peer debug!("Connecting to explicit peer {:?}", peer_id); + let handler = self.new_handler(); self.events.push_back(NetworkBehaviourAction::DialPeer { peer_id: *peer_id, condition: DialPeerCondition::Disconnected, + handler, }); } } @@ -1493,9 +1495,11 @@ where self.px_peers.insert(peer_id); // dial peer + let handler = self.new_handler(); self.events.push_back(NetworkBehaviourAction::DialPeer { peer_id, condition: DialPeerCondition::Disconnected, + handler, }); } } @@ -2969,6 +2973,7 @@ where peer_id: &PeerId, connection_id: &ConnectionId, endpoint: &ConnectedPoint, + _: ::Handler, ) { // Remove IP from peer scoring system if let Some((peer_score, ..)) = &mut self.peer_score { @@ -3169,47 +3174,13 @@ where &mut self, cx: &mut Context<'_>, _: &mut impl PollParameters, - ) -> Poll< - NetworkBehaviourAction< - ::InEvent, - Self::OutEvent, - >, - > { + ) -> Poll> { if let Some(event) = self.events.pop_front() { - return Poll::Ready(match event { - NetworkBehaviourAction::NotifyHandler { - peer_id, - handler, - event: send_event, - } => { - // clone send event reference if others references are present - let event = Arc::try_unwrap(send_event).unwrap_or_else(|e| (*e).clone()); - NetworkBehaviourAction::NotifyHandler { - peer_id, - event, - handler, - } - } - NetworkBehaviourAction::GenerateEvent(e) => { - NetworkBehaviourAction::GenerateEvent(e) - } - NetworkBehaviourAction::DialAddress { address } => { - NetworkBehaviourAction::DialAddress { address } - } - NetworkBehaviourAction::DialPeer { peer_id, condition } => { - NetworkBehaviourAction::DialPeer { peer_id, condition } - } - NetworkBehaviourAction::ReportObservedAddr { address, score } => { - NetworkBehaviourAction::ReportObservedAddr { address, score } - } - NetworkBehaviourAction::CloseConnection { - peer_id, - connection, - } => NetworkBehaviourAction::CloseConnection { - peer_id, - connection, - }, - }); + let event: NetworkBehaviourAction> = event; + return Poll::Ready(event.map_in(|e: Arc| { + // clone send event reference if others references are present + Arc::try_unwrap(e).unwrap_or_else(|e| (*e).clone()) + })); } // update scores @@ -3396,7 +3367,7 @@ impl fmt::Debug for Gossipsub) -> fmt::Result { f.debug_struct("Gossipsub") .field("config", &self.config) - .field("events", &self.events) + .field("events", &self.events.len()) .field("control_pool", &self.control_pool) .field("publish_config", &self.publish_config) .field("topic_peers", &self.topic_peers) diff --git a/protocols/gossipsub/src/behaviour/tests.rs b/protocols/gossipsub/src/behaviour/tests.rs index 463452b0cb6..ead3e6d89a9 100644 --- a/protocols/gossipsub/src/behaviour/tests.rs +++ b/protocols/gossipsub/src/behaviour/tests.rs @@ -67,7 +67,7 @@ mod tests { F: TopicSubscriptionFilter + Clone + Default + Send + 'static, { pub fn create_network(self) -> (Gossipsub, Vec, Vec) { - let keypair = libp2p_core::identity::Keypair::generate_secp256k1(); + let keypair = libp2p_core::identity::Keypair::generate_ed25519(); // create a gossipsub struct let mut gs: Gossipsub = Gossipsub::new_with_subscription_filter_and_transform( MessageAuthenticity::Signed(keypair), diff --git a/protocols/gossipsub/src/protocol.rs b/protocols/gossipsub/src/protocol.rs index 199d210452a..d62605f6807 100644 --- a/protocols/gossipsub/src/protocol.rs +++ b/protocols/gossipsub/src/protocol.rs @@ -600,7 +600,7 @@ mod tests { fn arbitrary(g: &mut G) -> Self { let keypair = if g.gen() { // Small enough to be inlined. - Keypair::generate_secp256k1() + Keypair::generate_ed25519() } else { // Too large to be inlined. let mut rsa_key = hex::decode("308204bd020100300d06092a864886f70d0101010500048204a7308204a30201000282010100ef930f41a71288b643c1cbecbf5f72ab53992249e2b00835bf07390b6745419f3848cbcc5b030faa127bc88cdcda1c1d6f3ff699f0524c15ab9d2c9d8015f5d4bd09881069aad4e9f91b8b0d2964d215cdbbae83ddd31a7622a8228acee07079f6e501aea95508fa26c6122816ef7b00ac526d422bd12aed347c37fff6c1c307f3ba57bb28a7f28609e0bdcc839da4eedca39f5d2fa855ba4b0f9c763e9764937db929a1839054642175312a3de2d3405c9d27bdf6505ef471ce85c5e015eee85bf7874b3d512f715de58d0794fd8afe021c197fbd385bb88a930342fac8da31c27166e2edab00fa55dc1c3814448ba38363077f4e8fe2bdea1c081f85f1aa6f02030100010282010028ff427a1aac1a470e7b4879601a6656193d3857ea79f33db74df61e14730e92bf9ffd78200efb0c40937c3356cbe049cd32e5f15be5c96d5febcaa9bd3484d7fded76a25062d282a3856a1b3b7d2c525cdd8434beae147628e21adf241dd64198d5819f310d033743915ba40ea0b6acdbd0533022ad6daa1ff42de51885f9e8bab2306c6ef1181902d1cd7709006eba1ab0587842b724e0519f295c24f6d848907f772ae9a0953fc931f4af16a07df450fb8bfa94572562437056613647818c238a6ff3f606cffa0533e4b8755da33418dfbc64a85110b1a036623c947400a536bb8df65e5ebe46f2dfd0cfc86e7aeeddd7574c253e8fbf755562b3669525d902818100f9fff30c6677b78dd31ec7a634361438457e80be7a7faf390903067ea8355faa78a1204a82b6e99cb7d9058d23c1ecf6cfe4a900137a00cecc0113fd68c5931602980267ea9a95d182d48ba0a6b4d5dd32fdac685cb2e5d8b42509b2eb59c9579ea6a67ccc7547427e2bd1fb1f23b0ccb4dd6ba7d206c8dd93253d70a451701302818100f5530dfef678d73ce6a401ae47043af10a2e3f224c71ae933035ecd68ccbc4df52d72bc6ca2b17e8faf3e548b483a2506c0369ab80df3b137b54d53fac98f95547c2bc245b416e650ce617e0d29db36066f1335a9ba02ad3e0edf9dc3d58fd835835042663edebce81803972696c789012847cb1f854ab2ac0a1bd3867ac7fb502818029c53010d456105f2bf52a9a8482bca2224a5eac74bf3cc1a4d5d291fafcdffd15a6a6448cce8efdd661f6617ca5fc37c8c885cc3374e109ac6049bcbf72b37eabf44602a2da2d4a1237fd145c863e6d75059976de762d9d258c42b0984e2a2befa01c95217c3ee9c736ff209c355466ff99375194eff943bc402ea1d172a1ed02818027175bf493bbbfb8719c12b47d967bf9eac061c90a5b5711172e9095c38bb8cc493c063abffe4bea110b0a2f22ac9311b3947ba31b7ef6bfecf8209eebd6d86c316a2366bbafda7279b2b47d5bb24b6202254f249205dcad347b574433f6593733b806f84316276c1990a016ce1bbdbe5f650325acc7791aefe515ecc60063bd02818100b6a2077f4adcf15a17092d9c4a346d6022ac48f3861b73cf714f84c440a07419a7ce75a73b9cbff4597c53c128bf81e87b272d70428a272d99f90cd9b9ea1033298e108f919c6477400145a102df3fb5601ffc4588203cf710002517bfa24e6ad32f4d09c6b1a995fa28a3104131bedd9072f3b4fb4a5c2056232643d310453f").unwrap(); diff --git a/protocols/identify/src/identify.rs b/protocols/identify/src/identify.rs index caa737f669c..57dcbd9f3b5 100644 --- a/protocols/identify/src/identify.rs +++ b/protocols/identify/src/identify.rs @@ -52,7 +52,7 @@ pub struct Identify { /// Pending replies to send. pending_replies: VecDeque, /// Pending events to be emitted when polled. - events: VecDeque>, + events: VecDeque>, /// Peers to which an active push with current information about /// the local peer should be sent. pending_push: HashSet, @@ -292,12 +292,7 @@ impl NetworkBehaviour for Identify { &mut self, cx: &mut Context<'_>, params: &mut impl PollParameters, - ) -> Poll< - NetworkBehaviourAction< - ::InEvent, - Self::OutEvent, - >, - > { + ) -> Poll> { if let Some(event) = self.events.pop_front() { return Poll::Ready(event); } diff --git a/protocols/kad/src/behaviour.rs b/protocols/kad/src/behaviour.rs index c7cd51b95b7..2f2aa07df9c 100644 --- a/protocols/kad/src/behaviour.rs +++ b/protocols/kad/src/behaviour.rs @@ -98,7 +98,7 @@ pub struct Kademlia { connection_idle_timeout: Duration, /// Queued events to return when the behaviour is being polled. - queued_events: VecDeque, KademliaEvent>>, + queued_events: VecDeque>>, /// The currently known addresses of the local node. local_addrs: HashSet, @@ -561,10 +561,12 @@ where RoutingUpdate::Failed } kbucket::InsertResult::Pending { disconnected } => { + let handler = self.new_handler(), self.queued_events .push_back(NetworkBehaviourAction::DialPeer { peer_id: disconnected.into_preimage(), condition: DialPeerCondition::Disconnected, + handler, }); RoutingUpdate::Pending } @@ -1140,10 +1142,12 @@ where // // Only try dialing peer if not currently connected. if !self.connected_peers.contains(disconnected.preimage()) { + let handler = self.new_handler(), self.queued_events .push_back(NetworkBehaviourAction::DialPeer { peer_id: disconnected.into_preimage(), condition: DialPeerCondition::Disconnected, + handler, }) } } @@ -1859,7 +1863,7 @@ where } } - fn inject_dial_failure(&mut self, peer_id: &PeerId) { + fn inject_dial_failure(&mut self, peer_id: &PeerId, _: Self::ProtocolsHandler) { for query in self.queries.iter_mut() { query.on_failure(peer_id); } @@ -2156,7 +2160,7 @@ where &mut self, cx: &mut Context<'_>, parameters: &mut impl PollParameters, - ) -> Poll, Self::OutEvent>> { + ) -> Poll> { let now = Instant::now(); // Calculate the available capacity for queries triggered by background jobs. @@ -2254,10 +2258,12 @@ where }); } else if &peer_id != self.kbuckets.local_key().preimage() { query.inner.pending_rpcs.push((peer_id, event)); + let handler = self.new_handler(), self.queued_events .push_back(NetworkBehaviourAction::DialPeer { peer_id, condition: DialPeerCondition::Disconnected, + handler, }); } } diff --git a/protocols/mdns/src/behaviour.rs b/protocols/mdns/src/behaviour.rs index 2a170e2d839..9e5a4b9ca20 100644 --- a/protocols/mdns/src/behaviour.rs +++ b/protocols/mdns/src/behaviour.rs @@ -289,8 +289,8 @@ impl NetworkBehaviour for Mdns { params: &mut impl PollParameters, ) -> Poll< NetworkBehaviourAction< - ::InEvent, Self::OutEvent, + DummyProtocolsHandler, >, > { while let Poll::Ready(event) = Pin::new(&mut self.if_watch).poll(cx) { diff --git a/protocols/ping/src/lib.rs b/protocols/ping/src/lib.rs index d4e3828f430..476ca5d7b01 100644 --- a/protocols/ping/src/lib.rs +++ b/protocols/ping/src/lib.rs @@ -103,7 +103,7 @@ impl NetworkBehaviour for Ping { &mut self, _: &mut Context<'_>, _: &mut impl PollParameters, - ) -> Poll> { + ) -> Poll> { if let Some(e) = self.events.pop_back() { Poll::Ready(NetworkBehaviourAction::GenerateEvent(e)) } else { diff --git a/protocols/relay/src/behaviour.rs b/protocols/relay/src/behaviour.rs index 9b17eca2c51..9d0186ff17e 100644 --- a/protocols/relay/src/behaviour.rs +++ b/protocols/relay/src/behaviour.rs @@ -29,7 +29,8 @@ use libp2p_core::connection::{ConnectedPoint, ConnectionId, ListenerId}; use libp2p_core::multiaddr::Multiaddr; use libp2p_core::PeerId; use libp2p_swarm::{ - DialPeerCondition, NetworkBehaviour, NetworkBehaviourAction, NotifyHandler, PollParameters, + DialPeerCondition, IntoProtocolsHandler, NetworkBehaviour, NetworkBehaviourAction, + NotifyHandler, PollParameters, }; use std::collections::{hash_map::Entry, HashMap, HashSet, VecDeque}; use std::task::{Context, Poll}; @@ -45,7 +46,7 @@ pub struct Relay { /// [`Self::listeners`] or [`Self::listener_any_relay`]. outbox_to_listeners: VecDeque<(PeerId, BehaviourToListenerMsg)>, /// Events that need to be yielded to the outside when polling. - outbox_to_swarm: VecDeque>, + outbox_to_swarm: VecDeque>, /// List of peers the network is connected to. connected_peers: HashMap>, @@ -301,7 +302,7 @@ impl NetworkBehaviour for Relay { } } - fn inject_dial_failure(&mut self, peer_id: &PeerId) { + fn inject_dial_failure(&mut self, peer_id: &PeerId, _: Self::ProtocolsHandler) { if let Entry::Occupied(o) = self.listeners.entry(*peer_id) { if matches!(o.get(), RelayListener::Connecting { .. }) { // By removing the entry, the channel to the listener is dropped and thus the @@ -340,6 +341,7 @@ impl NetworkBehaviour for Relay { peer: &PeerId, connection: &ConnectionId, _: &ConnectedPoint, + _: ::Handler, ) { // Remove connection from the set of connections for the given peer. In case the set is // empty it will be removed in `inject_disconnected`. @@ -476,6 +478,7 @@ impl NetworkBehaviour for Relay { .push_back(NetworkBehaviourAction::DialPeer { peer_id: dest_id, condition: DialPeerCondition::NotDialing, + handler: self.new_handler(), }); } else { self.outbox_to_swarm @@ -562,7 +565,7 @@ impl NetworkBehaviour for Relay { &mut self, cx: &mut Context<'_>, poll_parameters: &mut impl PollParameters, - ) -> Poll> { + ) -> Poll> { if !self.outbox_to_listeners.is_empty() { let relay_peer_id = self.outbox_to_listeners[0].0; @@ -668,6 +671,7 @@ impl NetworkBehaviour for Relay { return Poll::Ready(NetworkBehaviourAction::DialPeer { peer_id: relay_peer_id, condition: DialPeerCondition::Disconnected, + handler: self.new_handler(), }); } } @@ -734,6 +738,7 @@ impl NetworkBehaviour for Relay { return Poll::Ready(NetworkBehaviourAction::DialPeer { peer_id: relay_peer_id, condition: DialPeerCondition::Disconnected, + handler: self.new_handler(), }); } } diff --git a/protocols/relay/tests/lib.rs b/protocols/relay/tests/lib.rs index 0829ec87d7b..67259dae77a 100644 --- a/protocols/relay/tests/lib.rs +++ b/protocols/relay/tests/lib.rs @@ -1147,11 +1147,11 @@ enum CombinedEvent { } impl CombinedBehaviour { - fn poll( + fn poll( &mut self, _: &mut Context, _: &mut impl PollParameters, - ) -> Poll> { + ) -> Poll::ProtocolsHandler> { if !self.events.is_empty() { return Poll::Ready(NetworkBehaviourAction::GenerateEvent(self.events.remove(0))); } diff --git a/protocols/request-response/src/lib.rs b/protocols/request-response/src/lib.rs index a2277e4c8df..0f23eed7130 100644 --- a/protocols/request-response/src/lib.rs +++ b/protocols/request-response/src/lib.rs @@ -303,7 +303,7 @@ impl RequestResponseConfig { /// A request/response protocol for some message codec. pub struct RequestResponse where - TCodec: RequestResponseCodec, + TCodec: RequestResponseCodec + Send, { /// The supported inbound protocols. inbound_protocols: SmallVec<[TCodec::Protocol; 2]>, @@ -320,8 +320,8 @@ where /// Pending events to return from `poll`. pending_events: VecDeque< NetworkBehaviourAction< - RequestProtocol, RequestResponseEvent, + RequestResponseHandler, >, >, /// The currently connected peers, their pending outbound and inbound responses and their known, @@ -863,12 +863,7 @@ where &mut self, _: &mut Context<'_>, _: &mut impl PollParameters, - ) -> Poll< - NetworkBehaviourAction< - RequestProtocol, - RequestResponseEvent, - >, - > { + ) -> Poll> { if let Some(ev) = self.pending_events.pop_front() { return Poll::Ready(ev); } else if self.pending_events.capacity() > EMPTY_QUEUE_SHRINK_THRESHOLD { diff --git a/swarm/src/behaviour.rs b/swarm/src/behaviour.rs index 3dd02bac308..0bec20f032d 100644 --- a/swarm/src/behaviour.rs +++ b/swarm/src/behaviour.rs @@ -247,8 +247,11 @@ pub trait NetworkBehaviourEventProcess { // [`NetworkBehaviourAction::map_in`], mapping the handler `InEvent` leaving the // handler itself untouched. #[derive(Debug)] -pub enum NetworkBehaviourAction> -{ +pub enum NetworkBehaviourAction< + TOutEvent, + THandler: IntoProtocolsHandler, + TInEvent = THandlerInEvent, +> { /// Instructs the `Swarm` to return an event when it is being polled. GenerateEvent(TOutEvent), @@ -335,15 +338,14 @@ pub enum NetworkBehaviourAction NetworkBehaviourAction { +impl + NetworkBehaviourAction +{ /// Map the handler event. - pub fn map_in( + pub fn map_in( self, - f: impl FnOnce(THandlerInEvent) -> New, - ) -> NetworkBehaviourAction - where - Self: , - { + f: impl FnOnce(TInEventOld) -> TInEventNew, + ) -> NetworkBehaviourAction { match self { NetworkBehaviourAction::GenerateEvent(e) => NetworkBehaviourAction::GenerateEvent(e), NetworkBehaviourAction::DialAddress { address, handler } => { @@ -379,7 +381,9 @@ impl NetworkBehaviourAction NetworkBehaviourAction { /// Map the event the swarm will return. pub fn map_out(self, f: impl FnOnce(TOutEvent) -> E) -> NetworkBehaviourAction { match self { From 62c5e13072a97526ee46a54c760625ba6c216cb1 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Thu, 19 Aug 2021 21:46:58 +0200 Subject: [PATCH 12/38] protocols/*: Update --- Cargo.toml | 2 +- core/tests/connection_limits.rs | 6 +++--- misc/metrics/src/swarm.rs | 6 +++--- protocols/floodsub/src/layer.rs | 3 ++- protocols/identify/src/identify.rs | 10 +++++++--- protocols/kad/src/behaviour.rs | 7 ++++--- protocols/relay/src/behaviour.rs | 3 ++- swarm-derive/CHANGELOG.md | 2 ++ swarm-derive/Cargo.toml | 2 +- swarm-derive/src/lib.rs | 12 ++++++------ swarm/src/test.rs | 29 +++++++++++++++++------------ 11 files changed, 48 insertions(+), 34 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f53f0283846..4414f62adda 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -80,7 +80,7 @@ libp2p-pnet = { version = "0.21.0", path = "transports/pnet", optional = true } libp2p-relay = { version = "0.4.0", path = "protocols/relay", optional = true } libp2p-request-response = { version = "0.13.0", path = "protocols/request-response", optional = true } libp2p-swarm = { version = "0.31.0", path = "swarm" } -libp2p-swarm-derive = { version = "0.24.0", path = "swarm-derive" } +libp2p-swarm-derive = { version = "0.25.0", path = "swarm-derive" } libp2p-uds = { version = "0.30.0", path = "transports/uds", optional = true } libp2p-wasm-ext = { version = "0.30.0", path = "transports/wasm-ext", default-features = false, optional = true } libp2p-yamux = { version = "0.34.0", path = "muxers/yamux", optional = true } diff --git a/core/tests/connection_limits.rs b/core/tests/connection_limits.rs index 471f054caf3..11d61797f03 100644 --- a/core/tests/connection_limits.rs +++ b/core/tests/connection_limits.rs @@ -53,9 +53,9 @@ fn max_outgoing() { .dial(Multiaddr::empty(), Vec::new(), TestHandler()) .expect_err("Unexpected dialing success.") { - DialError::ConnectionLimit(err) => { - assert_eq!(err.current, outgoing_limit); - assert_eq!(err.limit, outgoing_limit); + DialError::ConnectionLimit{limit, handler: _} => { + assert_eq!(limit.current, outgoing_limit); + assert_eq!(limit.limit, outgoing_limit); } e => panic!("Unexpected error: {:?}", e), } diff --git a/misc/metrics/src/swarm.rs b/misc/metrics/src/swarm.rs index c5c4558ca7f..630479a2f9c 100644 --- a/misc/metrics/src/swarm.rs +++ b/misc/metrics/src/swarm.rs @@ -230,7 +230,7 @@ enum PendingConnectionError { InvalidPeerId, TransportErrorMultiaddrNotSupported, TransportErrorOther, - ConnectionLimit, + Aborted, Io, } @@ -248,8 +248,8 @@ impl From<&libp2p_core::connection::PendingConnectionError libp2p_core::connection::PendingConnectionError::Transport( libp2p_core::transport::TransportError::Other(_), ) => PendingConnectionError::TransportErrorOther, - libp2p_core::connection::PendingConnectionError::ConnectionLimit(_) => { - PendingConnectionError::ConnectionLimit + libp2p_core::connection::PendingConnectionError::Aborted => { + PendingConnectionError::Aborted } libp2p_core::connection::PendingConnectionError::IO(_) => PendingConnectionError::Io, } diff --git a/protocols/floodsub/src/layer.rs b/protocols/floodsub/src/layer.rs index a17fd5589f7..20101eb64e9 100644 --- a/protocols/floodsub/src/layer.rs +++ b/protocols/floodsub/src/layer.rs @@ -106,10 +106,11 @@ impl Floodsub { } if self.target_peers.insert(peer_id) { + let handler = self.new_handler(); self.events.push_back(NetworkBehaviourAction::DialPeer { peer_id, condition: DialPeerCondition::Disconnected, - handler: self.new_handler(), + handler, }); } } diff --git a/protocols/identify/src/identify.rs b/protocols/identify/src/identify.rs index 57dcbd9f3b5..f1d21d4c6db 100644 --- a/protocols/identify/src/identify.rs +++ b/protocols/identify/src/identify.rs @@ -27,8 +27,9 @@ use libp2p_core::{ ConnectedPoint, Multiaddr, PeerId, PublicKey, }; use libp2p_swarm::{ - AddressScore, DialPeerCondition, NegotiatedSubstream, NetworkBehaviour, NetworkBehaviourAction, - NotifyHandler, PollParameters, ProtocolsHandler, ProtocolsHandlerUpgrErr, + AddressScore, DialPeerCondition, IntoProtocolsHandler, NegotiatedSubstream, NetworkBehaviour, + NetworkBehaviourAction, NotifyHandler, PollParameters, ProtocolsHandler, + ProtocolsHandlerUpgrErr, }; use std::{ collections::{HashMap, HashSet, VecDeque}, @@ -173,9 +174,11 @@ impl Identify { for p in peers { if self.pending_push.insert(p) { if !self.connected.contains_key(&p) { + let handler = self.new_handler(); self.events.push_back(NetworkBehaviourAction::DialPeer { peer_id: p, condition: DialPeerCondition::Disconnected, + handler, }); } } @@ -213,13 +216,14 @@ impl NetworkBehaviour for Identify { peer_id: &PeerId, conn: &ConnectionId, _: &ConnectedPoint, + _: ::Handler, ) { if let Some(addrs) = self.connected.get_mut(peer_id) { addrs.remove(conn); } } - fn inject_dial_failure(&mut self, peer_id: &PeerId) { + fn inject_dial_failure(&mut self, peer_id: &PeerId, _: Self::ProtocolsHandler) { if !self.connected.contains_key(peer_id) { self.pending_push.remove(peer_id); } diff --git a/protocols/kad/src/behaviour.rs b/protocols/kad/src/behaviour.rs index 2f2aa07df9c..c28230a9060 100644 --- a/protocols/kad/src/behaviour.rs +++ b/protocols/kad/src/behaviour.rs @@ -394,6 +394,7 @@ impl KademliaConfig { impl Kademlia where for<'a> TStore: RecordStore<'a>, + TStore: Send + 'static, { /// Creates a new `Kademlia` network behaviour with a default configuration. pub fn new(id: PeerId, store: TStore) -> Self { @@ -561,7 +562,7 @@ where RoutingUpdate::Failed } kbucket::InsertResult::Pending { disconnected } => { - let handler = self.new_handler(), + let handler = self.new_handler(); self.queued_events .push_back(NetworkBehaviourAction::DialPeer { peer_id: disconnected.into_preimage(), @@ -1142,7 +1143,7 @@ where // // Only try dialing peer if not currently connected. if !self.connected_peers.contains(disconnected.preimage()) { - let handler = self.new_handler(), + let handler = self.new_handler(); self.queued_events .push_back(NetworkBehaviourAction::DialPeer { peer_id: disconnected.into_preimage(), @@ -2258,7 +2259,7 @@ where }); } else if &peer_id != self.kbuckets.local_key().preimage() { query.inner.pending_rpcs.push((peer_id, event)); - let handler = self.new_handler(), + let handler = self.new_handler(); self.queued_events .push_back(NetworkBehaviourAction::DialPeer { peer_id, diff --git a/protocols/relay/src/behaviour.rs b/protocols/relay/src/behaviour.rs index 9d0186ff17e..68653c5c88a 100644 --- a/protocols/relay/src/behaviour.rs +++ b/protocols/relay/src/behaviour.rs @@ -474,11 +474,12 @@ impl NetworkBehaviour for Relay { src_connection_id: connection, }, ); + let handler = self.new_handler(); self.outbox_to_swarm .push_back(NetworkBehaviourAction::DialPeer { peer_id: dest_id, condition: DialPeerCondition::NotDialing, - handler: self.new_handler(), + handler, }); } else { self.outbox_to_swarm diff --git a/swarm-derive/CHANGELOG.md b/swarm-derive/CHANGELOG.md index 94ea25f6b10..ff800bfe5eb 100644 --- a/swarm-derive/CHANGELOG.md +++ b/swarm-derive/CHANGELOG.md @@ -1,3 +1,5 @@ +# 0.25.0 [unreleased] + # 0.24.0 [2021-07-12] - Handle `NetworkBehaviourAction::CloseConnection`. See [PR 2110] for details. diff --git a/swarm-derive/Cargo.toml b/swarm-derive/Cargo.toml index e63b7a71dd8..2da7fc8a34b 100644 --- a/swarm-derive/Cargo.toml +++ b/swarm-derive/Cargo.toml @@ -2,7 +2,7 @@ name = "libp2p-swarm-derive" edition = "2018" description = "Procedural macros of libp2p-core" -version = "0.24.0" +version = "0.25.0" authors = ["Parity Technologies "] license = "MIT" repository = "https://github.com/libp2p/rust-libp2p" diff --git a/swarm-derive/src/lib.rs b/swarm-derive/src/lib.rs index 3f92e549fa9..7ce28977f73 100644 --- a/swarm-derive/src/lib.rs +++ b/swarm-derive/src/lib.rs @@ -518,11 +518,11 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { loop { match #trait_to_impl::poll(&mut #field_name, cx, poll_params) { #generate_event_match_arm - std::task::Poll::Ready(#network_behaviour_action::DialAddress { address }) => { - return std::task::Poll::Ready(#network_behaviour_action::DialAddress { address }); + std::task::Poll::Ready(#network_behaviour_action::DialAddress { address, handler }) => { + return std::task::Poll::Ready(#network_behaviour_action::DialAddress { address, handler }); } - std::task::Poll::Ready(#network_behaviour_action::DialPeer { peer_id, condition }) => { - return std::task::Poll::Ready(#network_behaviour_action::DialPeer { peer_id, condition }); + std::task::Poll::Ready(#network_behaviour_action::DialPeer { peer_id, condition, handler }) => { + return std::task::Poll::Ready(#network_behaviour_action::DialPeer { peer_id, condition, handler }); } std::task::Poll::Ready(#network_behaviour_action::NotifyHandler { peer_id, handler, event }) => { return std::task::Poll::Ready(#network_behaviour_action::NotifyHandler { @@ -586,7 +586,7 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { #(#inject_addr_reach_failure_stmts);* } - fn inject_dial_failure(&mut self, peer_id: &#peer_id) { + fn inject_dial_failure(&mut self, peer_id: &#peer_id, handler: Self::ProtocolsHandler) { #(#inject_dial_failure_stmts);* } @@ -629,7 +629,7 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { } } - fn poll(&mut self, cx: &mut std::task::Context, poll_params: &mut impl #poll_parameters) -> std::task::Poll<#network_behaviour_action<<::Handler as #protocols_handler>::InEvent, Self::OutEvent>> { + fn poll(&mut self, cx: &mut std::task::Context, poll_params: &mut impl #poll_parameters) -> std::task::Poll<#network_behaviour_action> { use libp2p::futures::prelude::*; #(#poll_stmts)* let f: std::task::Poll<#network_behaviour_action<<::Handler as #protocols_handler>::InEvent, Self::OutEvent>> = #poll_method; diff --git a/swarm/src/test.rs b/swarm/src/test.rs index 5cb05d7baf3..5fc3885ea29 100644 --- a/swarm/src/test.rs +++ b/swarm/src/test.rs @@ -45,7 +45,7 @@ where /// The next action to return from `poll`. /// /// An action is only returned once. - pub next_action: Option>, + pub next_action: Option>, } impl MockBehaviour @@ -84,7 +84,7 @@ where &mut self, _: &mut Context, _: &mut impl PollParameters, - ) -> Poll> { + ) -> Poll> { self.next_action.take().map_or(Poll::Pending, Poll::Ready) } } @@ -202,10 +202,16 @@ where self.inner.inject_disconnected(peer); } - fn inject_connection_closed(&mut self, p: &PeerId, c: &ConnectionId, e: &ConnectedPoint) { + fn inject_connection_closed( + &mut self, + p: &PeerId, + c: &ConnectionId, + e: &ConnectedPoint, + handler: ::Handler, + ) { self.inject_connection_closed .push((p.clone(), c.clone(), e.clone())); - self.inner.inject_connection_closed(p, c, e); + self.inner.inject_connection_closed(p, c, e, handler); } fn inject_event( @@ -228,9 +234,9 @@ where self.inner.inject_addr_reach_failure(p, a, e); } - fn inject_dial_failure(&mut self, p: &PeerId) { + fn inject_dial_failure(&mut self, p: &PeerId, handler: Self::ProtocolsHandler) { self.inject_dial_failure.push(p.clone()); - self.inner.inject_dial_failure(p); + self.inner.inject_dial_failure(p, handler); } fn inject_new_listener(&mut self, id: ListenerId) { @@ -268,12 +274,11 @@ where self.inner.inject_listener_closed(l, r); } - fn poll(&mut self, cx: &mut Context, args: &mut impl PollParameters) -> - Poll::Handler as ProtocolsHandler>::InEvent, - Self::OutEvent - >> - { + fn poll( + &mut self, + cx: &mut Context, + args: &mut impl PollParameters, + ) -> Poll> { self.poll += 1; self.inner.poll(cx, args) } From 174693af958c21a17acc1ddec8f8ae55c754d916 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Fri, 20 Aug 2021 13:44:07 +0200 Subject: [PATCH 13/38] swarm-derive: Adjust to changes --- swarm-derive/src/lib.rs | 101 ++++++++++++++++++++------ swarm-derive/tests/test.rs | 10 +-- swarm/src/protocols_handler/select.rs | 8 ++ 3 files changed, 93 insertions(+), 26 deletions(-) diff --git a/swarm-derive/src/lib.rs b/swarm-derive/src/lib.rs index 7ce28977f73..84ae1a131df 100644 --- a/swarm-derive/src/lib.rs +++ b/swarm-derive/src/lib.rs @@ -223,15 +223,31 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { // Build the list of statements to put in the body of `inject_connection_closed()`. let inject_connection_closed_stmts = { - data_struct.fields.iter().enumerate().filter_map(move |(field_n, field)| { - if is_ignored(&field) { - return None; - } - Some(match field.ident { - Some(ref i) => quote!{ self.#i.inject_connection_closed(peer_id, connection_id, endpoint); }, - None => quote!{ self.#field_n.inject_connection_closed(peer_id, connection_id, endpoint); }, + data_struct + .fields + .iter() + .enumerate() + .rev() + .filter(|f| !is_ignored(&f.1)) + .enumerate() + .map(move |(enum_n, (field_n, field))| { + let handler = if field_n == 0 { + quote! { let handler = handlers } + } else { + quote! { + let (handlers, handler) = handlers.into_inner() + } + }; + let inject = match field.ident { + Some(ref i) => quote!{ self.#i.inject_connection_closed(peer_id, connection_id, endpoint, handler) }, + None => quote!{ self.#enum_n.inject_connection_closed(peer_id, connection_id, endpoint, handler) }, + }; + + quote! { + #handler; + #inject; + } }) - }) }; // Build the list of statements to put in the body of `inject_addr_reach_failure()`. @@ -255,15 +271,27 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { .fields .iter() .enumerate() - .filter_map(move |(field_n, field)| { - if is_ignored(&field) { - return None; - } + .rev() + .filter(|f| !is_ignored(&f.1)) + .enumerate() + .map(move |(enum_n, (field_n, field))| { + let handler = if field_n == 0 { + quote! { let handler = handlers } + } else { + quote! { + let (handlers, handler) = handlers.into_inner() + } + }; - Some(match field.ident { - Some(ref i) => quote! { self.#i.inject_dial_failure(peer_id); }, - None => quote! { self.#field_n.inject_dial_failure(peer_id); }, - }) + let inject = match field.ident { + Some(ref i) => quote! { self.#i.inject_dial_failure(peer_id, handler) }, + None => quote! { self.#enum_n.inject_dial_failure(peer_id, handler) }, + }; + + quote! { + #handler; + #inject; + } }) }; @@ -426,6 +454,7 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { ref mut ev @ None => *ev = Some(field_info), } } + // ph_ty = Some(quote! ) ph_ty.unwrap_or(quote! {()}) // TODO: `!` instead }; @@ -500,6 +529,36 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { wrapped_event = quote!{ #either_ident::First(#wrapped_event) }; } + let wrapped_handler = { + let mut out_handler = None; + + for (f_n, f) in data_struct.fields.iter().enumerate() { + if is_ignored(&f) { + continue; + } + + let f_name = match f.ident { + Some(ref i) => quote! { self.#i }, + None => quote! { self.#f_n }, + }; + + let builder = if field_n == f_n { + quote! { handler } + } else { + quote! { #f_name.new_handler() } + }; + + match out_handler { + Some(h) => { + out_handler = Some(quote! { #into_protocols_handler::select(#h, #builder) }) + } + ref mut h @ None => *h = Some(builder), + } + } + + out_handler.unwrap() // _or(quote! {()}) // TODO: incorrect + }; + let generate_event_match_arm = if event_process { quote! { std::task::Poll::Ready(#network_behaviour_action::GenerateEvent(event)) => { @@ -519,10 +578,10 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { match #trait_to_impl::poll(&mut #field_name, cx, poll_params) { #generate_event_match_arm std::task::Poll::Ready(#network_behaviour_action::DialAddress { address, handler }) => { - return std::task::Poll::Ready(#network_behaviour_action::DialAddress { address, handler }); + return std::task::Poll::Ready(#network_behaviour_action::DialAddress { address, handler: #wrapped_handler }); } std::task::Poll::Ready(#network_behaviour_action::DialPeer { peer_id, condition, handler }) => { - return std::task::Poll::Ready(#network_behaviour_action::DialPeer { peer_id, condition, handler }); + return std::task::Poll::Ready(#network_behaviour_action::DialPeer { peer_id, condition, handler: #wrapped_handler }); } std::task::Poll::Ready(#network_behaviour_action::NotifyHandler { peer_id, handler, event }) => { return std::task::Poll::Ready(#network_behaviour_action::NotifyHandler { @@ -578,7 +637,7 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { #(#inject_address_change_stmts);* } - fn inject_connection_closed(&mut self, peer_id: &#peer_id, connection_id: &#connection_id, endpoint: &#connected_point) { + fn inject_connection_closed(&mut self, peer_id: &#peer_id, connection_id: &#connection_id, endpoint: &#connected_point, handlers: ::Handler) { #(#inject_connection_closed_stmts);* } @@ -586,7 +645,7 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { #(#inject_addr_reach_failure_stmts);* } - fn inject_dial_failure(&mut self, peer_id: &#peer_id, handler: Self::ProtocolsHandler) { + fn inject_dial_failure(&mut self, peer_id: &#peer_id, handlers: Self::ProtocolsHandler) { #(#inject_dial_failure_stmts);* } @@ -632,7 +691,7 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { fn poll(&mut self, cx: &mut std::task::Context, poll_params: &mut impl #poll_parameters) -> std::task::Poll<#network_behaviour_action> { use libp2p::futures::prelude::*; #(#poll_stmts)* - let f: std::task::Poll<#network_behaviour_action<<::Handler as #protocols_handler>::InEvent, Self::OutEvent>> = #poll_method; + let f: std::task::Poll<#network_behaviour_action> = #poll_method; f } } diff --git a/swarm-derive/tests/test.rs b/swarm-derive/tests/test.rs index 78a9ed985f9..6932d79b948 100644 --- a/swarm-derive/tests/test.rs +++ b/swarm-derive/tests/test.rs @@ -19,7 +19,7 @@ // DEALINGS IN THE SOFTWARE. use futures::prelude::*; -use libp2p::swarm::SwarmEvent; +use libp2p::swarm::{NetworkBehaviour, SwarmEvent}; use libp2p_swarm_derive::*; /// Small utility to check that a type implements `NetworkBehaviour`. @@ -149,11 +149,11 @@ fn custom_polling() { } impl Foo { - fn foo( + fn foo( &mut self, _: &mut std::task::Context, _: &mut impl libp2p::swarm::PollParameters, - ) -> std::task::Poll> { + ) -> std::task::Poll::OutEvent, ::ProtocolsHandler>> { std::task::Poll::Pending } } @@ -207,11 +207,11 @@ fn custom_event_and_polling() { } impl Foo { - fn foo( + fn foo( &mut self, _: &mut std::task::Context, _: &mut impl libp2p::swarm::PollParameters, - ) -> std::task::Poll> { + ) -> std::task::Poll::OutEvent, ::ProtocolsHandler>> { std::task::Poll::Pending } } diff --git a/swarm/src/protocols_handler/select.rs b/swarm/src/protocols_handler/select.rs index b5891c25d1f..cce84928b5e 100644 --- a/swarm/src/protocols_handler/select.rs +++ b/swarm/src/protocols_handler/select.rs @@ -45,6 +45,10 @@ impl IntoProtocolsHandlerSelect { pub(crate) fn new(proto1: TProto1, proto2: TProto2) -> Self { IntoProtocolsHandlerSelect { proto1, proto2 } } + + pub fn into_inner(self) -> (TProto1, TProto2) { + (self.proto1, self.proto2) + } } impl IntoProtocolsHandler for IntoProtocolsHandlerSelect @@ -87,6 +91,10 @@ impl ProtocolsHandlerSelect { pub(crate) fn new(proto1: TProto1, proto2: TProto2) -> Self { ProtocolsHandlerSelect { proto1, proto2 } } + + pub fn into_inner(self) -> (TProto1, TProto2) { + (self.proto1, self.proto2) + } } impl ProtocolsHandler for ProtocolsHandlerSelect From a78de13fd55b11ceff08ec58e6fff35b0157bedf Mon Sep 17 00:00:00 2001 From: Max Inden Date: Fri, 20 Aug 2021 14:24:32 +0200 Subject: [PATCH 14/38] core/: Fix ConectionClose and PendingAborted reporting --- core/src/connection/pool.rs | 12 ------------ protocols/relay/tests/lib.rs | 2 +- swarm/src/lib.rs | 10 ++++------ 3 files changed, 5 insertions(+), 19 deletions(-) diff --git a/core/src/connection/pool.rs b/core/src/connection/pool.rs index e365b0596aa..8b4aca1d7e3 100644 --- a/core/src/connection/pool.rs +++ b/core/src/connection/pool.rs @@ -384,32 +384,20 @@ impl Pool { /// > performing such an orderly close. pub fn disconnect(&mut self, peer: &PeerId) { if let Some(conns) = self.established.get(peer) { - // Count upwards because we push to / pop from the end. See also `Pool::poll`. for (&id, endpoint) in conns.iter() { if let Some(manager::Entry::Established(e)) = self.manager.entry(id) { e.start_close(None); - // TODO: I removed the disconnected logic, thus depending on start_close to - // eventually trigger a ConnectionClosed event. Make sure that is the case. } - self.counters.dec_established(endpoint); } } - self.established.remove(peer); - let mut aborted = Vec::new(); for (&id, (_endpoint, peer2)) in &self.pending { if Some(peer) == peer2.as_ref() { if let Some(manager::Entry::Pending(e)) = self.manager.entry(id) { e.abort(); - aborted.push(id); } } } - for id in aborted { - if let Some((endpoint, _)) = self.pending.remove(&id) { - self.counters.dec_pending(&endpoint); - } - } } /// Counts the number of established connections to the given peer. diff --git a/protocols/relay/tests/lib.rs b/protocols/relay/tests/lib.rs index 67259dae77a..ddfb26e729c 100644 --- a/protocols/relay/tests/lib.rs +++ b/protocols/relay/tests/lib.rs @@ -1151,7 +1151,7 @@ impl CombinedBehaviour { &mut self, _: &mut Context, _: &mut impl PollParameters, - ) -> Poll::ProtocolsHandler> { + ) -> Poll::ProtocolsHandler>> { if !self.events.is_empty() { return Poll::Ready(NetworkBehaviourAction::GenerateEvent(self.events.remove(0))); } diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index 232bb52e429..6e7c7baa3a9 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -1353,17 +1353,15 @@ mod tests { <::Handler as ProtocolsHandler>::OutEvent: Clone { for s in &[swarm1, swarm2] { - if s.behaviour.inject_connection_closed.len() < num_connections { - assert_eq!(s.behaviour.inject_disconnected.len(), 0); - } else { - assert_eq!(s.behaviour.inject_disconnected.len(), 1); - } assert_eq!(s.behaviour.inject_connection_established.len(), 0); assert_eq!(s.behaviour.inject_connected.len(), 0); } [swarm1, swarm2] .iter() - .all(|s| s.behaviour.inject_connection_closed.len() == num_connections) + .all(|s| s.behaviour.inject_connection_closed.len() == num_connections) && + [swarm1, swarm2] + .iter() + .all(|s| s.behaviour.inject_disconnected.len() == 1) } /// Establishes multiple connections between two peers, From d4960b7e73bc2d47d4e0a039ca1c25dce3800de4 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Fri, 20 Aug 2021 14:30:40 +0200 Subject: [PATCH 15/38] *: Format with rustfmt --- core/src/connection/pool.rs | 2 +- core/tests/connection_limits.rs | 2 +- core/tests/network_dial_error.rs | 5 ++++- protocols/gossipsub/src/behaviour.rs | 6 +++++- protocols/mdns/src/behaviour.rs | 7 +------ protocols/relay/tests/lib.rs | 3 ++- protocols/request-response/src/throttled.rs | 15 +++++++++++---- swarm-derive/tests/test.rs | 14 ++++++++++++-- swarm/src/lib.rs | 8 ++++---- 9 files changed, 41 insertions(+), 21 deletions(-) diff --git a/core/src/connection/pool.rs b/core/src/connection/pool.rs index 8b4aca1d7e3..a8a8b4ace38 100644 --- a/core/src/connection/pool.rs +++ b/core/src/connection/pool.rs @@ -384,7 +384,7 @@ impl Pool { /// > performing such an orderly close. pub fn disconnect(&mut self, peer: &PeerId) { if let Some(conns) = self.established.get(peer) { - for (&id, endpoint) in conns.iter() { + for (&id, _endpoint) in conns.iter() { if let Some(manager::Entry::Established(e)) = self.manager.entry(id) { e.start_close(None); } diff --git a/core/tests/connection_limits.rs b/core/tests/connection_limits.rs index 11d61797f03..d5156664ffb 100644 --- a/core/tests/connection_limits.rs +++ b/core/tests/connection_limits.rs @@ -53,7 +53,7 @@ fn max_outgoing() { .dial(Multiaddr::empty(), Vec::new(), TestHandler()) .expect_err("Unexpected dialing success.") { - DialError::ConnectionLimit{limit, handler: _} => { + DialError::ConnectionLimit { limit, handler: _ } => { assert_eq!(limit.current, outgoing_limit); assert_eq!(limit.limit, outgoing_limit); } diff --git a/core/tests/network_dial_error.rs b/core/tests/network_dial_error.rs index 04aba9fb420..ddbb7a9285e 100644 --- a/core/tests/network_dial_error.rs +++ b/core/tests/network_dial_error.rs @@ -205,7 +205,10 @@ fn multiple_addresses_err() { assert_eq!(Into::::into(&attempts_remaining), 0); return Poll::Ready(Ok(())); } else { - assert_eq!(Into::::into(&attempts_remaining), addresses.len() as u32); + assert_eq!( + Into::::into(&attempts_remaining), + addresses.len() as u32 + ); } } Poll::Ready(_) => unreachable!(), diff --git a/protocols/gossipsub/src/behaviour.rs b/protocols/gossipsub/src/behaviour.rs index ac96d82e4ee..700d3829df3 100644 --- a/protocols/gossipsub/src/behaviour.rs +++ b/protocols/gossipsub/src/behaviour.rs @@ -3176,7 +3176,11 @@ where _: &mut impl PollParameters, ) -> Poll> { if let Some(event) = self.events.pop_front() { - let event: NetworkBehaviourAction> = event; + let event: NetworkBehaviourAction< + Self::OutEvent, + Self::ProtocolsHandler, + Arc, + > = event; return Poll::Ready(event.map_in(|e: Arc| { // clone send event reference if others references are present Arc::try_unwrap(e).unwrap_or_else(|e| (*e).clone()) diff --git a/protocols/mdns/src/behaviour.rs b/protocols/mdns/src/behaviour.rs index 9e5a4b9ca20..215a00be21b 100644 --- a/protocols/mdns/src/behaviour.rs +++ b/protocols/mdns/src/behaviour.rs @@ -287,12 +287,7 @@ impl NetworkBehaviour for Mdns { &mut self, cx: &mut Context<'_>, params: &mut impl PollParameters, - ) -> Poll< - NetworkBehaviourAction< - Self::OutEvent, - DummyProtocolsHandler, - >, - > { + ) -> Poll> { while let Poll::Ready(event) = Pin::new(&mut self.if_watch).poll(cx) { let socket = self.recv_socket.get_ref(); match event { diff --git a/protocols/relay/tests/lib.rs b/protocols/relay/tests/lib.rs index ddfb26e729c..29a0cfd35ed 100644 --- a/protocols/relay/tests/lib.rs +++ b/protocols/relay/tests/lib.rs @@ -1151,7 +1151,8 @@ impl CombinedBehaviour { &mut self, _: &mut Context, _: &mut impl PollParameters, - ) -> Poll::ProtocolsHandler>> { + ) -> Poll::ProtocolsHandler>> + { if !self.events.is_empty() { return Poll::Ready(NetworkBehaviourAction::GenerateEvent(self.events.remove(0))); } diff --git a/protocols/request-response/src/throttled.rs b/protocols/request-response/src/throttled.rs index 46092ef9cc7..72b26d50cf7 100644 --- a/protocols/request-response/src/throttled.rs +++ b/protocols/request-response/src/throttled.rs @@ -448,7 +448,8 @@ where end: &ConnectedPoint, handler: ::Handler, ) { - self.behaviour.inject_connection_closed(peer, id, end, handler); + self.behaviour + .inject_connection_closed(peer, id, end, handler); if let Some(info) = self.peer_info.get_mut(peer) { if let Some(grant) = &mut info.recv_budget.grant { log::debug! { "{:08x}: resending credit grant {} to {} after connection closed", @@ -748,9 +749,15 @@ where NetworkBehaviourAction::DialAddress { address, handler } => { NetworkBehaviourAction::DialAddress { address, handler } } - NetworkBehaviourAction::DialPeer { peer_id, condition, handler } => { - NetworkBehaviourAction::DialPeer { peer_id, condition, handler } - } + NetworkBehaviourAction::DialPeer { + peer_id, + condition, + handler, + } => NetworkBehaviourAction::DialPeer { + peer_id, + condition, + handler, + }, NetworkBehaviourAction::NotifyHandler { peer_id, handler, diff --git a/swarm-derive/tests/test.rs b/swarm-derive/tests/test.rs index 6932d79b948..ef457c83776 100644 --- a/swarm-derive/tests/test.rs +++ b/swarm-derive/tests/test.rs @@ -153,7 +153,12 @@ fn custom_polling() { &mut self, _: &mut std::task::Context, _: &mut impl libp2p::swarm::PollParameters, - ) -> std::task::Poll::OutEvent, ::ProtocolsHandler>> { + ) -> std::task::Poll< + libp2p::swarm::NetworkBehaviourAction< + ::OutEvent, + ::ProtocolsHandler, + >, + > { std::task::Poll::Pending } } @@ -211,7 +216,12 @@ fn custom_event_and_polling() { &mut self, _: &mut std::task::Context, _: &mut impl libp2p::swarm::PollParameters, - ) -> std::task::Poll::OutEvent, ::ProtocolsHandler>> { + ) -> std::task::Poll< + libp2p::swarm::NetworkBehaviourAction< + ::OutEvent, + ::ProtocolsHandler, + >, + > { std::task::Poll::Pending } } diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index 6e7c7baa3a9..b9200b5022b 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -1358,10 +1358,10 @@ mod tests { } [swarm1, swarm2] .iter() - .all(|s| s.behaviour.inject_connection_closed.len() == num_connections) && - [swarm1, swarm2] - .iter() - .all(|s| s.behaviour.inject_disconnected.len() == 1) + .all(|s| s.behaviour.inject_connection_closed.len() == num_connections) + && [swarm1, swarm2] + .iter() + .all(|s| s.behaviour.inject_disconnected.len() == 1) } /// Establishes multiple connections between two peers, From b73139a90badcabda62fb12b0dc3d35c1202fe33 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Fri, 20 Aug 2021 14:37:36 +0200 Subject: [PATCH 16/38] core/src/connection: Remove outdated doc comment --- core/src/connection/pool.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/core/src/connection/pool.rs b/core/src/connection/pool.rs index a8a8b4ace38..85366cbf5fc 100644 --- a/core/src/connection/pool.rs +++ b/core/src/connection/pool.rs @@ -376,12 +376,8 @@ impl Pool { /// (Forcefully) close all connections to the given peer. /// /// All connections to the peer, whether pending or established are - /// dropped asap and no more events from these connections are emitted + /// closed asap and no more events from these connections are emitted /// by the pool effective immediately. - /// - /// > **Note**: Established connections are dropped without performing - /// > an orderly close. See [`EstablishedConnection::start_close`] for - /// > performing such an orderly close. pub fn disconnect(&mut self, peer: &PeerId) { if let Some(conns) = self.established.get(peer) { for (&id, _endpoint) in conns.iter() { From aa02e5fc59acde1652ed2b75e9e2d38455e7179a Mon Sep 17 00:00:00 2001 From: Max Inden Date: Fri, 20 Aug 2021 14:57:22 +0200 Subject: [PATCH 17/38] swarm/src/toggle: Fix TODOs --- swarm/src/behaviour.rs | 54 ++++++++++++++++++++++++++++++++++++++++++ swarm/src/toggle.rs | 15 +++++++----- 2 files changed, 63 insertions(+), 6 deletions(-) diff --git a/swarm/src/behaviour.rs b/swarm/src/behaviour.rs index 0bec20f032d..1257bd3000b 100644 --- a/swarm/src/behaviour.rs +++ b/swarm/src/behaviour.rs @@ -423,6 +423,60 @@ impl NetworkBehaviourAction NetworkBehaviourAction +where + THandlerOld: IntoProtocolsHandler, + ::Handler: ProtocolsHandler, +{ + /// Map the handler event. + pub fn map_handler( + self, + f: impl FnOnce(THandlerOld) -> THandlerNew, + ) -> NetworkBehaviourAction + where + THandlerNew: IntoProtocolsHandler, + ::Handler: ProtocolsHandler, + { + match self { + NetworkBehaviourAction::GenerateEvent(e) => NetworkBehaviourAction::GenerateEvent(e), + NetworkBehaviourAction::DialAddress { address, handler } => { + NetworkBehaviourAction::DialAddress { + address, + handler: f(handler), + } + } + NetworkBehaviourAction::DialPeer { + peer_id, + condition, + handler, + } => NetworkBehaviourAction::DialPeer { + peer_id, + condition, + handler: f(handler), + }, + NetworkBehaviourAction::NotifyHandler { + peer_id, + handler, + event, + } => NetworkBehaviourAction::NotifyHandler { + peer_id, + handler, + event: event, + }, + NetworkBehaviourAction::ReportObservedAddr { address, score } => { + NetworkBehaviourAction::ReportObservedAddr { address, score } + } + NetworkBehaviourAction::CloseConnection { + peer_id, + connection, + } => NetworkBehaviourAction::CloseConnection { + peer_id, + connection, + }, + } + } +} + /// The options w.r.t. which connection handler to notify of an event. #[derive(Debug, Clone)] pub enum NotifyHandler { diff --git a/swarm/src/toggle.rs b/swarm/src/toggle.rs index 84cdbfd3163..578868929d9 100644 --- a/swarm/src/toggle.rs +++ b/swarm/src/toggle.rs @@ -116,8 +116,9 @@ where handler: ::Handler, ) { if let Some(inner) = self.inner.as_mut() { - // TODO: Unwrap safe here? - inner.inject_connection_closed(peer_id, connection, endpoint, handler.inner.unwrap()) + if let Some(handler) = handler.inner { + inner.inject_connection_closed(peer_id, connection, endpoint, handler) + } } } @@ -157,8 +158,9 @@ where fn inject_dial_failure(&mut self, peer_id: &PeerId, handler: Self::ProtocolsHandler) { if let Some(inner) = self.inner.as_mut() { - // TODO: Unwrap safe here? - inner.inject_dial_failure(peer_id, handler.inner.unwrap()) + if let Some(handler) = handler.inner { + inner.inject_dial_failure(peer_id, handler) + } } } @@ -210,8 +212,9 @@ where params: &mut impl PollParameters, ) -> Poll> { if let Some(inner) = self.inner.as_mut() { - // inner.poll(cx, params) - todo!("Wrap handler in Dial events with ToggleHandler."); + inner + .poll(cx, params) + .map(|action| action.map_handler(|h| ToggleIntoProtoHandler { inner: Some(h) })) } else { Poll::Pending } From 2c9f0d3be60b62a112121c19095a2fc281615dfa Mon Sep 17 00:00:00 2001 From: Max Inden Date: Mon, 23 Aug 2021 14:53:42 +0200 Subject: [PATCH 18/38] protocols/: Remove unused imports --- protocols/floodsub/src/layer.rs | 2 +- protocols/ping/src/lib.rs | 1 - protocols/request-response/src/throttled.rs | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/protocols/floodsub/src/layer.rs b/protocols/floodsub/src/layer.rs index 20101eb64e9..25b235e3364 100644 --- a/protocols/floodsub/src/layer.rs +++ b/protocols/floodsub/src/layer.rs @@ -29,7 +29,7 @@ use fnv::FnvHashSet; use libp2p_core::{connection::ConnectionId, PeerId}; use libp2p_swarm::{ DialPeerCondition, NetworkBehaviour, NetworkBehaviourAction, NotifyHandler, OneShotHandler, - PollParameters, ProtocolsHandler, + PollParameters, }; use log::warn; use smallvec::SmallVec; diff --git a/protocols/ping/src/lib.rs b/protocols/ping/src/lib.rs index 476ca5d7b01..70345dd0472 100644 --- a/protocols/ping/src/lib.rs +++ b/protocols/ping/src/lib.rs @@ -49,7 +49,6 @@ pub use handler::{PingConfig, PingFailure, PingResult, PingSuccess}; use libp2p_core::{connection::ConnectionId, PeerId}; use libp2p_swarm::{NetworkBehaviour, NetworkBehaviourAction, PollParameters}; use std::{collections::VecDeque, task::Context, task::Poll}; -use void::Void; /// `Ping` is a [`NetworkBehaviour`] that responds to inbound pings and /// periodically sends outbound pings on every established connection. diff --git a/protocols/request-response/src/throttled.rs b/protocols/request-response/src/throttled.rs index 72b26d50cf7..61a43c4412e 100644 --- a/protocols/request-response/src/throttled.rs +++ b/protocols/request-response/src/throttled.rs @@ -40,7 +40,7 @@ use super::{ ProtocolSupport, RequestId, RequestResponse, RequestResponseCodec, RequestResponseConfig, RequestResponseEvent, RequestResponseMessage, }; -use crate::handler::{RequestProtocol, RequestResponseHandler, RequestResponseHandlerEvent}; +use crate::handler::{RequestResponseHandler, RequestResponseHandlerEvent}; use codec::{Codec, Message, ProtocolWrapper, Type}; use futures::ready; use libp2p_core::{connection::ConnectionId, ConnectedPoint, Multiaddr, PeerId}; From a56980e21f597c344d94a5a62ffd98cb7abd45e8 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Tue, 24 Aug 2021 10:13:37 +0200 Subject: [PATCH 19/38] core/src/network/event: Use NoZeroU32 --- core/src/network.rs | 5 ++--- core/src/network/event.rs | 5 ++--- swarm/src/lib.rs | 28 ++++++++++++++++++---------- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/core/src/network.rs b/core/src/network.rs index df04821da29..8993787f884 100644 --- a/core/src/network.rs +++ b/core/src/network.rs @@ -45,7 +45,7 @@ use std::{ collections::hash_map, convert::TryFrom as _, error, fmt, - num::NonZeroUsize, + num::{NonZeroU32, NonZeroUsize}, pin::Pin, task::{Context, Poll}, }; @@ -599,7 +599,7 @@ where let num_remain = u32::try_from(attempt.remaining.len()).unwrap(); let failed_addr = attempt.current.1.clone(); - let (opts, attempts_remaining) = if num_remain > 0 { + let (opts, attempts_remaining) = if let Some(num_remain) = NonZeroU32::new(num_remain) { let next_attempt = attempt.remaining.remove(0); let opts = DialingOpts { peer: peer_id, @@ -614,7 +614,6 @@ where ( opts, - // TODO: This is the place to return the handler. NetworkEvent::DialError { attempts_remaining, peer_id, diff --git a/core/src/network/event.rs b/core/src/network/event.rs index a6f840be249..6e42625e58a 100644 --- a/core/src/network/event.rs +++ b/core/src/network/event.rs @@ -174,15 +174,14 @@ where } pub enum DialAttemptsRemaining { - // TODO: Make this a NonZeroU32. - Some(u32), + Some(NonZeroU32), None(THandler), } impl From<&DialAttemptsRemaining> for u32 { fn from(attempts_remaining: &DialAttemptsRemaining) -> Self { match attempts_remaining { - DialAttemptsRemaining::Some(attempts) => *attempts, + DialAttemptsRemaining::Some(attempts) => (*attempts).into(), DialAttemptsRemaining::None(_) => 0, } } diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index b9200b5022b..8340db49f2f 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -703,23 +703,31 @@ where error, attempts_remaining, }) => { - log::debug!( - "Connection attempt to {:?} via {:?} failed with {:?}. Attempts remaining: {}.", - // TODO: Can we do better on conversion? - peer_id, multiaddr, error, Into::::into(&attempts_remaining)); this.behaviour .inject_addr_reach_failure(Some(&peer_id), &multiaddr, &error); - let attempts_remaining_num = (&attempts_remaining).into(); - if let DialAttemptsRemaining::None(handler) = attempts_remaining { - this.behaviour - .inject_dial_failure(&peer_id, handler.into_protocol_handler()); + + let num_remaining: u32; + match attempts_remaining { + DialAttemptsRemaining::Some(n) => { + num_remaining = n.into(); + } + DialAttemptsRemaining::None(handler) => { + num_remaining = 0; + this.behaviour + .inject_dial_failure(&peer_id, handler.into_protocol_handler()); + } } + + log::debug!( + "Connection attempt to {:?} via {:?} failed with {:?}. Attempts remaining: {}.", + peer_id, multiaddr, error, num_remaining, + ); + return Poll::Ready(SwarmEvent::UnreachableAddr { peer_id, address: multiaddr, error, - // TODO: Can we do better? - attempts_remaining: attempts_remaining_num, + attempts_remaining: num_remaining, }); } Poll::Ready(NetworkEvent::UnknownPeerDialError { From 1c3ed2e9b9eb5ef3dafd9064f05df1b158d05b87 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Tue, 24 Aug 2021 10:54:09 +0200 Subject: [PATCH 20/38] swarm/src/protocols_handler: Rename to into_protocols_handler --- core/src/network.rs | 3 --- core/src/network/event.rs | 1 - swarm-derive/src/lib.rs | 2 +- swarm/src/lib.rs | 10 ++++++---- swarm/src/protocols_handler/node_handler.rs | 7 ++----- 5 files changed, 9 insertions(+), 14 deletions(-) diff --git a/core/src/network.rs b/core/src/network.rs index 8993787f884..c47080074aa 100644 --- a/core/src/network.rs +++ b/core/src/network.rs @@ -434,7 +434,6 @@ where log::warn!("Dialing aborted: {:?}", e); } } - // TODO: Include handler in event. event } Poll::Ready(PoolEvent::ConnectionClosed { @@ -626,7 +625,6 @@ where match endpoint { ConnectedPoint::Dialer { address } => ( None, - // TODO: This is the place to return the handler. NetworkEvent::UnknownPeerDialError { multiaddr: address, error, @@ -638,7 +636,6 @@ where send_back_addr, } => ( None, - // TODO: This is the place to return the handler. NetworkEvent::IncomingConnectionError { local_addr, send_back_addr, diff --git a/core/src/network/event.rs b/core/src/network/event.rs index 6e42625e58a..1bd62d84bc2 100644 --- a/core/src/network/event.rs +++ b/core/src/network/event.rs @@ -239,7 +239,6 @@ where local_addr, send_back_addr, error, - // TODO: Should this be printed as well? handler: _, } => f .debug_struct("IncomingConnectionError") diff --git a/swarm-derive/src/lib.rs b/swarm-derive/src/lib.rs index 84ae1a131df..864226a9149 100644 --- a/swarm-derive/src/lib.rs +++ b/swarm-derive/src/lib.rs @@ -556,7 +556,7 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { } } - out_handler.unwrap() // _or(quote! {()}) // TODO: incorrect + out_handler.unwrap_or(quote! {()}) // TODO: incorrect }; let generate_event_match_arm = if event_process { diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index 8340db49f2f..91c6644d691 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -388,7 +388,7 @@ where Err(error) => { let (error, handler) = DialError::from_network_dial_error(error); self.behaviour - .inject_dial_failure(&peer_id, handler.into_protocol_handler()); + .inject_dial_failure(&peer_id, handler.into_protocols_handler()); Err(error) } } @@ -595,7 +595,7 @@ where &peer_id, &id, &endpoint, - handler.into_protocol_handler(), + handler.into_protocols_handler(), ); if num_established == 0 { this.behaviour.inject_disconnected(&peer_id); @@ -714,7 +714,7 @@ where DialAttemptsRemaining::None(handler) => { num_remaining = 0; this.behaviour - .inject_dial_failure(&peer_id, handler.into_protocol_handler()); + .inject_dial_failure(&peer_id, handler.into_protocols_handler()); } } @@ -835,7 +835,9 @@ where } } } - // TODO: Return the handler to the behaviour. + // TODO: Return the handler to the behaviour. Though keep in mind that + // one can not just call dial_failure, as it is not really a failure on + // all DialPeerConditions. } } } diff --git a/swarm/src/protocols_handler/node_handler.rs b/swarm/src/protocols_handler/node_handler.rs index c12a7e6a689..8254968c6e8 100644 --- a/swarm/src/protocols_handler/node_handler.rs +++ b/swarm/src/protocols_handler/node_handler.rs @@ -66,8 +66,7 @@ where self } - // TODO: Rethink this method. - pub(crate) fn into_protocol_handler(self) -> TIntoProtoHandler { + pub(crate) fn into_protocols_handler(self) -> TIntoProtoHandler { self.handler } } @@ -136,9 +135,7 @@ where } impl NodeHandlerWrapper { - // TODO: Should this be a From impl? - // TODO: Find better name. - pub fn into_protocol_handler(self) -> TProtoHandler { + pub(crate) fn into_protocols_handler(self) -> TProtoHandler { self.handler } } From 32fc84e2709abb6c9c2fff0b9e1e6388809a855e Mon Sep 17 00:00:00 2001 From: Max Inden Date: Tue, 24 Aug 2021 17:16:29 +0200 Subject: [PATCH 21/38] swarm/src/behaviour: Introduce NetworkBehaviour::inject_listen_failure --- swarm-derive/src/lib.rs | 34 ++++++++++++++++++++++++++++++++++ swarm/src/behaviour.rs | 12 ++++++++++++ swarm/src/lib.rs | 8 ++++++-- swarm/src/toggle.rs | 13 +++++++++++++ 4 files changed, 65 insertions(+), 2 deletions(-) diff --git a/swarm-derive/src/lib.rs b/swarm-derive/src/lib.rs index 864226a9149..43b8f3855aa 100644 --- a/swarm-derive/src/lib.rs +++ b/swarm-derive/src/lib.rs @@ -295,6 +295,36 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { }) }; + // Build the list of statements to put in the body of `inject_listen_failure()`. + let inject_listen_failure_stmts = { + data_struct + .fields + .iter() + .enumerate() + .rev() + .filter(|f| !is_ignored(&f.1)) + .enumerate() + .map(move |(enum_n, (field_n, field))| { + let handler = if field_n == 0 { + quote! { let handler = handlers } + } else { + quote! { + let (handlers, handler) = handlers.into_inner() + } + }; + + let inject = match field.ident { + Some(ref i) => quote! { self.#i.inject_listen_failure(local_addr, send_back_addr, handler) }, + None => quote! { self.#enum_n.inject_listen_failure(local_addr, send_back_addr, handler) }, + }; + + quote! { + #handler; + #inject; + } + }) + }; + // Build the list of statements to put in the body of `inject_new_listener()`. let inject_new_listener_stmts = { data_struct @@ -649,6 +679,10 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { #(#inject_dial_failure_stmts);* } + fn inject_listen_failure(&mut self, local_addr: &#multiaddr, send_back_addr: &#multiaddr, handlers: Self::ProtocolsHandler) { + #(#inject_listen_failure_stmts);* + } + fn inject_new_listener(&mut self, id: #listener_id) { #(#inject_new_listener_stmts);* } diff --git a/swarm/src/behaviour.rs b/swarm/src/behaviour.rs index 1257bd3000b..223a064b1c1 100644 --- a/swarm/src/behaviour.rs +++ b/swarm/src/behaviour.rs @@ -166,6 +166,18 @@ pub trait NetworkBehaviour: Send + 'static { /// `inject_connected` has not been called, or `inject_disconnected` has been called since then. fn inject_dial_failure(&mut self, _peer_id: &PeerId, _handler: Self::ProtocolsHandler) {} + /// Indicates to the behaviour that an error happened on an incoming connection during its + /// initial handshake. + /// + /// This can include, for example, an error during the handshake of the encryption layer, or the + /// connection unexpectedly closed. + fn inject_listen_failure( + &mut self, + _local_addr: &Multiaddr, + _send_back_addr: &Multiaddr, + _handler: Self::ProtocolsHandler, + ) {} + /// Indicates to the behaviour that a new listener was created. fn inject_new_listener(&mut self, _id: ListenerId) {} diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index 91c6644d691..816b5ea7eb5 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -687,10 +687,14 @@ where local_addr, send_back_addr, error, - handler: _, + handler, }) => { - // TODO: Should handler not be injected into behaviour? log::debug!("Incoming connection failed: {:?}", error); + this.behaviour.inject_listen_failure( + &local_addr, + &send_back_addr, + handler.into_protocols_handler(), + ); return Poll::Ready(SwarmEvent::IncomingConnectionError { local_addr, send_back_addr, diff --git a/swarm/src/toggle.rs b/swarm/src/toggle.rs index 578868929d9..f21a84721d4 100644 --- a/swarm/src/toggle.rs +++ b/swarm/src/toggle.rs @@ -164,6 +164,19 @@ where } } + fn inject_listen_failure( + &mut self, + local_addr: &Multiaddr, + send_back_addr: &Multiaddr, + handler: Self::ProtocolsHandler, + ) { + if let Some(inner) = self.inner.as_mut() { + if let Some(handler) = handler.inner { + inner.inject_listen_failure(local_addr, send_back_addr, handler) + } + } + } + fn inject_new_listener(&mut self, id: ListenerId) { if let Some(inner) = self.inner.as_mut() { inner.inject_new_listener(id) From 7ea4908eb7ddf74f714e109d191c52014bb0ccfd Mon Sep 17 00:00:00 2001 From: Max Inden Date: Tue, 24 Aug 2021 22:32:46 +0200 Subject: [PATCH 22/38] swarm/src/lib: Inject handler on DialPeerCondition false --- protocols/identify/src/identify.rs | 6 +- protocols/kad/src/behaviour.rs | 32 +++++- protocols/relay/src/behaviour.rs | 17 +++- protocols/request-response/src/lib.rs | 4 +- protocols/request-response/src/throttled.rs | 11 ++- swarm-derive/src/lib.rs | 7 +- swarm/CHANGELOG.md | 2 + swarm/src/behaviour.rs | 14 ++- swarm/src/lib.rs | 102 ++++++++++++-------- swarm/src/test.rs | 5 +- swarm/src/toggle.rs | 12 ++- 11 files changed, 144 insertions(+), 68 deletions(-) diff --git a/protocols/identify/src/identify.rs b/protocols/identify/src/identify.rs index f1d21d4c6db..d75dfb72054 100644 --- a/protocols/identify/src/identify.rs +++ b/protocols/identify/src/identify.rs @@ -27,8 +27,8 @@ use libp2p_core::{ ConnectedPoint, Multiaddr, PeerId, PublicKey, }; use libp2p_swarm::{ - AddressScore, DialPeerCondition, IntoProtocolsHandler, NegotiatedSubstream, NetworkBehaviour, - NetworkBehaviourAction, NotifyHandler, PollParameters, ProtocolsHandler, + AddressScore, DialError, DialPeerCondition, IntoProtocolsHandler, NegotiatedSubstream, + NetworkBehaviour, NetworkBehaviourAction, NotifyHandler, PollParameters, ProtocolsHandler, ProtocolsHandlerUpgrErr, }; use std::{ @@ -223,7 +223,7 @@ impl NetworkBehaviour for Identify { } } - fn inject_dial_failure(&mut self, peer_id: &PeerId, _: Self::ProtocolsHandler) { + fn inject_dial_failure(&mut self, peer_id: &PeerId, _: Self::ProtocolsHandler, _: DialError) { if !self.connected.contains_key(peer_id) { self.pending_push.remove(peer_id); } diff --git a/protocols/kad/src/behaviour.rs b/protocols/kad/src/behaviour.rs index c28230a9060..74b5616ff91 100644 --- a/protocols/kad/src/behaviour.rs +++ b/protocols/kad/src/behaviour.rs @@ -43,7 +43,8 @@ use libp2p_core::{ ConnectedPoint, Multiaddr, PeerId, }; use libp2p_swarm::{ - DialPeerCondition, NetworkBehaviour, NetworkBehaviourAction, NotifyHandler, PollParameters, + DialError, DialPeerCondition, NetworkBehaviour, NetworkBehaviourAction, NotifyHandler, + PollParameters, }; use log::{debug, info, warn}; use smallvec::SmallVec; @@ -1864,9 +1865,32 @@ where } } - fn inject_dial_failure(&mut self, peer_id: &PeerId, _: Self::ProtocolsHandler) { - for query in self.queries.iter_mut() { - query.on_failure(peer_id); + fn inject_dial_failure( + &mut self, + peer_id: &PeerId, + _: Self::ProtocolsHandler, + error: DialError, + ) { + match error { + DialError::Banned + | DialError::ConnectionLimit(_) + | DialError::InvalidAddress(_) + | DialError::UnreachableAddr(_) + | DialError::LocalPeerId + | DialError::NoAddresses => { + for query in self.queries.iter_mut() { + query.on_failure(peer_id); + } + } + DialError::DialPeerConditionFalse( + DialPeerCondition::Disconnected | DialPeerCondition::NotDialing, + ) => { + // We might (still) be connected, or about to be connected, thus do not report the + // failure to the queries. + } + DialError::DialPeerConditionFalse(DialPeerCondition::Always) => { + unreachable!("DialPeerCondition::Always can not trigger DialPeerConditionFalse."); + } } } diff --git a/protocols/relay/src/behaviour.rs b/protocols/relay/src/behaviour.rs index 68653c5c88a..35f97647efc 100644 --- a/protocols/relay/src/behaviour.rs +++ b/protocols/relay/src/behaviour.rs @@ -29,7 +29,7 @@ use libp2p_core::connection::{ConnectedPoint, ConnectionId, ListenerId}; use libp2p_core::multiaddr::Multiaddr; use libp2p_core::PeerId; use libp2p_swarm::{ - DialPeerCondition, IntoProtocolsHandler, NetworkBehaviour, NetworkBehaviourAction, + DialError, DialPeerCondition, IntoProtocolsHandler, NetworkBehaviour, NetworkBehaviourAction, NotifyHandler, PollParameters, }; use std::collections::{hash_map::Entry, HashMap, HashSet, VecDeque}; @@ -302,7 +302,20 @@ impl NetworkBehaviour for Relay { } } - fn inject_dial_failure(&mut self, peer_id: &PeerId, _: Self::ProtocolsHandler) { + fn inject_dial_failure( + &mut self, + peer_id: &PeerId, + _: Self::ProtocolsHandler, + error: DialError, + ) { + if let DialError::DialPeerConditionFalse( + DialPeerCondition::Disconnected | DialPeerCondition::NotDialing, + ) = error + { + // Return early. The dial, that this dial was canceled for, might still succeed. + return; + } + if let Entry::Occupied(o) = self.listeners.entry(*peer_id) { if matches!(o.get(), RelayListener::Connecting { .. }) { // By removing the entry, the channel to the listener is dropped and thus the diff --git a/protocols/request-response/src/lib.rs b/protocols/request-response/src/lib.rs index 4c998931336..ef3913c1efb 100644 --- a/protocols/request-response/src/lib.rs +++ b/protocols/request-response/src/lib.rs @@ -68,7 +68,7 @@ use futures::channel::oneshot; use handler::{RequestProtocol, RequestResponseHandler, RequestResponseHandlerEvent}; use libp2p_core::{connection::ConnectionId, ConnectedPoint, Multiaddr, PeerId}; use libp2p_swarm::{ - DialPeerCondition, IntoProtocolsHandler, NetworkBehaviour, NetworkBehaviourAction, + DialError, DialPeerCondition, IntoProtocolsHandler, NetworkBehaviour, NetworkBehaviourAction, NotifyHandler, PollParameters, }; use smallvec::SmallVec; @@ -686,7 +686,7 @@ where self.connected.remove(peer); } - fn inject_dial_failure(&mut self, peer: &PeerId, _: Self::ProtocolsHandler) { + fn inject_dial_failure(&mut self, peer: &PeerId, _: Self::ProtocolsHandler, _: DialError) { // If there are pending outgoing requests when a dial failure occurs, // it is implied that we are not connected to the peer, since pending // outgoing requests are drained when a connection is established and diff --git a/protocols/request-response/src/throttled.rs b/protocols/request-response/src/throttled.rs index 61a43c4412e..2b8693bc437 100644 --- a/protocols/request-response/src/throttled.rs +++ b/protocols/request-response/src/throttled.rs @@ -45,7 +45,7 @@ use codec::{Codec, Message, ProtocolWrapper, Type}; use futures::ready; use libp2p_core::{connection::ConnectionId, ConnectedPoint, Multiaddr, PeerId}; use libp2p_swarm::{ - IntoProtocolsHandler, NetworkBehaviour, NetworkBehaviourAction, PollParameters, + DialError, IntoProtocolsHandler, NetworkBehaviour, NetworkBehaviourAction, PollParameters, }; use lru::LruCache; use std::{cmp::max, num::NonZeroU16}; @@ -493,8 +493,13 @@ where self.behaviour.inject_disconnected(p) } - fn inject_dial_failure(&mut self, p: &PeerId, handler: Self::ProtocolsHandler) { - self.behaviour.inject_dial_failure(p, handler) + fn inject_dial_failure( + &mut self, + p: &PeerId, + handler: Self::ProtocolsHandler, + error: DialError, + ) { + self.behaviour.inject_dial_failure(p, handler, error) } fn inject_event( diff --git a/swarm-derive/src/lib.rs b/swarm-derive/src/lib.rs index 43b8f3855aa..63235094b68 100644 --- a/swarm-derive/src/lib.rs +++ b/swarm-derive/src/lib.rs @@ -57,6 +57,7 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { let connection_id = quote! {::libp2p::core::connection::ConnectionId}; let connected_point = quote! {::libp2p::core::ConnectedPoint}; let listener_id = quote! {::libp2p::core::connection::ListenerId}; + let dial_error = quote! {::libp2p::swarm::DialError}; let poll_parameters = quote! {::libp2p::swarm::PollParameters}; @@ -284,8 +285,8 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { }; let inject = match field.ident { - Some(ref i) => quote! { self.#i.inject_dial_failure(peer_id, handler) }, - None => quote! { self.#enum_n.inject_dial_failure(peer_id, handler) }, + Some(ref i) => quote! { self.#i.inject_dial_failure(peer_id, handler, error.clone()) }, + None => quote! { self.#enum_n.inject_dial_failure(peer_id, handler, error.clone()) }, }; quote! { @@ -675,7 +676,7 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { #(#inject_addr_reach_failure_stmts);* } - fn inject_dial_failure(&mut self, peer_id: &#peer_id, handlers: Self::ProtocolsHandler) { + fn inject_dial_failure(&mut self, peer_id: &#peer_id, handlers: Self::ProtocolsHandler, error: #dial_error) { #(#inject_dial_failure_stmts);* } diff --git a/swarm/CHANGELOG.md b/swarm/CHANGELOG.md index 4eacf3dfe45..cf19d7978cc 100644 --- a/swarm/CHANGELOG.md +++ b/swarm/CHANGELOG.md @@ -19,6 +19,8 @@ - Implement `ProtocolsHandler` on `either::Either`representing either of two `ProtocolsHandler` implementations (see [PR 2192]). +TODO: Mention `inject_dial_failure` now called on DialPeerCondition not met. + [PR 2150]: https://github.com/libp2p/rust-libp2p/pull/2150 [PR 2182]: https://github.com/libp2p/rust-libp2p/pull/2182 [PR 2183]: https://github.com/libp2p/rust-libp2p/pull/2183 diff --git a/swarm/src/behaviour.rs b/swarm/src/behaviour.rs index 223a064b1c1..c605d97ca22 100644 --- a/swarm/src/behaviour.rs +++ b/swarm/src/behaviour.rs @@ -19,7 +19,7 @@ // DEALINGS IN THE SOFTWARE. use crate::protocols_handler::{IntoProtocolsHandler, ProtocolsHandler}; -use crate::{AddressRecord, AddressScore}; +use crate::{AddressRecord, AddressScore, DialError}; use libp2p_core::{ connection::{ConnectionId, ListenerId}, ConnectedPoint, Multiaddr, PeerId, @@ -164,7 +164,13 @@ pub trait NetworkBehaviour: Send + 'static { /// /// The `peer_id` is guaranteed to be in a disconnected state. In other words, /// `inject_connected` has not been called, or `inject_disconnected` has been called since then. - fn inject_dial_failure(&mut self, _peer_id: &PeerId, _handler: Self::ProtocolsHandler) {} + fn inject_dial_failure( + &mut self, + _peer_id: &PeerId, + _handler: Self::ProtocolsHandler, + _error: DialError, + ) { + } /// Indicates to the behaviour that an error happened on an incoming connection during its /// initial handshake. @@ -176,7 +182,8 @@ pub trait NetworkBehaviour: Send + 'static { _local_addr: &Multiaddr, _send_back_addr: &Multiaddr, _handler: Self::ProtocolsHandler, - ) {} + ) { + } /// Indicates to the behaviour that a new listener was created. fn inject_new_listener(&mut self, _id: ListenerId) {} @@ -501,7 +508,6 @@ pub enum NotifyHandler { /// The available conditions under which a new dialing attempt to /// a peer is initiated when requested by [`NetworkBehaviourAction::DialPeer`]. #[derive(Debug, Copy, Clone)] -#[non_exhaustive] pub enum DialPeerCondition { /// A new dialing attempt is initiated _only if_ the peer is currently /// considered disconnected, i.e. there is no established connection diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index 816b5ea7eb5..ab66c9ec1cc 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -361,8 +361,10 @@ where handler: ::ProtocolsHandler, ) -> Result<(), DialError> { if self.banned_peers.contains(peer_id) { - self.behaviour.inject_dial_failure(peer_id, handler); - return Err(DialError::Banned); + let error = DialError::Banned; + self.behaviour + .inject_dial_failure(peer_id, handler, error.clone()); + return Err(error); } let self_listening = &self.listened_addrs; @@ -375,8 +377,10 @@ where let first = match addrs.next() { Some(first) => first, None => { - self.behaviour.inject_dial_failure(peer_id, handler); - return Err(DialError::NoAddresses); + let error = DialError::NoAddresses; + self.behaviour + .inject_dial_failure(peer_id, handler, error.clone()); + return Err(error); } }; @@ -387,8 +391,11 @@ where Ok(_connection_id) => Ok(()), Err(error) => { let (error, handler) = DialError::from_network_dial_error(error); - self.behaviour - .inject_dial_failure(&peer_id, handler.into_protocols_handler()); + self.behaviour.inject_dial_failure( + &peer_id, + handler.into_protocols_handler(), + error.clone(), + ); Err(error) } } @@ -717,8 +724,11 @@ where } DialAttemptsRemaining::None(handler) => { num_remaining = 0; - this.behaviour - .inject_dial_failure(&peer_id, handler.into_protocols_handler()); + this.behaviour.inject_dial_failure( + &peer_id, + handler.into_protocols_handler(), + DialError::UnreachableAddr(multiaddr.clone()), + ); } } @@ -806,43 +816,40 @@ where condition, handler, }) => { - if this.banned_peers.contains(&peer_id) { - this.behaviour.inject_dial_failure(&peer_id, handler); + let condition_matched = match condition { + DialPeerCondition::Disconnected => this.network.is_disconnected(&peer_id), + DialPeerCondition::NotDialing => !this.network.is_dialing(&peer_id), + DialPeerCondition::Always => true, + }; + if condition_matched { + if Swarm::dial_with_handler(this, &peer_id, handler).is_ok() { + return Poll::Ready(SwarmEvent::Dialing(peer_id)); + } } else { - let condition_matched = match condition { - DialPeerCondition::Disconnected => { - this.network.is_disconnected(&peer_id) - } - DialPeerCondition::NotDialing => !this.network.is_dialing(&peer_id), - DialPeerCondition::Always => true, - }; - if condition_matched { - if Swarm::dial_with_handler(this, &peer_id, handler).is_ok() { - return Poll::Ready(SwarmEvent::Dialing(peer_id)); - } - } else { - // Even if the condition for a _new_ dialing attempt is not met, - // we always add any potentially new addresses of the peer to an - // ongoing dialing attempt, if there is one. - log::trace!( - "Condition for new dialing attempt to {:?} not met: {:?}", - peer_id, - condition - ); - let self_listening = &this.listened_addrs; - if let Some(mut peer) = this.network.peer(peer_id).into_dialing() { - let addrs = this.behaviour.addresses_of_peer(peer.id()); - let mut attempt = peer.some_attempt(); - for a in addrs { - if !self_listening.contains(&a) { - attempt.add_address(a); - } + // Even if the condition for a _new_ dialing attempt is not met, + // we always add any potentially new addresses of the peer to an + // ongoing dialing attempt, if there is one. + log::trace!( + "Condition for new dialing attempt to {:?} not met: {:?}", + peer_id, + condition + ); + let self_listening = &this.listened_addrs; + if let Some(mut peer) = this.network.peer(peer_id).into_dialing() { + let addrs = this.behaviour.addresses_of_peer(peer.id()); + let mut attempt = peer.some_attempt(); + for a in addrs { + if !self_listening.contains(&a) { + attempt.add_address(a); } } - // TODO: Return the handler to the behaviour. Though keep in mind that - // one can not just call dial_failure, as it is not really a failure on - // all DialPeerConditions. } + + this.behaviour.inject_dial_failure( + &peer_id, + handler, + DialError::DialPeerConditionFalse(condition), + ); } } Poll::Ready(NetworkBehaviourAction::NotifyHandler { @@ -1192,7 +1199,7 @@ where } } -/// The possible failures of [`Swarm::dial`]. +/// The possible failures of dialing. #[derive(Debug, Clone)] pub enum DialError { /// The peer is currently banned. @@ -1202,10 +1209,15 @@ pub enum DialError { ConnectionLimit(ConnectionLimit), /// The address given for dialing is invalid. InvalidAddress(Multiaddr), + // TODO: Document + UnreachableAddr(Multiaddr), + // TODO: Document LocalPeerId, /// [`NetworkBehaviour::addresses_of_peer`] returned no addresses /// for the peer to dial. NoAddresses, + // TODO: Document + DialPeerConditionFalse(DialPeerCondition), } impl DialError { @@ -1229,7 +1241,11 @@ impl fmt::Display for DialError { DialError::NoAddresses => write!(f, "Dial error: no addresses for peer."), DialError::LocalPeerId => write!(f, "Dial error: tried to dial local peer id."), DialError::InvalidAddress(a) => write!(f, "Dial error: invalid address: {}", a), + DialError::UnreachableAddr(a) => write!(f, "Dial error: unreachable address: {}", a), DialError::Banned => write!(f, "Dial error: peer is banned."), + DialError::DialPeerConditionFalse(c) => { + write!(f, "Dial error: condition {:?} for dialing peer was false.", c) + } } } } @@ -1239,9 +1255,11 @@ impl error::Error for DialError { match self { DialError::ConnectionLimit(err) => Some(err), DialError::InvalidAddress(_) => None, + DialError::UnreachableAddr(_) => None, DialError::LocalPeerId => None, DialError::NoAddresses => None, DialError::Banned => None, + DialError::DialPeerConditionFalse(_) => None, } } } diff --git a/swarm/src/test.rs b/swarm/src/test.rs index 5fc3885ea29..8bc4897b83f 100644 --- a/swarm/src/test.rs +++ b/swarm/src/test.rs @@ -19,6 +19,7 @@ // DEALINGS IN THE SOFTWARE. use crate::{ + DialError, IntoProtocolsHandler, NetworkBehaviour, NetworkBehaviourAction, PollParameters, ProtocolsHandler, }; @@ -234,9 +235,9 @@ where self.inner.inject_addr_reach_failure(p, a, e); } - fn inject_dial_failure(&mut self, p: &PeerId, handler: Self::ProtocolsHandler) { + fn inject_dial_failure(&mut self, p: &PeerId, handler: Self::ProtocolsHandler, error: DialError) { self.inject_dial_failure.push(p.clone()); - self.inner.inject_dial_failure(p, handler); + self.inner.inject_dial_failure(p, handler, error); } fn inject_new_listener(&mut self, id: ListenerId) { diff --git a/swarm/src/toggle.rs b/swarm/src/toggle.rs index f21a84721d4..575d4e46809 100644 --- a/swarm/src/toggle.rs +++ b/swarm/src/toggle.rs @@ -24,7 +24,8 @@ use crate::protocols_handler::{ }; use crate::upgrade::{InboundUpgradeSend, OutboundUpgradeSend, SendWrapper}; use crate::{ - NetworkBehaviour, NetworkBehaviourAction, NetworkBehaviourEventProcess, PollParameters, + DialError, NetworkBehaviour, NetworkBehaviourAction, NetworkBehaviourEventProcess, + PollParameters, }; use either::Either; use libp2p_core::{ @@ -156,10 +157,15 @@ where } } - fn inject_dial_failure(&mut self, peer_id: &PeerId, handler: Self::ProtocolsHandler) { + fn inject_dial_failure( + &mut self, + peer_id: &PeerId, + handler: Self::ProtocolsHandler, + error: DialError, + ) { if let Some(inner) = self.inner.as_mut() { if let Some(handler) = handler.inner { - inner.inject_dial_failure(peer_id, handler) + inner.inject_dial_failure(peer_id, handler, error) } } } From fbd4681ab48aa732bbcbf8ee2e03eb53101ccae3 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Wed, 25 Aug 2021 09:41:52 +0200 Subject: [PATCH 23/38] core/src/connection: Assume manager to always close handler --- core/src/connection/manager/task.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/core/src/connection/manager/task.rs b/core/src/connection/manager/task.rs index 1d7ca3c515b..0ed331c6625 100644 --- a/core/src/connection/manager/task.rs +++ b/core/src/connection/manager/task.rs @@ -270,10 +270,8 @@ where continue 'poll; } Poll::Ready(None) => { - todo!("Safe to assume that this should never happen?"); - // The manager has dropped the task or disappeared; abort. - // TODO: Should we return the handler in this case? - // return Poll::Ready(()); + // The manager has disappeared; abort. + return Poll::Ready(()); } } } From 6787e7709223e9b1edf99386a7886da7cdf3f807 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Wed, 25 Aug 2021 10:11:12 +0200 Subject: [PATCH 24/38] swarm-derive: Add comments --- swarm-derive/src/lib.rs | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/swarm-derive/src/lib.rs b/swarm-derive/src/lib.rs index 63235094b68..e826a2a46a5 100644 --- a/swarm-derive/src/lib.rs +++ b/swarm-derive/src/lib.rs @@ -228,11 +228,13 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { .fields .iter() .enumerate() + // The outmost handler belongs to the last behaviour. .rev() .filter(|f| !is_ignored(&f.1)) .enumerate() .map(move |(enum_n, (field_n, field))| { let handler = if field_n == 0 { + // Given that the iterator is reversed, this is the innermost handler only. quote! { let handler = handlers } } else { quote! { @@ -272,11 +274,13 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { .fields .iter() .enumerate() + // The outmost handler belongs to the last behaviour. .rev() .filter(|f| !is_ignored(&f.1)) .enumerate() .map(move |(enum_n, (field_n, field))| { let handler = if field_n == 0 { + // Given that the iterator is reversed, this is the innermost handler only. quote! { let handler = handlers } } else { quote! { @@ -285,8 +289,12 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { }; let inject = match field.ident { - Some(ref i) => quote! { self.#i.inject_dial_failure(peer_id, handler, error.clone()) }, - None => quote! { self.#enum_n.inject_dial_failure(peer_id, handler, error.clone()) }, + Some(ref i) => { + quote! { self.#i.inject_dial_failure(peer_id, handler, error.clone()) } + } + None => { + quote! { self.#enum_n.inject_dial_failure(peer_id, handler, error.clone()) } + } }; quote! { @@ -516,7 +524,7 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { } } - out_handler.unwrap_or(quote! {()}) // TODO: incorrect + out_handler.unwrap_or(quote! {()}) // TODO: See test `empty`. }; // The method to use to poll. @@ -560,7 +568,11 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { wrapped_event = quote!{ #either_ident::First(#wrapped_event) }; } - let wrapped_handler = { + // `DialPeer` and `DialAddress` each provide a handler of the specific + // behaviour triggering the event. Though in order for the final handler + // to be able to handle protocols of all behaviours, the provided + // handler needs to be combined with handlers of all other behaviours. + let provided_handler_and_new_handlers = { let mut out_handler = None; for (f_n, f) in data_struct.fields.iter().enumerate() { @@ -574,7 +586,9 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { }; let builder = if field_n == f_n { - quote! { handler } + // The behaviour that triggered the event. Thus, instead of + // creating a new handler, use the provided handler. + quote! { provided_handler } } else { quote! { #f_name.new_handler() } }; @@ -587,7 +601,7 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { } } - out_handler.unwrap_or(quote! {()}) // TODO: incorrect + out_handler.unwrap_or(quote! {()}) // TODO: See test `empty`. }; let generate_event_match_arm = if event_process { @@ -608,11 +622,11 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { loop { match #trait_to_impl::poll(&mut #field_name, cx, poll_params) { #generate_event_match_arm - std::task::Poll::Ready(#network_behaviour_action::DialAddress { address, handler }) => { - return std::task::Poll::Ready(#network_behaviour_action::DialAddress { address, handler: #wrapped_handler }); + std::task::Poll::Ready(#network_behaviour_action::DialAddress { address, providedhandler }) => { + return std::task::Poll::Ready(#network_behaviour_action::DialAddress { address, handler: #provided_handler_and_new_handlers }); } - std::task::Poll::Ready(#network_behaviour_action::DialPeer { peer_id, condition, handler }) => { - return std::task::Poll::Ready(#network_behaviour_action::DialPeer { peer_id, condition, handler: #wrapped_handler }); + std::task::Poll::Ready(#network_behaviour_action::DialPeer { peer_id, condition, provided_handler }) => { + return std::task::Poll::Ready(#network_behaviour_action::DialPeer { peer_id, condition, handler: #provided_handler_and_new_handlers }); } std::task::Poll::Ready(#network_behaviour_action::NotifyHandler { peer_id, handler, event }) => { return std::task::Poll::Ready(#network_behaviour_action::NotifyHandler { From b8641331145661a0c58ee639e34d5bc6a8694e25 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Wed, 25 Aug 2021 10:31:06 +0200 Subject: [PATCH 25/38] swarm: Add documentation --- core/src/network.rs | 1 + swarm/src/behaviour.rs | 35 ++++++++++++++++++++++++++++------- swarm/src/lib.rs | 6 +++--- 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/core/src/network.rs b/core/src/network.rs index c47080074aa..d0ba799da8e 100644 --- a/core/src/network.rs +++ b/core/src/network.rs @@ -766,6 +766,7 @@ pub enum DialError { address: Multiaddr, handler: THandler, }, + /// The dialing attempt is rejected because the peer being dialed is the local peer. LocalPeerId { handler: THandler, }, diff --git a/swarm/src/behaviour.rs b/swarm/src/behaviour.rs index c605d97ca22..3c7c7e50bc8 100644 --- a/swarm/src/behaviour.rs +++ b/swarm/src/behaviour.rs @@ -69,16 +69,20 @@ pub trait NetworkBehaviour: Send + 'static { /// Creates a new `ProtocolsHandler` for a connection with a peer. /// - /// Every time an incoming connection is opened, and every time we start dialing a node, this - /// method is called. + /// Every time an incoming connection is opened, and every time another [`NetworkBehaviour`] + /// emitted a dial request, this method is called. /// /// The returned object is a handler for that specific connection, and will be moved to a /// background task dedicated to that connection. /// - /// The network behaviour (ie. the implementation of this trait) and the handlers it has - /// spawned (ie. the objects returned by `new_handler`) can communicate by passing messages. - /// Messages sent from the handler to the behaviour are injected with `inject_event`, and - /// the behaviour can send a message to the handler by making `poll` return `SendEvent`. + /// The network behaviour (ie. the implementation of this trait) and the handlers it has spawned + /// (ie. the objects returned by `new_handler`) can communicate by passing messages. Messages + /// sent from the handler to the behaviour are injected with [`NetworkBehaviour::inject_event`], + /// and the behaviour can send a message to the handler by making [`NetworkBehaviour::poll`] + /// return [`NetworkBehaviourAction::SendEvent`]. + /// + /// Note that the handler is returned to the [`NetworkBehaviour`] on connection failure and + /// connection closing. fn new_handler(&mut self) -> Self::ProtocolsHandler; /// Addresses that this behaviour is aware of for this specific peer, and that may allow @@ -275,9 +279,19 @@ pub enum NetworkBehaviourAction< GenerateEvent(TOutEvent), /// Instructs the swarm to dial the given multiaddress optionally including a [`PeerId`]. + /// + /// On success, [`NetworkBehaviour::inject_connection_established`] is invoked. + /// On failure, [`NetworkBehaviour::inject_dial_failure`] is invoked. DialAddress { /// The address to dial. address: Multiaddr, + /// The handler to be used to handle the connection to the peer. + /// + /// Note that the handler is returned to the [`NetworkBehaviour`] on connection failure and + /// connection closing. Thus it can be used to carry state, which otherwise would have to be + /// tracked in the [`NetworkBehaviour`] itself. E.g. a message destined to an unconnected + /// peer can be included in the handler, and thus directly send on connection success or + /// extracted by the [`NetworkBehaviour`] on connection failure. handler: THandler, }, @@ -289,13 +303,20 @@ pub enum NetworkBehaviourAction< /// If we were already trying to dial this node, the addresses that are not yet in the queue of /// addresses to try are added back to this queue. /// - /// On success, [`NetworkBehaviour::inject_connected`] is invoked. + /// On success, [`NetworkBehaviour::inject_connection_established`] is invoked. /// On failure, [`NetworkBehaviour::inject_dial_failure`] is invoked. DialPeer { /// The peer to try reach. peer_id: PeerId, /// The condition for initiating a new dialing attempt. condition: DialPeerCondition, + /// The handler to be used to handle the connection to the peer. + /// + /// Note that the handler is returned to the [`NetworkBehaviour`] on connection failure and + /// connection closing. Thus it can be used to carry state, which otherwise would have to be + /// tracked in the [`NetworkBehaviour`] itself. E.g. a message destined to an unconnected + /// peer can be included in the handler, and thus directly send on connection success or + /// extracted by the [`NetworkBehaviour`] on connection failure. handler: THandler, }, diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index ab66c9ec1cc..9c15fe6e519 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -1209,14 +1209,14 @@ pub enum DialError { ConnectionLimit(ConnectionLimit), /// The address given for dialing is invalid. InvalidAddress(Multiaddr), - // TODO: Document + /// Tried to dial an address but it ended up being unreachaable. UnreachableAddr(Multiaddr), - // TODO: Document + /// The peer being dialed is the local peer and thus the dial was aborted. LocalPeerId, /// [`NetworkBehaviour::addresses_of_peer`] returned no addresses /// for the peer to dial. NoAddresses, - // TODO: Document + /// The provided [`DialPeerCondition`] evaluated to false and thus the dial was aborted. DialPeerConditionFalse(DialPeerCondition), } From b2bf380c7704be86def97ae34fde37cfd544989e Mon Sep 17 00:00:00 2001 From: Max Inden Date: Wed, 25 Aug 2021 10:31:27 +0200 Subject: [PATCH 26/38] *: Format with rustfmt --- core/src/network.rs | 4 +--- swarm/src/lib.rs | 6 +++++- swarm/src/test.rs | 10 +++++++--- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/core/src/network.rs b/core/src/network.rs index d0ba799da8e..831a99c4b01 100644 --- a/core/src/network.rs +++ b/core/src/network.rs @@ -767,9 +767,7 @@ pub enum DialError { handler: THandler, }, /// The dialing attempt is rejected because the peer being dialed is the local peer. - LocalPeerId { - handler: THandler, - }, + LocalPeerId { handler: THandler }, } impl fmt::Debug for DialError { diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index 9c15fe6e519..925e47d644a 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -1244,7 +1244,11 @@ impl fmt::Display for DialError { DialError::UnreachableAddr(a) => write!(f, "Dial error: unreachable address: {}", a), DialError::Banned => write!(f, "Dial error: peer is banned."), DialError::DialPeerConditionFalse(c) => { - write!(f, "Dial error: condition {:?} for dialing peer was false.", c) + write!( + f, + "Dial error: condition {:?} for dialing peer was false.", + c + ) } } } diff --git a/swarm/src/test.rs b/swarm/src/test.rs index 8bc4897b83f..457701899a8 100644 --- a/swarm/src/test.rs +++ b/swarm/src/test.rs @@ -19,8 +19,7 @@ // DEALINGS IN THE SOFTWARE. use crate::{ - DialError, - IntoProtocolsHandler, NetworkBehaviour, NetworkBehaviourAction, PollParameters, + DialError, IntoProtocolsHandler, NetworkBehaviour, NetworkBehaviourAction, PollParameters, ProtocolsHandler, }; use libp2p_core::{ @@ -235,7 +234,12 @@ where self.inner.inject_addr_reach_failure(p, a, e); } - fn inject_dial_failure(&mut self, p: &PeerId, handler: Self::ProtocolsHandler, error: DialError) { + fn inject_dial_failure( + &mut self, + p: &PeerId, + handler: Self::ProtocolsHandler, + error: DialError, + ) { self.inject_dial_failure.push(p.clone()); self.inner.inject_dial_failure(p, handler, error); } From 7d342b663740a7d8ddaba021a47e9dc603226c37 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Wed, 25 Aug 2021 10:40:05 +0200 Subject: [PATCH 27/38] swar/src/behaviour: Link to NotifyHandler not SendEvent --- swarm/src/behaviour.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/swarm/src/behaviour.rs b/swarm/src/behaviour.rs index 3c7c7e50bc8..d13cc240279 100644 --- a/swarm/src/behaviour.rs +++ b/swarm/src/behaviour.rs @@ -79,7 +79,7 @@ pub trait NetworkBehaviour: Send + 'static { /// (ie. the objects returned by `new_handler`) can communicate by passing messages. Messages /// sent from the handler to the behaviour are injected with [`NetworkBehaviour::inject_event`], /// and the behaviour can send a message to the handler by making [`NetworkBehaviour::poll`] - /// return [`NetworkBehaviourAction::SendEvent`]. + /// return [`NetworkBehaviourAction::NotifyHandler`]. /// /// Note that the handler is returned to the [`NetworkBehaviour`] on connection failure and /// connection closing. From 5853890ba9c1a84285793648e6b3d3d87ca5eb85 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Wed, 25 Aug 2021 11:16:50 +0200 Subject: [PATCH 28/38] *: Update changelogs --- core/CHANGELOG.md | 5 ++++- swarm-derive/CHANGELOG.md | 4 ++++ swarm/CHANGELOG.md | 24 +++++++++++++++++++++++- swarm/src/behaviour.rs | 2 +- 4 files changed, 32 insertions(+), 3 deletions(-) diff --git a/core/CHANGELOG.md b/core/CHANGELOG.md index e04884147cf..3b1ec3c43e1 100644 --- a/core/CHANGELOG.md +++ b/core/CHANGELOG.md @@ -20,12 +20,15 @@ - Require `ConnectionHandler::{InEvent,OutEvent,Error}` to implement `Debug` (see [PR 2183]). +- Remove `DisconnectedPeer::set_connected` and `Pool::add` (see [PR 2195]). + - Report `ConnectionLimit` error through `ConnectionError` and thus through `NetworkEvent::ConnectionClosed` instead of previously through `PendingConnectionError` and thus `NetworkEvent::{IncomingConnectionError, DialError}` (see [PR 2191]). -- Remove `DisconnectedPeer::set_connected` and `Pool::add` (see [PR 2195]). +- Report abortion of pending connection through `DialError`, + `UnknownPeerDialError` or `IncomingConnectionError` (see [PR 2191]). [PR 2145]: https://github.com/libp2p/rust-libp2p/pull/2145 [PR 2142]: https://github.com/libp2p/rust-libp2p/pull/2142 diff --git a/swarm-derive/CHANGELOG.md b/swarm-derive/CHANGELOG.md index ff800bfe5eb..335d0fb28e8 100644 --- a/swarm-derive/CHANGELOG.md +++ b/swarm-derive/CHANGELOG.md @@ -1,5 +1,9 @@ # 0.25.0 [unreleased] +- Update to latest `libp2p-swarm` changes (see [PR 2191]). + +[PR 2191]: https://github.com/libp2p/rust-libp2p/pull/2191 + # 0.24.0 [2021-07-12] - Handle `NetworkBehaviourAction::CloseConnection`. See [PR 2110] for details. diff --git a/swarm/CHANGELOG.md b/swarm/CHANGELOG.md index cf19d7978cc..5894591c3bd 100644 --- a/swarm/CHANGELOG.md +++ b/swarm/CHANGELOG.md @@ -19,12 +19,34 @@ - Implement `ProtocolsHandler` on `either::Either`representing either of two `ProtocolsHandler` implementations (see [PR 2192]). -TODO: Mention `inject_dial_failure` now called on DialPeerCondition not met. +- Require implementation to provide handler in + `NetworkBehaviourAction::DialPeer` and `NetworkBehaviourAction::DialAddress`. + Note that the handler is returned to the `NetworkBehaviour` on connection + failure and connection closing. Thus it can be used to carry state, which + otherwise would have to be tracked in the `NetworkBehaviour` itself. E.g. a + message destined to an unconnected peer can be included in the handler, and + thus directly send on connection success or extracted by the + `NetworkBehaviour` on connection failure (see [PR 2191]). + +- Include handler in `NetworkBehaviour::inject_dial_failure`, + `NetworkBehaviour::inject_connection_closed`, + `NetworkBehaviour::inject_listen_failure` (see [PR 2191]). + +- Include error in `NetworkBehaviour::inject_dial_failure` and call + `NetworkBehaviour::inject_dial_failure` on `DialPeerCondition` evaluating to + false. To emulate the previous behaviour, return early within + `inject_dial_failure` on `DialError::DialPeerConditionFalse`. See [PR 2191]. + +- Make `NetworkBehaviourAction` generic over `NetworkBehaviour::OutEvent` and + `NetworkBehaviour::ProtocolsHandler`. In most cases, change your generic type + parameters to `NetworkBehaviourAction`. See [PR 2191]. [PR 2150]: https://github.com/libp2p/rust-libp2p/pull/2150 [PR 2182]: https://github.com/libp2p/rust-libp2p/pull/2182 [PR 2183]: https://github.com/libp2p/rust-libp2p/pull/2183 [PR 2192]: https://github.com/libp2p/rust-libp2p/pull/2192 +[PR 2191]: https://github.com/libp2p/rust-libp2p/pull/2191 # 0.30.0 [2021-07-12] diff --git a/swarm/src/behaviour.rs b/swarm/src/behaviour.rs index d13cc240279..fc5edd35ca6 100644 --- a/swarm/src/behaviour.rs +++ b/swarm/src/behaviour.rs @@ -468,7 +468,7 @@ where THandlerOld: IntoProtocolsHandler, ::Handler: ProtocolsHandler, { - /// Map the handler event. + /// Map the handler. pub fn map_handler( self, f: impl FnOnce(THandlerOld) -> THandlerNew, From 4d0faf9af81a5b1f3373b062e2a7cb498eb97dba Mon Sep 17 00:00:00 2001 From: Max Inden Date: Wed, 25 Aug 2021 11:41:16 +0200 Subject: [PATCH 29/38] swarm-derive: Fix typo --- swarm-derive/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/swarm-derive/src/lib.rs b/swarm-derive/src/lib.rs index e826a2a46a5..eef08d15c07 100644 --- a/swarm-derive/src/lib.rs +++ b/swarm-derive/src/lib.rs @@ -622,10 +622,10 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { loop { match #trait_to_impl::poll(&mut #field_name, cx, poll_params) { #generate_event_match_arm - std::task::Poll::Ready(#network_behaviour_action::DialAddress { address, providedhandler }) => { + std::task::Poll::Ready(#network_behaviour_action::DialAddress { address, handler: provided_handler }) => { return std::task::Poll::Ready(#network_behaviour_action::DialAddress { address, handler: #provided_handler_and_new_handlers }); } - std::task::Poll::Ready(#network_behaviour_action::DialPeer { peer_id, condition, provided_handler }) => { + std::task::Poll::Ready(#network_behaviour_action::DialPeer { peer_id, condition, handler: provided_handler }) => { return std::task::Poll::Ready(#network_behaviour_action::DialPeer { peer_id, condition, handler: #provided_handler_and_new_handlers }); } std::task::Poll::Ready(#network_behaviour_action::NotifyHandler { peer_id, handler, event }) => { From 7a45e7b93f044f9d304a00a04d62fa78a97e80bc Mon Sep 17 00:00:00 2001 From: Max Inden Date: Thu, 26 Aug 2021 17:25:02 +0200 Subject: [PATCH 30/38] Apply suggestions from code review Co-authored-by: Thomas Eizinger --- core/src/connection/manager.rs | 2 +- core/src/connection/pool.rs | 2 +- core/src/network.rs | 13 ++++--------- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/core/src/connection/manager.rs b/core/src/connection/manager.rs index 437e2680c4d..51c08024202 100644 --- a/core/src/connection/manager.rs +++ b/core/src/connection/manager.rs @@ -433,7 +433,7 @@ impl<'a, I> EstablishedEntry<'a, I> { } /// Sends a close command to the associated background task, - /// thus initiating a graceful active close of the connectione + /// thus initiating a graceful active close of the connection. /// /// Has no effect if the connection is already closing. /// diff --git a/core/src/connection/pool.rs b/core/src/connection/pool.rs index 85366cbf5fc..11861600c21 100644 --- a/core/src/connection/pool.rs +++ b/core/src/connection/pool.rs @@ -494,7 +494,7 @@ impl Pool { id, endpoint, error, - handler: handler, + handler, peer, pool: self, }); diff --git a/core/src/network.rs b/core/src/network.rs index 831a99c4b01..bc9c67a8ed1 100644 --- a/core/src/network.rs +++ b/core/src/network.rs @@ -518,15 +518,10 @@ where // Ensure the address to dial encapsulates the `p2p` protocol for the // targeted peer, so that the transport has a "fully qualified" address // to work with. - let addr = match p2p_addr(opts.peer, opts.address) { - Ok(address) => address, - Err(address) => { - return Err(DialError::InvalidAddress { - address, - handler: opts.handler, - }) - } - }; + let addr = p2p_addr(opts.peer, opts.address).map_err(|address| DialError::InvalidAddress { + address, + handler: opts.handler, + })?; let result = match transport.dial(addr.clone()) { Ok(fut) => { From cef949c843cddfc7e710732ed63714913db896f9 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Thu, 26 Aug 2021 17:38:43 +0200 Subject: [PATCH 31/38] core/src/network: Revert map_err --- core/src/network.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/core/src/network.rs b/core/src/network.rs index bc9c67a8ed1..831a99c4b01 100644 --- a/core/src/network.rs +++ b/core/src/network.rs @@ -518,10 +518,15 @@ where // Ensure the address to dial encapsulates the `p2p` protocol for the // targeted peer, so that the transport has a "fully qualified" address // to work with. - let addr = p2p_addr(opts.peer, opts.address).map_err(|address| DialError::InvalidAddress { - address, - handler: opts.handler, - })?; + let addr = match p2p_addr(opts.peer, opts.address) { + Ok(address) => address, + Err(address) => { + return Err(DialError::InvalidAddress { + address, + handler: opts.handler, + }) + } + }; let result = match transport.dial(addr.clone()) { Ok(fut) => { From ceb77e5a94fa41df437200fd29575716346b8a05 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Thu, 26 Aug 2021 18:05:33 +0200 Subject: [PATCH 32/38] core/src/network: Use custom method on DialAttemptsRemaining --- core/src/network/event.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/network/event.rs b/core/src/network/event.rs index 1bd62d84bc2..98793395165 100644 --- a/core/src/network/event.rs +++ b/core/src/network/event.rs @@ -178,9 +178,9 @@ pub enum DialAttemptsRemaining { None(THandler), } -impl From<&DialAttemptsRemaining> for u32 { - fn from(attempts_remaining: &DialAttemptsRemaining) -> Self { - match attempts_remaining { +impl DialAttemptsRemaining { + fn get_attempts(&self) -> u32 { + match self { DialAttemptsRemaining::Some(attempts) => (*attempts).into(), DialAttemptsRemaining::None(_) => 0, } @@ -268,7 +268,7 @@ where error, } => f .debug_struct("DialError") - .field("attempts_remaining", &Into::::into(attempts_remaining)) + .field("attempts_remaining", &attempts_remaining.get_attempts()) .field("peer_id", peer_id) .field("multiaddr", multiaddr) .field("error", error) From 814ff4b2b981287d4ab03ac4ee8505b7d49c672a Mon Sep 17 00:00:00 2001 From: Max Inden Date: Thu, 26 Aug 2021 22:07:17 +0200 Subject: [PATCH 33/38] swarm: Add doc example for carrying state in handler --- swarm/Cargo.toml | 3 +- swarm/src/behaviour.rs | 198 ++++++++++++++++++++++++++++++++++++++--- swarm/src/lib.rs | 23 +++-- 3 files changed, 201 insertions(+), 23 deletions(-) diff --git a/swarm/Cargo.toml b/swarm/Cargo.toml index 33b482f2a0f..b8e5d77a242 100644 --- a/swarm/Cargo.toml +++ b/swarm/Cargo.toml @@ -20,7 +20,6 @@ wasm-timer = "0.2" void = "1" [dev-dependencies] -libp2p-mplex = { path = "../muxers/mplex" } -libp2p-noise = { path = "../transports/noise" } +libp2p = { path = "../", default-features = false, features = ["yamux", "plaintext"] } quickcheck = "0.9.0" rand = "0.7.2" diff --git a/swarm/src/behaviour.rs b/swarm/src/behaviour.rs index fc5edd35ca6..c3d121bdcc7 100644 --- a/swarm/src/behaviour.rs +++ b/swarm/src/behaviour.rs @@ -282,16 +282,17 @@ pub enum NetworkBehaviourAction< /// /// On success, [`NetworkBehaviour::inject_connection_established`] is invoked. /// On failure, [`NetworkBehaviour::inject_dial_failure`] is invoked. + /// + /// Note that the provided handler is returned to the [`NetworkBehaviour`] on connection failure + /// and connection closing. Thus it can be used to carry state, which otherwise would have to be + /// tracked in the [`NetworkBehaviour`] itself. E.g. a message destined to an unconnected peer + /// can be included in the handler, and thus directly send on connection success or extracted by + /// the [`NetworkBehaviour`] on connection failure. See [`NetworkBehaviourAction::DialPeer`] for + /// example. DialAddress { /// The address to dial. address: Multiaddr, /// The handler to be used to handle the connection to the peer. - /// - /// Note that the handler is returned to the [`NetworkBehaviour`] on connection failure and - /// connection closing. Thus it can be used to carry state, which otherwise would have to be - /// tracked in the [`NetworkBehaviour`] itself. E.g. a message destined to an unconnected - /// peer can be included in the handler, and thus directly send on connection success or - /// extracted by the [`NetworkBehaviour`] on connection failure. handler: THandler, }, @@ -305,18 +306,191 @@ pub enum NetworkBehaviourAction< /// /// On success, [`NetworkBehaviour::inject_connection_established`] is invoked. /// On failure, [`NetworkBehaviour::inject_dial_failure`] is invoked. + /// + /// Note that the provided handler is returned to the [`NetworkBehaviour`] on connection failure + /// and connection closing. Thus it can be used to carry state, which otherwise would have to be + /// tracked in the [`NetworkBehaviour`] itself. E.g. a message destined to an unconnected peer + /// can be included in the handler, and thus directly send on connection success or extracted by + /// the [`NetworkBehaviour`] on connection failure. + /// + /// Example showcasing usage of handler to carry state: + /// + /// ```rust + /// # use futures::executor::block_on; + /// # use futures::stream::StreamExt; + /// # use libp2p::core::connection::ConnectionId; + /// # use libp2p::core::identity; + /// # use libp2p::core::transport::{MemoryTransport, Transport}; + /// # use libp2p::core::upgrade::{self, DeniedUpgrade, InboundUpgrade, OutboundUpgrade}; + /// # use libp2p::core::PeerId; + /// # use libp2p::plaintext::PlainText2Config; + /// # use libp2p::swarm::{ + /// # DialError, DialPeerCondition, IntoProtocolsHandler, KeepAlive, NegotiatedSubstream, + /// # NetworkBehaviour, NetworkBehaviourAction, PollParameters, ProtocolsHandler, + /// # ProtocolsHandlerEvent, ProtocolsHandlerUpgrErr, SubstreamProtocol, Swarm, SwarmEvent, + /// # }; + /// # use libp2p::yamux; + /// # use std::collections::VecDeque; + /// # use std::task::{Context, Poll}; + /// # use void::Void; + /// # + /// # let local_key = identity::Keypair::generate_ed25519(); + /// # let local_public_key = local_key.public(); + /// # let local_peer_id = PeerId::from(local_public_key.clone()); + /// # + /// # let transport = MemoryTransport::default() + /// # .upgrade(upgrade::Version::V1) + /// # .authenticate(PlainText2Config { local_public_key }) + /// # .multiplex(yamux::YamuxConfig::default()) + /// # .boxed(); + /// # + /// # let mut swarm = Swarm::new(transport, MyBehaviour::default(), local_peer_id); + /// # + /// // Super precious message that we should better not lose. + /// let message = PreciousMessage("My precious message".to_string()); + /// + /// // Unfortunately this peer is offline, thus sending our message to it will fail. + /// let offline_peer = PeerId::random(); + /// + /// // Let's send it anyways. We should get it back in case connecting to the peer fails. + /// swarm.behaviour_mut().send(offline_peer, message); + /// + /// block_on(async { + /// // As expected, sending failed. But great news, we got our message back. + /// matches!( + /// swarm.next().await.expect("Infinite stream"), + /// SwarmEvent::Behaviour(PreciousMessage(_)) + /// ); + /// }); + /// + /// # #[derive(Default)] + /// # struct MyBehaviour { + /// # outbox_to_swarm: VecDeque>, + /// # } + /// # + /// # impl MyBehaviour { + /// # fn send(&mut self, peer_id: PeerId, msg: PreciousMessage) { + /// # self.outbox_to_swarm + /// # .push_back(NetworkBehaviourAction::DialPeer { + /// # peer_id, + /// # condition: DialPeerCondition::Always, + /// # handler: MyHandler { message: Some(msg) }, + /// # }); + /// # } + /// # } + /// # + /// impl NetworkBehaviour for MyBehaviour { + /// # type ProtocolsHandler = MyHandler; + /// # type OutEvent = PreciousMessage; + /// # + /// # fn new_handler(&mut self) -> Self::ProtocolsHandler { + /// # MyHandler { message: None } + /// # } + /// # + /// # + /// # fn inject_event( + /// # &mut self, + /// # _: PeerId, + /// # _: ConnectionId, + /// # _: <::Handler as ProtocolsHandler>::OutEvent, + /// # ) { + /// # unreachable!(); + /// # } + /// # + /// fn inject_dial_failure( + /// &mut self, + /// _: &PeerId, + /// handler: Self::ProtocolsHandler, + /// _: DialError, + /// ) { + /// // As expected, sending the message failed. But lucky us, we got the handler back, thus + /// // the precious message is not lost and we can return it back to the user. + /// let msg = handler.message.unwrap(); + /// self.outbox_to_swarm + /// .push_back(NetworkBehaviourAction::GenerateEvent(msg)) + /// } + /// # + /// # fn poll( + /// # &mut self, + /// # _: &mut Context<'_>, + /// # _: &mut impl PollParameters, + /// # ) -> Poll> { + /// # if let Some(action) = self.outbox_to_swarm.pop_front() { + /// # return Poll::Ready(action); + /// # } + /// # Poll::Pending + /// # } + /// } + /// + /// # struct MyHandler { + /// # message: Option, + /// # } + /// # + /// # impl ProtocolsHandler for MyHandler { + /// # type InEvent = Void; + /// # type OutEvent = Void; + /// # type Error = Void; + /// # type InboundProtocol = DeniedUpgrade; + /// # type OutboundProtocol = DeniedUpgrade; + /// # type InboundOpenInfo = (); + /// # type OutboundOpenInfo = Void; + /// # + /// # fn listen_protocol( + /// # &self, + /// # ) -> SubstreamProtocol { + /// # SubstreamProtocol::new(DeniedUpgrade, ()) + /// # } + /// # + /// # fn inject_fully_negotiated_inbound( + /// # &mut self, + /// # _: >::Output, + /// # _: Self::InboundOpenInfo, + /// # ) { + /// # } + /// # + /// # fn inject_fully_negotiated_outbound( + /// # &mut self, + /// # _: >::Output, + /// # _: Self::OutboundOpenInfo, + /// # ) { + /// # } + /// # + /// # fn inject_event(&mut self, _event: Self::InEvent) {} + /// # + /// # fn inject_dial_upgrade_error( + /// # &mut self, + /// # _: Self::OutboundOpenInfo, + /// # _: ProtocolsHandlerUpgrErr, + /// # ) { + /// # } + /// # + /// # fn connection_keep_alive(&self) -> KeepAlive { + /// # KeepAlive::Yes + /// # } + /// # + /// # fn poll( + /// # &mut self, + /// # _: &mut Context<'_>, + /// # ) -> Poll< + /// # ProtocolsHandlerEvent< + /// # Self::OutboundProtocol, + /// # Self::OutboundOpenInfo, + /// # Self::OutEvent, + /// # Self::Error, + /// # >, + /// # > { + /// # todo!("If `Self::message.is_some()` send the message to the remote.") + /// # } + /// # } + /// # #[derive(Debug, PartialEq, Eq)] + /// # struct PreciousMessage(String); + /// ``` DialPeer { /// The peer to try reach. peer_id: PeerId, /// The condition for initiating a new dialing attempt. condition: DialPeerCondition, /// The handler to be used to handle the connection to the peer. - /// - /// Note that the handler is returned to the [`NetworkBehaviour`] on connection failure and - /// connection closing. Thus it can be used to carry state, which otherwise would have to be - /// tracked in the [`NetworkBehaviour`] itself. E.g. a message destined to an unconnected - /// peer can be included in the handler, and thus directly send on connection success or - /// extracted by the [`NetworkBehaviour`] on connection failure. handler: THandler, }, diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index 925e47d644a..2fe2376c713 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -1326,8 +1326,9 @@ mod tests { use crate::protocols_handler::DummyProtocolsHandler; use crate::test::{CallTraceBehaviour, MockBehaviour}; use futures::{executor, future}; - use libp2p_core::{identity, multiaddr, transport, upgrade}; - use libp2p_noise as noise; + use libp2p::core::{identity, multiaddr, transport, upgrade}; + use libp2p::plaintext; + use libp2p::yamux; // Test execution state. // Connection => Disconnecting => Connecting. @@ -1343,17 +1344,16 @@ mod tests { O: Send + 'static, { let id_keys = identity::Keypair::generate_ed25519(); - let pubkey = id_keys.public(); - let noise_keys = noise::Keypair::::new() - .into_authentic(&id_keys) - .unwrap(); + let local_public_key = id_keys.public(); let transport = transport::MemoryTransport::default() .upgrade(upgrade::Version::V1) - .authenticate(noise::NoiseConfig::xx(noise_keys).into_authenticated()) - .multiplex(libp2p_mplex::MplexConfig::new()) + .authenticate(plaintext::PlainText2Config { + local_public_key: local_public_key.clone(), + }) + .multiplex(yamux::YamuxConfig::default()) .boxed(); let behaviour = CallTraceBehaviour::new(MockBehaviour::new(handler_proto)); - SwarmBuilder::new(transport, behaviour, pubkey.into()).build() + SwarmBuilder::new(transport, behaviour, local_public_key.into()).build() } fn swarms_connected( @@ -1704,4 +1704,9 @@ mod tests { } })) } + + /// [`NetworkBehaviourAction::DialAddress`] and [`NetworkBehaviourAction::DialPeer`] require a + /// handler. This handler can be used to carry state. See corresponding doc comments. + #[test] + fn use_handler_to_carry_state() {} } From 60c7261f665e3101e2c219104bcc3610c78cebe7 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Mon, 30 Aug 2021 10:43:49 +0200 Subject: [PATCH 34/38] swarm/src/lib: Remove use_handler_to_carry_state --- swarm/src/lib.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index 2fe2376c713..dcccd46af79 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -1704,9 +1704,4 @@ mod tests { } })) } - - /// [`NetworkBehaviourAction::DialAddress`] and [`NetworkBehaviourAction::DialPeer`] require a - /// handler. This handler can be used to carry state. See corresponding doc comments. - #[test] - fn use_handler_to_carry_state() {} } From a2f1819ddc6cc13c8699ca8a8d3714c483db619b Mon Sep 17 00:00:00 2001 From: Max Inden Date: Mon, 30 Aug 2021 10:50:35 +0200 Subject: [PATCH 35/38] core/tests/network_dial_error: Use get_attempts --- core/src/network/event.rs | 2 +- core/tests/network_dial_error.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/network/event.rs b/core/src/network/event.rs index 98793395165..cea5bbddc21 100644 --- a/core/src/network/event.rs +++ b/core/src/network/event.rs @@ -179,7 +179,7 @@ pub enum DialAttemptsRemaining { } impl DialAttemptsRemaining { - fn get_attempts(&self) -> u32 { + pub fn get_attempts(&self) -> u32 { match self { DialAttemptsRemaining::Some(attempts) => (*attempts).into(), DialAttemptsRemaining::None(_) => 0, diff --git a/core/tests/network_dial_error.rs b/core/tests/network_dial_error.rs index ddbb7a9285e..ff9f12a574f 100644 --- a/core/tests/network_dial_error.rs +++ b/core/tests/network_dial_error.rs @@ -70,7 +70,7 @@ fn deny_incoming_connec() { multiaddr, error: PendingConnectionError::Transport(_), }) => { - assert_eq!(0u32, (&attempts_remaining).into()); + assert_eq!(0u32, attempts_remaining.get_attempts()); assert_eq!(&peer_id, swarm1.local_peer_id()); assert_eq!( multiaddr, @@ -202,11 +202,11 @@ fn multiple_addresses_err() { .with(Protocol::P2p(target.clone().into())); assert_eq!(multiaddr, expected); if addresses.is_empty() { - assert_eq!(Into::::into(&attempts_remaining), 0); + assert_eq!(attempts_remaining.get_attempts(), 0); return Poll::Ready(Ok(())); } else { assert_eq!( - Into::::into(&attempts_remaining), + attempts_remaining.get_attempts(), addresses.len() as u32 ); } From b90554545bf6ff62eb56d3a1cd455b0cafb90f1a Mon Sep 17 00:00:00 2001 From: Max Inden Date: Mon, 30 Aug 2021 10:52:22 +0200 Subject: [PATCH 36/38] swarm/src/behaviour.rs: Use heading for doc example Co-authored-by: Thomas Eizinger --- swarm/src/behaviour.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/swarm/src/behaviour.rs b/swarm/src/behaviour.rs index c3d121bdcc7..085439aa1b1 100644 --- a/swarm/src/behaviour.rs +++ b/swarm/src/behaviour.rs @@ -313,7 +313,7 @@ pub enum NetworkBehaviourAction< /// can be included in the handler, and thus directly send on connection success or extracted by /// the [`NetworkBehaviour`] on connection failure. /// - /// Example showcasing usage of handler to carry state: + /// # Example /// /// ```rust /// # use futures::executor::block_on; From 99f81d0cf5ccba8062689a6fe0b3e3ecd61e5caf Mon Sep 17 00:00:00 2001 From: Max Inden Date: Mon, 30 Aug 2021 12:50:15 +0200 Subject: [PATCH 37/38] core/tests: Format with rustfmt --- core/tests/network_dial_error.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/core/tests/network_dial_error.rs b/core/tests/network_dial_error.rs index ff9f12a574f..bed4c06e023 100644 --- a/core/tests/network_dial_error.rs +++ b/core/tests/network_dial_error.rs @@ -205,10 +205,7 @@ fn multiple_addresses_err() { assert_eq!(attempts_remaining.get_attempts(), 0); return Poll::Ready(Ok(())); } else { - assert_eq!( - attempts_remaining.get_attempts(), - addresses.len() as u32 - ); + assert_eq!(attempts_remaining.get_attempts(), addresses.len() as u32); } } Poll::Ready(_) => unreachable!(), From c09198ecb085dbeac448eaf86a2f7a9bd08fb48b Mon Sep 17 00:00:00 2001 From: Max Inden Date: Tue, 31 Aug 2021 14:24:07 +0200 Subject: [PATCH 38/38] protocols/gossipsub/src/behaviour.rs: Remove unnecesary assignment Co-authored-by: Thomas Eizinger --- protocols/gossipsub/src/behaviour.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/protocols/gossipsub/src/behaviour.rs b/protocols/gossipsub/src/behaviour.rs index 700d3829df3..e3adfdcfc1a 100644 --- a/protocols/gossipsub/src/behaviour.rs +++ b/protocols/gossipsub/src/behaviour.rs @@ -3176,11 +3176,6 @@ where _: &mut impl PollParameters, ) -> Poll> { if let Some(event) = self.events.pop_front() { - let event: NetworkBehaviourAction< - Self::OutEvent, - Self::ProtocolsHandler, - Arc, - > = event; return Poll::Ready(event.map_in(|e: Arc| { // clone send event reference if others references are present Arc::try_unwrap(e).unwrap_or_else(|e| (*e).clone())