Merge bitcoin/bitcoin#22791: init: Fix asmap/addrman initialization order bug

724c4975622bc22cedc3f3814dfc8e66cf8371f7 [fuzz] Add ConsumeAsmap() function (John Newbery)
5840476714ffebb2599999c85a23b52ebcff6090 [addrman] Make m_asmap private (John Newbery)
f9002cb5dbd573cd9ca200de21319fa296e26055 [net] Rename the copyStats arg from m_asmap to asmap (John Newbery)
f572f2b2048994b3b50f4cfd5de19e40b1acfb22 [addrman] Set m_asmap in CAddrMan initializer list (John Newbery)
593247872decd6d483a76e96d79433247226ad14 [net] Remove CConnMan::SetAsmap() (John Newbery)
50fd77045e2f858a53486b5e02e1798c92ab946c [init] Read/decode asmap before constructing addrman (John Newbery)

Pull request description:

  Commit 181a1207 introduced an initialization order bug: CAddrMan's m_asmap must be set before deserializing peers.dat.

  The first commit restores the correct initialization order. The remaining commits make `CAddrMan::m_asmap` usage safer:

  - don't reach into `CAddrMan`'s internal data from `CConnMan`
  - set `m_asmap` in the initializer list and make it const
  - make `m_asmap` private, and access it (as a reference to const) from a getter.

  This ensures that peers.dat deserialization must happen after setting m_asmap, since m_asmap is set during CAddrMan construction.

ACKs for top commit:
  mzumsande:
    Tested ACK 724c4975622bc22cedc3f3814dfc8e66cf8371f7
  amitiuttarwar:
    code review but utACK 724c497562
  naumenkogs:
    utACK 724c4975622bc22cedc3f3814dfc8e66cf8371f7
  vasild:
    ACK 724c4975622bc22cedc3f3814dfc8e66cf8371f7
  MarcoFalke:
    review ACK 724c4975622bc22cedc3f3814dfc8e66cf8371f7 👫

Tree-SHA512: 684a4cf9e3d4496c9997fb2bc4ec874809987055c157ec3fad1d2143b8223df52b5a0af787d028930b27388c8efeba0aeb2446cb35c337a5552ae76112ade726

1 files changed, 7 insertions, 7 deletions

diff --git a/src/net.cpp b/src/net.cpp
index 9b1e17c587..35376b89ac 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -552,14 +552,14 @@ Network CNode::ConnectedThroughNetwork() const
 
 #undef X
 #define X(name) stats.name = name
-void CNode::copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap)
+void CNode::CopyStats(CNodeStats& stats, const std::vector<bool>& asmap)
 {
     stats.nodeid = this->GetId();
     X(nServices);
     X(addr);
     X(addrBind);
     stats.m_network = ConnectedThroughNetwork();
-    stats.m_mapped_as = addr.GetMappedAS(m_asmap);
+    stats.m_mapped_as = addr.GetMappedAS(asmap);
     if (m_tx_relay != nullptr) {
         LOCK(m_tx_relay->cs_filter);
         stats.fRelayTxes = m_tx_relay->fRelayTxes;
@@ -1921,7 +1921,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
                     case ConnectionType::BLOCK_RELAY:
                     case ConnectionType::ADDR_FETCH:
                     case ConnectionType::FEELER:
-                        setConnected.insert(pnode->addr.GetGroup(addrman.m_asmap));
+                        setConnected.insert(pnode->addr.GetGroup(addrman.GetAsmap()));
                 } // no default case, so the compiler can warn about missing cases
             }
         }
@@ -1995,7 +1995,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
                 m_anchors.pop_back();
                 if (!addr.IsValid() || IsLocal(addr) || !IsReachable(addr) ||
                     !HasAllDesirableServiceFlags(addr.nServices) ||
-                    setConnected.count(addr.GetGroup(addrman.m_asmap))) continue;
+                    setConnected.count(addr.GetGroup(addrman.GetAsmap()))) continue;
                 addrConnect = addr;
                 LogPrint(BCLog::NET, "Trying to make an anchor connection to %s\n", addrConnect.ToString());
                 break;
@@ -2035,7 +2035,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
             }
 
             // Require outbound connections, other than feelers, to be to distinct network groups
-            if (!fFeeler && setConnected.count(addr.GetGroup(addrman.m_asmap))) {
+            if (!fFeeler && setConnected.count(addr.GetGroup(addrman.GetAsmap()))) {
                 break;
             }
 
@@ -2804,7 +2804,7 @@ void CConnman::GetNodeStats(std::vector<CNodeStats>& vstats) const
     vstats.reserve(vNodes.size());
     for (CNode* pnode : vNodes) {
         vstats.emplace_back();
-        pnode->copyStats(vstats.back(), addrman.m_asmap);
+        pnode->CopyStats(vstats.back(), addrman.GetAsmap());
     }
 }
 
@@ -3067,7 +3067,7 @@ CSipHasher CConnman::GetDeterministicRandomizer(uint64_t id) const
 
 uint64_t CConnman::CalculateKeyedNetGroup(const CAddress& ad) const
 {
-    std::vector<unsigned char> vchNetGroup(ad.GetGroup(addrman.m_asmap));
+    std::vector<unsigned char> vchNetGroup(ad.GetGroup(addrman.GetAsmap()));
 
     return GetDeterministicRandomizer(RANDOMIZER_ID_NETGROUP).Write(vchNetGroup.data(), vchNetGroup.size()).Finalize();
 }