Merge bitcoin/bitcoin#29605: net: Favor peers from addrman over fetching seednodes

6eeb188d40fe8f6c3b0a3be6dca519ea9a7b2358 test: adds seednode functional tests (Sergi Delgado Segura)
3270f0adad6ccbb8c004fb222f420e9b3ea32ea6 net: Favor peers from addrman over fetching seednodes (Sergi Delgado Segura)

Pull request description:

  This is a follow-up of #28016 motivated by https://github.com/bitcoin/bitcoin/pull/28016#pullrequestreview-1913140932 and https://github.com/bitcoin/bitcoin/pull/28016#issuecomment-1984448937.

  The current behavior of seednode fetching is pretty eager: we do it as the first step under `ThreadOpenNetworkConnections` even if some peers may be queryable from our addrman. This poses two potential issues:

  - First, if permanently set (e.g. running with seednode in a config file) we'd be signaling such seed every time we restart our node
  - Second, we will be giving the seed node way too much influence over our addrman, populating the latter with data from the former even when unnecessary

  This changes the behavior to only add seednodes to `m_addr_fetch` if our addrman is empty, or little by little after we've spent some time trying addresses from our addrman. Also, seednodes are added to `m_addr_fetch` in random order, to avoid signaling the same node in case more than one seed is added and we happen to try them over multiple restarts

ACKs for top commit:
  achow101:
    ACK 6eeb188d40fe8f6c3b0a3be6dca519ea9a7b2358
  cbergqvist:
    ACK 6eeb188d40fe8f6c3b0a3be6dca519ea9a7b2358
  itornaza:
    Tested ACK 6eeb188d40fe8f6c3b0a3be6dca519ea9a7b2358
  tdb3:
    ACK 6eeb188d40fe8f6c3b0a3be6dca519ea9a7b2358

Tree-SHA512: b04445412f22018852d6bef4d3f1e88425ee6ddb434f61dcffa9e0c41b8e31f8c56f83858d5c7686289c86dc4c9476c437df15ea61a47082e2bb2e073cc62f15

2 files changed, 56 insertions, 0 deletions

diff --git a/test/functional/p2p_seednode.py b/test/functional/p2p_seednode.py
new file mode 100755
index 0000000000..6c510a6a0b
--- /dev/null
+++ b/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(__file__).main()
+
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index 59c37aa18f..86e8a10af8 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -406,6 +406,7 @@ BASE_SCRIPTS = [
     '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
 ]