test: make reusable SOCKS5 server starting
What changed, and why it matters
This commit is a test-only code cleanup. It moves the setup of a fake SOCKS5 proxy server used in automated tests into a shared helper function and lets the operating system pick an available network port instead of guessing one. It does not change any production Bitcoin Core code or fix a security vulnerability.
No security action needed. Treat as normal test maintenance.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change refactors test/functional/p2p_private_broadcast.py to use a new start_socks5_server() helper in test/functional/test_framework/socks5.py. The helper binds to ('127.0.0.1', 0) so the OS assigns an ephemeral port, removing the previous assumption that p2p_port(self.num_nodes) was free. This is purely a test-framework robustness/cleanup change; no node consensus, networking, or wallet code is modified.
Changed components
test/functional/p2p_private_broadcast.pytest/functional/test_framework/socks5.pyInspect captured patch +15 / −17
diff --git a/test/functional/p2p_private_broadcast.py b/test/functional/p2p_private_broadcast.py
index 1b4c393c..ce630fc1 100755
--- a/test/functional/p2p_private_broadcast.py
+++ b/test/functional/p2p_private_broadcast.py
@@ -29,8 +29,7 @@ from test_framework.netutil import (
)
from test_framework.script_util import build_malleated_tx_package
from test_framework.socks5 import (
- Socks5Configuration,
- Socks5Server,
+ start_socks5_server,
)
from test_framework.test_framework import (
BitcoinTestFramework,
@@ -40,7 +39,6 @@ from test_framework.util import (
assert_greater_than_or_equal,
assert_not_equal,
assert_raises_rpc_error,
- p2p_port,
tor_port,
)
from test_framework.wallet import (
@@ -166,18 +164,6 @@ class P2PPrivateBroadcast(BitcoinTestFramework):
self.num_nodes = 2
def setup_nodes(self):
- # Start a SOCKS5 proxy server.
- socks5_server_config = Socks5Configuration()
- # self.nodes[0] listens on p2p_port(0),
- # self.nodes[1] listens on p2p_port(1),
- # thus we tell the SOCKS5 server to listen on p2p_port(self.num_nodes) (self.num_nodes is 2)
- socks5_server_config.addr = ("127.0.0.1", p2p_port(self.num_nodes))
- socks5_server_config.unauth = True
- socks5_server_config.auth = True
-
- self.socks5_server = Socks5Server(socks5_server_config)
- self.socks5_server.start()
-
self.destinations = []
self.destinations_lock = threading.Lock()
@@ -268,7 +254,7 @@ class P2PPrivateBroadcast(BitcoinTestFramework):
"actual_to_port": actual_to_port,
}
- self.socks5_server.conf.destinations_factory = destinations_factory
+ self.socks5_server = start_socks5_server(destinations_factory)
self.extra_args = [
[
@@ -279,7 +265,7 @@ class P2PPrivateBroadcast(BitcoinTestFramework):
"-v2transport=0",
"-test=addrman",
"-privatebroadcast",
- f"-proxy={socks5_server_config.addr[0]}:{socks5_server_config.addr[1]}",
+ f"-proxy={self.socks5_server.conf.addr[0]}:{self.socks5_server.conf.addr[1]}",
# To increase coverage, make it think that the I2P network is reachable so that it
# selects such addresses as well. Pick a proxy address where nobody is listening
# and connection attempts fail quickly.
diff --git a/test/functional/test_framework/socks5.py b/test/functional/test_framework/socks5.py
index 28aa2e8e..930e0e67 100644
--- a/test/functional/test_framework/socks5.py
+++ b/test/functional/test_framework/socks5.py
@@ -327,3 +327,15 @@ class Socks5Server():
logger.debug(f"Stop(): Handler {i} thread joined")
else:
logger.warning(f"Stop(): Handler thread {i} didn't finish after force close")
+
+def start_socks5_server(destinations_factory):
+ config = Socks5Configuration()
+ config.addr = ("127.0.0.1", 0) # Use port=0 to let the OS pick one. The actual port is later in server.conf.addr[1].
+ config.unauth = True
+ config.auth = True
+ config.destinations_factory = destinations_factory
+
+ server = Socks5Server(config)
+ server.start()
+
+ return server
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.