Merge #19884: p2p: No delay in adding fixed seeds if -dnsseed=0 and peers.dat is empty

fe3e993968 [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 fe3e993968
  laanwj:
    re-ACK fe3e993968

Tree-SHA512: 79449bf4e83a315be6dbac9bdd226de89d2a3f7f76d9c5640a2cb3572866e6b0e8ed67e65674c9824054cf13119dc01c7e1a33848daac6b6c34dbc158b6dba8f

src/init.cpp

@@ -434,8 +434,9 @@ void SetupServerArgs(NodeContext& node)
     argsman.AddArg("-connect=<ip>", "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=<ip>", "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);

src/net.cpp

@@ -1772,11 +1772,19 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
     }
 
     // Initiate network connections
-    int64_t nStart = GetTime();
+    auto start = GetTime<std::chrono::seconds>();
 
     // 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();
@@ -1788,18 +1796,32 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> 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<std::chrono::seconds>() > 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<std::function<void()> >, "net", std::function<void()>(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<std::function<void()> >, "dnsseed", std::function<void()>(std::bind(&CConnman::ThreadDNSAddressSeed, this)));

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;
 

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 @@ class ConfArgsTest(BitcoinTestFramework):
             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()