fix(net): check for handshake disconnect (#596)

crates/net/eth-wire/src/error.rs

@@ -96,11 +96,13 @@ pub enum P2PStreamError {
 impl P2PStreamError {
     /// Returns the [`DisconnectReason`] if it is the `Disconnected` variant.
     pub fn as_disconnected(&self) -> Option<DisconnectReason> {
-        if let P2PStreamError::Disconnected(reason) = self {
-            Some(*reason)
-        } else {
-            None
-        }
+        let reason = match self {
+            P2PStreamError::HandshakeError(P2PHandshakeError::Disconnected(reason)) => reason,
+            P2PStreamError::Disconnected(reason) => reason,
+            _ => return None,
+        };
+
+        Some(*reason)
     }
 }
 

crates/net/network/src/error.rs

@@ -5,6 +5,7 @@ use reth_eth_wire::{
     error::{EthStreamError, HandshakeError, P2PHandshakeError, P2PStreamError},
     DisconnectReason,
 };
+use std::fmt;
 
 /// All error variants for the network
 #[derive(Debug, thiserror::Error)]
@@ -19,7 +20,7 @@ pub enum NetworkError {
 
 /// Abstraction over errors that can lead to a failed session
 #[auto_impl::auto_impl(&)]
-pub(crate) trait SessionError {
+pub(crate) trait SessionError: fmt::Debug {
     /// Returns true if the error indicates that the corresponding peer should be removed from peer
     /// discovery, for example if it's using a different genesis hash.
     fn merits_discovery_ban(&self) -> bool;
@@ -78,7 +79,14 @@ impl SessionError for EthStreamError {
     }
 
     fn should_backoff(&self) -> bool {
-        Some(DisconnectReason::TooManyPeers) == self.as_disconnected()
+        self.as_disconnected()
+            .map(|reason| {
+                matches!(
+                    reason,
+                    DisconnectReason::TooManyPeers | DisconnectReason::AlreadyConnected
+                )
+            })
+            .unwrap_or_default()
     }
 }
 
@@ -104,3 +112,18 @@ impl SessionError for PendingSessionHandshakeError {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_should_backoff() {
+        let err = EthStreamError::P2PStreamError(P2PStreamError::HandshakeError(
+            P2PHandshakeError::Disconnected(DisconnectReason::TooManyPeers),
+        ));
+
+        assert_eq!(err.as_disconnected(), Some(DisconnectReason::TooManyPeers));
+        assert!(err.should_backoff());
+    }
+}

crates/net/network/src/peers/manager.rs

@@ -197,6 +197,7 @@ impl PeersManager {
 
     /// Temporarily puts the peer in timeout
     fn backoff_peer(&mut self, peer_id: PeerId) {
+        trace!(target: "net::peers", ?peer_id, "backing off");
         self.ban_list.ban_peer_until(peer_id, std::time::Instant::now() + self.backoff_duration);
     }
 
@@ -291,7 +292,10 @@ impl PeersManager {
         err: impl SessionError,
         reputation_change: ReputationChangeKind,
     ) {
+        trace!(target: "net::peers", ?remote_addr, ?peer_id, ?err, "handling failed connection");
+
         if err.is_fatal_protocol_error() {
+            trace!(target: "net::peers", ?remote_addr, ?peer_id, ?err, "fatal connection error");
             // remove the peer to which we can't establish a connection due to protocol related
             // issues.
             if let Some(peer) = self.peers.remove(peer_id) {