From 90a1349c9be41c42f7b69f806a895bbcd1d23306 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Sat, 3 Sep 2022 18:00:21 +0900 Subject: [PATCH 1/7] swarm/behaviour: Replace inject_* with on_event Replace the various `NetworkBehaviour::inject_*` methods with a single `NetworkBehaviour::on_event` method and a `InEvent` `enum`. --- swarm/src/behaviour.rs | 361 +++++++++++++++++++++++++++++----- swarm/src/behaviour/either.rs | 188 ++---------------- swarm/src/behaviour/toggle.rs | 135 +------------ swarm/src/handler/either.rs | 23 +++ swarm/src/lib.rs | 188 +++++++++++++++--- 5 files changed, 521 insertions(+), 374 deletions(-) diff --git a/swarm/src/behaviour.rs b/swarm/src/behaviour.rs index a5bf0e06f06..1465e2ee401 100644 --- a/swarm/src/behaviour.rs +++ b/swarm/src/behaviour.rs @@ -152,7 +152,15 @@ pub trait NetworkBehaviour: 'static { vec![] } + /// Informs the behaviour about an event from the [Transport](libp2p_core::Transport) or the + /// [`ConnectionHandler`]. + fn on_event(&mut self, _event: InEvent); + /// Informs the behaviour about a newly established connection to a peer. + #[deprecated( + since = "0.39.0", + note = "Handle `InEvent::ConnectionEstablished` in `NetworkBehaviour::on_event` instead." + )] fn inject_connection_established( &mut self, _peer_id: &PeerId, @@ -163,21 +171,31 @@ pub trait NetworkBehaviour: 'static { ) { } - /// Informs the behaviour about a closed connection to a peer. - /// - /// A call to this method is always paired with an earlier call to - /// [`NetworkBehaviour::inject_connection_established`] with the same peer ID, connection ID and endpoint. - fn inject_connection_closed( - &mut self, - _: &PeerId, - _: &ConnectionId, - _: &ConnectedPoint, - _: ::Handler, - _remaining_established: usize, - ) { - } + // TODO: Have to remove this as it takes ownership of `Handler`. + // + // /// Informs the behaviour about a closed connection to a peer. + // /// + // /// A call to this method is always paired with an earlier call to + // /// [`NetworkBehaviour::inject_connection_established`] with the same peer ID, connection ID and endpoint. + // #[deprecated( + // since = "0.39.0", + // note = "Handle `InEvent::ConnectionClosed` in `NetworkBehaviour::on_event` instead." + // )] + // fn inject_connection_closed( + // &mut self, + // _: &PeerId, + // _: &ConnectionId, + // _: &ConnectedPoint, + // _: ::Handler, + // _remaining_established: usize, + // ) { + // } /// Informs the behaviour that the [`ConnectedPoint`] of an existing connection has changed. + #[deprecated( + since = "0.39.0", + note = "Handle `InEvent::AddressChange` in `NetworkBehaviour::on_event` instead." + )] fn inject_address_change( &mut self, _: &PeerId, @@ -187,61 +205,99 @@ pub trait NetworkBehaviour: 'static { ) { } - /// Informs the behaviour about an event generated by the handler dedicated to the peer identified by `peer_id`. - /// for the behaviour. - /// - /// The `peer_id` is guaranteed to be in a connected state. In other words, - /// [`NetworkBehaviour::inject_connection_established`] has previously been called with this `PeerId`. - fn inject_event( - &mut self, - peer_id: PeerId, - connection: ConnectionId, - event: <::Handler as ConnectionHandler>::OutEvent, - ); - - /// Indicates to the behaviour that the dial to a known or unknown node failed. - fn inject_dial_failure( - &mut self, - _peer_id: Option, - _handler: Self::ConnectionHandler, - _error: &DialError, - ) { - } - - /// 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::ConnectionHandler, - ) { - } + // TODO: Can not deprecate as it takes ownership of handler event. + // + // /// Informs the behaviour about an event generated by the handler dedicated to the peer identified by `peer_id`. + // /// for the behaviour. + // /// + // /// The `peer_id` is guaranteed to be in a connected state. In other words, + // /// [`NetworkBehaviour::inject_connection_established`] has previously been called with this `PeerId`. + // #[deprecated( + // since = "0.39.0", + // note = "Handle `InEvent::ConnectionHandler` in `NetworkBehaviour::on_event` instead." + // )] + // fn inject_event( + // &mut self, + // peer_id: PeerId, + // connection: ConnectionId, + // event: <::Handler as ConnectionHandler>::OutEvent, + // ); + + // TODO: Can not deprecate as it takes ownership of handler. + // + // /// Indicates to the behaviour that the dial to a known or unknown node failed. + // fn inject_dial_failure( + // &mut self, + // _peer_id: Option, + // _handler: Self::ConnectionHandler, + // _error: &DialError, + // ) { + // } + + // TODO: Can not deprecate as it takes ownership of handler. + // + // /// 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::ConnectionHandler, + // ) { + // } /// Indicates to the behaviour that a new listener was created. + #[deprecated( + since = "0.39.0", + note = "Handle `InEvent::NewListener` in `NetworkBehaviour::on_event` instead." + )] fn inject_new_listener(&mut self, _id: ListenerId) {} /// Indicates to the behaviour that we have started listening on a new multiaddr. + #[deprecated( + since = "0.39.0", + note = "Handle `InEvent::NewListenAddr` in `NetworkBehaviour::on_event` instead." + )] fn inject_new_listen_addr(&mut self, _id: ListenerId, _addr: &Multiaddr) {} /// Indicates to the behaviour that a multiaddr we were listening on has expired, - /// which means that we are no longer listening in it. + /// which means that we are no longer listening on it. + #[deprecated( + since = "0.39.0", + note = "Handle `InEvent::ExpiredListenAddr` in `NetworkBehaviour::on_event` instead." + )] fn inject_expired_listen_addr(&mut self, _id: ListenerId, _addr: &Multiaddr) {} /// A listener experienced an error. + #[deprecated( + since = "0.39.0", + note = "Handle `InEvent::ListenerError` in `NetworkBehaviour::on_event` instead." + )] fn inject_listener_error(&mut self, _id: ListenerId, _err: &(dyn std::error::Error + 'static)) { } /// A listener closed. + #[deprecated( + since = "0.39.0", + note = "Handle `InEvent::ListenerClosed` in `NetworkBehaviour::on_event` instead." + )] fn inject_listener_closed(&mut self, _id: ListenerId, _reason: Result<(), &std::io::Error>) {} /// Indicates to the behaviour that we have discovered a new external address for us. + #[deprecated( + since = "0.39.0", + note = "Handle `InEvent::NewExternalAddr` in `NetworkBehaviour::on_event` instead." + )] fn inject_new_external_addr(&mut self, _addr: &Multiaddr) {} /// Indicates to the behaviour that an external address was removed. + #[deprecated( + since = "0.39.0", + note = "Handle `InEvent::ExpiredExternalAddr` in `NetworkBehaviour::on_event` instead." + )] fn inject_expired_external_addr(&mut self, _addr: &Multiaddr) {} /// Polls for things that swarm should do. @@ -721,3 +777,214 @@ impl Default for CloseConnection { CloseConnection::All } } + +pub enum InEvent<'a, Handler: IntoConnectionHandler> { + /// Informs the behaviour about a newly established connection to a peer. + ConnectionEstablished { + peer_id: PeerId, + connection_id: ConnectionId, + endpoint: &'a ConnectedPoint, + // TODO: Would a slice not be better? + failed_addresses: Option<&'a Vec>, + other_established: usize, + }, + /// Informs the behaviour about a closed connection to a peer. + /// + /// This event is always paired with an earlier call to + /// [`InEvent::ConnectionEstablished`] with the same peer ID, connection ID + /// and endpoint. + ConnectionClosed { + peer_id: PeerId, + connection_id: ConnectionId, + endpoint: &'a ConnectedPoint, + handler: ::Handler, + remaining_established: usize, + }, + /// Informs the behaviour that the [`ConnectedPoint`] of an existing + /// connection has changed. + AddressChange { + peer_id: PeerId, + connection_id: ConnectionId, + old: &'a ConnectedPoint, + new: &'a ConnectedPoint, + }, + /// Informs the behaviour about an event generated by the handler dedicated to the peer + /// identified by `peer_id`. for the behaviour. + /// + /// The `peer_id` is guaranteed to be in a connected state. In other words, + /// [`InEvent::ConnectionEstablished`] has previously been received with this `PeerId`. + ConnectionHandler { + peer_id: PeerId, + connection: ConnectionId, + event: <::Handler as ConnectionHandler>::OutEvent, + }, + /// Indicates to the behaviour that the dial to a known or unknown node failed. + DialFailure { + peer_id: Option, + handler: Handler, + error: &'a DialError, + }, + /// 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. + ListenFailure { + local_addr: &'a Multiaddr, + send_back_addr: &'a Multiaddr, + handler: Handler, + }, + /// Indicates to the behaviour that a new listener was created. + NewListener { listener_id: ListenerId }, + /// Indicates to the behaviour that we have started listening on a new multiaddr. + NewListenAddr { + listener_id: ListenerId, + addr: &'a Multiaddr, + }, + /// Indicates to the behaviour that a multiaddr we were listening on has expired, + /// which means that we are no longer listening on it. + ExpiredListenAddr { + listener_id: ListenerId, + addr: &'a Multiaddr, + }, + /// A listener experienced an error. + ListenerError { + listener_id: ListenerId, + err: &'a (dyn std::error::Error + 'static), + }, + /// A listener closed. + ListenerClosed { + listener_id: ListenerId, + reason: Result<(), &'a std::io::Error>, + }, + /// Indicates to the behaviour that we have discovered a new external address for us. + NewExternalAddr { addr: &'a Multiaddr }, + /// Indicates to the behaviour that an external address was removed. + ExpiredExternalAddr { addr: &'a Multiaddr }, +} + +impl<'a, Handler: IntoConnectionHandler> InEvent<'a, Handler> { + fn map_handler( + self, + map_into_handler: impl FnOnce(Handler) -> NewHandler, + map_handler: impl FnOnce( + ::Handler, + ) -> ::Handler, + map_event: impl FnOnce( + <::Handler as ConnectionHandler>::OutEvent, + ) -> + <::Handler as ConnectionHandler>::OutEvent, + ) -> InEvent<'a, NewHandler> + where + NewHandler: IntoConnectionHandler, + { + self.try_map_handler( + |h| Some(map_into_handler(h)), + |h| Some(map_handler(h)), + |e| Some(map_event(e)), + ) + .expect("To return Some as all closures return Some.") + } + + fn try_map_handler( + self, + map_into_handler: impl FnOnce(Handler) -> Option, + map_handler: impl FnOnce( + ::Handler, + ) -> Option<::Handler>, + map_event: impl FnOnce( + <::Handler as ConnectionHandler>::OutEvent, + ) -> Option< + <::Handler as ConnectionHandler>::OutEvent, + >, + ) -> Option> + where + NewHandler: IntoConnectionHandler, + { + match self { + InEvent::ConnectionClosed { + peer_id, + connection_id, + endpoint, + handler, + remaining_established, + } => Some(InEvent::ConnectionClosed { + peer_id, + connection_id, + endpoint, + handler: map_handler(handler)?, + remaining_established, + }), + InEvent::ConnectionEstablished { + peer_id, + connection_id, + endpoint, + failed_addresses, + other_established, + } => Some(InEvent::ConnectionEstablished { + peer_id, + connection_id, + endpoint, + failed_addresses, + other_established, + }), + InEvent::AddressChange { + peer_id, + connection_id, + old, + new, + } => Some(InEvent::AddressChange { + peer_id, + connection_id, + old, + new, + }), + InEvent::ConnectionHandler { + peer_id, + connection, + event, + } => Some(InEvent::ConnectionHandler { + peer_id, + connection, + event: map_event(event)?, + }), + InEvent::DialFailure { + peer_id, + handler, + error, + } => Some(InEvent::DialFailure { + peer_id, + handler: map_into_handler(handler)?, + error, + }), + InEvent::ListenFailure { + local_addr, + send_back_addr, + handler, + } => Some(InEvent::ListenFailure { + local_addr, + send_back_addr, + handler: map_into_handler(handler)?, + }), + InEvent::NewListener { listener_id } => Some(InEvent::NewListener { listener_id }), + InEvent::NewListenAddr { listener_id, addr } => { + Some(InEvent::NewListenAddr { listener_id, addr }) + } + InEvent::ExpiredListenAddr { listener_id, addr } => { + Some(InEvent::ExpiredListenAddr { listener_id, addr }) + } + InEvent::ListenerError { listener_id, err } => { + Some(InEvent::ListenerError { listener_id, err }) + } + InEvent::ListenerClosed { + listener_id, + reason, + } => Some(InEvent::ListenerClosed { + listener_id, + reason, + }), + InEvent::NewExternalAddr { addr } => Some(InEvent::NewExternalAddr { addr }), + InEvent::ExpiredExternalAddr { addr } => Some(InEvent::ExpiredExternalAddr { addr }), + } + } +} diff --git a/swarm/src/behaviour/either.rs b/swarm/src/behaviour/either.rs index 6bb1d95a519..e9677ff419a 100644 --- a/swarm/src/behaviour/either.rs +++ b/swarm/src/behaviour/either.rs @@ -18,12 +18,10 @@ // FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // DEALINGS IN THE SOFTWARE. -use crate::handler::{either::IntoEitherHandler, ConnectionHandler, IntoConnectionHandler}; -use crate::{DialError, NetworkBehaviour, NetworkBehaviourAction, PollParameters}; +use crate::handler::either::IntoEitherHandler; +use crate::{NetworkBehaviour, NetworkBehaviourAction, PollParameters}; use either::Either; -use libp2p_core::{ - connection::ConnectionId, transport::ListenerId, ConnectedPoint, Multiaddr, PeerId, -}; +use libp2p_core::{Multiaddr, PeerId}; use std::{task::Context, task::Poll}; /// Implementation of [`NetworkBehaviour`] that can be either of two implementations. @@ -49,170 +47,24 @@ where } } - fn inject_connection_established( - &mut self, - peer_id: &PeerId, - connection: &ConnectionId, - endpoint: &ConnectedPoint, - errors: Option<&Vec>, - other_established: usize, - ) { - match self { - Either::Left(a) => a.inject_connection_established( - peer_id, - connection, - endpoint, - errors, - other_established, - ), - Either::Right(b) => b.inject_connection_established( - peer_id, - connection, - endpoint, - errors, - other_established, - ), - } - } - - fn inject_connection_closed( - &mut self, - peer_id: &PeerId, - connection: &ConnectionId, - endpoint: &ConnectedPoint, - handler: ::Handler, - remaining_established: usize, - ) { - match (self, handler) { - (Either::Left(behaviour), Either::Left(handler)) => behaviour.inject_connection_closed( - peer_id, - connection, - endpoint, - handler, - remaining_established, - ), - (Either::Right(behaviour), Either::Right(handler)) => behaviour - .inject_connection_closed( - peer_id, - connection, - endpoint, - handler, - remaining_established, - ), - _ => unreachable!(), - } - } - - fn inject_address_change( - &mut self, - peer_id: &PeerId, - connection: &ConnectionId, - old: &ConnectedPoint, - new: &ConnectedPoint, - ) { - match self { - Either::Left(a) => a.inject_address_change(peer_id, connection, old, new), - Either::Right(b) => b.inject_address_change(peer_id, connection, old, new), - } - } - - fn inject_event( - &mut self, - peer_id: PeerId, - connection: ConnectionId, - event: <::Handler as ConnectionHandler>::OutEvent, - ) { - match (self, event) { - (Either::Left(behaviour), Either::Left(event)) => { - behaviour.inject_event(peer_id, connection, event) - } - (Either::Right(behaviour), Either::Right(event)) => { - behaviour.inject_event(peer_id, connection, event) - } - _ => unreachable!(), - } - } - - fn inject_dial_failure( - &mut self, - peer_id: Option, - handler: Self::ConnectionHandler, - error: &DialError, - ) { - match (self, handler) { - (Either::Left(behaviour), IntoEitherHandler::Left(handler)) => { - behaviour.inject_dial_failure(peer_id, handler, error) - } - (Either::Right(behaviour), IntoEitherHandler::Right(handler)) => { - behaviour.inject_dial_failure(peer_id, handler, error) - } - _ => unreachable!(), - } - } - - fn inject_listen_failure( - &mut self, - local_addr: &Multiaddr, - send_back_addr: &Multiaddr, - handler: Self::ConnectionHandler, - ) { - match (self, handler) { - (Either::Left(behaviour), IntoEitherHandler::Left(handler)) => { - behaviour.inject_listen_failure(local_addr, send_back_addr, handler) - } - (Either::Right(behaviour), IntoEitherHandler::Right(handler)) => { - behaviour.inject_listen_failure(local_addr, send_back_addr, handler) - } - _ => unreachable!(), - } - } - - fn inject_new_listener(&mut self, id: ListenerId) { - match self { - Either::Left(a) => a.inject_new_listener(id), - Either::Right(b) => b.inject_new_listener(id), - } - } - - fn inject_new_listen_addr(&mut self, id: ListenerId, addr: &Multiaddr) { - match self { - Either::Left(a) => a.inject_new_listen_addr(id, addr), - Either::Right(b) => b.inject_new_listen_addr(id, addr), - } - } - - fn inject_expired_listen_addr(&mut self, id: ListenerId, addr: &Multiaddr) { - match self { - Either::Left(a) => a.inject_expired_listen_addr(id, addr), - Either::Right(b) => b.inject_expired_listen_addr(id, addr), - } - } - - fn inject_new_external_addr(&mut self, addr: &Multiaddr) { - match self { - Either::Left(a) => a.inject_new_external_addr(addr), - Either::Right(b) => b.inject_new_external_addr(addr), - } - } - - fn inject_expired_external_addr(&mut self, addr: &Multiaddr) { - match self { - Either::Left(a) => a.inject_expired_external_addr(addr), - Either::Right(b) => b.inject_expired_external_addr(addr), - } - } - - fn inject_listener_error(&mut self, id: ListenerId, err: &(dyn std::error::Error + 'static)) { - match self { - Either::Left(a) => a.inject_listener_error(id, err), - Either::Right(b) => b.inject_listener_error(id, err), - } - } - - fn inject_listener_closed(&mut self, id: ListenerId, reason: Result<(), &std::io::Error>) { + fn on_event(&mut self, event: super::InEvent) { match self { - Either::Left(a) => a.inject_listener_closed(id, reason), - Either::Right(b) => b.inject_listener_closed(id, reason), + Either::Left(b) => b.on_event(event.map_handler( + |h| h.unwrap_left(), + |h| match h { + Either::Left(h) => h, + Either::Right(_) => unreachable!(), + }, + |e| e.unwrap_left(), + )), + Either::Right(b) => b.on_event(event.map_handler( + |h| h.unwrap_right(), + |h| match h { + Either::Right(h) => h, + Either::Left(_) => unreachable!(), + }, + |e| e.unwrap_right(), + )), } } diff --git a/swarm/src/behaviour/toggle.rs b/swarm/src/behaviour/toggle.rs index 07a15d56da9..6561876e555 100644 --- a/swarm/src/behaviour/toggle.rs +++ b/swarm/src/behaviour/toggle.rs @@ -23,12 +23,10 @@ use crate::handler::{ KeepAlive, SubstreamProtocol, }; use crate::upgrade::{InboundUpgradeSend, OutboundUpgradeSend, SendWrapper}; -use crate::{DialError, NetworkBehaviour, NetworkBehaviourAction, PollParameters}; +use crate::{NetworkBehaviour, NetworkBehaviourAction, PollParameters}; use either::Either; use libp2p_core::{ - connection::ConnectionId, either::{EitherError, EitherOutput}, - transport::ListenerId, upgrade::{DeniedUpgrade, EitherUpgrade}, ConnectedPoint, Multiaddr, PeerId, }; @@ -84,137 +82,14 @@ where .unwrap_or_else(Vec::new) } - fn inject_connection_established( - &mut self, - peer_id: &PeerId, - connection: &ConnectionId, - endpoint: &ConnectedPoint, - errors: Option<&Vec>, - other_established: usize, - ) { - if let Some(inner) = self.inner.as_mut() { - inner.inject_connection_established( - peer_id, - connection, - endpoint, - errors, - other_established, - ) - } - } - - fn inject_connection_closed( - &mut self, - peer_id: &PeerId, - connection: &ConnectionId, - endpoint: &ConnectedPoint, - handler: ::Handler, - remaining_established: usize, - ) { - if let Some(inner) = self.inner.as_mut() { - if let Some(handler) = handler.inner { - inner.inject_connection_closed( - peer_id, - connection, - endpoint, - handler, - remaining_established, - ) - } - } - } - - fn inject_address_change( - &mut self, - peer_id: &PeerId, - connection: &ConnectionId, - old: &ConnectedPoint, - new: &ConnectedPoint, - ) { - if let Some(inner) = self.inner.as_mut() { - inner.inject_address_change(peer_id, connection, old, new) - } - } - - fn inject_event( - &mut self, - peer_id: PeerId, - connection: ConnectionId, - event: <::Handler as ConnectionHandler>::OutEvent, - ) { - if let Some(inner) = self.inner.as_mut() { - inner.inject_event(peer_id, connection, event); - } - } - - fn inject_dial_failure( - &mut self, - peer_id: Option, - handler: Self::ConnectionHandler, - error: &DialError, - ) { - if let Some(inner) = self.inner.as_mut() { - if let Some(handler) = handler.inner { - inner.inject_dial_failure(peer_id, handler, error) - } - } - } - - fn inject_listen_failure( - &mut self, - local_addr: &Multiaddr, - send_back_addr: &Multiaddr, - handler: Self::ConnectionHandler, - ) { - 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 on_event(&mut self, event: super::InEvent) { + if let Some(behaviour) = &mut self.inner { + if let Some(event) = event.try_map_handler(|h| h.inner, |h| h.inner, |e| Some(e)) { + behaviour.on_event(event); } } } - fn inject_new_listener(&mut self, id: ListenerId) { - if let Some(inner) = self.inner.as_mut() { - inner.inject_new_listener(id) - } - } - - fn inject_new_listen_addr(&mut self, id: ListenerId, addr: &Multiaddr) { - if let Some(inner) = self.inner.as_mut() { - inner.inject_new_listen_addr(id, addr) - } - } - - fn inject_expired_listen_addr(&mut self, id: ListenerId, addr: &Multiaddr) { - if let Some(inner) = self.inner.as_mut() { - inner.inject_expired_listen_addr(id, addr) - } - } - - fn inject_new_external_addr(&mut self, addr: &Multiaddr) { - if let Some(inner) = self.inner.as_mut() { - inner.inject_new_external_addr(addr) - } - } - - fn inject_expired_external_addr(&mut self, addr: &Multiaddr) { - if let Some(inner) = self.inner.as_mut() { - inner.inject_expired_external_addr(addr) - } - } - - fn inject_listener_error(&mut self, id: ListenerId, err: &(dyn std::error::Error + 'static)) { - if let Some(inner) = self.inner.as_mut() { - inner.inject_listener_error(id, err) - } - } - - fn inject_listener_closed(&mut self, id: ListenerId, reason: Result<(), &std::io::Error>) { - if let Some(inner) = self.inner.as_mut() { - inner.inject_listener_closed(id, reason) - } - } - fn poll( &mut self, cx: &mut Context<'_>, diff --git a/swarm/src/handler/either.rs b/swarm/src/handler/either.rs index 21d386ec56a..2823b6ed824 100644 --- a/swarm/src/handler/either.rs +++ b/swarm/src/handler/either.rs @@ -64,6 +64,29 @@ where } } +// Taken from https://github.com/bluss/either. +impl IntoEitherHandler { + /// Returns the left value. + pub fn unwrap_left(self) -> L { + match self { + IntoEitherHandler::Left(l) => l, + IntoEitherHandler::Right(_) => { + panic!("called `IntoEitherHandler::unwrap_left()` on a `Right` value.",) + } + } + } + + /// Returns the right value. + pub fn unwrap_right(self) -> R { + match self { + IntoEitherHandler::Right(r) => r, + IntoEitherHandler::Left(_) => { + panic!("called `IntoEitherHandler::unwrap_right()` on a `Left` value.",) + } + } + } +} + /// Implementation of a [`ConnectionHandler`] that represents either of two [`ConnectionHandler`] /// implementations. impl ConnectionHandler for Either diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index 230de4631db..ed2ecd5c6ce 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -63,6 +63,7 @@ pub mod behaviour; pub mod dial_opts; pub mod handler; +use behaviour::InEvent; pub use behaviour::{ CloseConnection, NetworkBehaviour, NetworkBehaviourAction, NotifyHandler, PollParameters, }; @@ -327,6 +328,9 @@ where /// Depending on the underlying transport, one listener may have multiple listening addresses. pub fn listen_on(&mut self, addr: Multiaddr) -> Result> { let id = self.transport.listen_on(addr)?; + self.behaviour + .on_event(InEvent::NewListener { listener_id: id }); + #[allow(deprecated)] self.behaviour.inject_new_listener(id); Ok(id) } @@ -403,11 +407,11 @@ where PeerCondition::Always => true, }; if !condition_matched { - self.behaviour.inject_dial_failure( - Some(peer_id), + self.behaviour.on_event(InEvent::DialFailure { + peer_id: Some(peer_id), handler, - &DialError::DialPeerConditionFalse(condition), - ); + error: &DialError::DialPeerConditionFalse(condition), + }); return Err(DialError::DialPeerConditionFalse(condition)); } @@ -415,8 +419,11 @@ where // Check if peer is banned. if self.banned_peers.contains(&peer_id) { let error = DialError::Banned; - self.behaviour - .inject_dial_failure(Some(peer_id), handler, &error); + self.behaviour.on_event(InEvent::DialFailure { + peer_id: Some(peer_id), + handler, + error: &error, + }); return Err(error); } @@ -452,8 +459,11 @@ where if addresses.is_empty() { let error = DialError::NoAddresses; - self.behaviour - .inject_dial_failure(Some(peer_id), handler, &error); + self.behaviour.on_event(InEvent::DialFailure { + peer_id: Some(peer_id), + handler, + error: &error, + }); return Err(error); }; @@ -535,7 +545,11 @@ where Ok(_connection_id) => Ok(()), Err((connection_limit, handler)) => { let error = DialError::ConnectionLimit(connection_limit); - self.behaviour.inject_dial_failure(None, handler, &error); + self.behaviour.on_event(InEvent::DialFailure { + peer_id: None, + handler, + error: &error, + }); Err(error) } } @@ -576,12 +590,18 @@ where let result = self.external_addrs.add(a.clone(), s); let expired = match &result { AddAddressResult::Inserted { expired } => { + self.behaviour + .on_event(InEvent::NewExternalAddr { addr: &a }); + #[allow(deprecated)] self.behaviour.inject_new_external_addr(&a); expired } AddAddressResult::Updated { expired } => expired, }; for a in expired { + self.behaviour + .on_event(InEvent::ExpiredExternalAddr { addr: &a.addr }); + #[allow(deprecated)] self.behaviour.inject_expired_external_addr(&a.addr); } result @@ -595,6 +615,9 @@ where /// otherwise. pub fn remove_external_address(&mut self, addr: &Multiaddr) -> bool { if self.external_addrs.remove(addr) { + self.behaviour + .on_event(InEvent::ExpiredExternalAddr { addr }); + #[allow(deprecated)] self.behaviour.inject_expired_external_addr(addr); true } else { @@ -701,6 +724,15 @@ where let failed_addresses = concurrent_dial_errors .as_ref() .map(|es| es.iter().map(|(a, _)| a).cloned().collect()); + self.behaviour + .on_event(behaviour::InEvent::ConnectionEstablished { + peer_id, + connection_id: id, + endpoint: &endpoint, + failed_addresses: failed_addresses.as_ref(), + other_established: non_banned_established, + }); + #[allow(deprecated)] self.behaviour.inject_connection_established( &peer_id, &id, @@ -724,7 +756,11 @@ where } => { let error = error.into(); - self.behaviour.inject_dial_failure(peer, handler, &error); + self.behaviour.on_event(InEvent::DialFailure { + peer_id: peer, + handler, + error: &error, + }); if let Some(peer) = peer { log::debug!("Connection attempt to {:?} failed with {:?}.", peer, error,); @@ -745,8 +781,11 @@ where handler, } => { log::debug!("Incoming connection failed: {:?}", error); - self.behaviour - .inject_listen_failure(&local_addr, &send_back_addr, handler); + self.behaviour.on_event(InEvent::ListenFailure { + local_addr: &local_addr, + send_back_addr: &send_back_addr, + handler, + }); return Some(SwarmEvent::IncomingConnectionError { local_addr, send_back_addr, @@ -785,13 +824,14 @@ where .into_iter() .filter(|conn_id| !self.banned_peer_connections.contains(conn_id)) .count(); - self.behaviour.inject_connection_closed( - &peer_id, - &id, - &endpoint, - handler, - remaining_non_banned, - ); + self.behaviour + .on_event(behaviour::InEvent::ConnectionClosed { + peer_id, + connection_id: id, + endpoint: &endpoint, + handler, + remaining_established: remaining_non_banned, + }); } return Some(SwarmEvent::ConnectionClosed { peer_id, @@ -804,7 +844,11 @@ where if self.banned_peer_connections.contains(&id) { log::debug!("Ignoring event from banned peer: {} {:?}.", peer_id, id); } else { - self.behaviour.inject_event(peer_id, id, event); + self.behaviour.on_event(InEvent::ConnectionHandler { + peer_id, + connection: id, + event, + }); } } PoolEvent::AddressChange { @@ -814,6 +858,13 @@ where old_endpoint, } => { if !self.banned_peer_connections.contains(&id) { + self.behaviour.on_event(behaviour::InEvent::AddressChange { + peer_id, + connection_id: id, + old: &old_endpoint, + new: &new_endpoint, + }); + #[allow(deprecated)] self.behaviour.inject_address_change( &peer_id, &id, @@ -857,8 +908,11 @@ where }); } Err((connection_limit, handler)) => { - self.behaviour - .inject_listen_failure(&local_addr, &send_back_addr, handler); + self.behaviour.on_event(InEvent::ListenFailure { + local_addr: &local_addr, + send_back_addr: &send_back_addr, + handler, + }); log::warn!("Incoming connection rejected: {:?}", connection_limit); } }; @@ -872,6 +926,11 @@ where if !addrs.contains(&listen_addr) { addrs.push(listen_addr.clone()) } + self.behaviour.on_event(InEvent::NewListenAddr { + listener_id, + addr: &listen_addr, + }); + #[allow(deprecated)] self.behaviour .inject_new_listen_addr(listener_id, &listen_addr); return Some(SwarmEvent::NewListenAddr { @@ -891,6 +950,11 @@ where if let Some(addrs) = self.listened_addrs.get_mut(&listener_id) { addrs.retain(|a| a != &listen_addr); } + self.behaviour.on_event(InEvent::ExpiredListenAddr { + listener_id, + addr: &listen_addr, + }); + #[allow(deprecated)] self.behaviour .inject_expired_listen_addr(listener_id, &listen_addr); return Some(SwarmEvent::ExpiredListenAddr { @@ -905,8 +969,19 @@ where log::debug!("Listener {:?}; Closed by {:?}.", listener_id, reason); let addrs = self.listened_addrs.remove(&listener_id).unwrap_or_default(); for addr in addrs.iter() { + self.behaviour + .on_event(InEvent::ExpiredListenAddr { listener_id, addr }); + #[allow(deprecated)] self.behaviour.inject_expired_listen_addr(listener_id, addr); } + self.behaviour.on_event(InEvent::ListenerClosed { + listener_id, + reason: match &reason { + Ok(()) => Ok(()), + Err(err) => Err(err), + }, + }); + #[allow(deprecated)] self.behaviour.inject_listener_closed( listener_id, match &reason { @@ -921,6 +996,11 @@ where }); } TransportEvent::ListenerError { listener_id, error } => { + self.behaviour.on_event(InEvent::ListenerError { + listener_id, + err: &error, + }); + #[allow(deprecated)] self.behaviour.inject_listener_error(listener_id, &error); return Some(SwarmEvent::ListenerError { listener_id, error }); } @@ -1542,13 +1622,63 @@ impl NetworkBehaviour for DummyBehaviour { } } - fn inject_event( - &mut self, - _: PeerId, - _: ConnectionId, - event: ::OutEvent, - ) { - void::unreachable(event) + fn on_event(&mut self, event: InEvent) { + match event { + InEvent::ConnectionHandler { + peer_id: _, + connection: _, + event, + } => void::unreachable(event), + InEvent::ConnectionEstablished { + peer_id: _, + connection_id: _, + endpoint: _, + failed_addresses: _, + other_established: _, + } => {} + InEvent::ConnectionClosed { + peer_id: _, + connection_id: _, + endpoint: _, + handler: _, + remaining_established: _, + } => {} + InEvent::AddressChange { + peer_id: _, + connection_id: _, + old: _, + new: _, + } => {} + InEvent::DialFailure { + peer_id: _, + handler: _, + error: _, + } => {} + InEvent::ListenFailure { + local_addr: _, + send_back_addr: _, + handler: _, + } => {} + InEvent::NewListener { listener_id: _ } => {} + InEvent::NewListenAddr { + listener_id: _, + addr: _, + } => {} + InEvent::ExpiredListenAddr { + listener_id: _, + addr: _, + } => {} + InEvent::ListenerError { + listener_id: _, + err: _, + } => {} + InEvent::ListenerClosed { + listener_id: _, + reason: _, + } => {} + InEvent::NewExternalAddr { addr: _ } => {} + InEvent::ExpiredExternalAddr { addr: _ } => {} + } } fn poll( From de5c46dbe198c756c27de467196324084dee86b8 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Wed, 7 Sep 2022 15:11:36 +0900 Subject: [PATCH 2/7] swarm/behaviour: Delegate to on_event from inject_* --- swarm/src/behaviour.rs | 235 ++++++++++++++++++++++++++--------------- swarm/src/lib.rs | 121 ++++++--------------- 2 files changed, 179 insertions(+), 177 deletions(-) diff --git a/swarm/src/behaviour.rs b/swarm/src/behaviour.rs index 1465e2ee401..a8716eeaef9 100644 --- a/swarm/src/behaviour.rs +++ b/swarm/src/behaviour.rs @@ -159,146 +159,207 @@ pub trait NetworkBehaviour: 'static { /// Informs the behaviour about a newly established connection to a peer. #[deprecated( since = "0.39.0", - note = "Handle `InEvent::ConnectionEstablished` in `NetworkBehaviour::on_event` instead." + note = "Handle `InEvent::ConnectionEstablished` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." )] fn inject_connection_established( &mut self, - _peer_id: &PeerId, - _connection_id: &ConnectionId, - _endpoint: &ConnectedPoint, - _failed_addresses: Option<&Vec>, - _other_established: usize, + peer_id: &PeerId, + connection_id: &ConnectionId, + endpoint: &ConnectedPoint, + failed_addresses: Option<&Vec>, + other_established: usize, ) { + self.on_event(InEvent::ConnectionEstablished { + peer_id: *peer_id, + connection_id: *connection_id, + endpoint, + failed_addresses, + other_established, + }); } - // TODO: Have to remove this as it takes ownership of `Handler`. - // - // /// Informs the behaviour about a closed connection to a peer. - // /// - // /// A call to this method is always paired with an earlier call to - // /// [`NetworkBehaviour::inject_connection_established`] with the same peer ID, connection ID and endpoint. - // #[deprecated( - // since = "0.39.0", - // note = "Handle `InEvent::ConnectionClosed` in `NetworkBehaviour::on_event` instead." - // )] - // fn inject_connection_closed( - // &mut self, - // _: &PeerId, - // _: &ConnectionId, - // _: &ConnectedPoint, - // _: ::Handler, - // _remaining_established: usize, - // ) { - // } + /// Informs the behaviour about a closed connection to a peer. + /// + /// A call to this method is always paired with an earlier call to + /// [`NetworkBehaviour::inject_connection_established`] with the same peer ID, connection ID and endpoint. + #[deprecated( + since = "0.39.0", + note = "Handle `InEvent::ConnectionClosed` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." + )] + fn inject_connection_closed( + &mut self, + peer_id: &PeerId, + connection_id: &ConnectionId, + endpoint: &ConnectedPoint, + handler: ::Handler, + remaining_established: usize, + ) { + self.on_event(InEvent::ConnectionClosed { + peer_id: *peer_id, + connection_id: *connection_id, + endpoint, + handler, + remaining_established, + }); + } /// Informs the behaviour that the [`ConnectedPoint`] of an existing connection has changed. #[deprecated( since = "0.39.0", - note = "Handle `InEvent::AddressChange` in `NetworkBehaviour::on_event` instead." + note = "Handle `InEvent::AddressChange` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." )] fn inject_address_change( &mut self, - _: &PeerId, - _: &ConnectionId, - _old: &ConnectedPoint, - _new: &ConnectedPoint, + peer_id: &PeerId, + connection_id: &ConnectionId, + old: &ConnectedPoint, + new: &ConnectedPoint, ) { + self.on_event(InEvent::AddressChange { + peer_id: *peer_id, + connection_id: *connection_id, + old, + new, + }); } - // TODO: Can not deprecate as it takes ownership of handler event. - // - // /// Informs the behaviour about an event generated by the handler dedicated to the peer identified by `peer_id`. - // /// for the behaviour. - // /// - // /// The `peer_id` is guaranteed to be in a connected state. In other words, - // /// [`NetworkBehaviour::inject_connection_established`] has previously been called with this `PeerId`. - // #[deprecated( - // since = "0.39.0", - // note = "Handle `InEvent::ConnectionHandler` in `NetworkBehaviour::on_event` instead." - // )] - // fn inject_event( - // &mut self, - // peer_id: PeerId, - // connection: ConnectionId, - // event: <::Handler as ConnectionHandler>::OutEvent, - // ); - - // TODO: Can not deprecate as it takes ownership of handler. - // - // /// Indicates to the behaviour that the dial to a known or unknown node failed. - // fn inject_dial_failure( - // &mut self, - // _peer_id: Option, - // _handler: Self::ConnectionHandler, - // _error: &DialError, - // ) { - // } - - // TODO: Can not deprecate as it takes ownership of handler. - // - // /// 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::ConnectionHandler, - // ) { - // } + /// Informs the behaviour about an event generated by the handler dedicated to the peer identified by `peer_id`. + /// for the behaviour. + /// + /// The `peer_id` is guaranteed to be in a connected state. In other words, + /// [`NetworkBehaviour::inject_connection_established`] has previously been called with this `PeerId`. + #[deprecated( + since = "0.39.0", + note = "Handle `InEvent::ConnectionHandler` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." + )] + fn inject_event( + &mut self, + peer_id: PeerId, + connection: ConnectionId, + event: <::Handler as ConnectionHandler>::OutEvent, + ) { + self.on_event(InEvent::ConnectionHandler { + peer_id, + connection, + event, + }); + } + + /// Indicates to the behaviour that the dial to a known or unknown node failed. + #[deprecated( + since = "0.39.0", + note = "Handle `InEvent::DialFailure` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." + )] + fn inject_dial_failure( + &mut self, + peer_id: Option, + handler: Self::ConnectionHandler, + error: &DialError, + ) { + self.on_event(InEvent::DialFailure { + peer_id, + handler, + error, + }); + } + + /// 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. + #[deprecated( + since = "0.39.0", + note = "Handle `InEvent::ListenFailure` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." + )] + fn inject_listen_failure( + &mut self, + local_addr: &Multiaddr, + send_back_addr: &Multiaddr, + handler: Self::ConnectionHandler, + ) { + self.on_event(InEvent::ListenFailure { + local_addr, + send_back_addr, + handler, + }); + } /// Indicates to the behaviour that a new listener was created. #[deprecated( since = "0.39.0", - note = "Handle `InEvent::NewListener` in `NetworkBehaviour::on_event` instead." + note = "Handle `InEvent::NewListener` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." )] - fn inject_new_listener(&mut self, _id: ListenerId) {} + fn inject_new_listener(&mut self, id: ListenerId) { + self.on_event(InEvent::NewListener { listener_id: id }); + } /// Indicates to the behaviour that we have started listening on a new multiaddr. #[deprecated( since = "0.39.0", - note = "Handle `InEvent::NewListenAddr` in `NetworkBehaviour::on_event` instead." + note = "Handle `InEvent::NewListenAddr` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." )] - fn inject_new_listen_addr(&mut self, _id: ListenerId, _addr: &Multiaddr) {} + fn inject_new_listen_addr(&mut self, id: ListenerId, addr: &Multiaddr) { + self.on_event(InEvent::NewListenAddr { + listener_id: id, + addr, + }); + } /// Indicates to the behaviour that a multiaddr we were listening on has expired, /// which means that we are no longer listening on it. #[deprecated( since = "0.39.0", - note = "Handle `InEvent::ExpiredListenAddr` in `NetworkBehaviour::on_event` instead." + note = "Handle `InEvent::ExpiredListenAddr` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." )] - fn inject_expired_listen_addr(&mut self, _id: ListenerId, _addr: &Multiaddr) {} + fn inject_expired_listen_addr(&mut self, id: ListenerId, addr: &Multiaddr) { + self.on_event(InEvent::ExpiredListenAddr { + listener_id: id, + addr, + }); + } /// A listener experienced an error. #[deprecated( since = "0.39.0", - note = "Handle `InEvent::ListenerError` in `NetworkBehaviour::on_event` instead." + note = "Handle `InEvent::ListenerError` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." )] - fn inject_listener_error(&mut self, _id: ListenerId, _err: &(dyn std::error::Error + 'static)) { + fn inject_listener_error(&mut self, id: ListenerId, err: &(dyn std::error::Error + 'static)) { + self.on_event(InEvent::ListenerError { + listener_id: id, + err, + }); } /// A listener closed. #[deprecated( since = "0.39.0", - note = "Handle `InEvent::ListenerClosed` in `NetworkBehaviour::on_event` instead." + note = "Handle `InEvent::ListenerClosed` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." )] - fn inject_listener_closed(&mut self, _id: ListenerId, _reason: Result<(), &std::io::Error>) {} + fn inject_listener_closed(&mut self, id: ListenerId, reason: Result<(), &std::io::Error>) { + self.on_event(InEvent::ListenerClosed { + listener_id: id, + reason, + }); + } /// Indicates to the behaviour that we have discovered a new external address for us. #[deprecated( since = "0.39.0", - note = "Handle `InEvent::NewExternalAddr` in `NetworkBehaviour::on_event` instead." + note = "Handle `InEvent::NewExternalAddr` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." )] - fn inject_new_external_addr(&mut self, _addr: &Multiaddr) {} + fn inject_new_external_addr(&mut self, addr: &Multiaddr) { + self.on_event(InEvent::NewExternalAddr { addr }); + } /// Indicates to the behaviour that an external address was removed. #[deprecated( since = "0.39.0", - note = "Handle `InEvent::ExpiredExternalAddr` in `NetworkBehaviour::on_event` instead." + note = "Handle `InEvent::ExpiredExternalAddr` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." )] - fn inject_expired_external_addr(&mut self, _addr: &Multiaddr) {} + fn inject_expired_external_addr(&mut self, addr: &Multiaddr) { + self.on_event(InEvent::ExpiredExternalAddr { addr }); + } /// Polls for things that swarm should do. /// diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index ed2ecd5c6ce..00a26bf3c78 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -328,8 +328,6 @@ where /// Depending on the underlying transport, one listener may have multiple listening addresses. pub fn listen_on(&mut self, addr: Multiaddr) -> Result> { let id = self.transport.listen_on(addr)?; - self.behaviour - .on_event(InEvent::NewListener { listener_id: id }); #[allow(deprecated)] self.behaviour.inject_new_listener(id); Ok(id) @@ -407,11 +405,12 @@ where PeerCondition::Always => true, }; if !condition_matched { - self.behaviour.on_event(InEvent::DialFailure { - peer_id: Some(peer_id), + #[allow(deprecated)] + self.behaviour.inject_dial_failure( + Some(peer_id), handler, - error: &DialError::DialPeerConditionFalse(condition), - }); + &DialError::DialPeerConditionFalse(condition), + ); return Err(DialError::DialPeerConditionFalse(condition)); } @@ -419,11 +418,9 @@ where // Check if peer is banned. if self.banned_peers.contains(&peer_id) { let error = DialError::Banned; - self.behaviour.on_event(InEvent::DialFailure { - peer_id: Some(peer_id), - handler, - error: &error, - }); + #[allow(deprecated)] + self.behaviour + .inject_dial_failure(Some(peer_id), handler, &error); return Err(error); } @@ -459,11 +456,9 @@ where if addresses.is_empty() { let error = DialError::NoAddresses; - self.behaviour.on_event(InEvent::DialFailure { - peer_id: Some(peer_id), - handler, - error: &error, - }); + #[allow(deprecated)] + self.behaviour + .inject_dial_failure(Some(peer_id), handler, &error); return Err(error); }; @@ -545,11 +540,8 @@ where Ok(_connection_id) => Ok(()), Err((connection_limit, handler)) => { let error = DialError::ConnectionLimit(connection_limit); - self.behaviour.on_event(InEvent::DialFailure { - peer_id: None, - handler, - error: &error, - }); + #[allow(deprecated)] + self.behaviour.inject_dial_failure(None, handler, &error); Err(error) } } @@ -590,8 +582,6 @@ where let result = self.external_addrs.add(a.clone(), s); let expired = match &result { AddAddressResult::Inserted { expired } => { - self.behaviour - .on_event(InEvent::NewExternalAddr { addr: &a }); #[allow(deprecated)] self.behaviour.inject_new_external_addr(&a); expired @@ -599,8 +589,6 @@ where AddAddressResult::Updated { expired } => expired, }; for a in expired { - self.behaviour - .on_event(InEvent::ExpiredExternalAddr { addr: &a.addr }); #[allow(deprecated)] self.behaviour.inject_expired_external_addr(&a.addr); } @@ -615,8 +603,6 @@ where /// otherwise. pub fn remove_external_address(&mut self, addr: &Multiaddr) -> bool { if self.external_addrs.remove(addr) { - self.behaviour - .on_event(InEvent::ExpiredExternalAddr { addr }); #[allow(deprecated)] self.behaviour.inject_expired_external_addr(addr); true @@ -724,14 +710,6 @@ where let failed_addresses = concurrent_dial_errors .as_ref() .map(|es| es.iter().map(|(a, _)| a).cloned().collect()); - self.behaviour - .on_event(behaviour::InEvent::ConnectionEstablished { - peer_id, - connection_id: id, - endpoint: &endpoint, - failed_addresses: failed_addresses.as_ref(), - other_established: non_banned_established, - }); #[allow(deprecated)] self.behaviour.inject_connection_established( &peer_id, @@ -756,11 +734,8 @@ where } => { let error = error.into(); - self.behaviour.on_event(InEvent::DialFailure { - peer_id: peer, - handler, - error: &error, - }); + #[allow(deprecated)] + self.behaviour.inject_dial_failure(peer, handler, &error); if let Some(peer) = peer { log::debug!("Connection attempt to {:?} failed with {:?}.", peer, error,); @@ -781,11 +756,9 @@ where handler, } => { log::debug!("Incoming connection failed: {:?}", error); - self.behaviour.on_event(InEvent::ListenFailure { - local_addr: &local_addr, - send_back_addr: &send_back_addr, - handler, - }); + #[allow(deprecated)] + self.behaviour + .inject_listen_failure(&local_addr, &send_back_addr, handler); return Some(SwarmEvent::IncomingConnectionError { local_addr, send_back_addr, @@ -824,14 +797,14 @@ where .into_iter() .filter(|conn_id| !self.banned_peer_connections.contains(conn_id)) .count(); - self.behaviour - .on_event(behaviour::InEvent::ConnectionClosed { - peer_id, - connection_id: id, - endpoint: &endpoint, - handler, - remaining_established: remaining_non_banned, - }); + #[allow(deprecated)] + self.behaviour.inject_connection_closed( + &peer_id, + &id, + &endpoint, + handler, + remaining_non_banned, + ); } return Some(SwarmEvent::ConnectionClosed { peer_id, @@ -844,11 +817,8 @@ where if self.banned_peer_connections.contains(&id) { log::debug!("Ignoring event from banned peer: {} {:?}.", peer_id, id); } else { - self.behaviour.on_event(InEvent::ConnectionHandler { - peer_id, - connection: id, - event, - }); + #[allow(deprecated)] + self.behaviour.inject_event(peer_id, id, event); } } PoolEvent::AddressChange { @@ -858,12 +828,6 @@ where old_endpoint, } => { if !self.banned_peer_connections.contains(&id) { - self.behaviour.on_event(behaviour::InEvent::AddressChange { - peer_id, - connection_id: id, - old: &old_endpoint, - new: &new_endpoint, - }); #[allow(deprecated)] self.behaviour.inject_address_change( &peer_id, @@ -908,11 +872,9 @@ where }); } Err((connection_limit, handler)) => { - self.behaviour.on_event(InEvent::ListenFailure { - local_addr: &local_addr, - send_back_addr: &send_back_addr, - handler, - }); + #[allow(deprecated)] + self.behaviour + .inject_listen_failure(&local_addr, &send_back_addr, handler); log::warn!("Incoming connection rejected: {:?}", connection_limit); } }; @@ -926,10 +888,6 @@ where if !addrs.contains(&listen_addr) { addrs.push(listen_addr.clone()) } - self.behaviour.on_event(InEvent::NewListenAddr { - listener_id, - addr: &listen_addr, - }); #[allow(deprecated)] self.behaviour .inject_new_listen_addr(listener_id, &listen_addr); @@ -950,10 +908,6 @@ where if let Some(addrs) = self.listened_addrs.get_mut(&listener_id) { addrs.retain(|a| a != &listen_addr); } - self.behaviour.on_event(InEvent::ExpiredListenAddr { - listener_id, - addr: &listen_addr, - }); #[allow(deprecated)] self.behaviour .inject_expired_listen_addr(listener_id, &listen_addr); @@ -969,18 +923,9 @@ where log::debug!("Listener {:?}; Closed by {:?}.", listener_id, reason); let addrs = self.listened_addrs.remove(&listener_id).unwrap_or_default(); for addr in addrs.iter() { - self.behaviour - .on_event(InEvent::ExpiredListenAddr { listener_id, addr }); #[allow(deprecated)] self.behaviour.inject_expired_listen_addr(listener_id, addr); } - self.behaviour.on_event(InEvent::ListenerClosed { - listener_id, - reason: match &reason { - Ok(()) => Ok(()), - Err(err) => Err(err), - }, - }); #[allow(deprecated)] self.behaviour.inject_listener_closed( listener_id, @@ -996,10 +941,6 @@ where }); } TransportEvent::ListenerError { listener_id, error } => { - self.behaviour.on_event(InEvent::ListenerError { - listener_id, - err: &error, - }); #[allow(deprecated)] self.behaviour.inject_listener_error(listener_id, &error); return Some(SwarmEvent::ListenerError { listener_id, error }); From 021a7f1338fed87338fb27430502cd6eb1604fe4 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Wed, 7 Sep 2022 15:25:31 +0900 Subject: [PATCH 3/7] swarm/behaviour: Address code review --- swarm/src/behaviour.rs | 4 ++-- swarm/src/behaviour/either.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/swarm/src/behaviour.rs b/swarm/src/behaviour.rs index a8716eeaef9..2582d72ecf8 100644 --- a/swarm/src/behaviour.rs +++ b/swarm/src/behaviour.rs @@ -152,7 +152,7 @@ pub trait NetworkBehaviour: 'static { vec![] } - /// Informs the behaviour about an event from the [Transport](libp2p_core::Transport) or the + /// Informs the behaviour about an event from the [`Transport`](libp2p_core::Transport) or the /// [`ConnectionHandler`]. fn on_event(&mut self, _event: InEvent); @@ -851,7 +851,7 @@ pub enum InEvent<'a, Handler: IntoConnectionHandler> { }, /// Informs the behaviour about a closed connection to a peer. /// - /// This event is always paired with an earlier call to + /// This event is always paired with an earlier /// [`InEvent::ConnectionEstablished`] with the same peer ID, connection ID /// and endpoint. ConnectionClosed { diff --git a/swarm/src/behaviour/either.rs b/swarm/src/behaviour/either.rs index e9677ff419a..3daf5c13c18 100644 --- a/swarm/src/behaviour/either.rs +++ b/swarm/src/behaviour/either.rs @@ -18,8 +18,8 @@ // FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // DEALINGS IN THE SOFTWARE. +use crate::behaviour::{self, NetworkBehaviour, NetworkBehaviourAction, PollParameters}; use crate::handler::either::IntoEitherHandler; -use crate::{NetworkBehaviour, NetworkBehaviourAction, PollParameters}; use either::Either; use libp2p_core::{Multiaddr, PeerId}; use std::{task::Context, task::Poll}; @@ -47,7 +47,7 @@ where } } - fn on_event(&mut self, event: super::InEvent) { + fn on_event(&mut self, event: behaviour::InEvent) { match self { Either::Left(b) => b.on_event(event.map_handler( |h| h.unwrap_left(), From 07a4f201b78978cb56a6f24fd1426de5664447cc Mon Sep 17 00:00:00 2001 From: Max Inden Date: Sat, 17 Sep 2022 17:35:27 +0200 Subject: [PATCH 4/7] swarm/behaviour: Add on_connection_handler_event --- swarm/src/behaviour.rs | 114 ++++++++++++++-------------------- swarm/src/behaviour/either.rs | 25 ++++++-- swarm/src/behaviour/toggle.rs | 17 ++++- swarm/src/lib.rs | 16 +++-- 4 files changed, 91 insertions(+), 81 deletions(-) diff --git a/swarm/src/behaviour.rs b/swarm/src/behaviour.rs index 2582d72ecf8..b99e0603f9e 100644 --- a/swarm/src/behaviour.rs +++ b/swarm/src/behaviour.rs @@ -152,14 +152,29 @@ pub trait NetworkBehaviour: 'static { vec![] } - /// Informs the behaviour about an event from the [`Transport`](libp2p_core::Transport) or the - /// [`ConnectionHandler`]. - fn on_event(&mut self, _event: InEvent); + // TODO: Should we add an empty default implementation? That way this is not a breaking change. + /// Informs the behaviour about an event from the [`Swarm`](crate::Swarm). + fn on_swarm_event(&mut self, _event: InEvent); + + // TODO: Should we add an empty default implementation? That way this is not a breaking change. + /// Informs the behaviour about an event generated by the [`ConnectionHandler`] dedicated to the + /// peer identified by `peer_id`. for the behaviour. + /// + /// The [`PeerId`] is guaranteed to be in a connected state. In other words, + /// [`InEvent::ConnectionEstablished`] has previously been received with this [`PeerId`]. + fn on_connection_handler_event( + &mut self, + peer_id: PeerId, + connection_id: ConnectionId, + // TODO: Instead of the type alias THandlerOutEvent the full definition might be more + // intuitive for users outside of libp2p-swarm. + event: crate::THandlerOutEvent, + ); /// Informs the behaviour about a newly established connection to a peer. #[deprecated( since = "0.39.0", - note = "Handle `InEvent::ConnectionEstablished` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." + note = "Handle `InEvent::ConnectionEstablished` in `NetworkBehaviour::on_swarm_event` instead. The default implementation of this `inject_*` method delegates to it." )] fn inject_connection_established( &mut self, @@ -169,7 +184,7 @@ pub trait NetworkBehaviour: 'static { failed_addresses: Option<&Vec>, other_established: usize, ) { - self.on_event(InEvent::ConnectionEstablished { + self.on_swarm_event(InEvent::ConnectionEstablished { peer_id: *peer_id, connection_id: *connection_id, endpoint, @@ -184,7 +199,7 @@ pub trait NetworkBehaviour: 'static { /// [`NetworkBehaviour::inject_connection_established`] with the same peer ID, connection ID and endpoint. #[deprecated( since = "0.39.0", - note = "Handle `InEvent::ConnectionClosed` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." + note = "Handle `InEvent::ConnectionClosed` in `NetworkBehaviour::on_swarm_event` instead. The default implementation of this `inject_*` method delegates to it." )] fn inject_connection_closed( &mut self, @@ -194,7 +209,7 @@ pub trait NetworkBehaviour: 'static { handler: ::Handler, remaining_established: usize, ) { - self.on_event(InEvent::ConnectionClosed { + self.on_swarm_event(InEvent::ConnectionClosed { peer_id: *peer_id, connection_id: *connection_id, endpoint, @@ -206,7 +221,7 @@ pub trait NetworkBehaviour: 'static { /// Informs the behaviour that the [`ConnectedPoint`] of an existing connection has changed. #[deprecated( since = "0.39.0", - note = "Handle `InEvent::AddressChange` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." + note = "Handle `InEvent::AddressChange` in `NetworkBehaviour::on_swarm_event` instead. The default implementation of this `inject_*` method delegates to it." )] fn inject_address_change( &mut self, @@ -215,7 +230,7 @@ pub trait NetworkBehaviour: 'static { old: &ConnectedPoint, new: &ConnectedPoint, ) { - self.on_event(InEvent::AddressChange { + self.on_swarm_event(InEvent::AddressChange { peer_id: *peer_id, connection_id: *connection_id, old, @@ -230,7 +245,7 @@ pub trait NetworkBehaviour: 'static { /// [`NetworkBehaviour::inject_connection_established`] has previously been called with this `PeerId`. #[deprecated( since = "0.39.0", - note = "Handle `InEvent::ConnectionHandler` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." + note = "Implement `NetworkBehaviour::on_connection_handler_event` instead. The default implementation of this `inject_*` method delegates to it." )] fn inject_event( &mut self, @@ -238,17 +253,13 @@ pub trait NetworkBehaviour: 'static { connection: ConnectionId, event: <::Handler as ConnectionHandler>::OutEvent, ) { - self.on_event(InEvent::ConnectionHandler { - peer_id, - connection, - event, - }); + self.on_connection_handler_event(peer_id, connection, event); } /// Indicates to the behaviour that the dial to a known or unknown node failed. #[deprecated( since = "0.39.0", - note = "Handle `InEvent::DialFailure` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." + note = "Handle `InEvent::DialFailure` in `NetworkBehaviour::on_swarm_event` instead. The default implementation of this `inject_*` method delegates to it." )] fn inject_dial_failure( &mut self, @@ -256,7 +267,7 @@ pub trait NetworkBehaviour: 'static { handler: Self::ConnectionHandler, error: &DialError, ) { - self.on_event(InEvent::DialFailure { + self.on_swarm_event(InEvent::DialFailure { peer_id, handler, error, @@ -270,7 +281,7 @@ pub trait NetworkBehaviour: 'static { /// connection unexpectedly closed. #[deprecated( since = "0.39.0", - note = "Handle `InEvent::ListenFailure` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." + note = "Handle `InEvent::ListenFailure` in `NetworkBehaviour::on_swarm_event` instead. The default implementation of this `inject_*` method delegates to it." )] fn inject_listen_failure( &mut self, @@ -278,7 +289,7 @@ pub trait NetworkBehaviour: 'static { send_back_addr: &Multiaddr, handler: Self::ConnectionHandler, ) { - self.on_event(InEvent::ListenFailure { + self.on_swarm_event(InEvent::ListenFailure { local_addr, send_back_addr, handler, @@ -288,19 +299,19 @@ pub trait NetworkBehaviour: 'static { /// Indicates to the behaviour that a new listener was created. #[deprecated( since = "0.39.0", - note = "Handle `InEvent::NewListener` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." + note = "Handle `InEvent::NewListener` in `NetworkBehaviour::on_swarm_event` instead. The default implementation of this `inject_*` method delegates to it." )] fn inject_new_listener(&mut self, id: ListenerId) { - self.on_event(InEvent::NewListener { listener_id: id }); + self.on_swarm_event(InEvent::NewListener { listener_id: id }); } /// Indicates to the behaviour that we have started listening on a new multiaddr. #[deprecated( since = "0.39.0", - note = "Handle `InEvent::NewListenAddr` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." + note = "Handle `InEvent::NewListenAddr` in `NetworkBehaviour::on_swarm_event` instead. The default implementation of this `inject_*` method delegates to it." )] fn inject_new_listen_addr(&mut self, id: ListenerId, addr: &Multiaddr) { - self.on_event(InEvent::NewListenAddr { + self.on_swarm_event(InEvent::NewListenAddr { listener_id: id, addr, }); @@ -310,10 +321,10 @@ pub trait NetworkBehaviour: 'static { /// which means that we are no longer listening on it. #[deprecated( since = "0.39.0", - note = "Handle `InEvent::ExpiredListenAddr` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." + note = "Handle `InEvent::ExpiredListenAddr` in `NetworkBehaviour::on_swarm_event` instead. The default implementation of this `inject_*` method delegates to it." )] fn inject_expired_listen_addr(&mut self, id: ListenerId, addr: &Multiaddr) { - self.on_event(InEvent::ExpiredListenAddr { + self.on_swarm_event(InEvent::ExpiredListenAddr { listener_id: id, addr, }); @@ -322,10 +333,10 @@ pub trait NetworkBehaviour: 'static { /// A listener experienced an error. #[deprecated( since = "0.39.0", - note = "Handle `InEvent::ListenerError` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." + note = "Handle `InEvent::ListenerError` in `NetworkBehaviour::on_swarm_event` instead. The default implementation of this `inject_*` method delegates to it." )] fn inject_listener_error(&mut self, id: ListenerId, err: &(dyn std::error::Error + 'static)) { - self.on_event(InEvent::ListenerError { + self.on_swarm_event(InEvent::ListenerError { listener_id: id, err, }); @@ -334,10 +345,10 @@ pub trait NetworkBehaviour: 'static { /// A listener closed. #[deprecated( since = "0.39.0", - note = "Handle `InEvent::ListenerClosed` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." + note = "Handle `InEvent::ListenerClosed` in `NetworkBehaviour::on_swarm_event` instead. The default implementation of this `inject_*` method delegates to it." )] fn inject_listener_closed(&mut self, id: ListenerId, reason: Result<(), &std::io::Error>) { - self.on_event(InEvent::ListenerClosed { + self.on_swarm_event(InEvent::ListenerClosed { listener_id: id, reason, }); @@ -346,19 +357,19 @@ pub trait NetworkBehaviour: 'static { /// Indicates to the behaviour that we have discovered a new external address for us. #[deprecated( since = "0.39.0", - note = "Handle `InEvent::NewExternalAddr` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." + note = "Handle `InEvent::NewExternalAddr` in `NetworkBehaviour::on_swarm_event` instead. The default implementation of this `inject_*` method delegates to it." )] fn inject_new_external_addr(&mut self, addr: &Multiaddr) { - self.on_event(InEvent::NewExternalAddr { addr }); + self.on_swarm_event(InEvent::NewExternalAddr { addr }); } /// Indicates to the behaviour that an external address was removed. #[deprecated( since = "0.39.0", - note = "Handle `InEvent::ExpiredExternalAddr` in `NetworkBehaviour::on_event` instead. The default implementation of this `inject_*` method delegates to it." + note = "Handle `InEvent::ExpiredExternalAddr` in `NetworkBehaviour::on_swarm_event` instead. The default implementation of this `inject_*` method delegates to it." )] fn inject_expired_external_addr(&mut self, addr: &Multiaddr) { - self.on_event(InEvent::ExpiredExternalAddr { addr }); + self.on_swarm_event(InEvent::ExpiredExternalAddr { addr }); } /// Polls for things that swarm should do. @@ -839,6 +850,7 @@ impl Default for CloseConnection { } } +// TODO: Rename to FromSwarm pub enum InEvent<'a, Handler: IntoConnectionHandler> { /// Informs the behaviour about a newly established connection to a peer. ConnectionEstablished { @@ -869,16 +881,6 @@ pub enum InEvent<'a, Handler: IntoConnectionHandler> { old: &'a ConnectedPoint, new: &'a ConnectedPoint, }, - /// Informs the behaviour about an event generated by the handler dedicated to the peer - /// identified by `peer_id`. for the behaviour. - /// - /// The `peer_id` is guaranteed to be in a connected state. In other words, - /// [`InEvent::ConnectionEstablished`] has previously been received with this `PeerId`. - ConnectionHandler { - peer_id: PeerId, - connection: ConnectionId, - event: <::Handler as ConnectionHandler>::OutEvent, - }, /// Indicates to the behaviour that the dial to a known or unknown node failed. DialFailure { peer_id: Option, @@ -931,20 +933,12 @@ impl<'a, Handler: IntoConnectionHandler> InEvent<'a, Handler> { map_handler: impl FnOnce( ::Handler, ) -> ::Handler, - map_event: impl FnOnce( - <::Handler as ConnectionHandler>::OutEvent, - ) -> - <::Handler as ConnectionHandler>::OutEvent, ) -> InEvent<'a, NewHandler> where NewHandler: IntoConnectionHandler, { - self.try_map_handler( - |h| Some(map_into_handler(h)), - |h| Some(map_handler(h)), - |e| Some(map_event(e)), - ) - .expect("To return Some as all closures return Some.") + self.try_map_handler(|h| Some(map_into_handler(h)), |h| Some(map_handler(h))) + .expect("To return Some as all closures return Some.") } fn try_map_handler( @@ -953,11 +947,6 @@ impl<'a, Handler: IntoConnectionHandler> InEvent<'a, Handler> { map_handler: impl FnOnce( ::Handler, ) -> Option<::Handler>, - map_event: impl FnOnce( - <::Handler as ConnectionHandler>::OutEvent, - ) -> Option< - <::Handler as ConnectionHandler>::OutEvent, - >, ) -> Option> where NewHandler: IntoConnectionHandler, @@ -1000,15 +989,6 @@ impl<'a, Handler: IntoConnectionHandler> InEvent<'a, Handler> { old, new, }), - InEvent::ConnectionHandler { - peer_id, - connection, - event, - } => Some(InEvent::ConnectionHandler { - peer_id, - connection, - event: map_event(event)?, - }), InEvent::DialFailure { peer_id, handler, diff --git a/swarm/src/behaviour/either.rs b/swarm/src/behaviour/either.rs index 3daf5c13c18..59bc0f389e1 100644 --- a/swarm/src/behaviour/either.rs +++ b/swarm/src/behaviour/either.rs @@ -47,27 +47,42 @@ where } } - fn on_event(&mut self, event: behaviour::InEvent) { + fn on_swarm_event(&mut self, event: behaviour::InEvent) { match self { - Either::Left(b) => b.on_event(event.map_handler( + Either::Left(b) => b.on_swarm_event(event.map_handler( |h| h.unwrap_left(), |h| match h { Either::Left(h) => h, Either::Right(_) => unreachable!(), }, - |e| e.unwrap_left(), )), - Either::Right(b) => b.on_event(event.map_handler( + Either::Right(b) => b.on_swarm_event(event.map_handler( |h| h.unwrap_right(), |h| match h { Either::Right(h) => h, Either::Left(_) => unreachable!(), }, - |e| e.unwrap_right(), )), } } + fn on_connection_handler_event( + &mut self, + peer_id: PeerId, + connection_id: libp2p_core::connection::ConnectionId, + event: crate::THandlerOutEvent, + ) { + match (self, event) { + (Either::Left(left), Either::Left(event)) => { + left.on_connection_handler_event(peer_id, connection_id, event) + } + (Either::Right(right), Either::Right(event)) => { + right.on_connection_handler_event(peer_id, connection_id, event) + } + _ => unreachable!(), + } + } + fn poll( &mut self, cx: &mut Context<'_>, diff --git a/swarm/src/behaviour/toggle.rs b/swarm/src/behaviour/toggle.rs index 6561876e555..bdc245eeadc 100644 --- a/swarm/src/behaviour/toggle.rs +++ b/swarm/src/behaviour/toggle.rs @@ -82,14 +82,25 @@ where .unwrap_or_else(Vec::new) } - fn on_event(&mut self, event: super::InEvent) { + fn on_swarm_event(&mut self, event: super::InEvent) { if let Some(behaviour) = &mut self.inner { - if let Some(event) = event.try_map_handler(|h| h.inner, |h| h.inner, |e| Some(e)) { - behaviour.on_event(event); + if let Some(event) = event.try_map_handler(|h| h.inner, |h| h.inner) { + behaviour.on_swarm_event(event); } } } + fn on_connection_handler_event( + &mut self, + peer_id: PeerId, + connection_id: libp2p_core::connection::ConnectionId, + event: crate::THandlerOutEvent, + ) { + if let Some(behaviour) = &mut self.inner { + behaviour.on_connection_handler_event(peer_id, connection_id, event) + } + } + fn poll( &mut self, cx: &mut Context<'_>, diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index 14799100ba4..b37715442ba 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -1563,13 +1563,8 @@ impl NetworkBehaviour for DummyBehaviour { } } - fn on_event(&mut self, event: InEvent) { + fn on_swarm_event(&mut self, event: InEvent) { match event { - InEvent::ConnectionHandler { - peer_id: _, - connection: _, - event, - } => void::unreachable(event), InEvent::ConnectionEstablished { peer_id: _, connection_id: _, @@ -1622,6 +1617,15 @@ impl NetworkBehaviour for DummyBehaviour { } } + fn on_connection_handler_event( + &mut self, + _peer_id: PeerId, + _connection_id: ConnectionId, + event: crate::THandlerOutEvent, + ) { + void::unreachable(event) + } + fn poll( &mut self, _: &mut Context<'_>, From 99a4b64f2a4c1715aa5da641c314955bd277e077 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Sat, 17 Sep 2022 16:53:38 +0100 Subject: [PATCH 5/7] swarm/behaviour: Use slice --- swarm/src/behaviour.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/swarm/src/behaviour.rs b/swarm/src/behaviour.rs index b99e0603f9e..e9735d204fe 100644 --- a/swarm/src/behaviour.rs +++ b/swarm/src/behaviour.rs @@ -188,7 +188,9 @@ pub trait NetworkBehaviour: 'static { peer_id: *peer_id, connection_id: *connection_id, endpoint, - failed_addresses, + failed_addresses: failed_addresses + .map(|v| v.as_slice()) + .unwrap_or_else(|| &[]), other_established, }); } @@ -857,8 +859,7 @@ pub enum InEvent<'a, Handler: IntoConnectionHandler> { peer_id: PeerId, connection_id: ConnectionId, endpoint: &'a ConnectedPoint, - // TODO: Would a slice not be better? - failed_addresses: Option<&'a Vec>, + failed_addresses: &'a [Multiaddr], other_established: usize, }, /// Informs the behaviour about a closed connection to a peer. From 46f20fab508c916eb02a50a84260ad427cd817e1 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Sat, 17 Sep 2022 17:55:52 +0200 Subject: [PATCH 6/7] swarm/behaviour: Rename to maybe_map_handler --- swarm/src/behaviour.rs | 4 ++-- swarm/src/behaviour/toggle.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/swarm/src/behaviour.rs b/swarm/src/behaviour.rs index e9735d204fe..d682ed1ead4 100644 --- a/swarm/src/behaviour.rs +++ b/swarm/src/behaviour.rs @@ -938,11 +938,11 @@ impl<'a, Handler: IntoConnectionHandler> InEvent<'a, Handler> { where NewHandler: IntoConnectionHandler, { - self.try_map_handler(|h| Some(map_into_handler(h)), |h| Some(map_handler(h))) + self.maybe_map_handler(|h| Some(map_into_handler(h)), |h| Some(map_handler(h))) .expect("To return Some as all closures return Some.") } - fn try_map_handler( + fn maybe_map_handler( self, map_into_handler: impl FnOnce(Handler) -> Option, map_handler: impl FnOnce( diff --git a/swarm/src/behaviour/toggle.rs b/swarm/src/behaviour/toggle.rs index bdc245eeadc..ea5a2d736ed 100644 --- a/swarm/src/behaviour/toggle.rs +++ b/swarm/src/behaviour/toggle.rs @@ -84,7 +84,7 @@ where fn on_swarm_event(&mut self, event: super::InEvent) { if let Some(behaviour) = &mut self.inner { - if let Some(event) = event.try_map_handler(|h| h.inner, |h| h.inner) { + if let Some(event) = event.maybe_map_handler(|h| h.inner, |h| h.inner) { behaviour.on_swarm_event(event); } } From 63b70fe60d723ce75510dc0cfea81302b92a7f58 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Sat, 17 Sep 2022 17:58:02 +0200 Subject: [PATCH 7/7] swarm/behaviour: Provide default for on_swarm and on_connection This allows rolling out the whole patch set in a non-breaking way. --- swarm/src/behaviour.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/swarm/src/behaviour.rs b/swarm/src/behaviour.rs index d682ed1ead4..887d1a11231 100644 --- a/swarm/src/behaviour.rs +++ b/swarm/src/behaviour.rs @@ -152,11 +152,9 @@ pub trait NetworkBehaviour: 'static { vec![] } - // TODO: Should we add an empty default implementation? That way this is not a breaking change. /// Informs the behaviour about an event from the [`Swarm`](crate::Swarm). - fn on_swarm_event(&mut self, _event: InEvent); + fn on_swarm_event(&mut self, _event: InEvent) {} - // TODO: Should we add an empty default implementation? That way this is not a breaking change. /// Informs the behaviour about an event generated by the [`ConnectionHandler`] dedicated to the /// peer identified by `peer_id`. for the behaviour. /// @@ -164,12 +162,13 @@ pub trait NetworkBehaviour: 'static { /// [`InEvent::ConnectionEstablished`] has previously been received with this [`PeerId`]. fn on_connection_handler_event( &mut self, - peer_id: PeerId, - connection_id: ConnectionId, + _peer_id: PeerId, + _connection_id: ConnectionId, // TODO: Instead of the type alias THandlerOutEvent the full definition might be more // intuitive for users outside of libp2p-swarm. - event: crate::THandlerOutEvent, - ); + _event: crate::THandlerOutEvent, + ) { + } /// Informs the behaviour about a newly established connection to a peer. #[deprecated(