From 22a28db957000a2c8244a0435b1d8572cbcc2d36 Mon Sep 17 00:00:00 2001
From: jolestar <jolestar@gmail.com>
Date: Tue, 1 Dec 2020 18:58:00 +0800
Subject: [PATCH] Fix cargo clippy warning in peerset. (#7641)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* Fix cargo clippy warning in peerset.
* Update client/peerset/src/lib.rs
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
* Apply suggestions from code review
Co-authored-by: Pierre Krieger <pierre.krieger1708@gmail.com>
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: Pierre Krieger <pierre.krieger1708@gmail.com>
---
substrate/client/peerset/src/lib.rs | 48 ++++++++-----------
substrate/client/peerset/src/peersstate.rs | 8 ++--
substrate/client/peerset/tests/fuzz.rs | 4 +-
substrate/primitives/utils/src/mpsc.rs | 2 +-
.../primitives/utils/src/status_sinks.rs | 6 +++
5 files changed, 33 insertions(+), 35 deletions(-)
diff --git a/substrate/client/peerset/src/lib.rs b/substrate/client/peerset/src/lib.rs
index 575743afa07..b3284533a80 100644
--- a/substrate/client/peerset/src/lib.rs
+++ b/substrate/client/peerset/src/lib.rs
@@ -36,7 +36,7 @@ const BANNED_THRESHOLD: i32 = 82 * (i32::min_value() / 100);
/// Reputation change for a node when we get disconnected from it.
const DISCONNECT_REPUTATION_CHANGE: i32 = -256;
/// Reserved peers group ID
-const RESERVED_NODES: &'static str = "reserved";
+const RESERVED_NODES: &str = "reserved";
/// Amount of time between the moment we disconnect from a node and the moment we remove it from
/// the list.
const FORGET_AFTER: Duration = Duration::from_secs(3600);
@@ -87,7 +87,7 @@ impl PeersetHandle {
/// Has no effect if the node was already a reserved peer.
///
/// > **Note**: Keep in mind that the networking has to know an address for this node,
- /// > otherwise it will not be able to connect to it.
+ /// > otherwise it will not be able to connect to it.
pub fn add_reserved_peer(&self, peer_id: PeerId) {
let _ = self.tx.unbounded_send(Action::AddReservedPeer(peer_id));
}
@@ -169,7 +169,7 @@ pub struct PeersetConfig {
/// List of bootstrap nodes to initialize the peer with.
///
/// > **Note**: Keep in mind that the networking has to know an address for these nodes,
- /// > otherwise it will not be able to connect to them.
+ /// > otherwise it will not be able to connect to them.
pub bootnodes: Vec<PeerId>,
/// If true, we only accept nodes in [`PeersetConfig::priority_groups`].
@@ -178,7 +178,7 @@ pub struct PeersetConfig {
/// Lists of nodes we should always be connected to.
///
/// > **Note**: Keep in mind that the networking has to know an address for these nodes,
- /// > otherwise it will not be able to connect to them.
+ /// > otherwise it will not be able to connect to them.
pub priority_groups: Vec<(String, HashSet<PeerId>)>,
}
@@ -430,10 +430,9 @@ impl Peerset {
.get(RESERVED_NODES)
.into_iter()
.flatten()
- .filter(move |n| {
+ .find(move |n| {
data.peer(n).into_connected().is_none()
})
- .next()
.cloned()
};
@@ -469,10 +468,9 @@ impl Peerset {
self.priority_groups
.values()
.flatten()
- .filter(move |n| {
+ .find(move |n| {
data.peer(n).into_connected().is_none()
})
- .next()
.cloned()
};
@@ -497,21 +495,17 @@ impl Peerset {
}
// Now, we try to connect to non-priority nodes.
- loop {
- // Try to grab the next node to attempt to connect to.
- let next = match self.data.highest_not_connected_peer() {
- Some(p) => p,
- None => break, // No known node to add.
- };
-
+ while let Some(next) = self.data.highest_not_connected_peer() {
// Don't connect to nodes with an abysmal reputation.
if next.reputation() < BANNED_THRESHOLD {
break;
}
match next.try_outgoing() {
- Ok(conn) => self.message_queue.push_back(Message::Connect(conn.into_peer_id())),
- Err(_) => break, // No more slots available.
+ Ok(conn) => self
+ .message_queue
+ .push_back(Message::Connect(conn.into_peer_id())),
+ Err(_) => break, // No more slots available.
}
}
}
@@ -530,11 +524,9 @@ impl Peerset {
trace!(target: "peerset", "Incoming {:?}", peer_id);
self.update_time();
- if self.reserved_only {
- if !self.priority_groups.get(RESERVED_NODES).map_or(false, |n| n.contains(&peer_id)) {
- self.message_queue.push_back(Message::Reject(index));
- return;
- }
+ if self.reserved_only && !self.priority_groups.get(RESERVED_NODES).map_or(false, |n| n.contains(&peer_id)) {
+ self.message_queue.push_back(Message::Reject(index));
+ return;
}
let not_connected = match self.data.peer(&peer_id) {
@@ -584,7 +576,7 @@ impl Peerset {
/// Adds discovered peer ids to the PSM.
///
/// > **Note**: There is no equivalent "expired" message, meaning that it is the responsibility
- /// > of the PSM to remove `PeerId`s that fail to dial too often.
+ /// > of the PSM to remove `PeerId`s that fail to dial too often.
pub fn discovered<I: IntoIterator<Item = PeerId>>(&mut self, peer_ids: I) {
let mut discovered_any = false;
@@ -747,12 +739,12 @@ mod tests {
let (mut peerset, _handle) = Peerset::from_config(config);
peerset.incoming(incoming.clone(), ii);
- peerset.incoming(incoming.clone(), ii4);
- peerset.incoming(incoming2.clone(), ii2);
- peerset.incoming(incoming3.clone(), ii3);
+ peerset.incoming(incoming, ii4);
+ peerset.incoming(incoming2, ii2);
+ peerset.incoming(incoming3, ii3);
assert_messages(peerset, vec![
- Message::Connect(bootnode.clone()),
+ Message::Connect(bootnode),
Message::Accept(ii),
Message::Accept(ii2),
Message::Reject(ii3),
@@ -772,7 +764,7 @@ mod tests {
};
let (mut peerset, _) = Peerset::from_config(config);
- peerset.incoming(incoming.clone(), ii);
+ peerset.incoming(incoming, ii);
assert_messages(peerset, vec![
Message::Reject(ii),
diff --git a/substrate/client/peerset/src/peersstate.rs b/substrate/client/peerset/src/peersstate.rs
index 59879f629e3..19b2489eff4 100644
--- a/substrate/client/peerset/src/peersstate.rs
+++ b/substrate/client/peerset/src/peersstate.rs
@@ -42,8 +42,8 @@ pub struct PeersState {
/// List of nodes that we know about.
///
/// > **Note**: This list should really be ordered by decreasing reputation, so that we can
- /// easily select the best node to connect to. As a first draft, however, we don't
- /// sort, to make the logic easier.
+ /// easily select the best node to connect to. As a first draft, however, we don't
+ /// sort, to make the logic easier.
nodes: HashMap<PeerId, Node>,
/// Number of slot-occupying nodes for which the `ConnectionState` is `In`.
@@ -130,7 +130,7 @@ impl PeersState {
/// Returns an object that grants access to the state of a peer.
pub fn peer<'a>(&'a mut self, peer_id: &'a PeerId) -> Peer<'a> {
match self.nodes.get_mut(peer_id) {
- None => return Peer::Unknown(UnknownPeer {
+ None => Peer::Unknown(UnknownPeer {
parent: self,
peer_id: Cow::Borrowed(peer_id),
}),
@@ -585,7 +585,7 @@ mod tests {
peers_state.peer(&id2).into_connected().unwrap().disconnect();
assert_eq!(peers_state.highest_not_connected_peer().map(|p| p.into_peer_id()), Some(id1.clone()));
peers_state.peer(&id1).into_not_connected().unwrap().set_reputation(-100);
- assert_eq!(peers_state.highest_not_connected_peer().map(|p| p.into_peer_id()), Some(id2.clone()));
+ assert_eq!(peers_state.highest_not_connected_peer().map(|p| p.into_peer_id()), Some(id2));
}
#[test]
diff --git a/substrate/client/peerset/tests/fuzz.rs b/substrate/client/peerset/tests/fuzz.rs
index 6fa29e3d834..e02742fc40a 100644
--- a/substrate/client/peerset/tests/fuzz.rs
+++ b/substrate/client/peerset/tests/fuzz.rs
@@ -115,8 +115,8 @@ fn test_once() {
4 => if let Some(id) = known_nodes.iter()
.filter(|n| incoming_nodes.values().all(|m| m != *n) && !connected_nodes.contains(*n))
.choose(&mut rng) {
- peerset.incoming(id.clone(), next_incoming_id.clone());
- incoming_nodes.insert(next_incoming_id.clone(), id.clone());
+ peerset.incoming(id.clone(), next_incoming_id);
+ incoming_nodes.insert(next_incoming_id, id.clone());
next_incoming_id.0 += 1;
}
diff --git a/substrate/primitives/utils/src/mpsc.rs b/substrate/primitives/utils/src/mpsc.rs
index 70baa006bdc..321ab72f0d2 100644
--- a/substrate/primitives/utils/src/mpsc.rs
+++ b/substrate/primitives/utils/src/mpsc.rs
@@ -63,7 +63,7 @@ mod inner {
/// `UNBOUNDED_CHANNELS_COUNTER`
pub fn tracing_unbounded<T>(key: &'static str) ->(TracingUnboundedSender<T>, TracingUnboundedReceiver<T>) {
let (s, r) = mpsc::unbounded();
- (TracingUnboundedSender(key.clone(), s), TracingUnboundedReceiver(key,r))
+ (TracingUnboundedSender(key, s), TracingUnboundedReceiver(key,r))
}
impl<T> TracingUnboundedSender<T> {
diff --git a/substrate/primitives/utils/src/status_sinks.rs b/substrate/primitives/utils/src/status_sinks.rs
index 65a560af4ea..6ca9452893f 100644
--- a/substrate/primitives/utils/src/status_sinks.rs
+++ b/substrate/primitives/utils/src/status_sinks.rs
@@ -43,6 +43,12 @@ struct YieldAfter<T> {
sender: Option<TracingUnboundedSender<T>>,
}
+impl <T> Default for StatusSinks<T> {
+ fn default() -> Self {
+ Self::new()
+ }
+}
+
impl<T> StatusSinks<T> {
/// Builds a new empty collection.
pub fn new() -> StatusSinks<T> {
--
GitLab