fix(comms): dont overwrite ban-reason in add_peer (tari-project#5720)

Description --- Fixes ban reason disappearing when banning on connect
brianp · Sep 1, 2023 · 3b9890b · 3b9890b
1 parent b3f2dca
commit 3b9890b
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 22 deletions.
diff --git 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
@@ -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());