From 3b9890ba5857cc8767be77a024d01bf4826e3956 Mon Sep 17 00:00:00 2001
From: Stan Bondi <sdbondi@users.noreply.github.com>
Date: Fri, 1 Sep 2023 19:16:11 +0400
Subject: [PATCH] fix(comms): dont overwrite ban-reason in add_peer (#5720)

Description
---

Fixes ban reason disappearing when banning on connect
---
 comms/core/src/connection_manager/dialer.rs | 32 ++++++++-------------
 comms/core/src/peer_manager/peer.rs         |  6 ++--
 2 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/comms/core/src/connection_manager/dialer.rs b/comms/core/src/connection_manager/dialer.rs
index 95b3ca0f0d..0276b3931d 100644
--- a/comms/core/src/connection_manager/dialer.rs
+++ b/comms/core/src/connection_manager/dialer.rs
@@ -238,16 +238,6 @@ where
                 peer.supported_protocols = peer_identity.metadata.supported_protocols;
                 peer.user_agent = peer_identity.metadata.user_agent;
 
-                let _ = self
-                    .peer_manager
-                    .add_peer(dial_state.peer().clone())
-                    .await
-                    .map_err(|e| {
-                        error!(target: LOG_TARGET, "Could not update peer data:{}", e);
-                        let _ = dial_state
-                            .send_reply(Err(ConnectionManagerError::PeerManagerError(e)))
-                            .map_err(|e| error!(target: LOG_TARGET, "Could not send reply to dial request: {:?}", e));
-                    });
                 debug!(target: LOG_TARGET, "Successfully dialed peer '{}'", node_id);
                 self.notify_connection_manager(ConnectionManagerEvent::PeerConnected(conn.clone().into()))
                     .await;
@@ -266,16 +256,6 @@ where
                     target: LOG_TARGET,
                     "Failed to dial peer '{}' because '{:?}'", node_id, err
                 );
-                let _ = self
-                    .peer_manager
-                    .add_peer(dial_state.peer().clone())
-                    .await
-                    .map_err(|e| {
-                        error!(target: LOG_TARGET, "Could not update peer data:{}", e);
-                        let _ = dial_state
-                            .send_reply(Err(ConnectionManagerError::PeerManagerError(e)))
-                            .map_err(|e| error!(target: LOG_TARGET, "Could not send reply to dial request: {:?}", e));
-                    });
                 self.notify_connection_manager(ConnectionManagerEvent::PeerConnectFailed(node_id.clone(), err.clone()))
                     .await;
 
@@ -289,6 +269,17 @@ where
             },
         }
 
+        let _ = self
+            .peer_manager
+            .add_peer(dial_state.peer().clone())
+            .await
+            .map_err(|e| {
+                error!(target: LOG_TARGET, "Could not update peer data: {}", e);
+                let _ = dial_state
+                    .send_reply(Err(ConnectionManagerError::PeerManagerError(e)))
+                    .map_err(|e| error!(target: LOG_TARGET, "Could not send reply to dial request: {:?}", e));
+            });
+
         metrics::pending_connections(Some(&node_id), ConnectionDirection::Outbound).dec();
 
         self.cancel_dial(&node_id);
@@ -451,6 +442,7 @@ where
             config.network_info.clone(),
         )
         .await;
+
         let peer_identity =
             common::ban_on_offence(peer_manager, &authenticated_public_key, peer_identity_result).await?;
 
diff --git a/comms/core/src/peer_manager/peer.rs b/comms/core/src/peer_manager/peer.rs
index 3c4d9268c3..a7ab165a42 100644
--- a/comms/core/src/peer_manager/peer.rs
+++ b/comms/core/src/peer_manager/peer.rs
@@ -132,9 +132,11 @@ impl Peer {
     /// database so that data is not overwritten
     pub fn merge(&mut self, other: &Peer) {
         self.addresses.merge(&other.addresses);
-        self.banned_reason = other.banned_reason.clone();
-        self.added_at = cmp::min(self.added_at, other.added_at);
+        if !other.banned_reason.is_empty() {
+            self.banned_reason = other.banned_reason.clone();
+        }
         self.banned_until = cmp::max(self.banned_until, other.banned_until);
+        self.added_at = cmp::min(self.added_at, other.added_at);
         for protocol in &other.supported_protocols {
             if !self.supported_protocols.contains(protocol) {
                 self.supported_protocols.push(protocol.clone());