bitcoin · ryanofsky · Sep 25, 2024 · Jul 30, 2024 · Aug 4, 2019 · Sep 21, 2022
diff --git a/doc/release-notes-30529.md b/doc/release-notes-30529.md
@@ -0,0 +1,4 @@
+Configuration
+-------------
+
+Some corner cases handling negated list options `-norpcwhitelist`, `-norpcallowip`, `-norpcbind`, `-nobind`, `-nowhitebind`, `-noexternalip`, `-noonlynet`, `-noseednode`, `-nosignetchallenge`, `-nosignetseednode`, `-notest`, and `-norpcwallet` have been fixed. Now negating these options is the same as not specifying them at all.
diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp
@@ -1184,7 +1184,7 @@ static void ParseGetInfoResult(UniValue& result)
 static UniValue GetNewAddress()
 {
     std::optional<std::string> wallet_name{};
-    if (gArgs.IsArgSet("-rpcwallet")) wallet_name = gArgs.GetArg("-rpcwallet", "");
+    if (gArgs.IsArgSet("-rpcwallet") && !gArgs.IsArgNegated("-rpcwallet")) wallet_name = gArgs.GetArg("-rpcwallet", "");
     DefaultRequestHandler rh;
     return ConnectAndCallRPC(&rh, "getnewaddress", /* args=*/{}, wallet_name);
 }
@@ -1292,15 +1292,15 @@ static int CommandLineRPC(int argc, char *argv[])
         if (nRet == 0) {
             // Perform RPC call
             std::optional<std::string> wallet_name{};
-            if (gArgs.IsArgSet("-rpcwallet")) wallet_name = gArgs.GetArg("-rpcwallet", "");
+            if (gArgs.IsArgSet("-rpcwallet") && !gArgs.IsArgNegated("-rpcwallet")) wallet_name = gArgs.GetArg("-rpcwallet", "");
             const UniValue reply = ConnectAndCallRPC(rh.get(), method, args, wallet_name);
 
             // Parse reply
             UniValue result = reply.find_value("result");
             const UniValue& error = reply.find_value("error");
             if (error.isNull()) {
                 if (gArgs.GetBoolArg("-getinfo", false)) {
-                    if (!gArgs.IsArgSet("-rpcwallet")) {
+                    if (!gArgs.IsArgSet("-rpcwallet") || gArgs.IsArgNegated("-rpcwallet")) {
                         GetWalletBalances(result); // fetch multiwallet balances and append to result
                     }
                     ParseGetInfoResult(result);

diff --git a/src/chainparams.cpp b/src/chainparams.cpp
@@ -25,10 +25,10 @@ using util::SplitString;
 
 void ReadSigNetArgs(const ArgsManager& args, CChainParams::SigNetOptions& options)
 {
-    if (args.IsArgSet("-signetseednode")) {
+    if (!args.GetArgs("-signetseednode").empty()) {
         options.seeds.emplace(args.GetArgs("-signetseednode"));
     }
-    if (args.IsArgSet("-signetchallenge")) {
+    if (!args.GetArgs("-signetchallenge").empty()) {
         const auto signet_challenge = args.GetArgs("-signetchallenge");
         if (signet_challenge.size() != 1) {
             throw std::runtime_error("-signetchallenge cannot be multiple values.");
@@ -66,8 +66,6 @@ void ReadRegTestArgs(const ArgsManager& args, CChainParams::RegTestOptions& opti
         }
     }
 
-    if (!args.IsArgSet("-vbparams")) return;
-
     for (const std::string& strDeployment : args.GetArgs("-vbparams")) {
         std::vector<std::string> vDeploymentParams = SplitString(strDeployment, ':');
         if (vDeploymentParams.size() < 3 || 4 < vDeploymentParams.size()) {

diff --git a/src/httprpc.cpp b/src/httprpc.cpp
@@ -334,7 +334,7 @@ static bool InitRPCAuthentication()
         }
     }
 
-    g_rpc_whitelist_default = gArgs.GetBoolArg("-rpcwhitelistdefault", gArgs.IsArgSet("-rpcwhitelist"));
+    g_rpc_whitelist_default = gArgs.GetBoolArg("-rpcwhitelistdefault", !gArgs.GetArgs("-rpcwhitelist").empty());
     for (const std::string& strRPCWhitelist : gArgs.GetArgs("-rpcwhitelist")) {
         auto pos = strRPCWhitelist.find(':');
         std::string strUser = strRPCWhitelist.substr(0, pos);

diff --git a/src/httpserver.cpp b/src/httpserver.cpp
@@ -362,16 +362,20 @@ static bool HTTPBindAddresses(struct evhttp* http)
     std::vector<std::pair<std::string, uint16_t>> endpoints;
 
     // Determine what addresses to bind to
-    if (!(gArgs.IsArgSet("-rpcallowip") && gArgs.IsArgSet("-rpcbind"))) { // Default to loopback if not allowing external IPs
+    // To prevent misconfiguration and accidental exposure of the RPC
+    // interface, require -rpcallowip and -rpcbind to both be specified
+    // together. If either is missing, ignore both values, bind to localhost
+    // instead, and log warnings.
+    if (gArgs.GetArgs("-rpcallowip").empty() || gArgs.GetArgs("-rpcbind").empty()) { // Default to loopback if not allowing external IPs
         endpoints.emplace_back("::1", http_port);
         endpoints.emplace_back("127.0.0.1", http_port);
-        if (gArgs.IsArgSet("-rpcallowip")) {
+        if (!gArgs.GetArgs("-rpcallowip").empty()) {
             LogPrintf("WARNING: option -rpcallowip was specified without -rpcbind; this doesn't usually make sense\n");
         }
-        if (gArgs.IsArgSet("-rpcbind")) {
+        if (!gArgs.GetArgs("-rpcbind").empty()) {
             LogPrintf("WARNING: option -rpcbind was ignored because -rpcallowip was not specified, refusing to allow everyone to connect\n");
         }
-    } else if (gArgs.IsArgSet("-rpcbind")) { // Specific bind address
+    } else { // Specific bind addresses
         for (const std::string& strRPCBind : gArgs.GetArgs("-rpcbind")) {
             uint16_t port{http_port};
             std::string host;

diff --git a/src/init.cpp b/src/init.cpp
@@ -714,17 +714,18 @@ void InitParameterInteraction(ArgsManager& args)
 {
     // when specifying an explicit binding address, you want to listen on it
     // even when -connect or -proxy is specified
-    if (args.IsArgSet("-bind")) {
+    if (!args.GetArgs("-bind").empty()) {
         if (args.SoftSetBoolArg("-listen", true))
             LogInfo("parameter interaction: -bind set -> setting -listen=1\n");
     }
-    if (args.IsArgSet("-whitebind")) {
+    if (!args.GetArgs("-whitebind").empty()) {
         if (args.SoftSetBoolArg("-listen", true))
             LogInfo("parameter interaction: -whitebind set -> setting -listen=1\n");
     }
 
-    if (args.IsArgSet("-connect") || args.GetIntArg("-maxconnections", DEFAULT_MAX_PEER_CONNECTIONS) <= 0) {
+    if (!args.GetArgs("-connect").empty() || args.IsArgNegated("-connect") || args.GetIntArg("-maxconnections", DEFAULT_MAX_PEER_CONNECTIONS) <= 0) {
         // when only connecting to trusted nodes, do not seed via DNS, or listen by default
+        // do the same when connections are disabled
         if (args.SoftSetBoolArg("-dnsseed", false))
             LogInfo("parameter interaction: -connect or -maxconnections=0 set -> setting -dnsseed=0\n");
         if (args.SoftSetBoolArg("-listen", false))
@@ -760,7 +761,7 @@ void InitParameterInteraction(ArgsManager& args)
         }
     }
 
-    if (args.IsArgSet("-externalip")) {
+    if (!args.GetArgs("-externalip").empty()) {
         // if an explicit public IP is specified, do not try to find others
         if (args.SoftSetBoolArg("-discover", false))
             LogInfo("parameter interaction: -externalip set -> setting -discover=0\n");
@@ -780,8 +781,8 @@ void InitParameterInteraction(ArgsManager& args)
         if (args.SoftSetBoolArg("-whitelistrelay", true))
             LogInfo("parameter interaction: -whitelistforcerelay=1 -> setting -whitelistrelay=1\n");
     }
-    if (args.IsArgSet("-onlynet")) {
-        const auto onlynets = args.GetArgs("-onlynet");
+    const auto onlynets = args.GetArgs("-onlynet");
+    if (!onlynets.empty()) {
         bool clearnet_reachable = std::any_of(onlynets.begin(), onlynets.end(), [](const auto& net) {
             const auto n = ParseNetwork(net);
             return n == NET_IPV4 || n == NET_IPV6;
@@ -1025,12 +1026,12 @@ bool AppInitParameterInteraction(const ArgsManager& args)
     if (args.GetBoolArg("-peerbloomfilters", DEFAULT_PEERBLOOMFILTERS))
         g_local_services = ServiceFlags(g_local_services | NODE_BLOOM);
 
-    if (args.IsArgSet("-test")) {
+    const std::vector<std::string> test_options = args.GetArgs("-test");
+    if (!test_options.empty()) {
         if (chainparams.GetChainType() != ChainType::REGTEST) {
             return InitError(Untranslated("-test=<option> can only be used with regtest"));
         }
-        const std::vector<std::string> options = args.GetArgs("-test");
-        for (const std::string& option : options) {
+        for (const std::string& option : test_options) {
             auto it = std::find_if(TEST_OPTIONS_DOC.begin(), TEST_OPTIONS_DOC.end(), [&option](const std::string& doc_option) {
                 size_t pos = doc_option.find(" (");
                 return (pos != std::string::npos) && (doc_option.substr(0, pos) == option);
@@ -1471,7 +1472,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
             strSubVersion.size(), MAX_SUBVERSION_LENGTH));
     }
 
-    if (args.IsArgSet("-onlynet")) {
+    if (!args.GetArgs("-onlynet").empty()) {
         g_reachable_nets.RemoveAll();
         for (const std::string& snet : args.GetArgs("-onlynet")) {
             enum Network net = ParseNetwork(snet);
@@ -1482,7 +1483,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     }
 
     if (!args.IsArgSet("-cjdnsreachable")) {
-        if (args.IsArgSet("-onlynet") && g_reachable_nets.Contains(NET_CJDNS)) {
+        if (!args.GetArgs("-onlynet").empty() && g_reachable_nets.Contains(NET_CJDNS)) {
             return InitError(
                 _("Outbound connections restricted to CJDNS (-onlynet=cjdns) but "
                   "-cjdnsreachable is not provided"));
@@ -1533,7 +1534,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
         onion_proxy = addrProxy;
     }
 
-    const bool onlynet_used_with_onion{args.IsArgSet("-onlynet") && g_reachable_nets.Contains(NET_ONION)};
+    const bool onlynet_used_with_onion{!args.GetArgs("-onlynet").empty() && g_reachable_nets.Contains(NET_ONION)};
 
     // -onion can be used to set only a proxy for .onion, or override normal proxy for .onion addresses
     // -noonion (or -onion=0) disables connecting to .onion entirely
@@ -1942,10 +1943,12 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
 
     connOptions.vSeedNodes = args.GetArgs("-seednode");
 
-    // Initiate outbound connections unless connect=0
-    connOptions.m_use_addrman_outgoing = !args.IsArgSet("-connect");
-    if (!connOptions.m_use_addrman_outgoing) {
-        const auto connect = args.GetArgs("-connect");
+    const auto connect = args.GetArgs("-connect");
+    if (!connect.empty() || args.IsArgNegated("-connect")) {
+        // Do not initiate other outgoing connections when connecting to trusted
+        // nodes, or when -noconnect is specified.
+        connOptions.m_use_addrman_outgoing = false;
+
         if (connect.size() != 1 || connect[0] != "0") {
             connOptions.m_specified_outgoing = connect;
         }
@@ -1966,7 +1969,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
         }
         SetProxy(NET_I2P, Proxy{addr.value()});
     } else {
-        if (args.IsArgSet("-onlynet") && g_reachable_nets.Contains(NET_I2P)) {
+        if (!args.GetArgs("-onlynet").empty() && g_reachable_nets.Contains(NET_I2P)) {
             return InitError(
                 _("Outbound connections restricted to i2p (-onlynet=i2p) but "
                   "-i2psam is not provided"));

diff --git a/src/init/common.cpp b/src/init/common.cpp
@@ -58,7 +58,6 @@ void SetLoggingOptions(const ArgsManager& args)
 
 util::Result<void> SetLoggingLevel(const ArgsManager& args)
 {
-    if (args.IsArgSet("-loglevel")) {
         for (const std::string& level_str : args.GetArgs("-loglevel")) {
             if (level_str.find_first_of(':', 3) == std::string::npos) {
                 // user passed a global log level, i.e. -loglevel=<level>
@@ -73,14 +72,12 @@ util::Result<void> SetLoggingLevel(const ArgsManager& args)
                 }
             }
         }
-    }
     return {};
 }
 
 util::Result<void> SetLoggingCategories(const ArgsManager& args)
 {
-    if (args.IsArgSet("-debug")) {
-        // Special-case: if -debug=0/-nodebug is set, turn off debugging messages
+        // Special-case: if -debug=0/-debug=none is set, turn off debugging messages
         const std::vector<std::string> categories = args.GetArgs("-debug");
 
         if (std::none_of(categories.begin(), categories.end(),
@@ -91,7 +88,6 @@ util::Result<void> SetLoggingCategories(const ArgsManager& args)
                 }
             }
         }
-    }
 
     // Now remove the logging categories which were explicitly excluded
     for (const std::string& cat : args.GetArgs("-debugexclude")) {

diff --git a/src/net.cpp b/src/net.cpp
@@ -2190,7 +2190,7 @@ void CConnman::ThreadDNSAddressSeed()
 {
     int outbound_connection_count = 0;
 
-    if (gArgs.IsArgSet("-seednode")) {
+    if (!gArgs.GetArgs("-seednode").empty()) {
         auto start = NodeClock::now();
         constexpr std::chrono::seconds SEEDNODE_TIMEOUT = 30s;
         LogPrintf("-seednode enabled. Trying the provided seeds for %d seconds before defaulting to the dnsseeds.\n", SEEDNODE_TIMEOUT.count());
@@ -2493,7 +2493,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect, Spa
     auto next_extra_network_peer{start + rng.rand_exp_duration(EXTRA_NETWORK_PEER_INTERVAL)};
     const bool dnsseed = gArgs.GetBoolArg("-dnsseed", DEFAULT_DNSSEED);
     bool add_fixed_seeds = gArgs.GetBoolArg("-fixedseeds", DEFAULT_FIXEDSEEDS);
-    const bool use_seednodes{gArgs.IsArgSet("-seednode")};
+    const bool use_seednodes{!gArgs.GetArgs("-seednode").empty()};
 
     auto seed_node_timer = NodeClock::now();
     bool add_addr_fetch{addrman.Size() == 0 && !seed_nodes.empty()};

diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp
@@ -403,7 +403,7 @@ void TorController::get_socks_cb(TorControlConnection& _conn, const TorControlRe
     const auto onlynets = gArgs.GetArgs("-onlynet");
 
     const bool onion_allowed_by_onlynet{
-        !gArgs.IsArgSet("-onlynet") ||
+        onlynets.empty() ||
         std::any_of(onlynets.begin(), onlynets.end(), [](const auto& n) {
             return ParseNetwork(n) == NET_ONION;
         })};

diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py
@@ -367,6 +367,8 @@ def test_connect_with_seednode(self):
         seednode_ignored = ['-seednode is ignored when -connect is used\n']
         dnsseed_ignored = ['-dnsseed is ignored when -connect is used and -proxy is specified\n']
         addcon_thread_started = ['addcon thread start\n']
+        dnsseed_disabled = "parameter interaction: -connect or -maxconnections=0 set -> setting -dnsseed=0"
+        listen_disabled = "parameter interaction: -connect or -maxconnections=0 set -> setting -listen=0"
 
         # When -connect is supplied, expanding addrman via getaddr calls to ADDR_FETCH(-seednode)
         # nodes is irrelevant and -seednode is ignored.
@@ -391,6 +393,25 @@ def test_connect_with_seednode(self):
             with self.nodes[0].assert_debug_log(expected_msgs=addcon_thread_started,
                     unexpected_msgs=seednode_ignored):
                 self.restart_node(0, extra_args=[connect_arg, '-seednode=fakeaddress2'])
+
+        # Make sure -noconnect soft-disables -listen and -dnsseed.
+        # Need to temporarily remove these settings from the config file in
+        # order for the two log messages to appear
+        self.nodes[0].replace_in_config([("bind=", "#bind="), ("dnsseed=", "#dnsseed=")])
+        with self.nodes[0].assert_debug_log(expected_msgs=addcon_thread_started) as info:
+            self.restart_node(0, extra_args=[connect_arg])
+        for msg in [dnsseed_disabled, listen_disabled]:
+            if msg not in info.log:
+                raise AssertionError(f"Expected {msg!r} in -noconnect log message")
+        self.nodes[0].replace_in_config([("#bind=", "bind="), ("#dnsseed=", "dnsseed=")])
+
+        # Make sure -proxy and -noconnect warn about -dnsseed setting being
+        # ignored, just like -proxy and -connect do.
+        with self.nodes[0].assert_debug_log(expected_msgs=addcon_thread_started) as info:
+            self.restart_node(0, extra_args=[connect_arg, '-dnsseed', '-proxy=localhost:1080'])
+        for msg in dnsseed_ignored:
+            if msg not in info.log:
+                raise AssertionError(f"Expected {msg!r} in -noconnect log message")
         self.stop_node(0)
 
     def test_ignored_conf(self):

diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py
@@ -17,6 +17,7 @@
 import subprocess
 import tempfile
 import time
+import types
 import urllib.parse
 import collections
 import shlex
@@ -498,7 +499,8 @@ def assert_debug_log(self, expected_msgs, unexpected_msgs=None, timeout=2):
         time_end = time.time() + timeout * self.timeout_factor
         prev_size = self.debug_log_size(encoding="utf-8")  # Must use same encoding that is used to read() below
 
-        yield
+        info = types.SimpleNamespace()
+        yield info
 
         while True:
             found = True
@@ -513,6 +515,7 @@ def assert_debug_log(self, expected_msgs, unexpected_msgs=None, timeout=2):
                 if re.search(re.escape(expected_msg), log, flags=re.MULTILINE) is None:
                     found = False
             if found:
+                info.log = log
                 return
             if time.time() >= time_end:
                 break