From fe3e993968d6b46777d5a16a662cd22790ddf5bb Mon Sep 17 00:00:00 2001 From: Dhruv Mehta <856960+dhruv@users.noreply.github.com> Date: Sat, 5 Sep 2020 09:51:33 -0700 Subject: [PATCH] [p2p] No delay in adding fixed seeds if -dnsseed=0 and peers.dat is empty. Add -fixedseeds arg. --- src/init.cpp | 3 +- src/net.cpp | 48 ++++++++++++++++------ src/net.h | 2 + test/functional/feature_config_args.py | 57 ++++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 14 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index a77e2cf1cbf48..f71fae41e7103 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -434,8 +434,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 d004aace88f43..b3868b2210d23 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1769,11 +1769,19 @@ 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 nNextExtraBlockRelay = PoissonNextSend(nStart*1000*1000, EXTRA_BLOCK_RELAY_ONLY_PEER_INTERVAL); + + int64_t nNextFeeler = PoissonNextSend(count_microseconds(start), FEELER_INTERVAL); + int64_t nNextExtraBlockRelay = PoissonNextSend(count_microseconds(start), EXTRA_BLOCK_RELAY_ONLY_PEER_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) { ProcessAddrFetch(); @@ -1785,18 +1793,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(convertSeed6(Params().FixedSeeds()), local); - done = true; + add_fixed_seeds = false; } } @@ -2434,7 +2456,7 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) // Send and receive from sockets, accept connections threadSocketHandler = std::thread(&TraceThread >, "net", std::function(std::bind(&CConnman::ThreadSocketHandler, this))); - if (!gArgs.GetBoolArg("-dnsseed", true)) + if (!gArgs.GetBoolArg("-dnsseed", DEFAULT_DNSSEED)) LogPrintf("DNS seeding disabled\n"); else threadDNSAddressSeed = std::thread(&TraceThread >, "dnsseed", std::function(std::bind(&CConnman::ThreadDNSAddressSeed, this))); diff --git a/src/net.h b/src/net.h index a0aa15705840b..1678eda8e7c87 100644 --- a/src/net.h +++ b/src/net.h @@ -80,6 +80,8 @@ static const int64_t DEFAULT_PEER_CONNECT_TIMEOUT = 60; static const int NUM_FDS_MESSAGE_CAPTURE = 1; 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 2445b6d977a8b..82dd0e3b80911 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 @@ -147,11 +148,67 @@ def test_networkactive(self): self.start_node(0, extra_args=['-nonetworkactive=1']) 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_networkactive() self.test_config_file_parser()