From b76ba0647228b71d0ae53642fd5ddebeac5555c4 Mon Sep 17 00:00:00 2001
From: Pierre Krieger <pierre.krieger1708@gmail.com>
Date: Mon, 12 Nov 2018 17:54:57 +0100
Subject: [PATCH] Some minor TODOs removal (#1102)
* Only reconnect if topology changed
* Minor changes
---
.../core/network-libp2p/src/custom_proto.rs | 1 -
substrate/core/network-libp2p/src/lib.rs | 6 ++---
.../core/network-libp2p/src/service_task.rs | 13 +++++++---
substrate/core/network-libp2p/src/topology.rs | 24 +++++++++++++++----
4 files changed, 33 insertions(+), 11 deletions(-)
diff --git a/substrate/core/network-libp2p/src/custom_proto.rs b/substrate/core/network-libp2p/src/custom_proto.rs
index 863e2d68b33..fd76581d3ed 100644
--- a/substrate/core/network-libp2p/src/custom_proto.rs
+++ b/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());
diff --git a/substrate/core/network-libp2p/src/lib.rs b/substrate/core/network-libp2p/src/lib.rs
index 9d53b01b01e..4852f65d84e 100644
--- a/substrate/core/network-libp2p/src/lib.rs
+++ b/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> {
diff --git a/substrate/core/network-libp2p/src/service_task.rs b/substrate/core/network-libp2p/src/service_task.rs
index e370a68c381..f40170b00c1 100644
--- a/substrate/core/network-libp2p/src/service_task.rs
+++ b/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.
diff --git a/substrate/core/network-libp2p/src/topology.rs b/substrate/core/network-libp2p/src/topology.rs
index ad92448c6af..061a3d17956 100644
--- a/substrate/core/network-libp2p/src/topology.rs
+++ b/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.
--
GitLab