Merge #19771: net: Replace enum CConnMan::NumConnections with enum class ConnectionDirection

c77de62 net: Replace enum CConnMan::NumConnections with enum class ConnectionDirection (Luke Dashjr)

Pull request description:

  Refactor split out of #17167

ACKs for top commit:
  practicalswift:
    cr ACK c77de62: patch looks correct & `enum class` is strictly better

Tree-SHA512: 40a1bf69d8ab2651b04ba6adbab789369a5a1a29a64ba764c3e6aab575b7943ea8dfd6e35b0abf5bcffa10e7265f4b523a93aa899c0fd581a84fc51ae5377b90

Author: MarcoFalke

src/interfaces/node.h

@@ -6,9 +6,10 @@
 #define BITCOIN_INTERFACES_NODE_H
 
 #include <amount.h>     // For CAmount
-#include <net.h>        // For CConnman::NumConnections
+#include <net.h>        // For NodeId
 #include <net_types.h>  // For banmap_t
 #include <netaddress.h> // For Network
+#include <netbase.h>    // For ConnectionDirection
 #include <support/allocators/secure.h> // For SecureString
 #include <util/translation.h>
 
@@ -88,7 +89,7 @@ class Node
     virtual bool getProxy(Network net, proxyType& proxy_info) = 0;
 
     //! Get number of connections.
-    virtual size_t getNodeCount(CConnman::NumConnections flags) = 0;
+    virtual size_t getNodeCount(ConnectionDirection flags) = 0;
 
     //! Get stats for connected nodes.
     using NodesStats = std::vector<std::tuple<CNodeStats, bool, CNodeStateStats>>;

src/net.cpp

@@ -2725,15 +2725,15 @@ bool CConnman::RemoveAddedNode(const std::string& strNode)
     return false;
 }
 
-size_t CConnman::GetNodeCount(NumConnections flags)
+size_t CConnman::GetNodeCount(ConnectionDirection flags)
 {
     LOCK(cs_vNodes);
-    if (flags == CConnman::CONNECTIONS_ALL) // Shortcut if we want total
+    if (flags == ConnectionDirection::Both) // Shortcut if we want total
         return vNodes.size();
 
     int nNum = 0;
     for (const auto& pnode : vNodes) {
-        if (flags & (pnode->IsInboundConn() ? CONNECTIONS_IN : CONNECTIONS_OUT)) {
+        if (flags & (pnode->IsInboundConn() ? ConnectionDirection::In : ConnectionDirection::Out)) {
             nNum++;
         }
     }

src/net.h

@@ -17,6 +17,7 @@
 #include <i2p.h>
 #include <net_permissions.h>
 #include <netaddress.h>
+#include <netbase.h>
 #include <optional.h>
 #include <policy/feerate.h>
 #include <protocol.h>
@@ -801,13 +802,6 @@ class CConnman
 {
 public:
 
-    enum NumConnections {
-        CONNECTIONS_NONE = 0,
-        CONNECTIONS_IN = (1U << 0),
-        CONNECTIONS_OUT = (1U << 1),
-        CONNECTIONS_ALL = (CONNECTIONS_IN | CONNECTIONS_OUT),
-    };
-
     struct Options
     {
         ServiceFlags nLocalServices = NODE_NONE;
@@ -976,7 +970,7 @@ class CConnman
      */
     bool AddConnection(const std::string& address, ConnectionType conn_type);
 
-    size_t GetNodeCount(NumConnections num);
+    size_t GetNodeCount(ConnectionDirection);
     void GetNodeStats(std::vector<CNodeStats>& vstats);
     bool DisconnectNode(const std::string& node);
     bool DisconnectNode(const CSubNet& subnet);

src/netbase.h

@@ -18,6 +18,7 @@
 #include <memory>
 #include <stdint.h>
 #include <string>
+#include <type_traits>
 #include <vector>
 
 extern int nConnectTimeout;
@@ -28,6 +29,22 @@ static const int DEFAULT_CONNECT_TIMEOUT = 5000;
 //! -dns default
 static const int DEFAULT_NAME_LOOKUP = true;
 
+enum class ConnectionDirection {
+    None = 0,
+    In = (1U << 0),
+    Out = (1U << 1),
+    Both = (In | Out),
+};
+static inline ConnectionDirection& operator|=(ConnectionDirection& a, ConnectionDirection b) {
+    using underlying = typename std::underlying_type<ConnectionDirection>::type;
+    a = ConnectionDirection(underlying(a) | underlying(b));
+    return a;
+}
+static inline bool operator&(ConnectionDirection a, ConnectionDirection b) {
+    using underlying = typename std::underlying_type<ConnectionDirection>::type;
+    return (underlying(a) & underlying(b));
+}
+
 class proxyType
 {
 public:

src/node/interfaces.cpp

@@ -96,7 +96,7 @@ class NodeImpl : public Node
     bool shutdownRequested() override { return ShutdownRequested(); }
     void mapPort(bool use_upnp, bool use_natpmp) override { StartMapPort(use_upnp, use_natpmp); }
     bool getProxy(Network net, proxyType& proxy_info) override { return GetProxy(net, proxy_info); }
-    size_t getNodeCount(CConnman::NumConnections flags) override
+    size_t getNodeCount(ConnectionDirection flags) override
     {
         return m_context->connman ? m_context->connman->GetNodeCount(flags) : 0;
     }

src/qt/clientmodel.cpp

@@ -71,14 +71,14 @@ ClientModel::~ClientModel()
 
 int ClientModel::getNumConnections(unsigned int flags) const
 {
-    CConnman::NumConnections connections = CConnman::CONNECTIONS_NONE;
+    ConnectionDirection connections = ConnectionDirection::None;
 
     if(flags == CONNECTIONS_IN)
-        connections = CConnman::CONNECTIONS_IN;
+        connections = ConnectionDirection::In;
     else if (flags == CONNECTIONS_OUT)
-        connections = CConnman::CONNECTIONS_OUT;
+        connections = ConnectionDirection::Out;
     else if (flags == CONNECTIONS_ALL)
-        connections = CConnman::CONNECTIONS_ALL;
+        connections = ConnectionDirection::Both;
 
     return m_node.getNodeCount(connections);
 }

src/rpc/mining.cpp

@@ -659,7 +659,7 @@ static RPCHelpMan getblocktemplate()
         throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
 
     if (!Params().IsTestChain()) {
-        if (node.connman->GetNodeCount(CConnman::CONNECTIONS_ALL) == 0) {
+        if (node.connman->GetNodeCount(ConnectionDirection::Both) == 0) {
             throw JSONRPCError(RPC_CLIENT_NOT_CONNECTED, PACKAGE_NAME " is not connected!");
         }
 

src/rpc/net.cpp

@@ -57,7 +57,7 @@ static RPCHelpMan getconnectioncount()
     if(!node.connman)
         throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
 
-    return (int)node.connman->GetNodeCount(CConnman::CONNECTIONS_ALL);
+    return (int)node.connman->GetNodeCount(ConnectionDirection::Both);
 },
     };
 }
@@ -630,9 +630,9 @@ static RPCHelpMan getnetworkinfo()
     obj.pushKV("timeoffset",    GetTimeOffset());
     if (node.connman) {
         obj.pushKV("networkactive", node.connman->GetNetworkActive());
-        obj.pushKV("connections", (int)node.connman->GetNodeCount(CConnman::CONNECTIONS_ALL));
-        obj.pushKV("connections_in", (int)node.connman->GetNodeCount(CConnman::CONNECTIONS_IN));
-        obj.pushKV("connections_out", (int)node.connman->GetNodeCount(CConnman::CONNECTIONS_OUT));
+        obj.pushKV("connections", (int)node.connman->GetNodeCount(ConnectionDirection::Both));
+        obj.pushKV("connections_in", (int)node.connman->GetNodeCount(ConnectionDirection::In));
+        obj.pushKV("connections_out", (int)node.connman->GetNodeCount(ConnectionDirection::Out));
     }
     obj.pushKV("networks",      GetNetworksInfo());
     obj.pushKV("relayfee",      ValueFromAmount(::minRelayTxFee.GetFeePerK()));

src/test/fuzz/connman.cpp

@@ -95,7 +95,7 @@ FUZZ_TARGET_INIT(connman, initialize_connman)
                 (void)connman.GetDeterministicRandomizer(fuzzed_data_provider.ConsumeIntegral<uint64_t>());
             },
             [&] {
-                (void)connman.GetNodeCount(fuzzed_data_provider.PickValueInArray({CConnman::CONNECTIONS_NONE, CConnman::CONNECTIONS_IN, CConnman::CONNECTIONS_OUT, CConnman::CONNECTIONS_ALL}));
+                (void)connman.GetNodeCount(fuzzed_data_provider.PickValueInArray({ConnectionDirection::None, ConnectionDirection::In, ConnectionDirection::Out, ConnectionDirection::Both}));
             },
             [&] {
                 connman.MarkAddressGood(random_address);