Move LoadAddrman from init to addrdb

MarcoFalke · MarcoFalke · commit fa5aeec80c6c · 2021-09-09T09:11:41.000+02:00
Init should only concern itself with the initialization order, not the
detailed initialization logic of every module.

Also, inlining logic into a method that is ~800 lines of code, makes it
impossible to unit test on its own.
diff --git a/src/addrdb.cpp b/src/addrdb.cpp
@@ -18,6 +18,7 @@
 #include <univalue.h>
 #include <util/settings.h>
 #include <util/system.h>
+#include <util/translation.h>
 
 namespace {
 template <typename Stream, typename Data>
@@ -187,6 +188,23 @@ bool ReadFromStream(CAddrMan& addr, CDataStream& ssPeers)
     return DeserializeDB(ssPeers, addr, false);
 }
 
+std::optional<bilingual_str> LoadAddrman(const std::vector<bool>& asmap, const ArgsManager& args, std::unique_ptr<CAddrMan>& addrman)
+{
+    auto check_addrman = std::clamp<int32_t>(args.GetArg("-checkaddrman", DEFAULT_ADDRMAN_CONSISTENCY_CHECKS), 0, 1000000);
+    addrman = std::make_unique<CAddrMan>(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman);
+
+    int64_t nStart = GetTimeMillis();
+    if (ReadPeerAddresses(args, *addrman)) {
+        LogPrintf("Loaded %i addresses from peers.dat  %dms\n", addrman->size(), GetTimeMillis() - nStart);
+    } else {
+        // Addrman can be in an inconsistent state after failure, reset it
+        addrman = std::make_unique<CAddrMan>(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman);
+        LogPrintf("Recreating peers.dat\n");
+        DumpPeerAddresses(args, *addrman);
+    }
+    return std::nullopt;
+}
+
 void DumpAnchors(const fs::path& anchors_db_path, const std::vector<CAddress>& anchors)
 {
     LOG_TIME_SECONDS(strprintf("Flush %d outbound block-relay-only peer addresses to anchors.dat", anchors.size()));
diff --git a/src/addrdb.h b/src/addrdb.h
@@ -10,12 +10,14 @@
 #include <net_types.h> // For banmap_t
 #include <univalue.h>
 
+#include <optional>
 #include <vector>
 
 class ArgsManager;
 class CAddrMan;
 class CAddress;
 class CDataStream;
+struct bilingual_str;
 
 bool DumpPeerAddresses(const ArgsManager& args, const CAddrMan& addr);
 bool ReadPeerAddresses(const ArgsManager& args, CAddrMan& addr);
@@ -46,6 +48,9 @@ class CBanDB
     bool Read(banmap_t& banSet);
 };
 
+/** Returns an error string on failure */
+std::optional<bilingual_str> LoadAddrman(const std::vector<bool>& asmap, const ArgsManager& args, std::unique_ptr<CAddrMan>& addrman);
+
 /**
  * Dump the anchor IP address database (anchors.dat)
  *
diff --git a/src/init.cpp b/src/init.cpp
@@ -1200,19 +1200,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
             LogPrintf("Using /16 prefix for IP bucketing\n");
         }
 
-        auto check_addrman = std::clamp<int32_t>(args.GetArg("-checkaddrman", DEFAULT_ADDRMAN_CONSISTENCY_CHECKS), 0, 1000000);
-        node.addrman = std::make_unique<CAddrMan>(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman);
-
-        // Load addresses from peers.dat
         uiInterface.InitMessage(_("Loading P2P addresses…").translated);
-        int64_t nStart = GetTimeMillis();
-        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");
-            DumpPeerAddresses(args, *node.addrman);
+        if (const auto error{LoadAddrman(asmap, args, node.addrman)}) {
+            return InitError(*error);
         }
     }
 
