From c8e98212a0ce093dbe660e0a5bcf446c572b38dc Mon Sep 17 00:00:00 2001
From: Hansie Odendaal <pluto@tari.com>
Date: Fri, 23 Aug 2024 12:23:51 +0200
Subject: [PATCH] error handling

---
 comms/core/src/connection_manager/dialer.rs | 20 +++++++++++---
 comms/core/src/connection_manager/error.rs  |  7 ++++-
 comms/core/src/connectivity/manager.rs      | 29 ++++++++++++++++++++-
 3 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/comms/core/src/connection_manager/dialer.rs b/comms/core/src/connection_manager/dialer.rs
index 0451d8b5b4..924a8361cf 100644
--- a/comms/core/src/connection_manager/dialer.rs
+++ b/comms/core/src/connection_manager/dialer.rs
@@ -524,6 +524,10 @@ where
                         (state, Err(ConnectionManagerError::DialCancelled)) => break (state, Err(ConnectionManagerError::DialCancelled)),
                         (state, Err(err)) => {
                             debug!(target: LOG_TARGET, "Failed to dial peer {} | Attempt {} | Error: {}", state.peer().node_id.short_str(), state.num_attempts(), err);
+                            if let ConnectionManagerError::NoiseHandshakeError(_) = err {
+                                // If the noise handshake failed, we should not retry the dial
+                                break (state, Err(err));
+                            }
                             if state.num_attempts() >= config.max_dial_attempts {
                                 break (state, Err(ConnectionManagerError::ConnectFailedMaximumAttemptsReached));
                             }
@@ -546,6 +550,7 @@ where
     /// Attempts to dial a peer sequentially on all addresses; if connections are to be minimized only.
     /// Returns ownership of the given `DialState` and a success or failure result for the dial,
     /// or None if the dial was cancelled inflight
+    #[allow(clippy::too_many_lines)]
     async fn dial_peer(
         mut dial_state: DialState,
         noise_config: &NoiseConfig,
@@ -596,7 +601,7 @@ where
                 let initial_dial_time = timer.elapsed();
 
                 debug!(
-                    "Dialed peer: {} on address: {} on tcp after: {}",
+                    "Dialed peer: {} on address: {} on tcp after: {} ms",
                     node_id.short_str(),
                     moved_address,
                     timer.elapsed().as_millis()
@@ -644,13 +649,20 @@ where
                         dial_state.peer().node_id.short_str(),
                         err,
                     );
-
                     dial_state
                         .peer_mut()
                         .addresses
                         .mark_failed_connection_attempt(&address, err.to_string());
-                    // Try the next address
-                    continue;
+
+                    match err {
+                        ConnectionManagerError::NoiseHandshakeError(_) => {
+                            return (dial_state, Err(err));
+                        },
+                        _ => {
+                            // Try the next address
+                            continue;
+                        },
+                    }
                 },
                 // Canceled
                 Either::Right(_) => {
diff --git a/comms/core/src/connection_manager/error.rs b/comms/core/src/connection_manager/error.rs
index 7d2bb4af76..25e2443b91 100644
--- a/comms/core/src/connection_manager/error.rs
+++ b/comms/core/src/connection_manager/error.rs
@@ -68,6 +68,8 @@ pub enum ConnectionManagerError {
     // send the same response to multiple requesters
     #[error("Noise error: {0}")]
     NoiseError(String),
+    #[error("Noise handshake error: {0}")]
+    NoiseHandshakeError(String),
     #[error("Peer is banned, denying connection")]
     PeerBanned,
     #[error("Identity protocol failed: {0}")]
@@ -94,7 +96,10 @@ impl From<yamux::ConnectionError> for ConnectionManagerError {
 
 impl From<noise::NoiseError> for ConnectionManagerError {
     fn from(err: noise::NoiseError) -> Self {
-        ConnectionManagerError::NoiseError(err.to_string())
+        match err {
+            noise::NoiseError::HandshakeFailed(_) => ConnectionManagerError::NoiseHandshakeError(err.to_string()),
+            _ => ConnectionManagerError::NoiseError(err.to_string()),
+        }
     }
 }
 
diff --git a/comms/core/src/connectivity/manager.rs b/comms/core/src/connectivity/manager.rs
index c6ccb00500..ea0d3be916 100644
--- a/comms/core/src/connectivity/manager.rs
+++ b/comms/core/src/connectivity/manager.rs
@@ -602,6 +602,29 @@ impl ConnectivityManagerActor {
         Ok(())
     }
 
+    async fn on_peer_handshake_failure(&mut self, node_id: &NodeId) -> Result<(), ConnectivityError> {
+        let num_failed = self.mark_peer_failed(node_id.clone());
+
+        if (self.peer_manager.find_by_node_id(node_id).await?).is_some() {
+            debug!(
+                target: LOG_TARGET,
+                "Peer `{}` was marked as offline after {} attempts due to incompatible handshake noise prologue. \
+                Removing peer from peer list",
+                node_id,
+                num_failed,
+            );
+            self.peer_manager.delete_peer(node_id).await?;
+            self.ban_peer(
+                node_id,
+                Duration::from_secs(3 * 24 * 60 * 60),
+                "Incompatible handshake noise prologue".to_string(),
+            )
+            .await?;
+        }
+
+        Ok(())
+    }
+
     async fn handle_connection_manager_event(
         &mut self,
         event: &ConnectionManagerEvent,
@@ -684,7 +707,11 @@ impl ConnectivityManagerActor {
                     target: LOG_TARGET,
                     "Connection to peer '{}' failed because '{:?}'", node_id, err
                 );
-                self.on_peer_connection_failure(node_id).await?;
+                if let ConnectionManagerError::NoiseHandshakeError(_) = err {
+                    self.on_peer_handshake_failure(node_id).await?;
+                } else {
+                    self.on_peer_connection_failure(node_id).await?;
+                }
                 (node_id, ConnectionStatus::Failed, None)
             },
             _ => return Ok(()),