Skip to content

Commit 10fbb37

Browse files
committed
Merge bitcoin/bitcoin#22098: [test, init] DNS seed querying logic
82b6f89 [style] Small style improvements to DNS parameters (Amiti Uttarwar) 4c89e24 [test] Test the delay before querying DNS seeds (Amiti Uttarwar) 6395c8e [test] Test the interactions between -forcednsseed and -dnsseed (Amiti Uttarwar) 6f6b7df [init] Disallow starting up with conflicting paramters for -dnsseed and -forcednsseed (Amiti Uttarwar) 26d0ffe [test] Test -forcednsseed causes querying DNS seeds (Amiti Uttarwar) 3585145 [test] Test the interactions between -connect and -dnsseed (Amiti Uttarwar) 75c05af [test] Test logic to query DNS seeds with block-relay-only connections (Amiti Uttarwar) 9c08719 [test] Introduce test logic to query DNS seeds (Amiti Uttarwar) Pull request description: This PR adds a DNS seed to the regtest chain params to enable testing the DNS seed querying logic of `CConnman::ThreadDNSAddressSeed` and relevant startup parameters. Adds coverage for the changes in #22013 (and then some). The main behavioral change to bitcoind is that this PR disallows starting up with conflicting parameters for `-dnsseed` and `-forcednsseed`. The tests include: * parameter interactions of different combinations of `-connect`, `-dnsseed` and `-forcednsseed` * the delay before querying DNS seeds depending on how many addresses are in the addrman * the behavior of `-forcednsseed` * skipping DNS querying if we have outbound full relay connections & not block-relay-only connections Huge props to mzumsande for identifying the timing technique for testing successful connections before running `ThreadDNSAddressSeed` 🙌🏽 ACKs for top commit: mzumsande: ACK 82b6f89 jnewbery: reACK 82b6f89 Tree-SHA512: 9f0c29bfbf99426727e79c0a25606ae09deab91a92e3c5cee7f84c3ca7503a8ac9ab85a85c51841d40b164ef8c991326070f0b2f41d075fb7985df26f6e95d6d
2 parents 06788c6 + 82b6f89 commit 10fbb37

File tree

6 files changed

+144
-7
lines changed

6 files changed

+144
-7
lines changed

src/chainparams.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,8 @@ class CRegTestParams : public CChainParams {
435435
assert(genesis.hashMerkleRoot == uint256S("0x4a5e1e4baab89f3a32518a88c31bc87f618f76673e2cc77ab2127b7afdeda33b"));
436436

437437
vFixedSeeds.clear(); //!< Regtest mode doesn't have any fixed seeds.
438-
vSeeds.clear(); //!< Regtest mode doesn't have any DNS seeds.
438+
vSeeds.clear();
439+
vSeeds.emplace_back("dummySeed.invalid.");
439440

440441
fDefaultConsistencyChecks = true;
441442
fRequireStandard = true;

src/init.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,7 @@ void SetupServerArgs(ArgsManager& argsman)
426426
argsman.AddArg("-dnsseed", strprintf("Query for peer addresses via DNS lookup, if low on addresses (default: %u unless -connect used)", DEFAULT_DNSSEED), ArgsManager::ALLOW_BOOL, OptionsCategory::CONNECTION);
427427
argsman.AddArg("-externalip=<ip>", "Specify your own public address", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
428428
argsman.AddArg("-fixedseeds", strprintf("Allow fixed seeds if DNS seeds don't provide peers (default: %u)", DEFAULT_FIXEDSEEDS), ArgsManager::ALLOW_BOOL, OptionsCategory::CONNECTION);
429-
argsman.AddArg("-forcednsseed", strprintf("Always query for peer addresses via DNS lookup (default: %u)", DEFAULT_FORCEDNSSEED), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
429+
argsman.AddArg("-forcednsseed", strprintf("Always query for peer addresses via DNS lookup (default: %u)", DEFAULT_FORCEDNSSEED), ArgsManager::ALLOW_BOOL, OptionsCategory::CONNECTION);
430430
argsman.AddArg("-listen", "Accept connections from outside (default: 1 if no -proxy or -connect)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
431431
argsman.AddArg("-listenonion", strprintf("Automatically create Tor onion service (default: %d)", DEFAULT_LISTEN_ONION), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
432432
argsman.AddArg("-maxconnections=<n>", strprintf("Maintain at most <n> connections to peers (default: %u). This limit does not apply to connections manually added via -addnode or the addnode RPC, which have a separate limit of %u.", DEFAULT_MAX_PEER_CONNECTIONS, MAX_ADDNODE_CONNECTIONS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
@@ -848,6 +848,11 @@ bool AppInitParameterInteraction(const ArgsManager& args)
848848
return InitError(_("Prune mode is incompatible with -coinstatsindex."));
849849
}
850850

851+
// If -forcednsseed is set to true, ensure -dnsseed has not been set to false
852+
if (args.GetBoolArg("-forcednsseed", DEFAULT_FORCEDNSSEED) && !args.GetBoolArg("-dnsseed", DEFAULT_DNSSEED)){
853+
return InitError(_("Cannot set -forcednsseed to true when setting -dnsseed to false."));
854+
}
855+
851856
// -bind and -whitebind can't be set when not listening
852857
size_t nUserBind = args.GetArgs("-bind").size() + args.GetArgs("-whitebind").size();
853858
if (nUserBind != 0 && !args.GetBoolArg("-listen", DEFAULT_LISTEN)) {

src/net.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,9 @@ static const int64_t DEFAULT_PEER_CONNECT_TIMEOUT = 60;
7979
/** Number of file descriptors required for message capture **/
8080
static const int NUM_FDS_MESSAGE_CAPTURE = 1;
8181

82-
static const bool DEFAULT_FORCEDNSSEED = false;
83-
static const bool DEFAULT_DNSSEED = true;
84-
static const bool DEFAULT_FIXEDSEEDS = true;
82+
static constexpr bool DEFAULT_FORCEDNSSEED{false};
83+
static constexpr bool DEFAULT_DNSSEED{true};
84+
static constexpr bool DEFAULT_FIXEDSEEDS{true};
8585
static const size_t DEFAULT_MAXRECEIVEBUFFER = 5 * 1000;
8686
static const size_t DEFAULT_MAXSENDBUFFER = 1 * 1000;
8787

test/functional/feature_config_args.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,8 +159,9 @@ def test_seed_peers(self):
159159
self.stop_node(0)
160160

161161
# No peers.dat exists and -dnsseed=1
162-
# We expect the node will use DNS Seeds, but Regtest mode has 0 DNS seeds
163-
# So after 60 seconds, the node should fallback to fixed seeds (this is a slow test)
162+
# We expect the node will use DNS Seeds, but Regtest mode does not have
163+
# any valid DNS seeds. So after 60 seconds, the node should fallback to
164+
# fixed seeds
164165
assert not os.path.exists(os.path.join(default_data_dir, "peers.dat"))
165166
start = int(time.time())
166167
with self.nodes[0].assert_debug_log(expected_msgs=[

test/functional/p2p_dns_seeds.py

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
#!/usr/bin/env python3
2+
# Copyright (c) 2021 The Bitcoin Core developers
3+
# Distributed under the MIT software license, see the accompanying
4+
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
5+
"""Test ThreadDNSAddressSeed logic for querying DNS seeds."""
6+
7+
import itertools
8+
9+
from test_framework.p2p import P2PInterface
10+
from test_framework.test_framework import BitcoinTestFramework
11+
12+
13+
class P2PDNSSeeds(BitcoinTestFramework):
14+
def set_test_params(self):
15+
self.setup_clean_chain = True
16+
self.num_nodes = 1
17+
self.extra_args = [["-dnsseed=1"]]
18+
19+
def run_test(self):
20+
self.init_arg_tests()
21+
self.existing_outbound_connections_test()
22+
self.existing_block_relay_connections_test()
23+
self.force_dns_test()
24+
self.wait_time_tests()
25+
26+
def init_arg_tests(self):
27+
fakeaddr = "fakenodeaddr.fakedomain.invalid."
28+
29+
self.log.info("Check that setting -connect disables -dnsseed by default")
30+
self.nodes[0].stop_node()
31+
with self.nodes[0].assert_debug_log(expected_msgs=["DNS seeding disabled"]):
32+
self.start_node(0, [f"-connect={fakeaddr}"])
33+
34+
self.log.info("Check that running -connect and -dnsseed means DNS logic runs.")
35+
with self.nodes[0].assert_debug_log(expected_msgs=["Loading addresses from DNS seed"], timeout=12):
36+
self.restart_node(0, [f"-connect={fakeaddr}", "-dnsseed=1"])
37+
38+
self.log.info("Check that running -forcednsseed and -dnsseed=0 throws an error.")
39+
self.nodes[0].stop_node()
40+
self.nodes[0].assert_start_raises_init_error(
41+
expected_msg="Error: Cannot set -forcednsseed to true when setting -dnsseed to false.",
42+
extra_args=["-forcednsseed=1", "-dnsseed=0"],
43+
)
44+
45+
self.log.info("Check that running -forcednsseed and -connect throws an error.")
46+
# -connect soft sets -dnsseed to false, so throws the same error
47+
self.nodes[0].stop_node()
48+
self.nodes[0].assert_start_raises_init_error(
49+
expected_msg="Error: Cannot set -forcednsseed to true when setting -dnsseed to false.",
50+
extra_args=["-forcednsseed=1", f"-connect={fakeaddr}"],
51+
)
52+
53+
# Restore default bitcoind settings
54+
self.restart_node(0)
55+
56+
def existing_outbound_connections_test(self):
57+
# Make sure addrman is populated to enter the conditional where we
58+
# delay and potentially skip DNS seeding.
59+
self.nodes[0].addpeeraddress("192.0.0.8", 8333)
60+
61+
self.log.info("Check that we *do not* query DNS seeds if we have 2 outbound connections")
62+
63+
self.restart_node(0)
64+
with self.nodes[0].assert_debug_log(expected_msgs=["P2P peers available. Skipped DNS seeding."], timeout=12):
65+
for i in range(2):
66+
self.nodes[0].add_outbound_p2p_connection(P2PInterface(), p2p_idx=i, connection_type="outbound-full-relay")
67+
68+
def existing_block_relay_connections_test(self):
69+
# Make sure addrman is populated to enter the conditional where we
70+
# delay and potentially skip DNS seeding. No-op when run after
71+
# existing_outbound_connections_test.
72+
self.nodes[0].addpeeraddress("192.0.0.8", 8333)
73+
74+
self.log.info("Check that we *do* query DNS seeds if we only have 2 block-relay-only connections")
75+
76+
self.restart_node(0)
77+
with self.nodes[0].assert_debug_log(expected_msgs=["Loading addresses from DNS seed"], timeout=12):
78+
# This mimics the "anchors" logic where nodes are likely to
79+
# reconnect to block-relay-only connections on startup.
80+
# Since we do not participate in addr relay with these connections,
81+
# we still want to query the DNS seeds.
82+
for i in range(2):
83+
self.nodes[0].add_outbound_p2p_connection(P2PInterface(), p2p_idx=i, connection_type="block-relay-only")
84+
85+
def force_dns_test(self):
86+
self.log.info("Check that we query DNS seeds if -forcednsseed param is set")
87+
88+
with self.nodes[0].assert_debug_log(expected_msgs=["Loading addresses from DNS seed"], timeout=12):
89+
# -dnsseed defaults to 1 in bitcoind, but 0 in the test framework,
90+
# so pass it explicitly here
91+
self.restart_node(0, ["-forcednsseed", "-dnsseed=1"])
92+
93+
# Restore default for subsequent tests
94+
self.restart_node(0)
95+
96+
def wait_time_tests(self):
97+
self.log.info("Check the delay before querying DNS seeds")
98+
99+
# Populate addrman with < 1000 addresses
100+
for i in range(5):
101+
a = f"192.0.0.{i}"
102+
self.nodes[0].addpeeraddress(a, 8333)
103+
104+
# The delay should be 11 seconds
105+
with self.nodes[0].assert_debug_log(expected_msgs=["Waiting 11 seconds before querying DNS seeds.\n"]):
106+
self.restart_node(0)
107+
108+
# Populate addrman with > 1000 addresses
109+
for i in itertools.count():
110+
first_octet = i % 2 + 1
111+
second_octet = i % 256
112+
third_octet = i % 100
113+
a = f"{first_octet}.{second_octet}.{third_octet}.1"
114+
self.nodes[0].addpeeraddress(a, 8333)
115+
if (i > 1000 and i % 100 == 0):
116+
# The addrman size is non-deterministic because new addresses
117+
# are sorted into buckets, potentially displacing existing
118+
# addresses. Periodically check if we have met the desired
119+
# threshold.
120+
if len(self.nodes[0].getnodeaddresses(0)) > 1000:
121+
break
122+
123+
# The delay should be 5 mins
124+
with self.nodes[0].assert_debug_log(expected_msgs=["Waiting 300 seconds before querying DNS seeds.\n"]):
125+
self.restart_node(0)
126+
127+
128+
if __name__ == '__main__':
129+
P2PDNSSeeds().main()

test/functional/test_runner.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@
121121
'wallet_listreceivedby.py --legacy-wallet',
122122
'wallet_listreceivedby.py --descriptors',
123123
'wallet_abandonconflict.py --legacy-wallet',
124+
'p2p_dns_seeds.py',
124125
'wallet_abandonconflict.py --descriptors',
125126
'feature_csv_activation.py',
126127
'wallet_address_types.py --legacy-wallet',

0 commit comments

Comments
 (0)