From 55aada006f831e8140a7a2a6b16e280061408699 Mon Sep 17 00:00:00 2001
From: Age Manning <Age@AgeManning.com>
Date: Wed, 26 May 2021 14:21:44 +1000
Subject: [PATCH] More stringent dialing (#2363)

* More stringent dialing

* Cover cached enr dialing
---
 .../eth2_libp2p/src/peer_manager/mod.rs       | 21 +++++--------------
 .../eth2_libp2p/src/peer_manager/peerdb.rs    | 12 ++++++++++-
 2 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/beacon_node/eth2_libp2p/src/peer_manager/mod.rs b/beacon_node/eth2_libp2p/src/peer_manager/mod.rs
index 3269fbf26..659c8d9a3 100644
--- a/beacon_node/eth2_libp2p/src/peer_manager/mod.rs
+++ b/beacon_node/eth2_libp2p/src/peer_manager/mod.rs
@@ -668,18 +668,16 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
             .cached_enrs()
             .filter_map(|(peer_id, enr)| {
                 let peers = self.network_globals.peers.read();
-                if predicate(enr)
-                    && !peers.is_connected_or_dialing(peer_id)
-                    && !peers.is_banned(peer_id)
-                {
+                if predicate(enr) && peers.should_dial(peer_id) {
                     Some(*peer_id)
                 } else {
                     None
                 }
             })
             .collect();
-        for peer in &peers_to_dial {
-            self.dial_peer(peer);
+        for peer_id in &peers_to_dial {
+            debug!(self.log, "Dialing cached ENR peer"; "peer_id" => %peer_id);
+            self.dial_peer(peer_id);
         }
     }
 
@@ -698,16 +696,7 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
             // we attempt a connection if this peer is a subnet peer or if the max peer count
             // is not yet filled (including dialing peers)
             if (min_ttl.is_some() || connected_or_dialing + to_dial_peers.len() < self.max_peers)
-                && !self
-                    .network_globals
-                    .peers
-                    .read()
-                    .is_connected_or_dialing(&peer_id)
-                && !self
-                    .network_globals
-                    .peers
-                    .read()
-                    .is_banned_or_disconnected(&peer_id)
+                && self.network_globals.peers.read().should_dial(&peer_id)
             {
                 // This should be updated with the peer dialing. In fact created once the peer is
                 // dialed
diff --git a/beacon_node/eth2_libp2p/src/peer_manager/peerdb.rs b/beacon_node/eth2_libp2p/src/peer_manager/peerdb.rs
index 6803f49ed..44fcedbbd 100644
--- a/beacon_node/eth2_libp2p/src/peer_manager/peerdb.rs
+++ b/beacon_node/eth2_libp2p/src/peer_manager/peerdb.rs
@@ -153,6 +153,16 @@ impl<TSpec: EthSpec> PeerDB<TSpec> {
         )
     }
 
+    /// Returns true if the peer should be dialed. This checks the connection state and the
+    /// score state and determines if the peer manager should dial this peer.
+    pub fn should_dial(&self, peer_id: &PeerId) -> bool {
+        matches!(
+            self.connection_status(peer_id),
+            Some(PeerConnectionStatus::Disconnected { .. })
+                | Some(PeerConnectionStatus::Unknown { .. })
+        ) && !self.is_banned_or_disconnected(peer_id)
+    }
+
     /// Returns true if the peer is synced at least to our current head.
     pub fn is_synced(&self, peer_id: &PeerId) -> bool {
         match self.peers.get(peer_id).map(|info| &info.sync_status) {
@@ -341,7 +351,7 @@ impl<TSpec: EthSpec> PeerDB<TSpec> {
         }
 
         if let Err(e) = info.dialing_peer() {
-            error!(self.log, "{}", e);
+            error!(self.log, "{}", e; "peer_id" => %peer_id);
         }
     }