Repository: bitcoin/bitcoin (language: cpp)

Issue:
net: Tor service target port collides when running multiple nodes, making bitcoind error out
Various people have reported this on IRC after the 28.0 release, but have not created a GH issue, so i'm creating one.

To be able to distinguish incoming localhost connections from connections on the Tor hidden (onion) service, we automatically create a separate "Tor service target" P2P port. This port is always opened, even if `-listenonion=0`, because the user might manually configure a Tor hidden service and wants the same behavior.

[DefaultOnionServiceTarget()](https://github.com/bitcoin/bitcoin/blob/master/src/torcontrol.cpp#L714) creates the CService for the Tor service target port. This number is fixed in the chainparams to [8334](https://github.com/bitcoin/bitcoin/blob/master/src/chainparamsbase.cpp#L44) for mainnet, on localhost.

Normally, this is no problem. However a common scenario is to run multiple bitcoin nodes on one machine (usually for testing) with different P2P and RPC ports. In this case, they will all try to bind to port 8334 and the latter ones will fail. This problem has existed since the Tor service port was introduced, however it became visible in v0.28 with #22729. After that PR it becomes a crash issue.
(it *was* a problem before because multiple `bitcoind` instances would try to bind on port `8334` while ignoring failures, but still have the wrong internal knowledge that they bound on port 8334, passing this to Tor. So all of Tor's connections would go to the first node)

*The current suggested workaround* is to manually set `-bind=...`. This removes the automatic onion binds (although explicit `-bind=.....=onion` can be used to create new ones). i have tagged this issue with milestone 28.1 as it could be considered a regression and would be nice to have this automatically work again (especially in cases where the user doesn't care  about Tor, like in regtest harnesses).

Various ideas to resolve this have been discussed, among which:

1. Revert #22729: i strongly dislike this, as it would only bury the problem again. Explicit failures are always better than having to debug silent loss of functionality.
2. Instead of hardcoding port 8334, make it `P2P_port + 1`:  When overriding the `-port`, the Tor service target port changes at the same time. This resolves the problem automatically, except in the edge case where the different `bitcoind` instances' P2P ports are specified very closely together (and maybe it's just better to avoid that in the first place).
3. Disable the Tor service port binding when `-listenonion=0`: This avoids the problem when the user doesn't care about Onion binds, however it means that an extra option has to be set, it's still not nice automatic behavior. Might as well set `-bind=` then.
4. Disable the Tor service port binding when `-regtest`: In case of regtest, it's uncommon to care about Tor. However not always; this would interfere with out own tests when we want to test interaction with Tor. It also doesn't solve the issue in case of sigtest/mainnet.

(some more, from @mzumsande in #31134)

5. Do nothing and tell affected users to switch to `-bind` - maybe even deprecate `-port`
6.  if `-port` is given, don't attempt to bind to the default onion port
7. disable the automatic tor binding silently if the port is already used

i personally prefer 2. It keeps the current behavior, except when port is overridden, and doesn't require any special workarounds.
net: Tor service target port collides when running multiple nodes, making bitcoind error out
Various people have reported this on IRC after the 28.0 release, but have not created a GH issue, so i'm creating one.

To be able to distinguish incoming localhost connections from connections on the Tor hidden (onion) service, we automatically create a separate "Tor service target" P2P port. This port is always opened, even if `-listenonion=0`, because the user might manually configure a Tor hidden service and wants the same behavior.

[DefaultOnionServiceTarget()](https://github.com/bitcoin/bitcoin/blob/master/src/torcontrol.cpp#L714) creates the CService for the Tor service target port. This number is fixed in the chainparams to [8334](https://github.com/bitcoin/bitcoin/blob/master/src/chainparamsbase.cpp#L44) for mainnet, on localhost.

Normally, this is no problem. However a common scenario is to run multiple bitcoin nodes on one machine (usually for testing) with different P2P and RPC ports. In this case, they will all try to bind to port 8334 and the latter ones will fail. This problem has existed since the Tor service port was introduced, however it became visible in v0.28 with #22729. After that PR it becomes a crash issue.
(it *was* a problem before because multiple `bitcoind` instances would try to bind on port `8334` while ignoring failures, but still have the wrong internal knowledge that they bound on port 8334, passing this to Tor. So all of Tor's connections would go to the first node)

*The current suggested workaround* is to manually set `-bind=...`. This removes the automatic onion binds (although explicit `-bind=.....=onion` can be used to create new ones). i have tagged this issue with milestone 28.1 as it could be considered a regression and would be nice to have this automatically work again (especially in cases where the user doesn't care  about Tor, like in regtest harnesses).

Various ideas to resolve this have been discussed, among which:

1. Revert #22729: i strongly dislike this, as it would only bury the problem again. Explicit failures are always better than having to debug silent loss of functionality.
2. Instead of hardcoding port 8334, make it `P2P_port + 1`:  When overriding the `-port`, the Tor service target port changes at the same time. This resolves the problem automatically, except in the edge case where the different `bitcoind` instances' P2P ports are specified very closely together (and maybe it's just better to avoid that in the first place).
3. Disable the Tor service port binding when `-listenonion=0`: This avoids the problem when the user doesn't care about Onion binds, however it means that an extra option has to be set, it's still not nice automatic behavior. Might as well set `-bind=` then.
4. Disable the Tor service port binding when `-regtest`: In case of regtest, it's uncommon to care about Tor. However not always; this would interfere with out own tests when we want to test interaction with Tor. It also doesn't solve the issue in case of sigtest/mainnet.

(some more, from @mzumsande in #31134)

5. Do nothing and tell affected users to switch to `-bind` - maybe even deprecate `-port`
6.  if `-port` is given, don't attempt to bind to the default onion port
7. disable the automatic tor binding silently if the port is already used

i personally prefer 2. It keeps the current behavior, except when port is overridden, and doesn't require any special workarounds.


Discussion / hints:
cc @stevenroose 
I do think that keeping `-port` around would be helpful, especially for users less knowledgeable about networking, who aren't sure which address to put in an equivalent `-bind` command.

I also like option 2, provided that the behavior will be well-documented, so that users specifying `-port` values for multiple nodes don't pick consecutive ones by accident (which would then again lead to collisions). Otherwise I don't see a downside.

I would also be ok with option 6, if I specify a port like that I probably wouldn't necessarily expect another bind to a fixed port for onion.

I'm in the camp of deprecating and then removing the `-port` setting. It seems we should should just get people to migrate to bind. Is `-port` strictly a subset of `-bind` functionality? If so it seems simplifying the config options would make sense. 
I am in favor of 2. (port+1) or 5. (do nothing) in the short term, for 28.1.

For long term, maybe deprecate and remove `port=` (yes, I think it is a subset of `bind=`, somebody correct me if I am wrong) in favor of `bind=...:port`. I agree that `port=` is "helpful, especially for users less knowledgeable about networking", maybe to resolve that extend the syntax of `bind=` to allow `bind=*:8333`. That is used in at least Apache configs: `<VirtualHost *:80>`.
cc @stevenroose 
I do think that keeping `-port` around would be helpful, especially for users less knowledgeable about networking, who aren't sure which address to put in an equivalent `-bind` command.

I also like option 2, provided that the behavior will be well-documented, so that users specifying `-port` values for multiple nodes don't pick consecutive ones by accident (which would then again lead to collisions). Otherwise I don't see a downside.

I would also be ok with option 6, if I specify a port like that I probably wouldn't necessarily expect another bind to a fixed port for onion.

I'm in the camp of deprecating and then removing the `-port` setting. It seems we should should just get people to migrate to bind. Is `-port` strictly a subset of `-bind` functionality? If so it seems simplifying the config options would make sense. 
I am in favor of 2. (port+1) or 5. (do nothing) in the short term, for 28.1.

For long term, maybe deprecate and remove `port=` (yes, I think it is a subset of `bind=`, somebody correct me if I am wrong) in favor of `bind=...:port`. I agree that `port=` is "helpful, especially for users less knowledgeable about networking", maybe to resolve that extend the syntax of `bind=` to allow `bind=*:8333`. That is used in at least Apache configs: `<VirtualHost *:80>`.

Fix patch:
diff --git a/doc/release-notes-31223.md b/doc/release-notes-31223.md
new file mode 100644
index 0000000000000..44f0552fd9335
--- /dev/null
+++ b/doc/release-notes-31223.md
@@ -0,0 +1,15 @@
+P2P and network changes
+-----------------------
+When the `-port` configuration option is used, the default onion listening port will now
+be derived to be that port + 1 instead of being set to a fixed value (8334 on mainnet).
+This re-allows setups with multiple local nodes using different `-port` and not using `-bind`,
+which would lead to a startup failure in v28.0 due to a port collision.
+
+Note that a `HiddenServicePort` manually configured in `torrc` may need adjustment if used in
+connection with the `-port` option.
+For example, if you are using `-port=5555` with a non-standard value and not using `-bind=...=onion`,
+previously Bitcoin Core would listen for incoming Tor connections on `127.0.0.1:8334`.
+Now it would listen on `127.0.0.1:5556` (`-port` plus one). If you configured the hidden service manually
+in torrc now you have to change it from `HiddenServicePort 8333 127.0.0.1:8334` to `HiddenServicePort 8333
+127.0.0.1:5556`, or configure bitcoind with `-bind=127.0.0.1:8334=onion` to get the previous behavior.
+(#31223)
diff --git a/src/chainparamsbase.cpp b/src/chainparamsbase.cpp
index aadd04e509f22..060d519d9207c 100644
--- a/src/chainparamsbase.cpp
+++ b/src/chainparamsbase.cpp
@@ -41,15 +41,15 @@ std::unique_ptr<CBaseChainParams> CreateBaseChainParams(const ChainType chain)
 {
     switch (chain) {
     case ChainType::MAIN:
-        return std::make_unique<CBaseChainParams>("", 8332, 8334);
+        return std::make_unique<CBaseChainParams>("", 8332);
     case ChainType::TESTNET:
-        return std::make_unique<CBaseChainParams>("testnet3", 18332, 18334);
+        return std::make_unique<CBaseChainParams>("testnet3", 18332);
     case ChainType::TESTNET4:
-        return std::make_unique<CBaseChainParams>("testnet4", 48332, 48334);
+        return std::make_unique<CBaseChainParams>("testnet4", 48332);
     case ChainType::SIGNET:
-        return std::make_unique<CBaseChainParams>("signet", 38332, 38334);
+        return std::make_unique<CBaseChainParams>("signet", 38332);
     case ChainType::REGTEST:
-        return std::make_unique<CBaseChainParams>("regtest", 18443, 18445);
+        return std::make_unique<CBaseChainParams>("regtest", 18443);
     }
     assert(false);
 }
diff --git a/src/chainparamsbase.h b/src/chainparamsbase.h
index c75a70cb96b00..adbd6a5174695 100644
--- a/src/chainparamsbase.h
+++ b/src/chainparamsbase.h
@@ -22,15 +22,13 @@ class CBaseChainParams
 public:
     const std::string& DataDir() const { return strDataDir; }
     uint16_t RPCPort() const { return m_rpc_port; }
-    uint16_t OnionServiceTargetPort() const { return m_onion_service_target_port; }
 
     CBaseChainParams() = delete;
-    CBaseChainParams(const std::string& data_dir, uint16_t rpc_port, uint16_t onion_service_target_port)
-        : m_rpc_port(rpc_port), m_onion_service_target_port(onion_service_target_port), strDataDir(data_dir) {}
+    CBaseChainParams(const std::string& data_dir, uint16_t rpc_port)
+        : m_rpc_port(rpc_port), strDataDir(data_dir) {}
 
 private:
     const uint16_t m_rpc_port;
-    const uint16_t m_onion_service_target_port;
     std::string strDataDir;
 };
 
diff --git a/src/init.cpp b/src/init.cpp
index 94a5a084638cc..8c4ae2e62d393 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -521,7 +521,7 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc)
     argsman.AddArg("-addnode=<ip>", strprintf("Add a node to connect to and attempt to keep the connection open (see the addnode RPC help for more info). This option can be specified multiple times to add multiple nodes; connections are limited to %u at a time and are counted separately from the -maxconnections limit.", MAX_ADDNODE_CONNECTIONS), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION);
     argsman.AddArg("-asmap=<file>", strprintf("Specify asn mapping used for bucketing of the peers (default: %s). Relative paths will be prefixed by the net-specific datadir location.", DEFAULT_ASMAP_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
     argsman.AddArg("-bantime=<n>", strprintf("Default duration (in seconds) of manually configured bans (default: %u)", DEFAULT_MISBEHAVING_BANTIME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
-    argsman.AddArg("-bind=<addr>[:<port>][=onion]", strprintf("Bind to given address and always listen on it (default: 0.0.0.0). Use [host]:port notation for IPv6. Append =onion to tag any incoming connections to that address and port as incoming Tor connections (default: 127.0.0.1:%u=onion, testnet3: 127.0.0.1:%u=onion, testnet4: 127.0.0.1:%u=onion, signet: 127.0.0.1:%u=onion, regtest: 127.0.0.1:%u=onion)", defaultBaseParams->OnionServiceTargetPort(), testnetBaseParams->OnionServiceTargetPort(), testnet4BaseParams->OnionServiceTargetPort(), signetBaseParams->OnionServiceTargetPort(), regtestBaseParams->OnionServiceTargetPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION);
+    argsman.AddArg("-bind=<addr>[:<port>][=onion]", strprintf("Bind to given address and always listen on it (default: 0.0.0.0). Use [host]:port notation for IPv6. Append =onion to tag any incoming connections to that address and port as incoming Tor connections (default: 127.0.0.1:%u=onion, testnet3: 127.0.0.1:%u=onion, testnet4: 127.0.0.1:%u=onion, signet: 127.0.0.1:%u=onion, regtest: 127.0.0.1:%u=onion)", defaultChainParams->GetDefaultPort() + 1, testnetChainParams->GetDefaultPort() + 1, testnet4ChainParams->GetDefaultPort() + 1, signetChainParams->GetDefaultPort() + 1, regtestChainParams->GetDefaultPort() + 1), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION);
     argsman.AddArg("-cjdnsreachable", "If set, then this host is configured for CJDNS (connecting to fc00::/8 addresses would lead us to the CJDNS network, see doc/cjdns.md) (default: 0)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
     argsman.AddArg("-connect=<ip>", "Connect only to the specified node; -noconnect disables automatic connections (the rules for this peer are the same as for -addnode). This option can be specified multiple times to connect to multiple nodes.", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION);
     argsman.AddArg("-discover", "Discover own IP addresses (default: 1 when listening and no -externalip or -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
@@ -548,7 +548,7 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc)
     argsman.AddArg("-peerbloomfilters", strprintf("Support filtering of blocks and transaction with bloom filters (default: %u)", DEFAULT_PEERBLOOMFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
     argsman.AddArg("-peerblockfilters", strprintf("Serve compact block filters to peers per BIP 157 (default: %u)", DEFAULT_PEERBLOCKFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
     argsman.AddArg("-txreconciliation", strprintf("Enable transaction reconciliations per BIP 330 (default: %d)", DEFAULT_TXRECONCILIATION_ENABLE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CONNECTION);
-    argsman.AddArg("-port=<port>", strprintf("Listen for connections on <port> (default: %u, testnet3: %u, testnet4: %u, signet: %u, regtest: %u). Not relevant for I2P (see doc/i2p.md).", defaultChainParams->GetDefaultPort(), testnetChainParams->GetDefaultPort(), testnet4ChainParams->GetDefaultPort(), signetChainParams->GetDefaultPort(), regtestChainParams->GetDefaultPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION);
+    argsman.AddArg("-port=<port>", strprintf("Listen for connections on <port> (default: %u, testnet3: %u, testnet4: %u, signet: %u, regtest: %u). Not relevant for I2P (see doc/i2p.md). If set to a value x, the default onion listening port will be set to x+1.", defaultChainParams->GetDefaultPort(), testnetChainParams->GetDefaultPort(), testnet4ChainParams->GetDefaultPort(), signetChainParams->GetDefaultPort(), regtestChainParams->GetDefaultPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION);
 #ifdef HAVE_SOCKADDR_UN
     argsman.AddArg("-proxy=<ip:port|path>", "Connect through SOCKS5 proxy, set -noproxy to disable (default: disabled). May be a local file path prefixed with 'unix:' if the proxy supports it.", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_ELISION, OptionsCategory::CONNECTION);
 #else
@@ -1846,6 +1846,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     const uint16_t default_bind_port =
         static_cast<uint16_t>(args.GetIntArg("-port", Params().GetDefaultPort()));
 
+    const uint16_t default_bind_port_onion = default_bind_port + 1;
+
     const auto BadPortWarning = [](const char* prefix, uint16_t port) {
         return strprintf(_("%s request to listen on port %u. This port is considered \"bad\" and "
                            "thus it is unlikely that any peer will connect to it. See "
@@ -1870,7 +1872,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
             const std::string network_type = bind_arg.substr(index + 1);
             if (network_type == "onion") {
                 const std::string truncated_bind_arg = bind_arg.substr(0, index);
-                bind_addr = Lookup(truncated_bind_arg, BaseParams().OnionServiceTargetPort(), false);
+                bind_addr = Lookup(truncated_bind_arg, default_bind_port_onion, false);
                 if (bind_addr.has_value()) {
                     connOptions.onion_binds.push_back(bind_addr.value());
                     continue;
@@ -1906,7 +1908,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     } else if (!connOptions.vBinds.empty()) {
         onion_service_target = connOptions.vBinds.front();
     } else {
-        onion_service_target = DefaultOnionServiceTarget();
+        onion_service_target = DefaultOnionServiceTarget(default_bind_port_onion);
         connOptions.onion_binds.push_back(onion_service_target);
     }
 
diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp
index 58fc1bdf2aa02..60df916f166c6 100644
--- a/src/torcontrol.cpp
+++ b/src/torcontrol.cpp
@@ -711,9 +711,9 @@ void StopTorControl()
     }
 }
 
-CService DefaultOnionServiceTarget()
+CService DefaultOnionServiceTarget(uint16_t port)
 {
     struct in_addr onion_service_target;
     onion_service_target.s_addr = htonl(INADDR_LOOPBACK);
-    return {onion_service_target, BaseParams().OnionServiceTargetPort()};
+    return {onion_service_target, port};
 }
diff --git a/src/torcontrol.h b/src/torcontrol.h
index 4a0eef223e41d..0b66201cf1a46 100644
--- a/src/torcontrol.h
+++ b/src/torcontrol.h
@@ -27,7 +27,7 @@ void StartTorControl(CService onion_service_target);
 void InterruptTorControl();
 void StopTorControl();
 
-CService DefaultOnionServiceTarget();
+CService DefaultOnionServiceTarget(uint16_t port);
 
 /** Reply from Tor, can be single or multi-line */
 class TorControlReply


Test patch:
diff --git a/test/functional/feature_port.py b/test/functional/feature_port.py
new file mode 100755
index 0000000000000..2746d7d79c1d6
--- /dev/null
+++ b/test/functional/feature_port.py
@@ -0,0 +1,60 @@
+#!/usr/bin/env python3
+# Copyright (c) 2024-present The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""
+Test the -port option and its interactions with
+-bind.
+"""
+
+from test_framework.test_framework import (
+    BitcoinTestFramework,
+)
+from test_framework.util import (
+    p2p_port,
+)
+
+
+class PortTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.setup_clean_chain = True
+        # Avoid any -bind= on the command line.
+        self.bind_to_localhost_only = False
+        self.num_nodes = 1
+
+    def run_test(self):
+        node = self.nodes[0]
+        node.has_explicit_bind = True
+        port1 = p2p_port(self.num_nodes)
+        port2 = p2p_port(self.num_nodes + 5)
+
+        self.log.info("When starting with -port, bitcoind binds to it and uses port + 1 for an onion bind")
+        with node.assert_debug_log(expected_msgs=[f'Bound to 0.0.0.0:{port1}', f'Bound to 127.0.0.1:{port1 + 1}']):
+            self.restart_node(0, extra_args=["-listen", f"-port={port1}"])
+
+        self.log.info("When specifying -port multiple times, only the last one is taken")
+        with node.assert_debug_log(expected_msgs=[f'Bound to 0.0.0.0:{port2}', f'Bound to 127.0.0.1:{port2 + 1}'], unexpected_msgs=[f'Bound to 0.0.0.0:{port1}']):
+            self.restart_node(0, extra_args=["-listen", f"-port={port1}", f"-port={port2}"])
+
+        self.log.info("When specifying ports with both -port and -bind, the one from -port is ignored")
+        with node.assert_debug_log(expected_msgs=[f'Bound to 0.0.0.0:{port2}'], unexpected_msgs=[f'Bound to 0.0.0.0:{port1}']):
+            self.restart_node(0, extra_args=["-listen", f"-port={port1}", f"-bind=0.0.0.0:{port2}"])
+
+        self.log.info("When -bind specifies no port, the values from -port and -bind are combined")
+        with self.nodes[0].assert_debug_log(expected_msgs=[f'Bound to 0.0.0.0:{port1}']):
+            self.restart_node(0, extra_args=["-listen", f"-port={port1}", "-bind=0.0.0.0"])
+
+        self.log.info("When an onion bind specifies no port, the value from -port, incremented by 1, is taken")
+        with self.nodes[0].assert_debug_log(expected_msgs=[f'Bound to 127.0.0.1:{port1 + 1}']):
+            self.restart_node(0, extra_args=["-listen", f"-port={port1}", "-bind=127.0.0.1=onion"])
+
+        self.log.info("Invalid values for -port raise errors")
+        self.stop_node(0)
+        node.extra_args = ["-listen", "-port=65536"]
+        node.assert_start_raises_init_error(expected_msg="Error: Invalid port specified in -port: '65536'")
+        node.extra_args = ["-listen", "-port=0"]
+        node.assert_start_raises_init_error(expected_msg="Error: Invalid port specified in -port: '0'")
+
+
+if __name__ == '__main__':
+    PortTest(__file__).main()
diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py
index f1083dc1ce6bf..0749860c8e662 100755
--- a/test/functional/test_framework/test_node.py
+++ b/test/functional/test_framework/test_node.py
@@ -221,10 +221,9 @@ def start(self, extra_args=None, *, cwd=None, stdout=None, stderr=None, env=None
             extra_args = self.extra_args
 
         # If listening and no -bind is given, then bitcoind would bind P2P ports on
-        # 0.0.0.0:P and 127.0.0.1:18445 (for incoming Tor connections), where P is
+        # 0.0.0.0:P and 127.0.0.1:P+1 (for incoming Tor connections), where P is
         # a unique port chosen by the test framework and configured as port=P in
-        # bitcoin.conf. To avoid collisions on 127.0.0.1:18445, change it to
-        # 127.0.0.1:tor_port().
+        # bitcoin.conf. To avoid collisions, change it to 127.0.0.1:tor_port().
         will_listen = all(e != "-nolisten" and e != "-listen=0" for e in extra_args)
         has_explicit_bind = self.has_explicit_bind or any(e.startswith("-bind=") for e in extra_args)
         if will_listen and not has_explicit_bind:
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index 701d81b9d25eb..cb621b537466f 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -339,6 +339,7 @@
     'feature_minchainwork.py',
     'rpc_estimatefee.py',
     'rpc_getblockstats.py',
+    'feature_port.py',
     'feature_bind_port_externalip.py',
     'wallet_create_tx.py --legacy-wallet',
     'wallet_send.py --legacy-wallet',