Some cleanup.

Author: Roman S. Borschel

client/network/src/protocol/generic_proto/behaviour.rs

@@ -74,13 +74,14 @@ use wasm_timer::Instant;
 /// manager. In other words, the peerset manager doesn't differentiate whether we are dialing a
 /// node or connected to it.
 ///
-/// For each peer, one connection is designated as the "control connection". If there are
-/// multiple connections to a peer, which may currently be at most two and only as a result
-/// of "simultaneous" dialing, the enabled/disabled status of all secondary connections
-/// follows the status of the control connection and hence maintain a unified status that
+/// For each peer, one connection is designated as the "primary connection". If there are
+/// multiple connections to a peer, which may currently practically be at most two and only
+/// as a result of "simultaneous" dialing, the enabled/disabled status of all secondary connections
+/// follows the status of the primary connection and hence maintain a unified status that
 /// is in sync with the peerset manager. Furtheremore, if a secondary connection fails
 /// due to an error, it has no impact on the current state of the peer on the external
-/// API.
+/// API. Secondary connections can thus serve traffic but do not affect the status of
+/// a peer w.r.t. this behaviour and the associated peerset manager.
 ///
 /// Additionally, there also exists a "banning" system. If we fail to dial a node, we "ban" it for
 /// a few seconds. If the PSM requests a node that is in the "banned" state, then we delay the
@@ -144,7 +145,7 @@ enum PeerState {
 	///
 	/// We may still have ongoing requests to that peer, but it should cease shortly.
 	Disabled {
-		/// The designated control connection.
+		/// The designated primary (usually the only) connection.
 		connection: Option<ConnectionId>,
 		/// How we are connected to this peer.
 		connected_point: ConnectedPoint,
@@ -160,7 +161,7 @@ enum PeerState {
 	/// will be enabled when `timer` fires. This peer can still perform Kademlia queries and such,
 	/// but should get disconnected in a few seconds.
 	DisabledPendingEnable {
-		/// The designated control connection.
+		/// The designated primary (usually the only) connection.
 		connection: Option<ConnectionId>,
 		/// How we are connected to this peer.
 		connected_point: ConnectedPoint,
@@ -177,7 +178,7 @@ enum PeerState {
 	/// We are connected to this peer and the peerset has accepted it. The handler is in the
 	/// enabled state.
 	Enabled {
-		/// The designated control connection.
+		/// The designated primary (usually the only) connection.
 		connection: Option<ConnectionId>,
 		/// How we are connected to this peer.
 		connected_point: ConnectedPoint,
@@ -189,7 +190,7 @@ enum PeerState {
 	/// is in initialization mode. We are waiting for the Accept or Reject from the peerset. There
 	/// is a corresponding entry in `incoming`.
 	Incoming {
-		/// The designated control connection.
+		/// The designated primary (usually the only) connection.
 		connection: Option<ConnectionId>,
 		/// How we are connected to this peer.
 		connected_point: ConnectedPoint,
@@ -211,6 +212,8 @@ impl PeerState {
 		}
 	}
 
+	/// The ID designated primary connection, if one is assigned
+	/// w.r.t. the current state of the peer.
 	fn connection(&self) -> &Option<ConnectionId> {
 		match self {
 			PeerState::Incoming { ref connection, .. } |
@@ -988,7 +991,7 @@ impl NetworkBehaviour for GenericProto {
 	fn inject_event(
 		&mut self,
 		source: PeerId,
-		_connection: ConnectionId,
+		connection: ConnectionId,
 		event: NotifsHandlerOut,
 	) {
 		match event {
@@ -1003,16 +1006,15 @@ impl NetworkBehaviour for GenericProto {
 				};
 
 				match entry.get_mut() {
-					PeerState::Enabled { connected_point, connection, .. } |
-					PeerState::Disabled { connected_point, connection, .. } |
-					PeerState::DisabledPendingEnable { connected_point, connection, .. } |
-					PeerState::Incoming { connected_point, connection, .. }
+					PeerState::Enabled { connected_point, connection: primary, .. } |
+					PeerState::Disabled { connected_point, connection: primary, .. } |
+					PeerState::DisabledPendingEnable { connected_point, connection: primary, .. } |
+					PeerState::Incoming { connected_point, connection: primary, .. }
 						if connected_point == &endpoint =>
 					{
-						// We take that handler as the handler of our control connection
+						// We take that handler as the one of our primary connection
 						// for this peer tracked in the `PeerState`.
-						*connection = connection.or(Some(_connection));
-						debug!(target: "sub-libp2p", "Control connection for {:?} set.", source);
+						*primary = primary.or(Some(connection));
 					}
 					_ => {}
 				}
@@ -1028,7 +1030,7 @@ impl NetworkBehaviour for GenericProto {
 				if let Some(event) = event {
 					self.events.push(NetworkBehaviourAction::NotifyHandler {
 						peer_id: source,
-						handler: NotifyHandler::One(_connection),
+						handler: NotifyHandler::One(connection),
 						event
 					});
 				}
@@ -1043,8 +1045,9 @@ impl NetworkBehaviour for GenericProto {
 					return
 				};
 
-				// Return early if this is not the connection we are tracking.
-				if entry.get().connection() != &Some(_connection) {
+				// Return early if this is not the primary connection w.r.t. the
+				// `PeerState` and PSM.
+				if entry.get().connection() != &Some(connection) {
 				    debug!(target: "sub-libp2p", "Secondary connection closed by {:?}", reason);
 				    return
 				}
@@ -1110,16 +1113,23 @@ impl NetworkBehaviour for GenericProto {
 			NotifsHandlerOut::Open => {
 				debug!(target: "sub-libp2p", "Handler({:?}) => Open", source);
 				let endpoint = match self.peers.get_mut(&source) {
-					Some(PeerState::Enabled { ref mut open, ref connected_point, connection }) |
-					Some(PeerState::DisabledPendingEnable { ref mut open, ref connected_point, connection, .. }) |
-					Some(PeerState::Disabled { ref mut open, ref connected_point, connection, .. }) => {
-						if connection != &Some(_connection) {
-							// It is a secondary connection.
-							debug!(target: "sub-libp2p", "Secondary connection opened custom protocol.");
+					Some(PeerState::Enabled {
+						ref mut open, connected_point, connection: primary
+					}) |
+					Some(PeerState::DisabledPendingEnable {
+						ref mut open, connected_point, connection: primary, ..
+					}) |
+					Some(PeerState::Disabled {
+						ref mut open, connected_point, connection: primary, ..
+					}) => {
+						if primary != &Some(connection) {
+							debug!(target: "sub-libp2p",
+								"Secondary connection opened custom protocol.");
 							return
 						}
 						if *open {
-							error!(target: "sub-libp2p", "Open: State mismatch in the custom protos handler");
+							error!(target: "sub-libp2p",
+								"Open: State mismatch in the custom protos handler");
 							return
 						}
 						*open = true;
@@ -1198,13 +1208,15 @@ impl NetworkBehaviour for GenericProto {
 			}
 
 			NotifsHandlerOut::ProtocolError { error, .. } => {
-				if self.peers.get(&source).and_then(|p| *p.connection()) != Some(_connection) {
-				    debug!(target: "sub-libp2p", "Handler({:?}) => Secondary connection error: {:?}",
+				if self.peers.get(&source).and_then(|p| *p.connection()) != Some(connection) {
+				    debug!(target: "sub-libp2p",
+						"Handler({:?}) => Secondary connection severe protocol error: {:?}",
 						source, error);
 				    return
 				}
 
-				debug!(target: "sub-libp2p", "Handler({:?}) => Severe protocol error: {:?}",
+				debug!(target: "sub-libp2p",
+					"Handler({:?}) => Severe protocol error: {:?}",
 					source, error);
 				// A severe protocol error happens when we detect a "bad" node, such as a node on
 				// a different chain, or a node that doesn't speak the same protocol(s). We