diff --git a/substrate/client/peerset/src/lib.rs b/substrate/client/peerset/src/lib.rs index 575743afa079c0f3c2ba2ddd1f0bf3bfdc195936..b3284533a80b5267a4f908b44d0f8b2b0d2d0f95 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 59879f629e31ecdbfbcb224a8a7dfb437f150746..19b2489eff486d7aae042c66771d73733981a3b9 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 6fa29e3d834cfcdaa0b035e03849432e20e7b3f3..e02742fc40ad4427cdbf037468d05b9f0e63f9ba 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 70baa006bdcdc0d86b99aec71859361806f9044c..321ab72f0d2724008770f9349de2d3401d5a9b33 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 65a560af4eaa52a8eb3fa49cd0d5e6bc38527be7..6ca9452893f3e133787a0d4d69b219a015b0273f 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> {