feat: move Identify I/O from NetworkBehaviour to ConnectionHandler

protocols/identify/src/behaviour.rs

@@ -18,24 +18,21 @@
 // FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
 // DEALINGS IN THE SOFTWARE.
 
-use crate::handler::{self, Proto, Push};
-use crate::protocol::{Info, ReplySubstream, UpgradeError};
-use futures::prelude::*;
+use crate::handler::{self, BehaviourInfo, InEvent, Proto};
+use crate::protocol::{Info, UpgradeError};
 use libp2p_core::{
     connection::ConnectionId, multiaddr::Protocol, ConnectedPoint, Multiaddr, PeerId, PublicKey,
 };
 use libp2p_swarm::behaviour::{ConnectionClosed, ConnectionEstablished, DialFailure, FromSwarm};
 use libp2p_swarm::{
     dial_opts::DialOpts, AddressScore, ConnectionHandler, ConnectionHandlerUpgrErr, DialError,
-    IntoConnectionHandler, NegotiatedSubstream, NetworkBehaviour, NetworkBehaviourAction,
-    NotifyHandler, PollParameters,
+    IntoConnectionHandler, NetworkBehaviour, NetworkBehaviourAction, NotifyHandler, PollParameters,
 };
 use lru::LruCache;
 use std::num::NonZeroUsize;
 use std::{
     collections::{HashMap, HashSet, VecDeque},
     iter::FromIterator,
-    pin::Pin,
     task::Context,
     task::Poll,
     time::Duration,
@@ -51,8 +48,8 @@ pub struct Behaviour {
     config: Config,
     /// For each peer we're connected to, the observed address to send back to it.
     connected: HashMap<PeerId, HashMap<ConnectionId, Multiaddr>>,
-    /// Pending replies to send.
-    pending_replies: VecDeque<Reply>,
+    /// Information requests from the handlers to be fullfiled.
+    requests: VecDeque<Request>,
     /// Pending events to be emitted when polled.
     events: VecDeque<NetworkBehaviourAction<Event, Proto>>,
     /// Peers to which an active push with current information about
@@ -62,19 +59,10 @@ pub struct Behaviour {
     discovered_peers: PeerCache,
 }
 
-/// A pending reply to an inbound identification request.
-enum Reply {
-    /// The reply is queued for sending.
-    Queued {
-        peer: PeerId,
-        io: ReplySubstream<NegotiatedSubstream>,
-        observed: Multiaddr,
-    },
-    /// The reply is being sent.
-    Sending {
-        peer: PeerId,
-        io: Pin<Box<dyn Future<Output = Result<(), UpgradeError>> + Send>>,
-    },
+/// A `Handler` request for `BehaviourInfo`.
+struct Request {
+    peer_id: PeerId,
+    connection_id: ConnectionId,
 }
 
 /// Configuration for the [`identify::Behaviour`](Behaviour).
@@ -184,7 +172,7 @@ impl Behaviour {
         Self {
             config,
             connected: HashMap::new(),
-            pending_replies: VecDeque::new(),
+            requests: VecDeque::new(),
             events: VecDeque::new(),
             pending_push: HashSet::new(),
             discovered_peers,
@@ -240,13 +228,19 @@ impl NetworkBehaviour for Behaviour {
     type OutEvent = Event;
 
     fn new_handler(&mut self) -> Self::ConnectionHandler {
-        Proto::new(self.config.initial_delay, self.config.interval)
+        Proto::new(
+            self.config.initial_delay,
+            self.config.interval,
+            self.config.local_public_key.clone(),
+            self.config.protocol_version.clone(),
+            self.config.agent_version.clone(),
+        )
     }
 
     fn on_connection_handler_event(
         &mut self,
         peer_id: PeerId,
-        connection: ConnectionId,
+        connection_id: ConnectionId,
         event: <<Self::ConnectionHandler as IntoConnectionHandler>::Handler as ConnectionHandler>::OutEvent,
     ) {
         match event {
@@ -271,26 +265,22 @@ impl NetworkBehaviour for Behaviour {
                         score: AddressScore::Finite(1),
                     });
             }
+            handler::Event::Identification(peer) => {
+                self.events
+                    .push_back(NetworkBehaviourAction::GenerateEvent(Event::Sent {
+                        peer_id: peer,
+                    }));
+            }
             handler::Event::IdentificationPushed => {
                 self.events
                     .push_back(NetworkBehaviourAction::GenerateEvent(Event::Pushed {
                         peer_id,
                     }));
             }
-            handler::Event::Identify(sender) => {
-                let observed = self
-                    .connected
-                    .get(&peer_id)
-                    .and_then(|addrs| addrs.get(&connection))
-                    .expect(
-                        "`on_connection_handler_event` is only called \
-                        with an established connection and calling `NetworkBehaviour::on_event` \
-                        with `FromSwarm::ConnectionEstablished ensures there is an entry; qed",
-                    );
-                self.pending_replies.push_back(Reply::Queued {
-                    peer: peer_id,
-                    io: sender,
-                    observed: observed.clone(),
+            handler::Event::Identify => {
+                self.requests.push_back(Request {
+                    peer_id,
+                    connection_id,
                 });
             }
             handler::Event::IdentificationError(error) => {
@@ -305,7 +295,7 @@ impl NetworkBehaviour for Behaviour {
 
     fn poll(
         &mut self,
-        cx: &mut Context<'_>,
+        _cx: &mut Context<'_>,
         params: &mut impl PollParameters,
     ) -> Poll<NetworkBehaviourAction<Self::OutEvent, Self::ConnectionHandler>> {
         if let Some(event) = self.events.pop_front() {
@@ -333,7 +323,7 @@ impl NetworkBehaviour for Behaviour {
                     observed_addr,
                 };
 
-                (*peer, Push(info))
+                (*peer, InEvent::Push(info))
             })
         });
 
@@ -346,55 +336,21 @@ impl NetworkBehaviour for Behaviour {
             });
         }
 
-        // Check for pending replies to send.
-        if let Some(r) = self.pending_replies.pop_front() {
-            let mut sending = 0;
-            let to_send = self.pending_replies.len() + 1;
-            let mut reply = Some(r);
-            loop {
-                match reply {
-                    Some(Reply::Queued { peer, io, observed }) => {
-                        let info = Info {
-                            listen_addrs: listen_addrs(params),
-                            protocols: supported_protocols(params),
-                            public_key: self.config.local_public_key.clone(),
-                            protocol_version: self.config.protocol_version.clone(),
-                            agent_version: self.config.agent_version.clone(),
-                            observed_addr: observed,
-                        };
-                        let io = Box::pin(io.send(info));
-                        reply = Some(Reply::Sending { peer, io });
-                    }
-                    Some(Reply::Sending { peer, mut io }) => {
-                        sending += 1;
-                        match Future::poll(Pin::new(&mut io), cx) {
-                            Poll::Ready(Ok(())) => {
-                                let event = Event::Sent { peer_id: peer };
-                                return Poll::Ready(NetworkBehaviourAction::GenerateEvent(event));
-                            }
-                            Poll::Pending => {
-                                self.pending_replies.push_back(Reply::Sending { peer, io });
-                                if sending == to_send {
-                                    // All remaining futures are NotReady
-                                    break;
-                                } else {
-                                    reply = self.pending_replies.pop_front();
-                                }
-                            }
-                            Poll::Ready(Err(err)) => {
-                                let event = Event::Error {
-                                    peer_id: peer,
-                                    error: ConnectionHandlerUpgrErr::Upgrade(
-                                        libp2p_core::upgrade::UpgradeError::Apply(err),
-                                    ),
-                                };
-                                return Poll::Ready(NetworkBehaviourAction::GenerateEvent(event));
-                            }
-                        }
-                    }
-                    None => unreachable!(),
-                }
-            }
+        // Check for information requests from the handlers.
+        if let Some(Request {
+            peer_id,
+            connection_id,
+        }) = self.requests.pop_front()
+        {
+            let info = BehaviourInfo {
+                listen_addrs: listen_addrs(params),
+                protocols: supported_protocols(params),
+            };
+            return Poll::Ready(NetworkBehaviourAction::NotifyHandler {
+                peer_id,
+                handler: NotifyHandler::One(connection_id),
+                event: InEvent::Identify(info),
+            });
         }
 
         Poll::Pending
@@ -557,6 +513,7 @@ impl PeerCache {
 mod tests {
     use super::*;
     use futures::pin_mut;
+    use futures::prelude::*;
     use libp2p::mplex::MplexConfig;
     use libp2p::noise;
     use libp2p::tcp;
@@ -618,7 +575,7 @@ mod tests {
 
         // nb. Either swarm may receive the `Identified` event first, upon which
         // it will permit the connection to be closed, as defined by
-        // `IdentifyHandler::connection_keep_alive`. Hence the test succeeds if
+        // `Handler::connection_keep_alive`. Hence the test succeeds if
         // either `Identified` event arrives correctly.
         async_std::task::block_on(async move {
             loop {