Merge bitcoin/bitcoin#22879: addrman: Fix format string in deserialize error

fab0b55cf0 addrman: Fix format string in deserialize error (MarcoFalke)
facce4ca44 test: Remove useless overwrite (MarcoFalke)

Pull request description:

  The format string is evaluated differently on modern compilers (clang 10 and later, as well as gcc 10 and later).

  Work around the behaviour change in compilers by pinning the underlying type of the format arguments.

  Can be tested by observing a failing test when running against master compiled with clang 10 or gcc 10 (or later).

ACKs for top commit:
  jonatack:
    ACK fab0b55cf0 verified the test fails on master as expected only at line 61 (assertion fixed by the code change); the last two test additions pass as expected
  mzumsande:
    ACK fab0b55cf0

Tree-SHA512: 07462901435107f3bc79098fd7d06446bfe8fe065fffdd35adfcba8f1dd3c499575006557afe7bc74b79d690c5ef7b58e3e031e908161be5529cf237e3b30609

src/addrman.cpp

@@ -243,9 +243,9 @@ void CAddrMan::Unserialize(Stream& s_)
     const uint8_t lowest_compatible = compat - INCOMPATIBILITY_BASE;
     if (lowest_compatible > FILE_FORMAT) {
         throw std::ios_base::failure(strprintf(
-                    "Unsupported format of addrman database: %u. It is compatible with formats >=%u, "
-                    "but the maximum supported by this version of %s is %u.",
-                    format, lowest_compatible, PACKAGE_NAME, static_cast<uint8_t>(FILE_FORMAT)));
+            "Unsupported format of addrman database: %u. It is compatible with formats >=%u, "
+            "but the maximum supported by this version of %s is %u.",
+            uint8_t{format}, uint8_t{lowest_compatible}, PACKAGE_NAME, uint8_t{FILE_FORMAT}));
     }
 
     s >> nKey;

test/functional/feature_addrman.py

@@ -0,0 +1,87 @@
+#!/usr/bin/env python3
+# Copyright (c) 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 addrman functionality"""
+
+import os
+import struct
+
+from test_framework.messages import ser_uint256, hash256
+from test_framework.p2p import MAGIC_BYTES
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import assert_equal
+
+
+def serialize_addrman(*, format=1, lowest_compatible=3):
+    new = []
+    tried = []
+    INCOMPATIBILITY_BASE = 32
+    r = MAGIC_BYTES["regtest"]
+    r += struct.pack("B", format)
+    r += struct.pack("B", INCOMPATIBILITY_BASE + lowest_compatible)
+    r += ser_uint256(1)
+    r += struct.pack("i", len(new))
+    r += struct.pack("i", len(tried))
+    ADDRMAN_NEW_BUCKET_COUNT = 1 << 10
+    r += struct.pack("i", ADDRMAN_NEW_BUCKET_COUNT ^ (1 << 30))
+    for _ in range(ADDRMAN_NEW_BUCKET_COUNT):
+        r += struct.pack("i", 0)
+    checksum = hash256(r)
+    r += checksum
+    return r
+
+
+def write_addrman(peers_dat, **kwargs):
+    with open(peers_dat, "wb") as f:
+        f.write(serialize_addrman(**kwargs))
+
+
+class AddrmanTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.num_nodes = 1
+
+    def run_test(self):
+        peers_dat = os.path.join(self.nodes[0].datadir, self.chain, "peers.dat")
+
+        self.log.info("Check that mocked addrman is valid")
+        self.stop_node(0)
+        write_addrman(peers_dat)
+        with self.nodes[0].assert_debug_log(["Loaded 0 addresses from peers.dat"]):
+            self.start_node(0, extra_args=["-checkaddrman=1"])
+        assert_equal(self.nodes[0].getnodeaddresses(), [])
+
+        self.log.info("Check that addrman from future cannot be read")
+        self.stop_node(0)
+        write_addrman(peers_dat, lowest_compatible=111)
+        with self.nodes[0].assert_debug_log([
+                f'ERROR: DeserializeDB: Deserialize or I/O error - Unsupported format of addrman database: 1. It is compatible with formats >=111, but the maximum supported by this version of {self.config["environment"]["PACKAGE_NAME"]} is 3.',
+                "Recreating peers.dat",
+        ]):
+            self.start_node(0)
+        assert_equal(self.nodes[0].getnodeaddresses(), [])
+
+        self.log.info("Check that corrupt addrman cannot be read")
+        self.stop_node(0)
+        with open(peers_dat, "wb") as f:
+            f.write(serialize_addrman()[:-1])
+        with self.nodes[0].assert_debug_log([
+                "ERROR: DeserializeDB: Deserialize or I/O error - CAutoFile::read: end of file",
+                "Recreating peers.dat",
+        ]):
+            self.start_node(0)
+        assert_equal(self.nodes[0].getnodeaddresses(), [])
+
+        self.log.info("Check that missing addrman is recreated")
+        self.stop_node(0)
+        os.remove(peers_dat)
+        with self.nodes[0].assert_debug_log([
+                f"Missing or invalid file {peers_dat}",
+                "Recreating peers.dat",
+        ]):
+            self.start_node(0)
+        assert_equal(self.nodes[0].getnodeaddresses(), [])
+
+
+if __name__ == "__main__":
+    AddrmanTest().main()

test/functional/feature_anchors.py

@@ -25,9 +25,6 @@ class AnchorsTest(BitcoinTestFramework):
         self.num_nodes = 1
         self.disable_autoconnect = False
 
-    def setup_network(self):
-        self.setup_nodes()
-
     def run_test(self):
         node_anchors_path = os.path.join(
             self.nodes[0].datadir, "regtest", "anchors.dat"

test/functional/test_runner.py

@@ -282,6 +282,7 @@ BASE_SCRIPTS = [
     'p2p_blockfilters.py',
     'p2p_message_capture.py',
     'feature_includeconf.py',
+    'feature_addrman.py',
     'feature_asmap.py',
     'mempool_unbroadcast.py',
     'mempool_compatibility.py',