From 65cb12ae8a0f87df69970ac0b884dba5308c23c6 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Fri, 12 Feb 2021 11:41:57 +0100 Subject: [PATCH] Merge #19884: p2p: No delay in adding fixed seeds if -dnsseed=0 and peers.dat is empty fe3e993968d6b46777d5a16a662cd22790ddf5bb [p2p] No delay in adding fixed seeds if -dnsseed=0 and peers.dat is empty. Add -fixedseeds arg. (Dhruv Mehta) Pull request description: Closes #19795 Before PR: If `peers.dat` is empty and `-dnsseed=0`, bitcoind will fallback on to fixed seeds but only after a 60 seconds delay. After PR: There's no 60 second delay. To reproduce: `rm ~/.bitcoin/peers.dat && src/bitcoind -dnsseed=0` without and with patch code Other changes in the PR: - `-fixedseeds` command line argument added: `-dnsseed=0 -fixedseeds=0 -addnode=X` provides a trusted peer only setup. `-dnsseed=0 -fixedseeds=0` allows for a `addnode` RPC to add a trusted peer without falling back to hardcoded seeds. ACKs for top commit: LarryRuane: re-ACK fe3e993968d6b46777d5a16a662cd22790ddf5bb laanwj: re-ACK fe3e993968d6b46777d5a16a662cd22790ddf5bb Tree-SHA512: 79449bf4e83a315be6dbac9bdd226de89d2a3f7f76d9c5640a2cb3572866e6b0e8ed67e65674c9824054cf13119dc01c7e1a33848daac6b6c34dbc158b6dba8f --- src/init.cpp | 3 +- src/net.cpp | 46 ++++++++++++++------ src/net.h | 2 + test/functional/feature_config_args.py | 58 +++++++++++++++++++++++++- 4 files changed, 95 insertions(+), 14 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index f38f8cfd24dde..c1687141b8d4f 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -569,8 +569,9 @@ void SetupServerArgs(NodeContext& node) argsman.AddArg("-connect=", "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); argsman.AddArg("-dns", strprintf("Allow DNS lookups for -addnode, -seednode and -connect (default: %u)", DEFAULT_NAME_LOOKUP), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-dnsseed", "Query for peer addresses via DNS lookup, if low on addresses (default: 1 unless -connect used)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + 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); argsman.AddArg("-externalip=", "Specify your own public address", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + argsman.AddArg("-fixedseeds", strprintf("Allow fixed seeds if DNS seeds don't provide peers (default: %u)", DEFAULT_FIXEDSEEDS), ArgsManager::ALLOW_BOOL, OptionsCategory::CONNECTION); argsman.AddArg("-forcednsseed", strprintf("Always query for peer addresses via DNS lookup (default: %u)", DEFAULT_FORCEDNSSEED), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-listen", "Accept connections from outside (default: 1 if no -proxy or -connect)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-listenonion", strprintf("Automatically create Tor onion service (default: %d)", DEFAULT_LISTEN_ONION), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); diff --git a/src/net.cpp b/src/net.cpp index e221974652b70..d9e2af775b0d3 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2244,10 +2244,18 @@ void CConnman::ThreadOpenConnections(const std::vector connect) } // Initiate network connections - int64_t nStart = GetTime(); + auto start = GetTime(); // Minimum time before next feeler connection (in microseconds). - int64_t nNextFeeler = PoissonNextSend(nStart*1000*1000, FEELER_INTERVAL); + int64_t nNextFeeler = PoissonNextSend(count_microseconds(start), FEELER_INTERVAL); + const bool dnsseed = gArgs.GetBoolArg("-dnsseed", DEFAULT_DNSSEED); + bool add_fixed_seeds = gArgs.GetBoolArg("-fixedseeds", DEFAULT_FIXEDSEEDS); + + if (!add_fixed_seeds) { + LogPrintf("Fixed seeds are disabled\n"); + } + + while (!interruptNet) { ProcessOneShot(); @@ -2259,18 +2267,32 @@ void CConnman::ThreadOpenConnections(const std::vector connect) if (interruptNet) return; - // Add seed nodes if DNS seeds are all down (an infrastructure attack?). - // Note that we only do this if we started with an empty peers.dat, - // (in which case we will query DNS seeds immediately) *and* the DNS - // seeds have not returned any results. - if (addrman.size() == 0 && (GetTime() - nStart > 60)) { - static bool done = false; - if (!done) { - LogPrintf("Adding fixed seed nodes as DNS doesn't seem to be available.\n"); + if (add_fixed_seeds && addrman.size() == 0) { + // When the node starts with an empty peers.dat, there are a few other sources of peers before + // we fallback on to fixed seeds: -dnsseed, -seednode, -addnode + // If none of those are available, we fallback on to fixed seeds immediately, else we allow + // 60 seconds for any of those sources to populate addrman. + bool add_fixed_seeds_now = false; + // It is cheapest to check if enough time has passed first. + if (GetTime() > start + std::chrono::minutes{1}) { + add_fixed_seeds_now = true; + LogPrintf("Adding fixed seeds as 60 seconds have passed and addrman is empty\n"); + } + + // Checking !dnsseed is cheaper before locking 2 mutexes. + if (!add_fixed_seeds_now && !dnsseed) { + LOCK2(m_addr_fetches_mutex, cs_vAddedNodes); + if (m_addr_fetches.empty() && vAddedNodes.empty()) { + add_fixed_seeds_now = true; + LogPrintf("Adding fixed seeds as -dnsseed=0, -addnode is not provided and and all -seednode(s) attempted\n"); + } + } + + if (add_fixed_seeds_now) { CNetAddr local; local.SetInternal("fixedseeds"); addrman.Add(ConvertSeeds(Params().FixedSeeds()), local); - done = true; + add_fixed_seeds = false; } } @@ -3233,7 +3255,7 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) // Send and receive from sockets, accept connections threadSocketHandler = std::thread(&util::TraceThread, "net", [this] { ThreadSocketHandler(); }); - if (!gArgs.GetBoolArg("-dnsseed", true)) + if (!gArgs.GetBoolArg("-dnsseed", DEFAULT_DNSSEED)) LogPrintf("DNS seeding disabled\n"); else threadDNSAddressSeed = std::thread(&util::TraceThread, "dnsseed", [this] { ThreadDNSAddressSeed(); }); diff --git a/src/net.h b/src/net.h index b2a73db5c154a..bd802e1f21275 100644 --- a/src/net.h +++ b/src/net.h @@ -93,6 +93,8 @@ static const bool DEFAULT_BLOCKSONLY = false; static const int64_t DEFAULT_PEER_CONNECT_TIMEOUT = 60; static const bool DEFAULT_FORCEDNSSEED = false; +static const bool DEFAULT_DNSSEED = true; +static const bool DEFAULT_FIXEDSEEDS = true; static const size_t DEFAULT_MAXRECEIVEBUFFER = 5 * 1000; static const size_t DEFAULT_MAXSENDBUFFER = 1 * 1000; diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py index f7e913320f4e0..d1da63fb6f417 100755 --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -5,6 +5,7 @@ """Test various command line arguments and configuration file parameters.""" import os +import time from test_framework.test_framework import BitcoinTestFramework from test_framework import util @@ -116,12 +117,67 @@ def test_args_log(self): ]) self.stop_node(0) + def test_seed_peers(self): + self.log.info('Test seed peers, this will take about 2 minutes') + default_data_dir = self.nodes[0].datadir + + # No peers.dat exists and -dnsseed=1 + # We expect the node will use DNS Seeds, but Regtest mode has 0 DNS seeds + # So after 60 seconds, the node should fallback to fixed seeds (this is a slow test) + assert not os.path.exists(os.path.join(default_data_dir, "peers.dat")) + start = time.time() + with self.nodes[0].assert_debug_log(expected_msgs=[ + "Loaded 0 addresses from peers.dat", + "0 addresses found from DNS seeds", + "Adding fixed seeds as 60 seconds have passed and addrman is empty"], timeout=80): + self.start_node(0, extra_args=['-dnsseed=1']) + assert time.time() - start >= 60 + self.stop_node(0) + + # No peers.dat exists and -dnsseed=0 + # We expect the node will fallback immediately to fixed seeds + assert not os.path.exists(os.path.join(default_data_dir, "peers.dat")) + start = time.time() + with self.nodes[0].assert_debug_log(expected_msgs=[ + "Loaded 0 addresses from peers.dat", + "DNS seeding disabled", + "Adding fixed seeds as -dnsseed=0, -addnode is not provided and and all -seednode(s) attempted\n"]): + self.start_node(0, extra_args=['-dnsseed=0']) + assert time.time() - start < 60 + self.stop_node(0) + + # No peers.dat exists and dns seeds are disabled. + # We expect the node will not add fixed seeds when explicitly disabled. + assert not os.path.exists(os.path.join(default_data_dir, "peers.dat")) + start = time.time() + with self.nodes[0].assert_debug_log(expected_msgs=[ + "Loaded 0 addresses from peers.dat", + "DNS seeding disabled", + "Fixed seeds are disabled"]): + self.start_node(0, extra_args=['-dnsseed=0', '-fixedseeds=0']) + assert time.time() - start < 60 + self.stop_node(0) + + # No peers.dat exists and -dnsseed=0, but a -addnode is provided + # We expect the node will allow 60 seconds prior to using fixed seeds + assert not os.path.exists(os.path.join(default_data_dir, "peers.dat")) + start = time.time() + with self.nodes[0].assert_debug_log(expected_msgs=[ + "Loaded 0 addresses from peers.dat", + "DNS seeding disabled", + "Adding fixed seeds as 60 seconds have passed and addrman is empty"], + timeout=80): + self.start_node(0, extra_args=['-dnsseed=0', '-addnode=fakenodeaddr']) + assert time.time() - start >= 60 + self.stop_node(0) + + def run_test(self): self.stop_node(0) self.test_log_buffer() self.test_args_log() - + self.test_seed_peers() self.test_config_file_parser()