Skip to content

swarm/behaviour: Inject events via single method #2832

Description

@mxinden

Description

Replace the various NetworkBehaviour::inject_xxx methods with a single NetworkBehaviour::on_event method and a InEvent enum.

Example replacing NetworkBehaviour::inject_connection_established:

diff --git a/swarm/src/behaviour.rs b/swarm/src/behaviour.rs
index c0ac5976..c10adb7d 100644
--- a/swarm/src/behaviour.rs
+++ b/swarm/src/behaviour.rs
@@ -191,7 +191,13 @@ pub trait NetworkBehaviour: 'static {
         vec![]
     }
 
+    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,
@@ -778,3 +784,15 @@ impl Default for CloseConnection {
         CloseConnection::All
     }
 }
+
+pub enum InEvent<'a> {
+    /// 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<Multiaddr>>,
+        other_established: usize,
+    },
+}
diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs
index 38df855c..dfefb050 100644
--- a/swarm/src/lib.rs
+++ b/swarm/src/lib.rs
@@ -710,6 +710,14 @@ where
                         failed_addresses.as_ref(),
                         non_banned_established,
                     );
+                    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,
+                        });
                     return Some(SwarmEvent::ConnectionEstablished {
                         peer_id,
                         num_established,

Motivation

  • Decision whether to ignore an event (e.g. the event of a new connection being established) is made at the NetworkBehaviour trait definition by providing a default implementation or not. I argue this decision should be made by the user not by libp2p, i.e. I think users should decide whether to exhaustively match on all event handlers, or ignore most through a wildcard match arm. This is especially relevant when introducing new event handlers in the future.
  • The amount of event handler methods is becoming too long, especially with swarm/: NetworkBehaviour/ConnectionHandler cross-communication #2680 and swarm/: Support generic connection management through NetworkBehaviour #2828
  • I find a single event handler multiplexing events through an enum more intuitive than many inject_xxx methods.
  • Having a single on_event method helps differentiate the event handling code in NetworkBehaviour from the non event handling code (new_handler, addresses_of_peer and poll).

What do folks think? Would you be in favor of this change?

Requirements

Open questions

Are you planning to do it yourself in a pull request?

Yes, unless someone else would like to.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions