fix: a couple of fixes for the way mnauth and probe nodes work (#5660)

## Issue being fixed or feature implemented
1. inactive MNs (`activeMasternodeInfo.proTxHash.IsNull() == true`)
should simply drop duplicated connections like regular nodes do.
2. we should not instantly drop inbound (potentially probe) connections
(even if `DeterministicOutboundConnection` results would say so), should
let `CMasternodeUtils::DoMaintenance` do that. This way a probing peer
should have a chance to get our `mnauth` back and mark this attempt as a
success. This should hopefully reduce the number of random unexplained
pose-punishments.
3. probe nodes must be disconnected ignoring everything else, quorum
nodes and relay members connect using their own logic which should not
interfere with the way probe nodes work. (meaningful changes only:
9134d964a0)

## What was done?
pls see individual commits

as a side-effect `activeMasternodeInfoCs` lock is moved out of
`ForEachNode`

## How Has This Been Tested?
run tests, run a testnet mn

## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_

src/evo/mnauth.cpp

@@ -133,6 +133,8 @@ void CMNAuth::ProcessMessage(CNode& peer, PeerManager& peerman, CConnman& connma
         }
     }
 
+    const uint256 myProTxHash = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash);
+
     connman.ForEachNode([&](CNode* pnode2) {
         if (peer.fDisconnect) {
             // we've already disconnected the new peer
@@ -140,17 +142,19 @@ void CMNAuth::ProcessMessage(CNode& peer, PeerManager& peerman, CConnman& connma
         }
 
         if (pnode2->GetVerifiedProRegTxHash() == mnauth.proRegTxHash) {
-            if (fMasternodeMode) {
+            if (fMasternodeMode && !myProTxHash.IsNull()) {
-                const auto deterministicOutbound = WITH_LOCK(activeMasternodeInfoCs, return llmq::utils::DeterministicOutboundConnection(activeMasternodeInfo.proTxHash, mnauth.proRegTxHash));
+                const auto deterministicOutbound = llmq::utils::DeterministicOutboundConnection(myProTxHash, mnauth.proRegTxHash);
                 LogPrint(BCLog::NET_NETCONN, "CMNAuth::ProcessMessage -- Masternode %s has already verified as peer %d, deterministicOutbound=%s. peer=%d\n",
                          mnauth.proRegTxHash.ToString(), pnode2->GetId(), deterministicOutbound.ToString(), peer.GetId());
-                if (WITH_LOCK(activeMasternodeInfoCs, return deterministicOutbound == activeMasternodeInfo.proTxHash)) {
+                if (deterministicOutbound == myProTxHash) {
+                    // NOTE: do not drop inbound nodes here, mark them as probes so that
+                    // they would be disconnected later in CMasternodeUtils::DoMaintenance
                     if (pnode2->fInbound) {
-                        LogPrint(BCLog::NET_NETCONN, "CMNAuth::ProcessMessage -- dropping old inbound, peer=%d\n", pnode2->GetId());
+                        LogPrint(BCLog::NET_NETCONN, "CMNAuth::ProcessMessage -- marking old inbound for dropping it later, peer=%d\n", pnode2->GetId());
-                        pnode2->fDisconnect = true;
+                        pnode2->m_masternode_probe_connection = true;
                     } else if (peer.fInbound) {
-                        LogPrint(BCLog::NET_NETCONN, "CMNAuth::ProcessMessage -- dropping new inbound, peer=%d\n", peer.GetId());
+                        LogPrint(BCLog::NET_NETCONN, "CMNAuth::ProcessMessage -- marking new inbound for dropping it later, peer=%d\n", peer.GetId());
-                        peer.fDisconnect = true;
+                        peer.m_masternode_probe_connection = true;
                     }
                 } else {
                     if (!pnode2->fInbound) {

src/masternode/utils.cpp

@@ -46,6 +46,10 @@ void CMasternodeUtils::DoMaintenance(CConnman& connman, const CMasternodeSync& m
     }
 
     connman.ForEachNode(CConnman::AllNodes, [&](CNode* pnode) {
+        if (pnode->m_masternode_probe_connection) {
+            // we're not disconnecting masternode probes for at least PROBE_WAIT_INTERVAL seconds
+            if (GetSystemTimeInSeconds() - pnode->nTimeConnected < PROBE_WAIT_INTERVAL) return;
+        } else {
             // we're only disconnecting m_masternode_connection connections
             if (!pnode->m_masternode_connection) return;
             if (!pnode->GetVerifiedProRegTxHash().IsNull()) {
@@ -68,8 +72,7 @@ void CMasternodeUtils::DoMaintenance(CConnman& connman, const CMasternodeSync& m
                 // keep watching nodes
                 return;
             }
-        // we're not disconnecting masternode probes for at least a few seconds
+        }
-        if (pnode->m_masternode_probe_connection && GetSystemTimeInSeconds() - pnode->nTimeConnected < 5) return;
 
 #ifdef ENABLE_WALLET
         bool fFound = ranges::any_of(vecDmns, [&pnode](const auto& dmn){ return pnode->addr == dmn->pdmnState->addr; });

src/net.h

@@ -55,6 +55,8 @@ static const bool DEFAULT_WHITELISTFORCERELAY = false;
 
 /** Time after which to disconnect, after waiting for a ping response (or inactivity). */
 static const int TIMEOUT_INTERVAL = 20 * 60;
+/** Time to wait since nTimeConnected before disconnecting a probe node. **/
+static const int PROBE_WAIT_INTERVAL = 5;
 /** Minimum time between warnings printed to log. */
 static const int WARNING_INTERVAL = 10 * 60;
 /** Run the feeler connection loop once every 2 minutes or 120 seconds. **/
@@ -1103,7 +1105,10 @@ public:
     bool fSentAddr{false};
     // If 'true' this node will be disconnected on CMasternodeMan::ProcessMasternodeConnections()
     std::atomic<bool> m_masternode_connection{false};
-    // If 'true' this node will be disconnected after MNAUTH
+    /**
+     * If 'true' this node will be disconnected after MNAUTH (outbound only) or
+     * after PROBE_WAIT_INTERVAL seconds since nTimeConnected
+     */
     std::atomic<bool> m_masternode_probe_connection{false};
     // If 'true', we identified it as an intra-quorum relay connection
     std::atomic<bool> m_masternode_iqr_connection{false};