fix: backport bitcoin#26909, allow for silent overwriting of inconsistent peers.dat

src/addrdb.cpp

@@ -33,10 +33,9 @@ bool SerializeDB(Stream& stream, const Data& data)
 {
     // Write and commit header, data
     try {
-        CHashWriter hasher(stream.GetType(), stream.GetVersion());
-        stream << Params().MessageStart() << data;
-        hasher << Params().MessageStart() << data;
-        stream << hasher.GetHash();
+        HashedSourceWriter hashwriter{stream};
+        hashwriter << Params().MessageStart() << data;
+        stream << hashwriter.GetHash();
     } catch (const std::exception& e) {
         return error("%s: Serialize or I/O error - %s", __func__, e.what());
     }
@@ -197,6 +196,15 @@ std::optional<bilingual_str> LoadAddrman(const std::vector<bool>& asmap, const A
         addrman = std::make_unique<AddrMan>(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman);
         LogPrintf("Creating peers.dat because the file was not found (%s)\n", path_addr);
         DumpPeerAddresses(args, *addrman);
+    } catch (const DbInconsistentError& e) {
+        // Addrman has shown a tendency to corrupt itself even with graceful shutdowns on known-good
+        // hardware. As the user would have to delete and recreate a new database regardless to cope
+        // with frequent corruption, we are restoring old behaviour that does the same, silently.
+        //
+        // TODO: Evaluate cause and fix, revert this change at some point.
+        addrman = std::make_unique<AddrMan>(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman);
+        LogPrintf("Creating peers.dat because of invalid or corrupt file (%s)\n", e.what());
+        DumpPeerAddresses(args, *addrman);
     } catch (const std::exception& e) {
         addrman = nullptr;
         return strprintf(_("Invalid or corrupt peers.dat (%s). If you believe this is a bug, please report it to %s. As a workaround, you can move the file (%s) out of the way (rename, move, or delete) to have a new one created on the next start."),

src/addrman.cpp

@@ -391,7 +391,7 @@ void AddrManImpl::Unserialize(Stream& s_)
 
     const int check_code{ForceCheckAddrman()};
     if (check_code != 0) {
-        throw std::ios_base::failure(strprintf(
+        throw DbInconsistentError(strprintf(
             "Corrupt data. Consistency check failed with code %s",
             check_code));
     }
@@ -1168,8 +1168,7 @@ void AddrMan::Unserialize(Stream& s_)
 }
 
 // explicit instantiation
-template void AddrMan::Serialize(CHashWriter& s) const;
-template void AddrMan::Serialize(CAutoFile& s) const;
+template void AddrMan::Serialize(HashedSourceWriter<CAutoFile>& s) const;
 template void AddrMan::Serialize(CDataStream& s) const;
 template void AddrMan::Unserialize(CAutoFile& s);
 template void AddrMan::Unserialize(CHashVerifier<CAutoFile>& s);

src/addrman.h

@@ -23,6 +23,16 @@ class AddrManImpl;
 /** Default for -checkaddrman */
 static constexpr int32_t DEFAULT_ADDRMAN_CONSISTENCY_CHECKS{0};
 
+class DbInconsistentError : public std::exception
+{
+    using std::exception::exception;
+    const std::string error;
+
+public:
+    explicit DbInconsistentError(const std::string _error) : error{_error} {}
+    const char* what() const noexcept override { return error.c_str(); }
+};
+
 /** Stochastic address manager
  *
  * Design goals:

src/hash.h

@@ -201,6 +201,30 @@ class CHashVerifier : public CHashWriter
     }
 };
 
+/** Writes data to an underlying source stream, while hashing the written data. */
+template <typename Source>
+class HashedSourceWriter : public CHashWriter
+{
+private:
+    Source& m_source;
+
+public:
+    explicit HashedSourceWriter(Source& source LIFETIMEBOUND) : CHashWriter{source.GetType(), source.GetVersion()}, m_source{source} {}
+
+    void write(Span<const std::byte> src)
+    {
+        m_source.write(src);
+        CHashWriter::write(src);
+    }
+
+    template <typename T>
+    HashedSourceWriter& operator<<(const T& obj)
+    {
+        ::Serialize(*this, obj);
+        return *this;
+    }
+};
+
 /** Compute the 256-bit hash of an object's serialization. */
 template<typename T>
 uint256 SerializeHash(const T& obj, int nType=SER_GETHASH, int nVersion=PROTOCOL_VERSION)

src/test/streams_tests.cpp

@@ -437,4 +437,18 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_rand)
     fs::remove("streams_test_tmp");
 }
 
+BOOST_AUTO_TEST_CASE(streams_hashed)
+{
+    CDataStream stream(SER_NETWORK, INIT_PROTO_VERSION);
+    HashedSourceWriter hash_writer{stream};
+    const std::string data{"bitcoin"};
+    hash_writer << data;
+
+    CHashVerifier hash_verifier{&stream};
+    std::string result;
+    hash_verifier >> result;
+    BOOST_CHECK_EQUAL(data, result);
+    BOOST_CHECK_EQUAL(hash_writer.GetHash(), hash_verifier.GetHash());
+}
+
 BOOST_AUTO_TEST_SUITE_END()

test/functional/feature_addrman.py

@@ -123,10 +123,8 @@ def run_test(self):
         self.log.info("Check that corrupt addrman cannot be read (failed check)")
         self.stop_node(0)
         write_addrman(peers_dat, bucket_key=0)
-        self.nodes[0].assert_start_raises_init_error(
-            expected_msg=init_error("Corrupt data. Consistency check failed with code -16: .*"),
-            match=ErrorMatch.FULL_REGEX,
-        )
+        with self.nodes[0].assert_debug_log(['Creating peers.dat because of invalid or corrupt file']):
+            self.start_node(0)
 
         self.log.info("Check that missing addrman is recreated")
         self.stop_node(0)