Skip to content

Commit 320c5d4

Browse files
MarcoFalkeknst
authored andcommitted
Merge bitcoin#22490: test: Disable automatic connections per default in the functional tests
8ca51af test: Disable automatic connections by default (Martin Zumsande) Pull request description: A node normally doesn't make automatic connections to peers in the functional tests because neither DNS seeds nor hardcoded peers are available on regtest. However, when random entries are inserted into addrman as part of a functional test (e.g. while testing addr relay), `ThreadOpenConnections` will periodically try to connect to them, resulting in log entries such as: `[opencon] [net.cpp:400] [ConnectNode] trying connection 18.166.1.1:8333 lastseen=0.0hrs` I don't think it's desirable that functional tests try to connect to random computers on the internet, aside from the possibility that at some point in time someone out there might actually answer in a way to ruin a test. This PR fixes this problem by disabling `ThreadOpenConnections` by adding `-connect=0` to the default args, and adding exceptions only when needed for the test to pass. ACKs for top commit: tryphe: Concept ACK, light code review ACK 8ca51af Tree-SHA512: bcfb2de610e6c35a97a2bd7ad6267e968b1ac7529638d99276993cd5bc93ce9919d54e22d6dc84e1b02ecd626ab6554e201693552ea065c29794eece38c43f7d
1 parent f6198ab commit 320c5d4

File tree

4 files changed

+17
-7
lines changed

4 files changed

+17
-7
lines changed

test/functional/feature_anchors.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
class AnchorsTest(BitcoinTestFramework):
2121
def set_test_params(self):
2222
self.num_nodes = 1
23+
self.disable_autoconnect = False
2324

2425
def run_test(self):
2526
node_anchors_path = os.path.join(

test/functional/feature_config_args.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ def set_test_params(self):
1717
self.num_nodes = 1
1818
self.supports_cli = False
1919
self.wallet_names = []
20+
self.disable_autoconnect = False
2021

2122
def test_config_file_parser(self):
2223
self.log.info('Test config file parser')

test/functional/test_framework/test_framework.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,9 @@ def __init__(self):
142142
# By default the wallet is not required. Set to true by skip_if_no_wallet().
143143
# When False, we ignore wallet_names regardless of what it is.
144144
self.requires_wallet = False
145+
# Disable ThreadOpenConnections by default, so that adding entries to
146+
# addrman will not result in automatic connections to them.
147+
self.disable_autoconnect = True
145148
self.set_test_params()
146149
assert self.wallet_names is None or len(self.wallet_names) <= self.num_nodes
147150
if self.options.timeout_scale != 1:
@@ -455,7 +458,7 @@ def setup_network(self):
455458
def setup_nodes(self):
456459
"""Override this method to customize test node setup"""
457460

458-
"""If this method is updated - backport changes to DashTestFramework.setup_nodes"""
461+
""" NOTE! If this method is updated - backport changes to DashTestFramework.setup_nodes"""
459462
self.add_nodes(self.num_nodes, self.extra_args)
460463
self.start_nodes()
461464
if self.requires_wallet:
@@ -947,7 +950,7 @@ def _initialize_chain(self):
947950
if not os.path.isdir(cache_node_dir):
948951
self.log.debug("Creating cache directory {}".format(cache_node_dir))
949952

950-
initialize_datadir(self.options.cachedir, CACHE_NODE_ID, self.chain)
953+
initialize_datadir(self.options.cachedir, CACHE_NODE_ID, self.chain, self.disable_autoconnect)
951954
self.nodes.append(
952955
TestNode(
953956
CACHE_NODE_ID,
@@ -1012,15 +1015,15 @@ def cache_path(*paths):
10121015
self.log.debug("Copy cache directory {} to node {}".format(cache_node_dir, i))
10131016
to_dir = get_datadir_path(self.options.tmpdir, i)
10141017
shutil.copytree(cache_node_dir, to_dir)
1015-
initialize_datadir(self.options.tmpdir, i, self.chain) # Overwrite port/rpcport in dash.conf
1018+
initialize_datadir(self.options.tmpdir, i, self.chain, self.disable_autoconnect) # Overwrite port/rpcport in dash.conf
10161019

10171020
def _initialize_chain_clean(self):
10181021
"""Initialize empty blockchain for use by the test.
10191022
10201023
Create an empty blockchain and num_nodes wallets.
10211024
Useful if a test case wants complete control over initialization."""
10221025
for i in range(self.num_nodes):
1023-
initialize_datadir(self.options.tmpdir, i, self.chain)
1026+
initialize_datadir(self.options.tmpdir, i, self.chain, self.disable_autoconnect)
10241027

10251028
def skip_if_no_py3_zmq(self):
10261029
"""Attempt to import the zmq package and skip the test if the import fails."""
@@ -1518,6 +1521,9 @@ def set_dash_test_params(self, num_nodes, masterodes_count, extra_args=None, evo
15181521
extra_args = [[]] * num_nodes
15191522
assert_equal(len(extra_args), num_nodes)
15201523
self.extra_args = [copy.deepcopy(a) for a in extra_args]
1524+
# masternodes creates connections for quorums by ThreadOpenConnections too
1525+
# it can't be disabled for DashTestFramework same as BitcoinTestFramework
1526+
self.disable_autoconnect = False
15211527

15221528
# LLMQ default test params (no need to pass -llmqtestparams)
15231529
self.llmq_size = 3

test/functional/test_framework/util.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -371,17 +371,17 @@ def rpc_url(datadir, i, chain, rpchost=None):
371371
################
372372

373373

374-
def initialize_datadir(dirname, n, chain):
374+
def initialize_datadir(dirname, n, chain, disable_autoconnect=True):
375375
datadir = get_datadir_path(dirname, n)
376376
if not os.path.isdir(datadir):
377377
os.makedirs(datadir)
378-
write_config(os.path.join(datadir, "dash.conf"), n=n, chain=chain)
378+
write_config(os.path.join(datadir, "dash.conf"), n=n, chain=chain, disable_autoconnect=disable_autoconnect)
379379
os.makedirs(os.path.join(datadir, 'stderr'), exist_ok=True)
380380
os.makedirs(os.path.join(datadir, 'stdout'), exist_ok=True)
381381
return datadir
382382

383383

384-
def write_config(config_path, *, n, chain, extra_config=""):
384+
def write_config(config_path, *, n, chain, extra_config="", disable_autoconnect=True):
385385
(chain_name_conf_arg, chain_name_conf_arg_value, chain_name_conf_section) = get_chain_conf_names(chain)
386386
with open(config_path, 'w', encoding='utf8') as f:
387387
if chain_name_conf_arg:
@@ -408,6 +408,8 @@ def write_config(config_path, *, n, chain, extra_config=""):
408408
f.write("shrinkdebugfile=0\n")
409409
# To improve SQLite wallet performance so that the tests don't timeout, use -unsafesqlitesync
410410
f.write("unsafesqlitesync=1\n")
411+
if disable_autoconnect:
412+
f.write("connect=0\n")
411413
f.write(extra_config)
412414

413415

0 commit comments

Comments
 (0)