Merge bitcoin/bitcoin#29007: test: create deterministic addrman in the functional tests

2cc8ca19f4185490f30a49516c890b2289fbab71 [test] Use deterministic addrman in addrman info tests (stratospher)
a8978661093500df8d8dcf2a9c90837afacd0aab [test] Restart a node with empty addrman (stratospher)
71c19915c0c716d6f8a539dd92b8ad41e8c447ee [test] Use deterministic addrman in addpeeraddress test (stratospher)
7b868e6b678502e86571976d696c0e3cb72c0884 Revert "test: avoid non-determinism in asmap-addrman test" (stratospher)
69e091f3e1f65b12cf1804e7d39651f55a01827a [init] Create deterministic addrman in tests using -test=addrman (stratospher)
be25ac3092b7755e26e1ec6c33a27cd0e3dd9eac [init] Remove -addrmantest command line arg (stratospher)
802e6e128bba5ffa6d4ec53ff45acccb7cb28f21 [init] Add new command line arg for use only in functional tests (stratospher)

Pull request description:

  An address is placed in a `[bucket,position]` in the addrman table (new table or tried table) using the `addpeeraddress` RPC. This `[bucket,position]` is calculated using `nKey`(and other metrics) for the addrman which is chosen randomly during every run.

  Supposing there are 2 addresses to be placed in an addrman table. During every test run, a different `[bucket,position]` would be calculated for each address.These calculated `[bucket,position]` could even be the same for the 2 addresses in some test runs and result in collisions in the addrman. We wouldn't be able to predict when the collisions are going to happen because we can't predict the `nKey` value which is chosen at random. This can cause flaky tests.

  Because of these non deterministic collisions, we are limited in what we can do to test addrman functionality. Currently in our tests don't add a second address to prevent these collisions from happening - we only keep 1 address in the new table and 1 address in the tried table. See https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1091145647, https://github.com/bitcoin/bitcoin/pull/23084, [#22831(comment)](https://github.com/bitcoin/bitcoin/pull/22831/files#r708302639).

  This PR lets us create a deterministic addrman with fixed `nKey` so that we can know the `[bucket,position]` collisions beforehand, safely add more addresses in an addrman table and write more extensive tests.

ACKs for top commit:
  amitiuttarwar:
    ACK 2cc8ca19f4185490f30a49516c890b2289fbab71
  achow101:
    ACK 2cc8ca19f4185490f30a49516c890b2289fbab71
  0xB10C:
    ACK 2cc8ca19f4185490f30a49516c890b2289fbab71
  mzumsande:
    Code Review ACK 2cc8ca19f4185490f30a49516c890b2289fbab71

Tree-SHA512: 8acd9bdfe7de1eb44d22373bf13533d8ecf602df966fdd5b8b78afcd8cc35a286c95d2712f67a89473a0d68dded7d38f5599f6e4bf95a6589475444545bfb189

1 files changed, 100 insertions, 79 deletions

diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py
index accb2439f2..265d9d959a 100755
--- a/test/functional/rpc_net.py
+++ b/test/functional/rpc_net.py
@@ -13,7 +13,6 @@ import platform
 import time
 
 import test_framework.messages
-from test_framework.netutil import ADDRMAN_NEW_BUCKET_COUNT, ADDRMAN_TRIED_BUCKET_COUNT, ADDRMAN_BUCKET_SIZE
 from test_framework.p2p import (
     P2PInterface,
     P2P_SERVICES,
@@ -42,6 +41,20 @@ def assert_net_servicesnames(servicesflag, servicenames):
     assert servicesflag_generated == servicesflag
 
 
+def seed_addrman(node):
+    """ Populate the addrman with addresses from different networks.
+    Here 2 ipv4, 2 ipv6, 1 cjdns, 2 onion and 1 i2p addresses are added.
+    """
+    node.addpeeraddress(address="1.2.3.4", tried=True, port=8333)
+    node.addpeeraddress(address="2.0.0.0", port=8333)
+    node.addpeeraddress(address="1233:3432:2434:2343:3234:2345:6546:4534", tried=True, port=8333)
+    node.addpeeraddress(address="2803:0:1234:abcd::1", port=45324)
+    node.addpeeraddress(address="fc00:1:2:3:4:5:6:7", port=8333)
+    node.addpeeraddress(address="pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion", tried=True, port=8333)
+    node.addpeeraddress(address="nrfj6inpyf73gpkyool35hcmne5zwfmse3jl3aw23vk7chdemalyaqad.onion", port=45324, tried=True)
+    node.addpeeraddress(address="c4gfnttsuwqomiygupdqqqyy5y5emnk5c73hrfvatri67prd7vyq.b32.i2p", port=8333)
+
+
 class NetTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 2
@@ -305,16 +318,8 @@ class NetTest(BitcoinTestFramework):
         assert_raises_rpc_error(-8, "Network not recognized: Foo", self.nodes[0].getnodeaddresses, 1, "Foo")
 
     def test_addpeeraddress(self):
-        """RPC addpeeraddress sets the source address equal to the destination address.
-        If an address with the same /16 as an existing new entry is passed, it will be
-        placed in the same new bucket and have a 1/64 chance of the bucket positions
-        colliding (depending on the value of nKey in the addrman), in which case the
-        new address won't be added.  The probability of collision can be reduced to
-        1/2^16 = 1/65536 by using an address from a different /16.  We avoid this here
-        by first testing adding a tried table entry before testing adding a new table one.
-        """
         self.log.info("Test addpeeraddress")
-        self.restart_node(1, ["-checkaddrman=1"])
+        self.restart_node(1, ["-checkaddrman=1", "-test=addrman"])
         node = self.nodes[1]
 
         self.log.debug("Test that addpeerinfo is a hidden RPC")
@@ -390,25 +395,33 @@ class NetTest(BitcoinTestFramework):
 
     def test_getaddrmaninfo(self):
         self.log.info("Test getaddrmaninfo")
+        self.restart_node(1, extra_args=["-cjdnsreachable", "-test=addrman"], clear_addrman=True)
         node = self.nodes[1]
+        seed_addrman(node)
+
+        expected_network_count = {
+            'all_networks': {'new': 4, 'tried': 4, 'total': 8},
+            'ipv4': {'new': 1, 'tried': 1, 'total': 2},
+            'ipv6': {'new': 1, 'tried': 1, 'total': 2},
+            'onion': {'new': 0, 'tried': 2, 'total': 2},
+            'i2p': {'new': 1, 'tried': 0, 'total': 1},
+            'cjdns': {'new': 1, 'tried': 0, 'total': 1},
+        }
 
-        # current count of ipv4 addresses in addrman is {'new':1, 'tried':1}
-        self.log.info("Test that count of addresses in addrman match expected values")
+        self.log.debug("Test that count of addresses in addrman match expected values")
         res = node.getaddrmaninfo()
-        assert_equal(res["ipv4"]["new"], 1)
-        assert_equal(res["ipv4"]["tried"], 1)
-        assert_equal(res["ipv4"]["total"], 2)
-        assert_equal(res["all_networks"]["new"], 1)
-        assert_equal(res["all_networks"]["tried"], 1)
-        assert_equal(res["all_networks"]["total"], 2)
-        for net in ["ipv6", "onion", "i2p", "cjdns"]:
-            assert_equal(res[net]["new"], 0)
-            assert_equal(res[net]["tried"], 0)
-            assert_equal(res[net]["total"], 0)
+        for network, count in expected_network_count.items():
+            assert_equal(res[network]['new'], count['new'])
+            assert_equal(res[network]['tried'], count['tried'])
+            assert_equal(res[network]['total'], count['total'])
 
     def test_getrawaddrman(self):
         self.log.info("Test getrawaddrman")
+        self.restart_node(1, extra_args=["-cjdnsreachable", "-test=addrman"], clear_addrman=True)
         node = self.nodes[1]
+        self.addr_time = int(time.time())
+        node.setmocktime(self.addr_time)
+        seed_addrman(node)
 
         self.log.debug("Test that getrawaddrman is a hidden RPC")
         # It is hidden from general help, but its detailed help may be called directly.
@@ -430,88 +443,96 @@ class NetTest(BitcoinTestFramework):
             getrawaddrman = node.getrawaddrman()
             getaddrmaninfo = node.getaddrmaninfo()
             for (table_name, table_info) in expected.items():
-                assert_equal(len(getrawaddrman[table_name]), len(table_info["entries"]))
+                assert_equal(len(getrawaddrman[table_name]), len(table_info))
                 assert_equal(len(getrawaddrman[table_name]), getaddrmaninfo["all_networks"][table_name])
 
                 for bucket_position in getrawaddrman[table_name].keys():
-                    bucket = int(bucket_position.split("/")[0])
-                    position = int(bucket_position.split("/")[1])
-
-                    # bucket and position only be sanity checked here as the
-                    # test-addrman isn't deterministic
-                    assert 0 <= int(bucket) < table_info["bucket_count"]
-                    assert 0 <= int(position) < ADDRMAN_BUCKET_SIZE
-
                     entry = getrawaddrman[table_name][bucket_position]
-                    expected_entry = list(filter(lambda e: e["address"] == entry["address"], table_info["entries"]))[0]
+                    expected_entry = list(filter(lambda e: e["address"] == entry["address"], table_info))[0]
+                    assert bucket_position == expected_entry["bucket_position"]
                     check_addr_information(entry, expected_entry)
 
-        # we expect one addrman new and tried table entry, which were added in a previous test
+        # we expect 4 new and 4 tried table entries in the addrman which were added using seed_addrman()
         expected = {
-            "new": {
-                "bucket_count": ADDRMAN_NEW_BUCKET_COUNT,
-                "entries": [
+            "new": [
                     {
+                        "bucket_position": "82/8",
                         "address": "2.0.0.0",
                         "port": 8333,
                         "services": 9,
                         "network": "ipv4",
                         "source": "2.0.0.0",
                         "source_network": "ipv4",
+                    },
+                    {
+                        "bucket_position": "336/24",
+                        "address": "fc00:1:2:3:4:5:6:7",
+                        "port": 8333,
+                        "services": 9,
+                        "network": "cjdns",
+                        "source": "fc00:1:2:3:4:5:6:7",
+                        "source_network": "cjdns",
+                    },
+                    {
+                        "bucket_position": "963/46",
+                        "address": "c4gfnttsuwqomiygupdqqqyy5y5emnk5c73hrfvatri67prd7vyq.b32.i2p",
+                        "port": 8333,
+                        "services": 9,
+                        "network": "i2p",
+                        "source": "c4gfnttsuwqomiygupdqqqyy5y5emnk5c73hrfvatri67prd7vyq.b32.i2p",
+                        "source_network": "i2p",
+                    },
+                    {
+                        "bucket_position": "613/6",
+                        "address": "2803:0:1234:abcd::1",
+                        "services": 9,
+                        "network": "ipv6",
+                        "source": "2803:0:1234:abcd::1",
+                        "source_network": "ipv6",
+                        "port": 45324,
                     }
-                ]
-            },
-            "tried": {
-                "bucket_count": ADDRMAN_TRIED_BUCKET_COUNT,
-                "entries": [
+            ],
+            "tried": [
                     {
+                        "bucket_position": "6/33",
                         "address": "1.2.3.4",
                         "port": 8333,
                         "services": 9,
                         "network": "ipv4",
                         "source": "1.2.3.4",
                         "source_network": "ipv4",
+                    },
+                    {
+                        "bucket_position": "197/34",
+                        "address": "1233:3432:2434:2343:3234:2345:6546:4534",
+                        "port": 8333,
+                        "services": 9,
+                        "network": "ipv6",
+                        "source": "1233:3432:2434:2343:3234:2345:6546:4534",
+                        "source_network": "ipv6",
+                    },
+                    {
+                        "bucket_position": "72/61",
+                        "address": "pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion",
+                        "port": 8333,
+                        "services": 9,
+                        "network": "onion",
+                        "source": "pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion",
+                        "source_network": "onion"
+                    },
+                    {
+                        "bucket_position": "139/46",
+                        "address": "nrfj6inpyf73gpkyool35hcmne5zwfmse3jl3aw23vk7chdemalyaqad.onion",
+                        "services": 9,
+                        "network": "onion",
+                        "source": "nrfj6inpyf73gpkyool35hcmne5zwfmse3jl3aw23vk7chdemalyaqad.onion",
+                        "source_network": "onion",
+                        "port": 45324,
                     }
-                ]
-            }
+            ]
         }
 
-        self.log.debug("Test that the getrawaddrman contains information about the addresses added in a previous test")
-        check_getrawaddrman_entries(expected)
-
-        self.log.debug("Add one new address to each addrman table")
-        expected["new"]["entries"].append({
-            "address": "2803:0:1234:abcd::1",
-            "services": 9,
-            "network": "ipv6",
-            "source": "2803:0:1234:abcd::1",
-            "source_network": "ipv6",
-            "port": -1,  # set once addpeeraddress is successful
-        })
-        expected["tried"]["entries"].append({
-            "address": "nrfj6inpyf73gpkyool35hcmne5zwfmse3jl3aw23vk7chdemalyaqad.onion",
-            "services": 9,
-            "network": "onion",
-            "source": "nrfj6inpyf73gpkyool35hcmne5zwfmse3jl3aw23vk7chdemalyaqad.onion",
-            "source_network": "onion",
-            "port": -1,  # set once addpeeraddress is successful
-        })
-
-        port = 0
-        for (table_name, table_info) in expected.items():
-            # There's a slight chance that the to-be-added address collides with an already
-            # present table entry. To avoid this, we increment the port until an address has been
-            # added. Incrementing the port changes the position in the new table bucket (bucket
-            # stays the same) and changes both the bucket and the position in the tried table.
-            while True:
-                if node.addpeeraddress(address=table_info["entries"][1]["address"], port=port, tried=table_name == "tried")["success"]:
-                    table_info["entries"][1]["port"] = port
-                    self.log.debug(f"Added {table_info['entries'][1]['address']} to {table_name} table")
-                    break
-                else:
-                    port += 1
-
-        self.log.debug("Test that the newly added addresses appear in getrawaddrman")
+        self.log.debug("Test that getrawaddrman contains information about newly added addresses in each addrman table")
         check_getrawaddrman_entries(expected)