[net/refactor] Simplify multiple-connection checks

amitiuttarwar · amitiuttarwar · commit 2f2e13b6c2c8 · 2020-08-07T17:18:16.000-07:00
Extract logic that check multiple connection types into interface functions &amp;
structure as switch statements. This makes it very clear what touch points are
for accessing `m_conn_type` &amp; using the switch statements enables the compiler
to warn if a new connection type is introduced but not handled for these cases.
diff --git a/src/net.cpp b/src/net.cpp
@@ -1648,7 +1648,7 @@ void CConnman::ThreadDNSAddressSeed()
                     {
                         LOCK(cs_vNodes);
                         for (const CNode* pnode : vNodes) {
-                            nRelevant += pnode->fSuccessfullyConnected && !pnode->IsFeelerConn() && !pnode->IsAddrFetchConn() && !pnode->IsManualConn() && !pnode->IsInboundConn();
+                            if (pnode->fSuccessfullyConnected && pnode->IsOutboundOrBlockRelayConn()) ++nRelevant;
                         }
                     }
                     if (nRelevant >= 2) {
@@ -1758,7 +1758,7 @@ int CConnman::GetExtraOutboundCount()
     {
         LOCK(cs_vNodes);
         for (const CNode* pnode : vNodes) {
-            if (!pnode->IsInboundConn() && !pnode->IsManualConn() && !pnode->IsFeelerConn() && !pnode->fDisconnect && !pnode->IsAddrFetchConn() && pnode->fSuccessfullyConnected) {
+            if (pnode->fSuccessfullyConnected && !pnode->fDisconnect && pnode->IsOutboundOrBlockRelayConn()) {
                 ++nOutbound;
             }
         }
diff --git a/src/net.h b/src/net.h
@@ -789,6 +789,21 @@ class CNode
     std::atomic_bool fPauseRecv{false};
     std::atomic_bool fPauseSend{false};
 
+    bool IsOutboundOrBlockRelayConn() const {
+        switch(m_conn_type) {
+            case ConnectionType::OUTBOUND:
+            case ConnectionType::BLOCK_RELAY:
+                return true;
+            case ConnectionType::INBOUND:
+            case ConnectionType::MANUAL:
+            case ConnectionType::ADDR_FETCH:
+            case ConnectionType::FEELER:
+                return false;
+        }
+
+        assert(false);
+    }
+
     bool IsFullOutboundConn() const {
         return m_conn_type == ConnectionType::OUTBOUND;
     }
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
@@ -829,7 +829,7 @@ void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds)
 
 static bool IsOutboundDisconnectionCandidate(const CNode& node)
 {
-    return !(node.IsInboundConn() || node.IsManualConn() || node.IsFeelerConn() || node.IsAddrFetchConn());
+    return node.IsOutboundOrBlockRelayConn();
 }
 
 void PeerLogicValidation::InitializeNode(CNode *pnode) {
@@ -2324,7 +2324,7 @@ void ProcessMessage(
         {
             connman.SetServices(pfrom.addr, nServices);
         }
-        if (!pfrom.IsInboundConn() && !pfrom.IsFeelerConn() && !pfrom.IsManualConn() && !HasAllDesirableServiceFlags(nServices))
+        if (pfrom.ExpectServicesFromConn() && !HasAllDesirableServiceFlags(nServices))
         {
             LogPrint(BCLog::NET, "peer=%d does not offer the expected services (%08x offered, %08x expected); disconnecting\n", pfrom.GetId(), nServices, GetDesirableServiceFlags(nServices));
             pfrom.fDisconnect = true;

Original file line number	Diff line number	Diff line change
`@@ -1648,7 +1648,7 @@ void CConnman::ThreadDNSAddressSeed()`
`1648`	`1648`	`{`
`1649`	`1649`	`LOCK(cs_vNodes);`
`1650`	`1650`	`for (const CNode* pnode : vNodes) {`
`1651`		`- nRelevant += pnode->fSuccessfullyConnected && !pnode->IsFeelerConn() && !pnode->IsAddrFetchConn() && !pnode->IsManualConn() && !pnode->IsInboundConn();`
	`1651`	`+ if (pnode->fSuccessfullyConnected && pnode->IsOutboundOrBlockRelayConn()) ++nRelevant;`
`1652`	`1652`	`}`
`1653`	`1653`	`}`
`1654`	`1654`	`if (nRelevant >= 2) {`
`@@ -1758,7 +1758,7 @@ int CConnman::GetExtraOutboundCount()`
`1758`	`1758`	`{`
`1759`	`1759`	`LOCK(cs_vNodes);`
`1760`	`1760`	`for (const CNode* pnode : vNodes) {`
`1761`		`- if (!pnode->IsInboundConn() && !pnode->IsManualConn() && !pnode->IsFeelerConn() && !pnode->fDisconnect && !pnode->IsAddrFetchConn() && pnode->fSuccessfullyConnected) {`
	`1761`	`+ if (pnode->fSuccessfullyConnected && !pnode->fDisconnect && pnode->IsOutboundOrBlockRelayConn()) {`
`1762`	`1762`	`++nOutbound;`
`1763`	`1763`	`}`
`1764`	`1764`	`}`
Original file line number	Diff line number	Diff line change
`@@ -829,7 +829,7 @@ void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds)`
`829`	`829`
`830`	`830`	`static bool IsOutboundDisconnectionCandidate(const CNode& node)`
`831`	`831`	`{`
`832`		`- return !(node.IsInboundConn() \|\| node.IsManualConn() \|\| node.IsFeelerConn() \|\| node.IsAddrFetchConn());`
	`832`	`+ return node.IsOutboundOrBlockRelayConn();`
`833`	`833`	`}`
`834`	`834`
`835`	`835`	`void PeerLogicValidation::InitializeNode(CNode *pnode) {`
`@@ -2324,7 +2324,7 @@ void ProcessMessage(`
`2324`	`2324`	`{`
`2325`	`2325`	`connman.SetServices(pfrom.addr, nServices);`
`2326`	`2326`	`}`
`2327`		`- if (!pfrom.IsInboundConn() && !pfrom.IsFeelerConn() && !pfrom.IsManualConn() && !HasAllDesirableServiceFlags(nServices))`
	`2327`	`+ if (pfrom.ExpectServicesFromConn() && !HasAllDesirableServiceFlags(nServices))`
`2328`	`2328`	`{`
`2329`	`2329`	`LogPrint(BCLog::NET, "peer=%d does not offer the expected services (%08x offered, %08x expected); disconnecting\n", pfrom.GetId(), nServices, GetDesirableServiceFlags(nServices));`
`2330`	`2330`	`pfrom.fDisconnect = true;`