From fa3f5d0dae2381038439b91ef2af85ec277c8294 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Tue, 7 Sep 2021 10:48:45 +0200
Subject: [PATCH 1/3] Move addrman includes from .h to .cpp

This is a follow-up to the code move in commit a820e79512b67b1bfda20bdc32b47086d2b0910d
---
 src/addrman.cpp               |  2 ++
 src/addrman.h                 | 11 ++---------
 src/net.cpp                   |  1 +
 src/qt/walletframe.cpp        |  1 +
 src/test/addrman_tests.cpp    |  7 ++++---
 src/test/fuzz/versionbits.cpp |  1 +
 6 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/src/addrman.cpp b/src/addrman.cpp
index 986a1a5d4b..bf1230110f 100644
--- a/src/addrman.cpp
+++ b/src/addrman.cpp
@@ -5,10 +5,12 @@
 
 #include <addrman.h>
 
+#include <clientversion.h>
 #include <hash.h>
 #include <logging.h>
 #include <netaddress.h>
 #include <serialize.h>
+#include <streams.h>
 
 #include <cmath>
 #include <optional>
diff --git a/src/addrman.h b/src/addrman.h
index 74bfe9748b..14cc113f94 100644
--- a/src/addrman.h
+++ b/src/addrman.h
@@ -6,23 +6,16 @@
 #ifndef BITCOIN_ADDRMAN_H
 #define BITCOIN_ADDRMAN_H
 
-#include <clientversion.h>
-#include <config/bitcoin-config.h>
 #include <fs.h>
-#include <hash.h>
+#include <logging.h>
 #include <netaddress.h>
 #include <protocol.h>
-#include <random.h>
-#include <streams.h>
 #include <sync.h>
 #include <timedata.h>
-#include <tinyformat.h>
-#include <util/system.h>
 
-#include <iostream>
+#include <cstdint>
 #include <optional>
 #include <set>
-#include <stdint.h>
 #include <unordered_map>
 #include <vector>
 
diff --git a/src/net.cpp b/src/net.cpp
index 35376b89ac..4a8ec55630 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -24,6 +24,7 @@
 #include <scheduler.h>
 #include <util/sock.h>
 #include <util/strencodings.h>
+#include <util/system.h>
 #include <util/thread.h>
 #include <util/trace.h>
 #include <util/translation.h>
diff --git a/src/qt/walletframe.cpp b/src/qt/walletframe.cpp
index f694fbecb5..5eeb2d5308 100644
--- a/src/qt/walletframe.cpp
+++ b/src/qt/walletframe.cpp
@@ -11,6 +11,7 @@
 #include <qt/psbtoperationsdialog.h>
 #include <qt/walletmodel.h>
 #include <qt/walletview.h>
+#include <util/system.h>
 
 #include <cassert>
 
diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp
index e1b5df9502..4ed06d588b 100644
--- a/src/test/addrman_tests.cpp
+++ b/src/test/addrman_tests.cpp
@@ -5,13 +5,14 @@
 #include <addrdb.h>
 #include <addrman.h>
 #include <chainparams.h>
+#include <clientversion.h>
+#include <hash.h>
+#include <netbase.h>
+#include <random.h>
 #include <test/data/asmap.raw.h>
 #include <test/util/setup_common.h>
 #include <util/asmap.h>
 #include <util/string.h>
-#include <hash.h>
-#include <netbase.h>
-#include <random.h>
 
 #include <boost/test/unit_test.hpp>
 
diff --git a/src/test/fuzz/versionbits.cpp b/src/test/fuzz/versionbits.cpp
index 9186821836..73a7d24971 100644
--- a/src/test/fuzz/versionbits.cpp
+++ b/src/test/fuzz/versionbits.cpp
@@ -6,6 +6,7 @@
 #include <chainparams.h>
 #include <consensus/params.h>
 #include <primitives/block.h>
+#include <util/system.h>
 #include <versionbits.h>
 
 #include <test/fuzz/FuzzedDataProvider.h>

From fa7f77b7d1709bf35808fced0d67b6e97b784d63 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Fri, 20 Aug 2021 12:33:24 +0200
Subject: [PATCH 2/3] Fix addrdb includes

---
 src/net.cpp                   | 1 +
 src/net.h                     | 1 -
 src/qt/bantablemodel.h        | 1 +
 src/test/fuzz/data_stream.cpp | 1 +
 src/test/fuzz/integer.cpp     | 1 +
 5 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/net.cpp b/src/net.cpp
index 4a8ec55630..48947ee967 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -9,6 +9,7 @@
 
 #include <net.h>
 
+#include <addrdb.h>
 #include <banman.h>
 #include <clientversion.h>
 #include <compat.h>
diff --git a/src/net.h b/src/net.h
index a884a4521d..5aca0bd75c 100644
--- a/src/net.h
+++ b/src/net.h
@@ -6,7 +6,6 @@
 #ifndef BITCOIN_NET_H
 #define BITCOIN_NET_H
 
-#include <addrdb.h>
 #include <addrman.h>
 #include <amount.h>
 #include <bloom.h>
diff --git a/src/qt/bantablemodel.h b/src/qt/bantablemodel.h
index 57f559fc14..4b5b38e43f 100644
--- a/src/qt/bantablemodel.h
+++ b/src/qt/bantablemodel.h
@@ -5,6 +5,7 @@
 #ifndef BITCOIN_QT_BANTABLEMODEL_H
 #define BITCOIN_QT_BANTABLEMODEL_H
 
+#include <addrdb.h>
 #include <net.h>
 
 #include <memory>
diff --git a/src/test/fuzz/data_stream.cpp b/src/test/fuzz/data_stream.cpp
index 8178878c30..9c1022047e 100644
--- a/src/test/fuzz/data_stream.cpp
+++ b/src/test/fuzz/data_stream.cpp
@@ -2,6 +2,7 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <addrdb.h>
 #include <addrman.h>
 #include <net.h>
 #include <test/fuzz/FuzzedDataProvider.h>
diff --git a/src/test/fuzz/integer.cpp b/src/test/fuzz/integer.cpp
index 5d26529837..5a732aeeff 100644
--- a/src/test/fuzz/integer.cpp
+++ b/src/test/fuzz/integer.cpp
@@ -23,6 +23,7 @@
 #include <test/fuzz/fuzz.h>
 #include <test/fuzz/util.h>
 #include <uint256.h>
+#include <univalue.h>
 #include <util/check.h>
 #include <util/moneystr.h>
 #include <util/strencodings.h>

From fade9a1a4db71241ccad03fdacfb626453952963 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Sat, 21 Aug 2021 11:22:21 +0200
Subject: [PATCH 3/3] Remove confusing CAddrDB

The class only stores the file path, reading it from a global. Globals
are confusing and make testing harder.

The method reading from a stream does not even use any class members, so
putting it in a class is also confusing.
---
 src/addrdb.cpp                | 13 +++++--------
 src/addrdb.h                  | 18 ++++++------------
 src/init.cpp                  |  5 ++---
 src/net.cpp                   |  3 +--
 src/test/addrman_tests.cpp    | 14 +++++++-------
 src/test/fuzz/data_stream.cpp |  2 +-
 6 files changed, 22 insertions(+), 33 deletions(-)

diff --git a/src/addrdb.cpp b/src/addrdb.cpp
index 345dbdfb16..856f318961 100644
--- a/src/addrdb.cpp
+++ b/src/addrdb.cpp
@@ -170,22 +170,19 @@ bool CBanDB::Read(banmap_t& banSet)
     return true;
 }
 
-CAddrDB::CAddrDB()
-{
-    pathAddr = gArgs.GetDataDirNet() / "peers.dat";
-}
-
-bool CAddrDB::Write(const CAddrMan& addr)
+bool DumpPeerAddresses(const ArgsManager& args, const CAddrMan& addr)
 {
+    const auto pathAddr = args.GetDataDirNet() / "peers.dat";
     return SerializeFileDB("peers", pathAddr, addr, CLIENT_VERSION);
 }
 
-bool CAddrDB::Read(CAddrMan& addr)
+bool ReadPeerAddresses(const ArgsManager& args, CAddrMan& addr)
 {
+    const auto pathAddr = args.GetDataDirNet() / "peers.dat";
     return DeserializeFileDB(pathAddr, addr, CLIENT_VERSION);
 }
 
-bool CAddrDB::Read(CAddrMan& addr, CDataStream& ssPeers)
+bool ReadFromStream(CAddrMan& addr, CDataStream& ssPeers)
 {
     return DeserializeDB(ssPeers, addr, false);
 }
diff --git a/src/addrdb.h b/src/addrdb.h
index 26b1c5880f..c31c126ee3 100644
--- a/src/addrdb.h
+++ b/src/addrdb.h
@@ -12,21 +12,15 @@
 
 #include <vector>
 
-class CAddress;
+class ArgsManager;
 class CAddrMan;
+class CAddress;
 class CDataStream;
 
-/** Access to the (IP) address database (peers.dat) */
-class CAddrDB
-{
-private:
-    fs::path pathAddr;
-public:
-    CAddrDB();
-    bool Write(const CAddrMan& addr);
-    bool Read(CAddrMan& addr);
-    static bool Read(CAddrMan& addr, CDataStream& ssPeers);
-};
+bool DumpPeerAddresses(const ArgsManager& args, const CAddrMan& addr);
+bool ReadPeerAddresses(const ArgsManager& args, CAddrMan& addr);
+/** Only used by tests. */
+bool ReadFromStream(CAddrMan& addr, CDataStream& ssPeers);
 
 /** Access to the banlist database (banlist.json) */
 class CBanDB
diff --git a/src/init.cpp b/src/init.cpp
index b744298667..2869cf4daf 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1206,14 +1206,13 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
         // Load addresses from peers.dat
         uiInterface.InitMessage(_("Loading P2P addresses…").translated);
         int64_t nStart = GetTimeMillis();
-        CAddrDB adb;
-        if (adb.Read(*node.addrman)) {
+        if (ReadPeerAddresses(args, *node.addrman)) {
             LogPrintf("Loaded %i addresses from peers.dat  %dms\n", node.addrman->size(), GetTimeMillis() - nStart);
         } else {
             // Addrman can be in an inconsistent state after failure, reset it
             node.addrman = std::make_unique<CAddrMan>(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman);
             LogPrintf("Recreating peers.dat\n");
-            adb.Write(*node.addrman);
+            DumpPeerAddresses(args, *node.addrman);
         }
     }
 
diff --git a/src/net.cpp b/src/net.cpp
index 48947ee967..c31b2d899e 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -1749,8 +1749,7 @@ void CConnman::DumpAddresses()
 {
     int64_t nStart = GetTimeMillis();
 
-    CAddrDB adb;
-    adb.Write(addrman);
+    DumpPeerAddresses(::gArgs, addrman);
 
     LogPrint(BCLog::NET, "Flushed %d addresses to peers.dat  %dms\n",
            addrman.size(), GetTimeMillis() - nStart);
diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp
index 4ed06d588b..835b18d42e 100644
--- a/src/test/addrman_tests.cpp
+++ b/src/test/addrman_tests.cpp
@@ -1003,7 +1003,7 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks)
     BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0");
 }
 
-BOOST_AUTO_TEST_CASE(caddrdb_read)
+BOOST_AUTO_TEST_CASE(load_addrman)
 {
     CAddrManUncorrupted addrmanUncorrupted;
 
@@ -1038,17 +1038,17 @@ BOOST_AUTO_TEST_CASE(caddrdb_read)
     BOOST_CHECK(addrman1.size() == 3);
     BOOST_CHECK(exceptionThrown == false);
 
-    // Test that CAddrDB::Read creates an addrman with the correct number of addrs.
+    // Test that ReadFromStream creates an addrman with the correct number of addrs.
     CDataStream ssPeers2 = AddrmanToStream(addrmanUncorrupted);
 
     CAddrMan addrman2(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 100);
     BOOST_CHECK(addrman2.size() == 0);
-    BOOST_CHECK(CAddrDB::Read(addrman2, ssPeers2));
+    BOOST_CHECK(ReadFromStream(addrman2, ssPeers2));
     BOOST_CHECK(addrman2.size() == 3);
 }
 
 
-BOOST_AUTO_TEST_CASE(caddrdb_read_corrupted)
+BOOST_AUTO_TEST_CASE(load_addrman_corrupted)
 {
     CAddrManCorrupted addrmanCorrupted;
 
@@ -1064,16 +1064,16 @@ BOOST_AUTO_TEST_CASE(caddrdb_read_corrupted)
     } catch (const std::exception&) {
         exceptionThrown = true;
     }
-    // Even through de-serialization failed addrman is not left in a clean state.
+    // Even though de-serialization failed addrman is not left in a clean state.
     BOOST_CHECK(addrman1.size() == 1);
     BOOST_CHECK(exceptionThrown);
 
-    // Test that CAddrDB::Read fails if peers.dat is corrupt
+    // Test that ReadFromStream fails if peers.dat is corrupt
     CDataStream ssPeers2 = AddrmanToStream(addrmanCorrupted);
 
     CAddrMan addrman2(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 100);
     BOOST_CHECK(addrman2.size() == 0);
-    BOOST_CHECK(!CAddrDB::Read(addrman2, ssPeers2));
+    BOOST_CHECK(!ReadFromStream(addrman2, ssPeers2));
 }
 
 
diff --git a/src/test/fuzz/data_stream.cpp b/src/test/fuzz/data_stream.cpp
index 9c1022047e..08e9c91ba3 100644
--- a/src/test/fuzz/data_stream.cpp
+++ b/src/test/fuzz/data_stream.cpp
@@ -23,5 +23,5 @@ FUZZ_TARGET_INIT(data_stream_addr_man, initialize_data_stream_addr_man)
     FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
     CDataStream data_stream = ConsumeDataStream(fuzzed_data_provider);
     CAddrMan addr_man(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 0);
-    CAddrDB::Read(addr_man, data_stream);
+    ReadFromStream(addr_man, data_stream);
 }