net: Break disconnecting out of Ban()

These are separate events which need to be carried out by separate subsystems.

This also cleans up some whitespace and tabs in qt to avoid getting flagged by
the linter.

Current behavior is preserved.

7 files changed, 55 insertions, 25 deletions

diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp
index acba05fd5e..09460ec43e 100644
--- a/src/interfaces/node.cpp
+++ b/src/interfaces/node.cpp
@@ -144,6 +144,13 @@ public:
         }
         return false;
     }
+    bool disconnect(const CNetAddr& net_addr) override
+    {
+        if (g_connman) {
+            return g_connman->DisconnectNode(net_addr);
+        }
+        return false;
+    }
     bool disconnect(NodeId id) override
     {
         if (g_connman) {
diff --git a/src/interfaces/node.h b/src/interfaces/node.h
index 7fa5958c51..6aa8ce0797 100644
--- a/src/interfaces/node.h
+++ b/src/interfaces/node.h
@@ -113,7 +113,10 @@ public:
     //! Unban node.
     virtual bool unban(const CSubNet& ip) = 0;
 
-    //! Disconnect node.
+    //! Disconnect node by address.
+    virtual bool disconnect(const CNetAddr& net_addr) = 0;
+
+    //! Disconnect node by id.
     virtual bool disconnect(NodeId id) = 0;
 
     //! Get total bytes recv.
diff --git a/src/net.cpp b/src/net.cpp
index 98bd518ecc..6f0d76ccf5 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -554,13 +554,6 @@ void CConnman::Ban(const CSubNet& subNet, const BanReason &banReason, int64_t ba
     }
     if(clientInterface)
         clientInterface->BannedListChanged();
-    {
-        LOCK(cs_vNodes);
-        for (CNode* pnode : vNodes) {
-            if (subNet.Match(static_cast<CNetAddr>(pnode->addr)))
-                pnode->fDisconnect = true;
-        }
-    }
     if(banReason == BanReasonManuallyAdded)
         DumpBanlist(); //store banlist to disk immediately if user requested ban
 }
@@ -2643,6 +2636,25 @@ bool CConnman::DisconnectNode(const std::string& strNode)
     }
     return false;
 }
+
+bool CConnman::DisconnectNode(const CSubNet& subnet)
+{
+    bool disconnected = false;
+    LOCK(cs_vNodes);
+    for (CNode* pnode : vNodes) {
+        if (subnet.Match(pnode->addr)) {
+            pnode->fDisconnect = true;
+            disconnected = true;
+        }
+    }
+    return disconnected;
+}
+
+bool CConnman::DisconnectNode(const CNetAddr& addr)
+{
+    return DisconnectNode(CSubNet(addr));
+}
+
 bool CConnman::DisconnectNode(NodeId id)
 {
     LOCK(cs_vNodes);
diff --git a/src/net.h b/src/net.h
index 430e148afa..c13af50ec4 100644
--- a/src/net.h
+++ b/src/net.h
@@ -282,6 +282,8 @@ public:
     size_t GetNodeCount(NumConnections num);
     void GetNodeStats(std::vector<CNodeStats>& vstats);
     bool DisconnectNode(const std::string& node);
+    bool DisconnectNode(const CSubNet& subnet);
+    bool DisconnectNode(const CNetAddr& addr);
     bool DisconnectNode(NodeId id);
 
     ServiceFlags GetLocalServices() const;
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 0e222bdfa4..56cd52ed21 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -2961,14 +2961,14 @@ static bool SendRejectsAndCheckIfBanned(CNode* pnode, CConnman* connman, bool en
             LogPrintf("Warning: not punishing whitelisted peer %s!\n", pnode->addr.ToString());
         else if (pnode->m_manual_connection)
             LogPrintf("Warning: not punishing manually-connected peer %s!\n", pnode->addr.ToString());
-        else {
+        else if (pnode->addr.IsLocal()) {
+            // Disconnect but don't ban _this_ local node
+            LogPrintf("Warning: disconnecting but not banning local peer %s!\n", pnode->addr.ToString());
             pnode->fDisconnect = true;
-            if (pnode->addr.IsLocal())
-                LogPrintf("Warning: not banning local peer %s!\n", pnode->addr.ToString());
-            else
-            {
-                connman->Ban(pnode->addr, BanReasonNodeMisbehaving);
-            }
+        } else {
+            // Disconnect and ban all nodes sharing the address
+            connman->Ban(pnode->addr, BanReasonNodeMisbehaving);
+            connman->DisconnectNode(pnode->addr);
         }
         return true;
     }
diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp
index 2949e43915..2989e1e9e5 100644
--- a/src/qt/rpcconsole.cpp
+++ b/src/qt/rpcconsole.cpp
@@ -1216,16 +1216,16 @@ void RPCConsole::banSelectedNode(int bantime)
         // Get currently selected peer address
         NodeId id = nodes.at(i).data().toLongLong();
 
-	// Get currently selected peer address
-	int detailNodeRow = clientModel->getPeerTableModel()->getRowByNodeId(id);
-	if(detailNodeRow < 0)
-	    return;
-
-	// Find possible nodes, ban it and clear the selected node
-	const CNodeCombinedStats *stats = clientModel->getPeerTableModel()->getNodeStats(detailNodeRow);
-	if(stats) {
+        // Get currently selected peer address
+        int detailNodeRow = clientModel->getPeerTableModel()->getRowByNodeId(id);
+        if (detailNodeRow < 0) return;
+
+        // Find possible nodes, ban it and clear the selected node
+        const CNodeCombinedStats *stats = clientModel->getPeerTableModel()->getNodeStats(detailNodeRow);
+        if (stats) {
             m_node.ban(stats->nodeStats.addr, BanReasonManuallyAdded, bantime);
-	}
+            m_node.disconnect(stats->nodeStats.addr);
+        }
     }
     clearSelectedNode();
     clientModel->getBanTableModel()->refresh();
diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp
index 6fdf80dc5f..59bc8e8091 100644
--- a/src/rpc/net.cpp
+++ b/src/rpc/net.cpp
@@ -565,7 +565,13 @@ static UniValue setban(const JSONRPCRequest& request)
         if (request.params[3].isTrue())
             absolute = true;
 
-        isSubnet ? g_connman->Ban(subNet, BanReasonManuallyAdded, banTime, absolute) : g_connman->Ban(netAddr, BanReasonManuallyAdded, banTime, absolute);
+        if (isSubnet) {
+            g_connman->Ban(subNet, BanReasonManuallyAdded, banTime, absolute);
+            g_connman->DisconnectNode(subNet);
+        } else {
+            g_connman->Ban(netAddr, BanReasonManuallyAdded, banTime, absolute);
+            g_connman->DisconnectNode(netAddr);
+        }
     }
     else if(strCommand == "remove")
     {