Some minor TODOs removal (#1102)

* Only reconnect if topology changed

* Minor changes

substrate/core/network-libp2p/src/custom_proto.rs

@@ -115,7 +115,6 @@ impl<TSubstream> RegisteredProtocolSubstream<TSubstream> {
 		self.send_queue.push_back(data);
 
 		// If the length of the queue goes over a certain arbitrary threshold, we print a warning.
-		// TODO: figure out a good threshold
 		if self.send_queue.len() >= 2048 {
 			warn!(target: "sub-libp2p", "Queue of packets to send over substream is pretty \
 				large: {}", self.send_queue.len());

substrate/core/network-libp2p/src/lib.rs

@@ -18,8 +18,7 @@
 //! TODO: Missing doc
 // end::description[]
 
-#![recursion_limit="128"]
-#![type_length_limit = "268435456"]
+#![recursion_limit = "128"]
 
 extern crate parking_lot;
 extern crate fnv;
@@ -58,7 +57,8 @@ pub use custom_proto::RegisteredProtocol;
 pub use error::{Error, ErrorKind, DisconnectReason};
 pub use libp2p::{Multiaddr, multiaddr::Protocol, PeerId};
 pub use service_task::{start_service, Service, ServiceEvent};
-pub use traits::*;	// TODO: expand to actual items
+pub use traits::{NetworkConfiguration, NodeIndex, NodeId, NonReservedPeerMode};
+pub use traits::{ProtocolId, Secret, Severity};
 
 /// Check if node url is valid
 pub fn validate_node_url(url: &str) -> Result<(), Error> {

substrate/core/network-libp2p/src/service_task.rs

@@ -567,6 +567,8 @@ impl Service {
 
 	/// Adds a list of peers to the network topology.
 	fn add_discovered_peers(&mut self, list: impl IntoIterator<Item = KadPeer>) {
+		let mut topology_has_changed = false;
+
 		for peer in list {
 			let connected = match peer.connection_ty {
 				KadConnectionType::NotConnected => false,
@@ -575,15 +577,20 @@ impl Service {
 				KadConnectionType::CannotConnect => continue,
 			};
 
-			self.topology.add_kademlia_discovered_addrs(
+			let changed = self.topology.add_kademlia_discovered_addrs(
 				&peer.node_id,
 				peer.multiaddrs.iter().map(|a| (a.clone(), connected))
 			);
+
+			if changed {
+				topology_has_changed = true;
+			}
 		}
 
 		// Potentially connect to the newly-discovered nodes.
-		// TODO: only do so if the topology reports that something new has been added
-		self.connect_to_nodes();
+		if topology_has_changed {
+			self.connect_to_nodes();
+		}
 	}
 
 	/// Handles the swarm opening a connection to the given peer.

substrate/core/network-libp2p/src/topology.rs

@@ -245,12 +245,16 @@ impl NetTopology {
 	/// Adds addresses that a node says it is listening on.
 	///
 	/// The addresses are most likely to be valid.
+	///
+	/// Returns `true` if the topology has changed in some way. Returns `false` if calling this
+	/// method was a no-op.
 	#[inline]
 	pub fn add_self_reported_listen_addrs<I>(
 		&mut self,
 		peer_id: &PeerId,
 		addrs: I,
-	) where I: Iterator<Item = Multiaddr> {
+	) -> bool
+		where I: Iterator<Item = Multiaddr> {
 		self.add_discovered_addrs(peer_id, addrs.map(|a| (a, true)))
 	}
 
@@ -260,21 +264,28 @@ impl NetTopology {
 	///
 	/// For each address, incorporates a boolean. If true, that means we have some sort of hint
 	/// that this address can be reached.
+	///
+	/// Returns `true` if the topology has changed in some way. Returns `false` if calling this
+	/// method was a no-op.
 	#[inline]
 	pub fn add_kademlia_discovered_addrs<I>(
 		&mut self,
 		peer_id: &PeerId,
 		addrs: I,
-	) where I: Iterator<Item = (Multiaddr, bool)> {
+	) -> bool
+		where I: Iterator<Item = (Multiaddr, bool)> {
 		self.add_discovered_addrs(peer_id, addrs)
 	}
 
-	/// Inner implementaiton of the `add_*_discovered_addrs`.
+	/// Inner implementaiton of the `add_*_discovered_addrs` methods.
+	/// Returns `true` if the topology has changed in some way. Returns `false` if calling this
+	/// method was a no-op.
 	fn add_discovered_addrs<I>(
 		&mut self,
 		peer_id: &PeerId,
 		addrs: I,
-	) where I: Iterator<Item = (Multiaddr, bool)> {
+	) -> bool
+		where I: Iterator<Item = (Multiaddr, bool)> {
 		let mut addrs: Vec<_> = addrs.collect();
 		let now_systime = SystemTime::now();
 		let now = Instant::now();
@@ -291,6 +302,8 @@ impl NetTopology {
 			true
 		});
 
+		let mut anything_changed = false;
+
 		if !addrs.is_empty() {
 			trace!(
 				target: "sub-libp2p",
@@ -317,6 +330,7 @@ impl NetTopology {
 				}
 			}
 
+			anything_changed = true;
 			peer.addrs.push(Addr {
 				addr,
 				expires: now_systime + KADEMLIA_DISCOVERY_EXPIRATION,
@@ -329,6 +343,8 @@ impl NetTopology {
 				}),
 			});
 		}
+
+		anything_changed
 	}
 
 	/// Indicates the peer store that we're connected to this given address.