net: Favor peers from addrman over fetching seednodes

src/net.cpp

@@ -67,6 +67,9 @@ static constexpr std::chrono::minutes DUMP_PEERS_INTERVAL{15};
 /** Number of DNS seeds to query when the number of connections is low. */
 static constexpr int DNSSEEDS_TO_QUERY_AT_ONCE = 3;
 
+/** Minimum number of outbound connections under which we will keep fetching our address seeds. */
+static constexpr int SEED_OUTBOUND_CONNECTION_THRESHOLD = 2;
+
 /** How long to delay before querying DNS seeds
  *
  * If we have more than THRESHOLD entries in addrman, then it's likely
@@ -2183,7 +2186,6 @@ void CConnman::WakeMessageHandler()
 
 void CConnman::ThreadDNSAddressSeed()
 {
-    constexpr int TARGET_OUTBOUND_CONNECTIONS = 2;
     int outbound_connection_count = 0;
 
     if (gArgs.IsArgSet("-seednode")) {
@@ -2202,7 +2204,7 @@ void CConnman::ThreadDNSAddressSeed()
             }
 
             outbound_connection_count = GetFullOutboundConnCount();
-            if (outbound_connection_count >= TARGET_OUTBOUND_CONNECTIONS) {
+            if (outbound_connection_count >= SEED_OUTBOUND_CONNECTION_THRESHOLD) {
                 LogPrintf("P2P peers available. Finished fetching data from seed nodes.\n");
                 break;
             }
@@ -2225,7 +2227,7 @@ void CConnman::ThreadDNSAddressSeed()
     }
 
     // Proceed with dnsseeds if seednodes hasn't reached the target or if forcednsseed is set
-    if (outbound_connection_count < TARGET_OUTBOUND_CONNECTIONS || seeds_right_now) {
+    if (outbound_connection_count < SEED_OUTBOUND_CONNECTION_THRESHOLD || seeds_right_now) {
         // goal: only query DNS seed if address need is acute
         // * If we have a reasonable number of peers in addrman, spend
         //   some time trying them first. This improves user privacy by
@@ -2256,7 +2258,7 @@ void CConnman::ThreadDNSAddressSeed()
                         if (!interruptNet.sleep_for(w)) return;
                         to_wait -= w;
 
-                        if (GetFullOutboundConnCount() >= TARGET_OUTBOUND_CONNECTIONS) {
+                        if (GetFullOutboundConnCount() >= SEED_OUTBOUND_CONNECTION_THRESHOLD) {
                             if (found > 0) {
                                 LogPrintf("%d addresses found from DNS seeds\n", found);
                                 LogPrintf("P2P peers available. Finished DNS seeding.\n");
@@ -2451,7 +2453,7 @@ bool CConnman::MaybePickPreferredNetwork(std::optional<Network>& network)
     return false;
 }
 
-void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
+void CConnman::ThreadOpenConnections(const std::vector<std::string> connect, Span<const std::string> seed_nodes)
 {
     AssertLockNotHeld(m_unused_i2p_sessions_mutex);
     AssertLockNotHeld(m_reconnections_mutex);
@@ -2491,12 +2493,28 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
     bool add_fixed_seeds = gArgs.GetBoolArg("-fixedseeds", DEFAULT_FIXEDSEEDS);
     const bool use_seednodes{gArgs.IsArgSet("-seednode")};
 
+    auto seed_node_timer = NodeClock::now();
+    bool add_addr_fetch{addrman.Size() == 0 && !seed_nodes.empty()};
+    constexpr std::chrono::seconds ADD_NEXT_SEEDNODE = 10s;
+
     if (!add_fixed_seeds) {
         LogPrintf("Fixed seeds are disabled\n");
     }
 
     while (!interruptNet)
     {
+        if (add_addr_fetch) {
+            add_addr_fetch = false;
+            const auto& seed{SpanPopBack(seed_nodes)};
+            AddAddrFetch(seed);
+
+            if (addrman.Size() == 0) {
+                LogInfo("Empty addrman, adding seednode (%s) to addrfetch\n", seed);
+            } else {
+                LogInfo("Couldn't connect to peers from addrman after %d seconds. Adding seednode (%s) to addrfetch\n", ADD_NEXT_SEEDNODE.count(), seed);
+            }
+        }
+
         ProcessAddrFetch();
 
         if (!interruptNet.sleep_for(std::chrono::milliseconds(500)))
@@ -2597,6 +2615,13 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
             }
         }
 
+        if (!seed_nodes.empty() && nOutboundFullRelay < SEED_OUTBOUND_CONNECTION_THRESHOLD) {
+            if (NodeClock::now() > seed_node_timer + ADD_NEXT_SEEDNODE) {
+                seed_node_timer = NodeClock::now();
+                add_addr_fetch = true;
+            }
+        }
+
         ConnectionType conn_type = ConnectionType::OUTBOUND_FULL_RELAY;
         auto now = GetTime<std::chrono::microseconds>();
         bool anchor = false;
@@ -3242,8 +3267,10 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions)
                                                                 i2p_sam, &interruptNet);
     }
 
-    for (const auto& strDest : connOptions.vSeedNodes) {
-        AddAddrFetch(strDest);
+    // Randomize the order in which we may query seednode to potentially prevent connecting to the same one every restart (and signal that we have restarted)
+    std::vector<std::string> seed_nodes = connOptions.vSeedNodes;
+    if (!seed_nodes.empty()) {
+        Shuffle(seed_nodes.begin(), seed_nodes.end(), FastRandomContext{});
     }
 
     if (m_use_addrman_outgoing) {
@@ -3304,7 +3331,7 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions)
     if (connOptions.m_use_addrman_outgoing || !connOptions.m_specified_outgoing.empty()) {
         threadOpenConnections = std::thread(
             &util::TraceThread, "opencon",
-            [this, connect = connOptions.m_specified_outgoing] { ThreadOpenConnections(connect); });
+            [this, connect = connOptions.m_specified_outgoing, seed_nodes = std::move(seed_nodes)] { ThreadOpenConnections(connect, seed_nodes); });
     }
 
     // Process messages

src/net.h

@@ -1273,7 +1273,7 @@ class CConnman
     void ThreadOpenAddedConnections() EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex, !m_unused_i2p_sessions_mutex, !m_reconnections_mutex);
     void AddAddrFetch(const std::string& strDest) EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex);
     void ProcessAddrFetch() EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_unused_i2p_sessions_mutex);
-    void ThreadOpenConnections(std::vector<std::string> connect) EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_added_nodes_mutex, !m_nodes_mutex, !m_unused_i2p_sessions_mutex, !m_reconnections_mutex);
+    void ThreadOpenConnections(std::vector<std::string> connect, Span<const std::string> seed_nodes) EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_added_nodes_mutex, !m_nodes_mutex, !m_unused_i2p_sessions_mutex, !m_reconnections_mutex);
     void ThreadMessageHandler() EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc);
     void ThreadI2PAcceptIncoming();
     void AcceptConnection(const ListenSocket& hListenSocket);

test/functional/p2p_seednode.py

@@ -0,0 +1,55 @@
+#!/usr/bin/env python3
+# Copyright (c) 2019-2021 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+"""
+Test seednode interaction with the AddrMan
+"""
+import random
+import time
+
+from test_framework.test_framework import BitcoinTestFramework
+
+ADD_NEXT_SEEDNODE = 10
+
+
+class P2PSeedNodes(BitcoinTestFramework):
+    def set_test_params(self):
+        self.num_nodes = 1
+        self.disable_autoconnect = False
+
+    def test_no_seednode(self):
+        # Check that if no seednode is provided, the node proceeds as usual (without waiting)
+        with self.nodes[0].assert_debug_log(expected_msgs=[], unexpected_msgs=["Empty addrman, adding seednode", f"Couldn't connect to peers from addrman after {ADD_NEXT_SEEDNODE} seconds. Adding seednode"], timeout=ADD_NEXT_SEEDNODE):
+            self.restart_node(0)
+
+    def test_seednode_empty_addrman(self):
+        seed_node = "0.0.0.1"
+        # Check that the seednode is added to m_addr_fetches on bootstrap on an empty addrman
+        with self.nodes[0].assert_debug_log(expected_msgs=[f"Empty addrman, adding seednode ({seed_node}) to addrfetch"], timeout=ADD_NEXT_SEEDNODE):
+            self.restart_node(0, extra_args=[f'-seednode={seed_node}'])
+
+    def test_seednode_addrman_unreachable_peers(self):
+        seed_node = "0.0.0.2"
+        node = self.nodes[0]
+        # Fill the addrman with unreachable nodes
+        for i in range(10):
+            ip = f"{random.randrange(128,169)}.{random.randrange(1,255)}.{random.randrange(1,255)}.{random.randrange(1,255)}"
+            port = 8333 + i
+            node.addpeeraddress(ip, port)
+
+        # Restart the node so seednode is processed again
+        with node.assert_debug_log(expected_msgs=[f"Couldn't connect to peers from addrman after {ADD_NEXT_SEEDNODE} seconds. Adding seednode ({seed_node}) to addrfetch"], unexpected_msgs=["Empty addrman, adding seednode"], timeout=ADD_NEXT_SEEDNODE * 1.5):
+            self.restart_node(0, extra_args=[f'-seednode={seed_node}'])
+            node.setmocktime(int(time.time()) + ADD_NEXT_SEEDNODE + 1)
+
+    def run_test(self):
+        self.test_no_seednode()
+        self.test_seednode_empty_addrman()
+        self.test_seednode_addrman_unreachable_peers()
+
+
+if __name__ == '__main__':
+    P2PSeedNodes().main()
+

test/functional/test_runner.py

@@ -391,6 +391,7 @@
     'feature_shutdown.py',
     'wallet_migration.py',
     'p2p_ibd_txrelay.py',
+    'p2p_seednode.py',
     # Don't append tests at the end to avoid merge conflicts
     # Put them in a random line within the section that fits their approximate run-time
 ]