Merge bitcoin/bitcoin#33231: net: Prevent node from binding to the same CService

4d4789d net: Prevent node from binding to the same CService (woltx)

Pull request description:

  Currently, if the node inadvertently starts with repeated `-bind` options (e.g. `./build/bin/bitcoind -listen -bind=0.0.0.0 -bind=0.0.0.0`), the user will receive a misleading message followed by the node shutdown:

  ```
  [net:error] Unable to bind to 0.0.0.0:8333 on this computer. Bitcoin Core is probably already running.
  [error] Unable to bind to 0.0.0.0:8333 on this computer. Bitcoin Core is probably already running.
  ```

  And the user might spend some time looking for a `bitcoind` process or what application is using port 8333, when what happens is that Bitcoin Core successfully connected to port 8333 and then tries again, generating this fatal error.

  This PR proposes that repeated `-bind` options have no effect.

ACKs for top commit:
  l0rinc:
    ACK 4d4789d
  yuvicc:
    re-ACK 4d4789d
  sipa:
    utACK 4d4789d
  achow101:
    ACK 4d4789d
  vasild:
    ACK 4d4789d
  naiyoma:
    Tested ACK 4d4789d

Tree-SHA512: f1042c00417da16550403cfcb75cb8b12740e67cf92a1d8e3c007ae81fcf741907088a633129ce12a6a48ad07fc9f320602792cafed73ec33f6306cd854514b4

Author: achow101

src/init.cpp

@@ -1233,6 +1233,40 @@ bool CheckHostPortOptions(const ArgsManager& args) {
     return true;
 }
 
+/**
+ * @brief Checks for duplicate bindings across all binding configurations.
+ *
+ * @param[in] conn_options Connection options containing the binding vectors to check
+ * @return std::optional<CService> containing the first duplicate found, or std::nullopt if no duplicates
+ */
+static std::optional<CService> CheckBindingConflicts(const CConnman::Options& conn_options)
+{
+    std::set<CService> seen;
+
+    // Check all whitelisted bindings
+    for (const auto& wb : conn_options.vWhiteBinds) {
+        if (!seen.insert(wb.m_service).second) {
+            return wb.m_service;
+        }
+    }
+
+    // Check regular bindings
+    for (const auto& bind : conn_options.vBinds) {
+        if (!seen.insert(bind).second) {
+            return bind;
+        }
+    }
+
+    // Check onion bindings
+    for (const auto& onion_bind : conn_options.onion_binds) {
+        if (!seen.insert(onion_bind).second) {
+            return onion_bind;
+        }
+    }
+
+    return std::nullopt;
+}
+
 // A GUI user may opt to retry once with do_reindex set if there is a failure during chainstate initialization.
 // The function therefore has to support re-entry.
 static ChainstateLoadResult InitAndLoadChainstate(
@@ -2142,6 +2176,13 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
 
     connOptions.m_i2p_accept_incoming = args.GetBoolArg("-i2pacceptincoming", DEFAULT_I2P_ACCEPT_INCOMING);
 
+    if (auto conflict = CheckBindingConflicts(connOptions)) {
+        return InitError(strprintf(
+            _("Duplicate binding configuration for address %s. "
+                "Please check your -bind, -bind=...=onion and -whitebind settings."),
+                    conflict->ToStringAddrPort()));
+    }
+
     if (!node.connman->Start(scheduler, connOptions)) {
         return false;
     }

test/functional/feature_bind_extra.py

@@ -3,24 +3,26 @@
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """
-Test starting bitcoind with -bind and/or -bind=...=onion and confirm
-that bind happens on the expected ports.
+Test starting bitcoind with -bind and/or -bind=...=onion, confirm that
+it binds to the expected ports, and verify that duplicate or conflicting
+-bind/-whitebind configurations are rejected with a descriptive error.
 """
 
+from itertools import combinations_with_replacement
 from test_framework.netutil import (
     addr_to_hex,
     get_bind_addrs,
 )
 from test_framework.test_framework import (
     BitcoinTestFramework,
 )
+from test_framework.test_node import ErrorMatch
 from test_framework.util import (
     assert_equal,
     p2p_port,
     rpc_port,
 )
 
-
 class BindExtraTest(BitcoinTestFramework):
     def set_test_params(self):
         self.setup_clean_chain = True
@@ -87,5 +89,14 @@ def run_test(self):
             binds = set(filter(lambda e: e[1] != rpc_port(i), binds))
             assert_equal(binds, set(expected_services))
 
+        self.stop_node(0)
+
+        addr = "127.0.0.1:11012"
+        for opt1, opt2 in combinations_with_replacement([f"-bind={addr}", f"-bind={addr}=onion", f"-whitebind=noban@{addr}"], 2):
+            self.nodes[0].assert_start_raises_init_error(
+                        [opt1, opt2],
+                        "Error: Duplicate binding configuration",
+                        match=ErrorMatch.PARTIAL_REGEX)
+
 if __name__ == '__main__':
     BindExtraTest(__file__).main()