Some improvements to the docs of NetworkBehaviour (#1152)

* Some improvements to the docs of NetworkBehaviour

* Apply suggestions from code review

Co-Authored-By: Roman Borschel <romanb@users.noreply.github.com>

core/src/swarm/behaviour.rs

@@ -32,36 +32,65 @@ use std::error;
 /// This trait has been designed to be composable. Multiple implementations can be combined into
 /// one that handles all the behaviours at once.
 pub trait NetworkBehaviour {
-    /// Handler for all the protocols the network supports.
+    /// Handler for all the protocols the network behaviour supports.
     type ProtocolsHandler: IntoProtocolsHandler;
-    /// Event generated by the swarm.
+
+    /// Event generated by the `NetworkBehaviour` and that the swarm will report back.
     type OutEvent;
 
     /// 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.
+    ///
+    /// 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_node_event`, and
+    /// the behaviour can send a message to the handler by making `poll` return `SendEvent`.
     fn new_handler(&mut self) -> Self::ProtocolsHandler;
 
     /// Addresses that this behaviour is aware of for this specific peer, and that may allow
     /// reaching the peer.
+    ///
+    /// The addresses will be tried in the order returned by this function, which means that they
+    /// should be ordered by decreasing likelihood of reachability. In other words, the first
+    /// address should be the most likely to be reachable.
     fn addresses_of_peer(&mut self, peer_id: &PeerId) -> Vec<Multiaddr>;
 
     /// Indicates the behaviour that we connected to the node with the given peer id through the
     /// given endpoint.
+    ///
+    /// This node now has a handler (as spawned by `new_handler`) running in the background.
     fn inject_connected(&mut self, peer_id: PeerId, endpoint: ConnectedPoint);
 
     /// Indicates the behaviour that we disconnected from the node with the given peer id. The
     /// endpoint is the one we used to be connected to.
+    ///
+    /// There is no handler running anymore for this node. Any event that has been sent to it may
+    /// or may not have been processed by the handler.
     fn inject_disconnected(&mut self, peer_id: &PeerId, endpoint: ConnectedPoint);
 
     /// Indicates the behaviour that we replace the connection from the node with another.
+    ///
+    /// The handler that used to be dedicated to this node has been destroyed and replaced with a
+    /// new one. Any event that has been sent to it may or may not have been processed.
+    ///
+    /// The default implementation of this method calls `inject_disconnected` followed with
+    /// `inject_connected`. This is a logically safe way to implement this behaviour. However, you
+    /// may want to overwrite this method in the situations where this isn't appropriate.
     fn inject_replaced(&mut self, peer_id: PeerId, closed_endpoint: ConnectedPoint, new_endpoint: ConnectedPoint) {
         self.inject_disconnected(&peer_id, closed_endpoint);
         self.inject_connected(peer_id, new_endpoint);
     }
 
-    /// Indicates the behaviour that the node with the given peer id has generated an event for
+    /// Informs the behaviour about an event generated by the handler dedicated to the peer identified by `peer_id`.
-    /// us.
+    /// for the behaviour.
     ///
-    /// > **Note**: This method is only called for events generated by the protocols handler.
+    /// The `peer_id` is guaranteed to be in a connected state. In other words, `inject_connected`
+    /// has previously been called with this `PeerId`.
     fn inject_node_event(
         &mut self,
         peer_id: PeerId,
@@ -77,6 +106,9 @@ pub trait NetworkBehaviour {
 
     /// Indicates to the behaviour that we tried to dial all the addresses known for a node, but
     /// failed.
+    ///
+    /// 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) {
     }
 
@@ -95,8 +127,10 @@ pub trait NetworkBehaviour {
 
     /// Polls for things that swarm should do.
     ///
-    /// This API mimics the API of the `Stream` trait.
+    /// This API mimics the API of the `Stream` trait. The method may register the current task in
-    fn poll(&mut self, params: &mut PollParameters<'_>) -> Async<NetworkBehaviourAction<<<Self::ProtocolsHandler as IntoProtocolsHandler>::Handler as ProtocolsHandler>::InEvent, Self::OutEvent>>;
+    /// order to wake it up at a later point in time.
+    fn poll(&mut self, params: &mut PollParameters<'_>)
+        -> Async<NetworkBehaviourAction<<<Self::ProtocolsHandler as IntoProtocolsHandler>::Handler as ProtocolsHandler>::InEvent, Self::OutEvent>>;
 }
 
 /// Used when deriving `NetworkBehaviour`. When deriving `NetworkBehaviour`, must be implemented
@@ -115,9 +149,8 @@ pub enum NetworkBehaviourAction<TInEvent, TOutEvent> {
     /// Instructs the `Swarm` to return an event when it is being polled.
     GenerateEvent(TOutEvent),
 
-    // TODO: report new raw connection for usage after intercepting an address dial
+    /// Instructs the swarm to dial the given multiaddress, with no knowledge of the `PeerId` that
-
+    /// may be reached.
-    /// Instructs the swarm to dial the given multiaddress, without a known `PeerId`.
     DialAddress {
         /// The address to dial.
         address: Multiaddr,
@@ -125,6 +158,11 @@ pub enum NetworkBehaviourAction<TInEvent, TOutEvent> {
 
     /// Instructs the swarm to dial a known `PeerId`.
     ///
+    /// The `addresses_of_peer` method is called to determine which addresses to attempt to reach.
+    ///
+    /// 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 failure, [`NetworkBehaviour::inject_dial_failure`] is invoked.
     DialPeer {
@@ -132,11 +170,14 @@ pub enum NetworkBehaviourAction<TInEvent, TOutEvent> {
         peer_id: PeerId,
     },
 
-    /// Instructs the `Swarm` to send a message to a connected peer.
+    /// Instructs the `Swarm` to send a message to the handler dedicated to the connection with the peer.
     ///
     /// If the `Swarm` is connected to the peer, the message is delivered to the remote's
     /// protocol handler. If there is no connection to the peer, the message is ignored.
     /// To ensure delivery, the `NetworkBehaviour` must keep track of connected peers.
+    ///
+    /// Note that even if the peer is currently connected, connections can get closed
+    /// at any time and thus the message may not reach its destination.
     SendEvent {
         /// The peer to which to send the message.
         peer_id: PeerId,