From 6625aa4afe2bc0aefbb23612dc4b95c1f58f422b Mon Sep 17 00:00:00 2001
From: Age Manning <Age@AgeManning.com>
Date: Sun, 28 Nov 2021 22:46:17 +0000
Subject: [PATCH] Status'd Peer Not Found (#2761)

## Issue Addressed

Users are experiencing `Status'd peer not found` errors

## Proposed Changes

Although I cannot reproduce this error, this is only one connection state change that is not addressed in the peer manager (that I could see). The error occurs because the number of disconnected peers in the peerdb becomes out of sync with the actual number of disconnected peers. From what I can tell almost all possible connection state changes are handled, except for the case when a disconnected peer changes to be disconnecting. This can potentially happen at the peer connection limit, where a previously connected peer switches to disconnecting.

This PR decrements the disconnected counter when this event occurs and from what I can tell, covers all possible disconnection state changes in the peer manager.
---
 .../src/peer_manager/peerdb.rs                | 61 +++++++++++--------
 .../src/peer_manager/peerdb/peer_info.rs      |  2 +-
 2 files changed, 36 insertions(+), 27 deletions(-)

diff --git a/beacon_node/lighthouse_network/src/peer_manager/peerdb.rs b/beacon_node/lighthouse_network/src/peer_manager/peerdb.rs
index 74d01c3239..4d69dc286f 100644
--- a/beacon_node/lighthouse_network/src/peer_manager/peerdb.rs
+++ b/beacon_node/lighthouse_network/src/peer_manager/peerdb.rs
@@ -534,32 +534,6 @@ impl<TSpec: EthSpec> PeerDB<TSpec> {
         }
     }
 
-    // Connection Status
-
-    /// A peer is being dialed.
-    // VISIBILITY: Only the peer manager can adjust the connection state
-    pub(super) fn dialing_peer(&mut self, peer_id: &PeerId, enr: Option<Enr>) {
-        let info = self.peers.entry(*peer_id).or_default();
-        if let Some(enr) = enr {
-            info.set_enr(enr);
-        }
-
-        if let Err(e) = info.dialing_peer() {
-            error!(self.log, "{}", e; "peer_id" => %peer_id);
-        }
-
-        // If the peer was banned, remove the banned peer and addresses.
-        if info.is_banned() {
-            self.banned_peers_count
-                .remove_banned_peer(info.seen_ip_addresses());
-        }
-
-        // If the peer was disconnected, reduce the disconnected peer count.
-        if info.is_disconnected() {
-            self.disconnected_peers = self.disconnected_peers().count().saturating_sub(1);
-        }
-    }
-
     /// Update min ttl of a peer.
     // VISIBILITY: Only the peer manager can update the min_ttl
     pub(super) fn update_min_ttl(&mut self, peer_id: &PeerId, min_ttl: Instant) {
@@ -614,6 +588,32 @@ impl<TSpec: EthSpec> PeerDB<TSpec> {
             });
     }
 
+    /// A peer is being dialed.
+    // VISIBILITY: Only the peer manager can adjust the connection state
+    // TODO: Remove the internal logic in favour of using the update_connection_state() function.
+    // This will be compatible once the ENR parameter is removed in the imminent behaviour tests PR.
+    pub(super) fn dialing_peer(&mut self, peer_id: &PeerId, enr: Option<Enr>) {
+        let info = self.peers.entry(*peer_id).or_default();
+        if let Some(enr) = enr {
+            info.set_enr(enr);
+        }
+
+        if let Err(e) = info.set_dialing_peer() {
+            error!(self.log, "{}", e; "peer_id" => %peer_id);
+        }
+
+        // If the peer was banned, remove the banned peer and addresses.
+        if info.is_banned() {
+            self.banned_peers_count
+                .remove_banned_peer(info.seen_ip_addresses());
+        }
+
+        // If the peer was disconnected, reduce the disconnected peer count.
+        if info.is_disconnected() {
+            self.disconnected_peers = self.disconnected_peers().count().saturating_sub(1);
+        }
+    }
+
     /// Sets a peer as connected with an ingoing connection.
     // VISIBILITY: Only the peer manager can adjust the connection state.
     pub(super) fn connect_ingoing(
@@ -786,6 +786,15 @@ impl<TSpec: EthSpec> PeerDB<TSpec> {
                 error!(self.log, "Disconnecting from a banned peer"; "peer_id" => %peer_id);
                 info.set_connection_status(PeerConnectionStatus::Disconnecting { to_ban });
             }
+            (
+                PeerConnectionStatus::Disconnected { .. },
+                NewConnectionState::Disconnecting { to_ban },
+            ) => {
+                // If the peer was previously disconnected and is now being disconnected, decrease
+                // the disconnected_peers counter.
+                self.disconnected_peers = self.disconnected_peers.saturating_sub(1);
+                info.set_connection_status(PeerConnectionStatus::Disconnecting { to_ban });
+            }
             (_, NewConnectionState::Disconnecting { to_ban }) => {
                 // We overwrite all states and set this peer to be disconnecting.
                 // NOTE: A peer can be in the disconnected state and transition straight to a
diff --git a/beacon_node/lighthouse_network/src/peer_manager/peerdb/peer_info.rs b/beacon_node/lighthouse_network/src/peer_manager/peerdb/peer_info.rs
index 82aaefc635..3ff5dc04ac 100644
--- a/beacon_node/lighthouse_network/src/peer_manager/peerdb/peer_info.rs
+++ b/beacon_node/lighthouse_network/src/peer_manager/peerdb/peer_info.rs
@@ -321,7 +321,7 @@ impl<T: EthSpec> PeerInfo<T> {
 
     /// Modifies the status to Dialing
     /// Returns an error if the current state is unexpected.
-    pub(super) fn dialing_peer(&mut self) -> Result<(), &'static str> {
+    pub(super) fn set_dialing_peer(&mut self) -> Result<(), &'static str> {
         match &mut self.connection_status {
             Connected { .. } => return Err("Dialing connected peer"),
             Dialing { .. } => return Err("Dialing an already dialing peer"),