From 3aad82d158590835d0737bfdd7125ca0b040a929 Mon Sep 17 00:00:00 2001
From: cloudhead <cloudhead@radicle.xyz>
Date: Fri, 10 Nov 2023 14:44:52 +0100
Subject: [PATCH] node: Fix various small logic issues

* Set peer's `last_active` even if they are being throttled
* Make sure not to break out of loop in wire module
* Add some more logging
* Use different variable for session filtering to make it clearer
---
 radicle-node/src/service.rs       | 14 ++++++++++++--
 radicle-node/src/wire/protocol.rs |  4 +---
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/radicle-node/src/service.rs b/radicle-node/src/service.rs
index fe058c35..bfb5f404 100644
--- a/radicle-node/src/service.rs
+++ b/radicle-node/src/service.rs
@@ -868,6 +868,8 @@ where
 
             self.outbox.wakeup(delay);
         } else {
+            debug!(target: "service", "Dropping peer {remote}..");
+
             self.sessions.remove(&remote);
             // Only re-attempt outbound connections, since we don't care if an inbound connection
             // is dropped.
@@ -1153,6 +1155,8 @@ where
             warn!(target: "service", "Session not found for {remote}");
             return Ok(());
         };
+        peer.last_active = self.clock;
+
         let limit = match peer.link {
             Link::Outbound => &self.config.limits.rate.outbound,
             Link::Inbound => &self.config.limits.rate.inbound,
@@ -1164,7 +1168,6 @@ where
             trace!(target: "service", "Rate limiting message from {remote} ({})", peer.addr);
             return Ok(());
         }
-        peer.last_active = self.clock;
         message.log(log::Level::Debug, remote, Link::Inbound);
 
         trace!(target: "service", "Received message {:?} from {}", &message, peer.id);
@@ -1184,7 +1187,7 @@ where
                     let relay_to = self
                         .sessions
                         .connected()
-                        .filter(|(id, _)| *id != remote && *id != &announcer)
+                        .filter(|(id, _)| *id != &relayer && *id != &announcer)
                         .map(|(_, p)| p);
 
                     self.outbox.relay(ann, relay_to);
@@ -1411,6 +1414,8 @@ where
     }
 
     fn connect(&mut self, nid: NodeId, addr: Address) -> bool {
+        debug!(target: "service", "Connecting to {nid} ({addr})..");
+
         if self.sessions.contains_key(&nid) {
             warn!(target: "service", "Attempted connection to peer {nid} which already has a session");
             return false;
@@ -1517,6 +1522,11 @@ where
             .filter(|(_, session)| *now - session.last_active >= STALE_CONNECTION_TIMEOUT);
 
         for (_, session) in stale {
+            debug!(target: "service", "Disconnecting unresponsive peer {}..", session.id);
+
+            // TODO: Should we switch the session state to "disconnected" even before receiving
+            // an official "disconnect"? Otherwise we keep pinging until we get the disconnection.
+
             self.outbox.disconnect(
                 session.id,
                 DisconnectReason::Session(session::Error::Timeout),
diff --git a/radicle-node/src/wire/protocol.rs b/radicle-node/src/wire/protocol.rs
index eb343992..d9af54e3 100644
--- a/radicle-node/src/wire/protocol.rs
+++ b/radicle-node/src/wire/protocol.rs
@@ -780,7 +780,7 @@ where
                             target: "wire",
                             "Attempt to connect to already connected peer {node_id}"
                         );
-                        break;
+                        continue;
                     }
 
                     match dial::<G>(
@@ -801,7 +801,6 @@ where
                                 transport.as_raw_fd(),
                                 Peer::outbound(addr.to_inner(), node_id),
                             );
-
                             self.actions
                                 .push_back(reactor::Action::RegisterTransport(transport));
                         }
@@ -810,7 +809,6 @@ where
 
                             self.service
                                 .disconnected(node_id, &DisconnectReason::Dial(Arc::new(err)));
-                            break;
                         }
                     }
                 }