merge bitcoin#26847: track AddrMan totals by network and table, impro…

…ve precision of adding fixed seeds

src/addrdb.cpp

@@ -192,7 +192,7 @@ std::optional<bilingual_str> LoadAddrman(const NetGroupManager& netgroupman, con
     const auto path_addr{gArgs.GetDataDirNet() / "peers.dat"};
     try {
         DeserializeFileDB(path_addr, *addrman, CLIENT_VERSION);
-        LogPrintf("Loaded %i addresses from peers.dat  %dms\n", addrman->size(), GetTimeMillis() - nStart);
+        LogPrintf("Loaded %i addresses from peers.dat  %dms\n", addrman->Size(), GetTimeMillis() - nStart);
     } catch (const DbNotFoundError&) {
         // Addrman can be in an inconsistent state after failure, reset it
         addrman = std::make_unique<AddrMan>(netgroupman, /*deterministic=*/false, /*consistency_check_ratio=*/check_addrman);

src/addrman.cpp

@@ -282,6 +282,7 @@ void AddrManImpl::Unserialize(Stream& s_)
         mapAddr[info] = n;
         info.nRandomPos = vRandom.size();
         vRandom.push_back(n);
+        m_network_counts[info.GetNetwork()].n_new++;
     }
     nIdCount = nNew;
 
@@ -301,6 +302,7 @@ void AddrManImpl::Unserialize(Stream& s_)
             mapAddr[info] = nIdCount;
             vvTried[nKBucket][nKBucketPos] = nIdCount;
             nIdCount++;
+            m_network_counts[info.GetNetwork()].n_tried++;
         } else {
             nLost++;
         }
@@ -414,6 +416,8 @@ AddrInfo* AddrManImpl::Create(const CAddress& addr, const CNetAddr& addrSource,
     mapAddr[addr] = nId;
     mapInfo[nId].nRandomPos = vRandom.size();
     vRandom.push_back(nId);
+    nNew++;
+    m_network_counts[addr.GetNetwork()].n_new++;
     if (pnId)
         *pnId = nId;
     return &mapInfo[nId];
@@ -453,6 +457,7 @@ void AddrManImpl::Delete(int nId)
     assert(info.nRefCount == 0);
 
     SwapRandom(info.nRandomPos, vRandom.size() - 1);
+    m_network_counts[info.GetNetwork()].n_new--;
     vRandom.pop_back();
     mapAddr.erase(info);
     mapInfo.erase(nId);
@@ -493,6 +498,7 @@ void AddrManImpl::MakeTried(AddrInfo& info, int nId)
         }
     }
     nNew--;
+    m_network_counts[info.GetNetwork()].n_new--;
 
     assert(info.nRefCount == 0);
 
@@ -511,6 +517,7 @@ void AddrManImpl::MakeTried(AddrInfo& info, int nId)
         infoOld.fInTried = false;
         vvTried[nKBucket][nKBucketPos] = -1;
         nTried--;
+        m_network_counts[infoOld.GetNetwork()].n_tried--;
 
         // find which new bucket it belongs to
         int nUBucket = infoOld.GetNewBucket(nKey, m_netgroupman);
@@ -522,6 +529,7 @@ void AddrManImpl::MakeTried(AddrInfo& info, int nId)
         infoOld.nRefCount = 1;
         vvNew[nUBucket][nUBucketPos] = nIdEvict;
         nNew++;
+        m_network_counts[infoOld.GetNetwork()].n_new++;
         LogPrint(BCLog::ADDRMAN, "Moved %s from tried[%i][%i] to new[%i][%i] to make space\n",
                  infoOld.ToString(), nKBucket, nKBucketPos, nUBucket, nUBucketPos);
     }
@@ -530,6 +538,7 @@ void AddrManImpl::MakeTried(AddrInfo& info, int nId)
     vvTried[nKBucket][nKBucketPos] = nId;
     nTried++;
     info.fInTried = true;
+    m_network_counts[info.GetNetwork()].n_tried++;
 }
 
 bool AddrManImpl::AddSingle(const CAddress& addr, const CNetAddr& source, int64_t nTimePenalty)
@@ -580,7 +589,6 @@ bool AddrManImpl::AddSingle(const CAddress& addr, const CNetAddr& source, int64_
     } else {
         pinfo = Create(addr, source, &nId);
         pinfo->nTime = std::max((int64_t)0, (int64_t)pinfo->nTime - nTimePenalty);
-        nNew++;
     }
 
     int nUBucket = pinfo->GetNewBucket(nKey, source, m_netgroupman);
@@ -965,6 +973,28 @@ std::optional<AddressPosition> AddrManImpl::FindAddressEntry_(const CAddress& ad
     }
 }
 
+size_t AddrManImpl::Size_(std::optional<Network> net, std::optional<bool> in_new) const
+{
+    AssertLockHeld(cs);
+
+    if (!net.has_value()) {
+        if (in_new.has_value()) {
+            return *in_new ? nNew : nTried;
+        } else {
+            return vRandom.size();
+        }
+    }
+    if (auto it = m_network_counts.find(*net); it != m_network_counts.end()) {
+        auto net_count = it->second;
+        if (in_new.has_value()) {
+            return *in_new ? net_count.n_new : net_count.n_tried;
+        } else {
+            return net_count.n_new + net_count.n_tried;
+        }
+    }
+    return 0;
+}
+
 void AddrManImpl::Check() const
 {
     AssertLockHeld(cs);
@@ -989,6 +1019,7 @@ int AddrManImpl::CheckAddrman() const
 
     std::unordered_set<int> setTried;
     std::unordered_map<int, int> mapNew;
+    std::unordered_map<Network, NewTriedCount> local_counts;
 
     if (vRandom.size() != (size_t)(nTried + nNew))
         return -7;
@@ -1002,12 +1033,14 @@ int AddrManImpl::CheckAddrman() const
             if (info.nRefCount)
                 return -2;
             setTried.insert(n);
+            local_counts[info.GetNetwork()].n_tried++;
         } else {
             if (info.nRefCount < 0 || info.nRefCount > ADDRMAN_NEW_BUCKETS_PER_ADDRESS)
                 return -3;
             if (!info.nRefCount)
                 return -4;
             mapNew[n] = info.nRefCount;
+            local_counts[info.GetNetwork()].n_new++;
         }
         const auto it{mapAddr.find(info)};
         if (it == mapAddr.end() || it->second != n) {
@@ -1065,13 +1098,27 @@ int AddrManImpl::CheckAddrman() const
     if (nKey.IsNull())
         return -16;
 
+    // It's possible that m_network_counts may have all-zero entries that local_counts
+    // doesn't have if addrs from a network were being added and then removed again in the past.
+    if (m_network_counts.size() < local_counts.size()) {
+        return -20;
+    }
+    for (const auto& [net, count] : m_network_counts) {
+        if (local_counts[net].n_new != count.n_new || local_counts[net].n_tried != count.n_tried) {
+            return -21;
+        }
+    }
+
     return 0;
 }
 
-size_t AddrManImpl::size() const
+size_t AddrManImpl::Size(std::optional<Network> net, std::optional<bool> in_new) const
 {
-    LOCK(cs); // TODO: Cache this in an atomic to avoid this overhead
-    return vRandom.size();
+    LOCK(cs);
+    Check();
+    auto ret = Size_(net, in_new);
+    Check();
+    return ret;
 }
 
 bool AddrManImpl::Add(const std::vector<CAddress>& vAddr, const CNetAddr& source, int64_t nTimePenalty)
@@ -1197,9 +1244,9 @@ template void AddrMan::Unserialize(CHashVerifier<CAutoFile>& s);
 template void AddrMan::Unserialize(CDataStream& s);
 template void AddrMan::Unserialize(CHashVerifier<CDataStream>& s);
 
-size_t AddrMan::size() const
+size_t AddrMan::Size(std::optional<Network> net, std::optional<bool> in_new) const
 {
-    return m_impl->size();
+    return m_impl->Size(net, in_new);
 }
 
 bool AddrMan::Add(const std::vector<CAddress>& vAddr, const CNetAddr& source, int64_t nTimePenalty)

src/addrman.h

@@ -110,8 +110,14 @@ class AddrMan
     template <typename Stream>
     void Unserialize(Stream& s_);
 
-    //! Return the number of (unique) addresses in all tables.
-    size_t size() const;
+    /**
+    * Return size information about addrman.
+    *
+    * @param[in] net              Select addresses only from specified network (nullopt = all)
+    * @param[in] in_new           Select addresses only from one table (true = new, false = tried, nullopt = both)
+    * @return                     Number of unique addresses that match specified options.
+    */
+    size_t Size(std::optional<Network> net = {}, std::optional<bool> in_new = {}) const;
 
     /**
      * Attempt to add one or more addresses to addrman's new table.

src/addrman_impl.h

@@ -110,7 +110,7 @@ class AddrManImpl
     template <typename Stream>
     void Unserialize(Stream& s_) EXCLUSIVE_LOCKS_REQUIRED(!cs);
 
-    size_t size() const EXCLUSIVE_LOCKS_REQUIRED(!cs);
+    size_t Size(std::optional<Network> net, std::optional<bool> in_new) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
 
     bool Add(const std::vector<CAddress>& vAddr, const CNetAddr& source, int64_t nTimePenalty)
         EXCLUSIVE_LOCKS_REQUIRED(!cs);
@@ -216,6 +216,14 @@ class AddrManImpl
     /** Reference to the netgroup manager. netgroupman must be constructed before addrman and destructed after. */
     const NetGroupManager& m_netgroupman;
 
+    struct NewTriedCount {
+        size_t n_new;
+        size_t n_tried;
+    };
+
+    /** Number of entries in addrman per network and new/tried table. */
+    std::unordered_map<Network, NewTriedCount> m_network_counts GUARDED_BY(cs);
+
     //! Find an entry.
     AddrInfo* Find(const CService& addr, int* pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs);
 
@@ -260,6 +268,8 @@ class AddrManImpl
 
     std::optional<AddressPosition> FindAddressEntry_(const CAddress& addr) EXCLUSIVE_LOCKS_REQUIRED(cs);
 
+    size_t Size_(std::optional<Network> net, std::optional<bool> in_new) const EXCLUSIVE_LOCKS_REQUIRED(cs);
+
     //! Consistency check, taking into account m_consistency_check_ratio.
     //! Will std::abort if an inconsistency is detected.
     void Check() const EXCLUSIVE_LOCKS_REQUIRED(cs);

src/net.cpp

@@ -2404,7 +2404,7 @@ void CConnman::ThreadDNSAddressSeed()
     if (gArgs.GetBoolArg("-forcednsseed", DEFAULT_FORCEDNSSEED)) {
         // When -forcednsseed is provided, query all.
         seeds_right_now = seeds.size();
-    } else if (addrman.size() == 0) {
+    } else if (addrman.Size() == 0) {
         // If we have no known peers, query all.
         // This will occur on the first run, or if peers.dat has been
         // deleted.
@@ -2423,13 +2423,13 @@ void CConnman::ThreadDNSAddressSeed()
     // * If we continue having problems, eventually query all the
     //   DNS seeds, and if that fails too, also try the fixed seeds.
     //   (done in ThreadOpenConnections)
-    const std::chrono::seconds seeds_wait_time = (addrman.size() >= DNSSEEDS_DELAY_PEER_THRESHOLD ? DNSSEEDS_DELAY_MANY_PEERS : DNSSEEDS_DELAY_FEW_PEERS);
+    const std::chrono::seconds seeds_wait_time = (addrman.Size() >= DNSSEEDS_DELAY_PEER_THRESHOLD ? DNSSEEDS_DELAY_MANY_PEERS : DNSSEEDS_DELAY_FEW_PEERS);
 
     for (const std::string& seed : seeds) {
         if (seeds_right_now == 0) {
             seeds_right_now += DNSSEEDS_TO_QUERY_AT_ONCE;
 
-            if (addrman.size() > 0) {
+            if (addrman.Size() > 0) {
                 LogPrintf("Waiting %d seconds before querying DNS seeds.\n", seeds_wait_time.count());
                 std::chrono::seconds to_wait = seeds_wait_time;
                 while (to_wait.count() > 0) {
@@ -2513,7 +2513,7 @@ void CConnman::DumpAddresses()
     DumpPeerAddresses(::gArgs, addrman);
 
     LogPrint(BCLog::NET, "Flushed %d addresses to peers.dat  %dms\n",
-           addrman.size(), GetTimeMillis() - nStart);
+             addrman.Size(), GetTimeMillis() - nStart);
 }
 
 void CConnman::ProcessAddrFetch()
@@ -2583,6 +2583,19 @@ int CConnman::GetExtraBlockRelayCount() const
     return std::max(block_relay_peers - m_max_outbound_block_relay, 0);
 }
 
+std::unordered_set<Network> CConnman::GetReachableEmptyNetworks() const
+{
+    std::unordered_set<Network> networks{};
+    for (int n = 0; n < NET_MAX; n++) {
+        enum Network net = (enum Network)n;
+        if (net == NET_UNROUTABLE || net == NET_INTERNAL) continue;
+        if (IsReachable(net) && addrman.Size(net, std::nullopt) == 0) {
+            networks.insert(net);
+        }
+    }
+    return networks;
+}
+
 void CConnman::ThreadOpenConnections(const std::vector<std::string> connect, CDeterministicMNManager& dmnman)
 {
     AssertLockNotHeld(m_unused_i2p_sessions_mutex);
@@ -2631,7 +2644,8 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect, CDe
         if (interruptNet)
             return;
 
-        if (add_fixed_seeds && addrman.size() == 0) {
+        const std::unordered_set<Network> fixed_seed_networks{GetReachableEmptyNetworks()};
+        if (add_fixed_seeds && !fixed_seed_networks.empty()) {
             // When the node starts with an empty peers.dat, there are a few other sources of peers before
             // we fallback on to fixed seeds: -dnsseed, -seednode, -addnode
             // If none of those are available, we fallback on to fixed seeds immediately, else we allow
@@ -2640,7 +2654,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect, CDe
             // It is cheapest to check if enough time has passed first.
             if (GetTime<std::chrono::seconds>() > start + std::chrono::minutes{1}) {
                 add_fixed_seeds_now = true;
-                LogPrintf("Adding fixed seeds as 60 seconds have passed and addrman is empty\n");
+                LogPrintf("Adding fixed seeds as 60 seconds have passed and addrman is empty for at least one reachable network\n");
             }
 
             // Checking !dnsseed is cheaper before locking 2 mutexes.
@@ -2657,14 +2671,12 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect, CDe
                 // We will not make outgoing connections to peers that are unreachable
                 // (e.g. because of -onlynet configuration).
                 // Therefore, we do not add them to addrman in the first place.
-                // Note that if you change -onlynet setting from one network to another,
-                // peers.dat will contain only peers of unreachable networks and
-                // manual intervention will be needed (either delete peers.dat after
-                // configuration change or manually add some reachable peer using addnode),
-                // see <https://github.com/bitcoin/bitcoin/issues/26035> for details.
+                // In case previously unreachable networks become reachable
+                // (e.g. in case of -onlynet changes by the user), fixed seeds will
+                // be loaded only for networks for which we have no addressses.
                 seed_addrs.erase(std::remove_if(seed_addrs.begin(), seed_addrs.end(),
-                                               [](const CAddress& addr) { return !IsReachable(addr); }),
-                                seed_addrs.end());
+                                                [&fixed_seed_networks](const CAddress& addr) { return fixed_seed_networks.count(addr.GetNetwork()) == 0; }),
+                                 seed_addrs.end());
                 CNetAddr local;
                 local.SetInternal("fixedseeds");
                 addrman.Add(seed_addrs, local);

src/net.h

@@ -45,6 +45,7 @@
 #include <optional>
 #include <queue>
 #include <thread>
+#include <unordered_set>
 #include <vector>
 
 class CConnman;
@@ -1476,6 +1477,12 @@ friend class CNode;
     void RecordBytesRecv(uint64_t bytes);
     void RecordBytesSent(uint64_t bytes) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex);
 
+    /**
+     Return reachable networks for which we have no addresses in addrman and therefore
+     may require loading fixed seeds.
+     */
+    std::unordered_set<Network> GetReachableEmptyNetworks() const;
+
     /**
      * Return vector of current BLOCK_RELAY peers.
      */