Set maximum/minimum connection thresholds

This uses the constant in src/net.h for the minimum allowed number of
connections.

The limit of new max connections is silently capped to the number of
available file descriptors. This value is not exposed in the UI or
RPC messages so as not to leak interesting or important system details.

The floor of maximum connections is set to the number of connections
required for this node to operate.

qa/rpc-tests/setmaxconnections.py

@@ -6,6 +6,8 @@
 #
 # Exercise setmaxconnections RPC command
 #
+# from the constant MAX_ADDNODE_CONNECTIONS in src/net.h
+MINIMUM_CONNECTIONS = 8
 
 from test_framework.mininode import *
 from test_framework.test_framework import BitcoinTestFramework
@@ -63,9 +65,9 @@ class SetMaxConnectionCountTest (BitcoinTestFramework):
     def run_test(self):
         self.test_rpc_argument_validation()
 
-        self.test_node_connection_changes(5)
-        self.test_node_connection_changes(10)
-        self.test_node_connection_changes(3)
+        self.test_node_connection_changes(12)
+        self.test_node_connection_changes(20)
+        self.test_node_connection_changes(9)
 
         # max_count has to be at least 20
         # min_count can be closer to 20
@@ -78,7 +80,7 @@ class SetMaxConnectionCountTest (BitcoinTestFramework):
             first_node.setmaxconnections()
             raise AssertionError("Must check for no parameter provided")
         except JSONRPCException as e:
-            assert("1. \"maxconnectioncount\"" in e.error['message'])
+            assert("1. maxconnectioncount" in e.error['message'])
 
         try:
             first_node.setmaxconnections("good doge bad doge")
@@ -88,21 +90,27 @@ class SetMaxConnectionCountTest (BitcoinTestFramework):
 
         try:
             first_node.setmaxconnections(-1)
-            raise AssertionError("Must check for parameter value >= 0")
+            raise AssertionError(f"Must check for parameter value >= {MINIMUM_CONNECTIONS}")
         except JSONRPCException as e:
-            assert("maxconnectioncount must be >= 0" in e.error['message'])
+            assert(f"maxconnectioncount must be >= {MINIMUM_CONNECTIONS}" in e.error['message'])
 
         try:
-            first_node.setmaxconnections(0)
+            first_node.setmaxconnections(7)
+            raise AssertionError(f"Must check for parameter value >= {MINIMUM_CONNECTIONS}")
+        except JSONRPCException as e:
+            assert(f"maxconnectioncount must be >= {MINIMUM_CONNECTIONS}" in e.error['message'])
+
+        try:
+            first_node.setmaxconnections(MINIMUM_CONNECTIONS)
             assert(True)
         except JSONRPCException as e:
-            raise AssertionError("Must allow parameter value >= 0")
+            raise AssertionError(f"Must allow parameter value >= {MINIMUM_CONNECTIONS}")
 
     def test_node_connection_changes(self, extras):
         first_node = self.nodes[0]
 
-        # 9 is 8 outgoing connections plus 1 feeler
-        first_node.setmaxconnections(9 + extras)
+        # MINIMUM_CONNECTIONS outgoing connections plus 1 feeler
+        first_node.setmaxconnections(1 + MINIMUM_CONNECTIONS + extras)
         client_nodes = []
 
         self.connect_nodes(client_nodes, extras)
@@ -119,17 +127,17 @@ class SetMaxConnectionCountTest (BitcoinTestFramework):
         for node in client_nodes:
             node.close()
 
-        first_node.setmaxconnections(0)
+        first_node.setmaxconnections(MINIMUM_CONNECTIONS)
         x = first_node.getconnectioncount()
-        assert(x == 0)
+        assert(x <= MINIMUM_CONNECTIONS)
 
     def test_node_disconnections(self, max_count, min_count):
         first_node = self.nodes[0]
 
         attempted_nodes = []
 
-        # 9 is 8 outgoing connections plus 1 feeler
-        first_node.setmaxconnections(9 + max_count)
+        # MINIMUM_CONNECTIONS outgoing connections plus 1 feeler
+        first_node.setmaxconnections(1 + MINIMUM_CONNECTIONS + max_count)
         client_nodes = []
 
         self.connect_nodes(client_nodes, max_count)

src/init.cpp

@@ -803,6 +803,7 @@ ServiceFlags nRelevantServices = NODE_NETWORK;
 int nMaxConnections;
 int nUserMaxConnections;
 int nFD;
+int nAvailableFds;
 ServiceFlags nLocalServices = NODE_NETWORK;
 
 }
@@ -903,7 +904,8 @@ bool AppInitParameterInteraction()
     nFD = RaiseFileDescriptorLimit(nMaxConnections + MIN_CORE_FILEDESCRIPTORS + MAX_ADDNODE_CONNECTIONS);
     if (nFD < MIN_CORE_FILEDESCRIPTORS)
         return InitError(_("Not enough file descriptors available."));
-    nMaxConnections = std::min(nFD - MIN_CORE_FILEDESCRIPTORS - MAX_ADDNODE_CONNECTIONS, nMaxConnections);
+    nAvailableFds = nFD - MIN_CORE_FILEDESCRIPTORS - MAX_ADDNODE_CONNECTIONS;
+    nMaxConnections = std::min(nAvailableFds, nMaxConnections);
 
     if (nMaxConnections < nUserMaxConnections)
         InitWarning(strprintf(_("Reducing -maxconnections from %d to %d, because of system limitations."), nUserMaxConnections, nMaxConnections));
@@ -1660,6 +1662,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
     connOptions.nLocalServices = nLocalServices;
     connOptions.nRelevantServices = nRelevantServices;
     connOptions.nMaxConnections = nMaxConnections;
+    connOptions.nAvailableFds = nAvailableFds;
     connOptions.nMaxOutbound = std::min(MAX_OUTBOUND_CONNECTIONS, connOptions.nMaxConnections);
     connOptions.nMaxAddnode = MAX_ADDNODE_CONNECTIONS;
     connOptions.nMaxFeeler = 1;

src/net.cpp

@@ -1408,7 +1408,7 @@ void CConnman::ThreadSocketHandler()
         //
         if (vNodesCopy.size() > (size_t)nMaxConnections)
         {
-            LogPrintf("%s: attempting to reduce connections: max=%u current=%u", __func__, vNodesCopy.size(), nMaxConnections);
+            LogPrintf("%s: attempting to reduce connections: max=%u current=%u\n", __func__, nMaxConnections, vNodesCopy.size());
             DisconnectUnusedNodes();
             DeleteDisconnectedNodes();
             AttemptToEvictConnection();
@@ -1424,7 +1424,11 @@ void CConnman::ThreadSocketHandler()
 
 void CConnman::SetMaxConnections(int newMaxConnections)
 {
-    nMaxConnections = newMaxConnections;
+    newMaxConnections = std::max(newMaxConnections, MAX_ADDNODE_CONNECTIONS);
+    nMaxConnections = std::min(newMaxConnections, nAvailableFds);
+    if (nMaxConnections != newMaxConnections) {
+        LogPrintf("%s: capped new maxconnections request of %d to %d\n", __func__, newMaxConnections, nMaxConnections);
+    }
 }
 
 
@@ -2226,6 +2230,7 @@ CConnman::CConnman(uint64_t nSeed0In, uint64_t nSeed1In) : nSeed0(nSeed0In), nSe
     semAddnode = NULL;
     nMaxConnections = 0;
     nMaxOutbound = 0;
+    nAvailableFds = 0;
     nMaxAddnode = 0;
     nBestHeight = 0;
     clientInterface = NULL;
@@ -2248,6 +2253,7 @@ bool CConnman::Start(CScheduler& scheduler, std::string& strNodeError, Options c
     nMaxOutbound = std::min((connOptions.nMaxOutbound), nMaxConnections);
     nMaxAddnode = connOptions.nMaxAddnode;
     nMaxFeeler = connOptions.nMaxFeeler;
+    nAvailableFds = connOptions.nAvailableFds;
 
     nSendBufferMaxSize = connOptions.nSendBufferMaxSize;
     nReceiveFloodSize = connOptions.nReceiveFloodSize;

src/net.h

@@ -140,6 +140,7 @@ public:
         int nMaxOutbound = 0;
         int nMaxAddnode = 0;
         int nMaxFeeler = 0;
+        int nAvailableFds = 0;
         int nBestHeight = 0;
         CClientUIInterface* uiInterface = nullptr;
         unsigned int nSendBufferMaxSize = 0;
@@ -388,6 +389,7 @@ private:
     int nMaxOutbound;
     int nMaxAddnode;
     int nMaxFeeler;
+    int nAvailableFds;
     std::atomic<int> nBestHeight;
     CClientUIInterface* clientInterface;
 

src/rpc/net.cpp

@@ -48,24 +48,25 @@ UniValue getconnectioncount(const JSONRPCRequest& request)
 UniValue setmaxconnections(const JSONRPCRequest& request)
 {
     int newMaxCount = 0;
+    const std::string minConnCount = to_string(MAX_ADDNODE_CONNECTIONS);
 
     if (request.fHelp || request.params.size() != 1)
         throw runtime_error(
             "setmaxconnections\n"
             "\nSets the maximum number of connections to other nodes.\n"
             "\nArguments:\n"
-            "1. \"maxconnectioncount\"     (numeric, required) The new maximum connection count (must be >= 0)\n"
+            "1. maxconnectioncount     (numeric, required) The new maximum connection count (must be >= " + minConnCount + ")\n"
             "\nResult:\n"
             "n          (boolean) True or false connection count\n"
             "\nExamples:\n"
             + HelpExampleCli("setmaxconnections", "20")
-            + HelpExampleRpc("setmaxconnections", "0")
+            + HelpExampleRpc("setmaxconnections", minConnCount)
         );
     else
         newMaxCount = request.params[0].get_int();
 
-    if (newMaxCount < 0)
-        throw JSONRPCError(RPC_INVALID_PARAMETER, "Error: maxconnectioncount must be >= 0");
+    if (newMaxCount < MAX_ADDNODE_CONNECTIONS)
+        throw JSONRPCError(RPC_INVALID_PARAMETER, "Error: maxconnectioncount must be >= " + minConnCount);
 
     if(!g_connman)
         throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");