Skip to content

Commit 2e9b48a

Browse files
kwvgkwvg
committed
merge bitcoin#26847: track AddrMan totals by network and table, improve precision of adding fixed seeds
1 parent 79a550e commit 2e9b48a

File tree

8 files changed

+182
-62
lines changed

8 files changed

+182
-62
lines changed

‎src/addrdb.cpp‎

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

‎src/addrman.cpp‎

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,7 @@ void AddrManImpl::Unserialize(Stream& s_)
282282
mapAddr[info] = n;
283283
info.nRandomPos = vRandom.size();
284284
vRandom.push_back(n);
285+
m_network_counts[info.GetNetwork()].n_new++;
285286
}
286287
nIdCount = nNew;
287288

@@ -301,6 +302,7 @@ void AddrManImpl::Unserialize(Stream& s_)
301302
mapAddr[info] = nIdCount;
302303
vvTried[nKBucket][nKBucketPos] = nIdCount;
303304
nIdCount++;
305+
m_network_counts[info.GetNetwork()].n_tried++;
304306
} else {
305307
nLost++;
306308
}
@@ -414,6 +416,8 @@ AddrInfo* AddrManImpl::Create(const CAddress& addr, const CNetAddr& addrSource,
414416
mapAddr[addr] = nId;
415417
mapInfo[nId].nRandomPos = vRandom.size();
416418
vRandom.push_back(nId);
419+
nNew++;
420+
m_network_counts[addr.GetNetwork()].n_new++;
417421
if (pnId)
418422
*pnId = nId;
419423
return &mapInfo[nId];
@@ -453,6 +457,7 @@ void AddrManImpl::Delete(int nId)
453457
assert(info.nRefCount == 0);
454458

455459
SwapRandom(info.nRandomPos, vRandom.size() - 1);
460+
m_network_counts[info.GetNetwork()].n_new--;
456461
vRandom.pop_back();
457462
mapAddr.erase(info);
458463
mapInfo.erase(nId);
@@ -493,6 +498,7 @@ void AddrManImpl::MakeTried(AddrInfo& info, int nId)
493498
}
494499
}
495500
nNew--;
501+
m_network_counts[info.GetNetwork()].n_new--;
496502

497503
assert(info.nRefCount == 0);
498504

@@ -511,6 +517,7 @@ void AddrManImpl::MakeTried(AddrInfo& info, int nId)
511517
infoOld.fInTried = false;
512518
vvTried[nKBucket][nKBucketPos] = -1;
513519
nTried--;
520+
m_network_counts[infoOld.GetNetwork()].n_tried--;
514521

515522
// find which new bucket it belongs to
516523
int nUBucket = infoOld.GetNewBucket(nKey, m_netgroupman);
@@ -522,6 +529,7 @@ void AddrManImpl::MakeTried(AddrInfo& info, int nId)
522529
infoOld.nRefCount = 1;
523530
vvNew[nUBucket][nUBucketPos] = nIdEvict;
524531
nNew++;
532+
m_network_counts[infoOld.GetNetwork()].n_new++;
525533
LogPrint(BCLog::ADDRMAN, "Moved %s from tried[%i][%i] to new[%i][%i] to make space\n",
526534
infoOld.ToString(), nKBucket, nKBucketPos, nUBucket, nUBucketPos);
527535
}
@@ -530,6 +538,7 @@ void AddrManImpl::MakeTried(AddrInfo& info, int nId)
530538
vvTried[nKBucket][nKBucketPos] = nId;
531539
nTried++;
532540
info.fInTried = true;
541+
m_network_counts[info.GetNetwork()].n_tried++;
533542
}
534543

535544
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_
580589
} else {
581590
pinfo = Create(addr, source, &nId);
582591
pinfo->nTime = std::max((int64_t)0, (int64_t)pinfo->nTime - nTimePenalty);
583-
nNew++;
584592
}
585593

586594
int nUBucket = pinfo->GetNewBucket(nKey, source, m_netgroupman);
@@ -965,6 +973,28 @@ std::optional<AddressPosition> AddrManImpl::FindAddressEntry_(const CAddress& ad
965973
}
966974
}
967975

976+
size_t AddrManImpl::Size_(std::optional<Network> net, std::optional<bool> in_new) const
977+
{
978+
AssertLockHeld(cs);
979+
980+
if (!net.has_value()) {
981+
if (in_new.has_value()) {
982+
return *in_new ? nNew : nTried;
983+
} else {
984+
return vRandom.size();
985+
}
986+
}
987+
if (auto it = m_network_counts.find(*net); it != m_network_counts.end()) {
988+
auto net_count = it->second;
989+
if (in_new.has_value()) {
990+
return *in_new ? net_count.n_new : net_count.n_tried;
991+
} else {
992+
return net_count.n_new + net_count.n_tried;
993+
}
994+
}
995+
return 0;
996+
}
997+
968998
void AddrManImpl::Check() const
969999
{
9701000
AssertLockHeld(cs);
@@ -989,6 +1019,7 @@ int AddrManImpl::CheckAddrman() const
9891019

9901020
std::unordered_set<int> setTried;
9911021
std::unordered_map<int, int> mapNew;
1022+
std::unordered_map<Network, NewTriedCount> local_counts;
9921023

9931024
if (vRandom.size() != (size_t)(nTried + nNew))
9941025
return -7;
@@ -1002,12 +1033,14 @@ int AddrManImpl::CheckAddrman() const
10021033
if (info.nRefCount)
10031034
return -2;
10041035
setTried.insert(n);
1036+
local_counts[info.GetNetwork()].n_tried++;
10051037
} else {
10061038
if (info.nRefCount < 0 || info.nRefCount > ADDRMAN_NEW_BUCKETS_PER_ADDRESS)
10071039
return -3;
10081040
if (!info.nRefCount)
10091041
return -4;
10101042
mapNew[n] = info.nRefCount;
1043+
local_counts[info.GetNetwork()].n_new++;
10111044
}
10121045
const auto it{mapAddr.find(info)};
10131046
if (it == mapAddr.end() || it->second != n) {
@@ -1065,13 +1098,27 @@ int AddrManImpl::CheckAddrman() const
10651098
if (nKey.IsNull())
10661099
return -16;
10671100

1101+
// It's possible that m_network_counts may have all-zero entries that local_counts
1102+
// doesn't have if addrs from a network were being added and then removed again in the past.
1103+
if (m_network_counts.size() < local_counts.size()) {
1104+
return -20;
1105+
}
1106+
for (const auto& [net, count] : m_network_counts) {
1107+
if (local_counts[net].n_new != count.n_new || local_counts[net].n_tried != count.n_tried) {
1108+
return -21;
1109+
}
1110+
}
1111+
10681112
return 0;
10691113
}
10701114

1071-
size_t AddrManImpl::size() const
1115+
size_t AddrManImpl::Size(std::optional<Network> net, std::optional<bool> in_new) const
10721116
{
1073-
LOCK(cs); // TODO: Cache this in an atomic to avoid this overhead
1074-
return vRandom.size();
1117+
LOCK(cs);
1118+
Check();
1119+
auto ret = Size_(net, in_new);
1120+
Check();
1121+
return ret;
10751122
}
10761123

10771124
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);
11971244
template void AddrMan::Unserialize(CDataStream& s);
11981245
template void AddrMan::Unserialize(CHashVerifier<CDataStream>& s);
11991246

1200-
size_t AddrMan::size() const
1247+
size_t AddrMan::Size(std::optional<Network> net, std::optional<bool> in_new) const
12011248
{
1202-
return m_impl->size();
1249+
return m_impl->Size(net, in_new);
12031250
}
12041251

12051252
bool AddrMan::Add(const std::vector<CAddress>& vAddr, const CNetAddr& source, int64_t nTimePenalty)

‎src/addrman.h‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,14 @@ class AddrMan
110110
template <typename Stream>
111111
void Unserialize(Stream& s_);
112112

113-
//! Return the number of (unique) addresses in all tables.
114-
size_t size() const;
113+
/**
114+
* Return size information about addrman.
115+
*
116+
* @param[in] net Select addresses only from specified network (nullopt = all)
117+
* @param[in] in_new Select addresses only from one table (true = new, false = tried, nullopt = both)
118+
* @return Number of unique addresses that match specified options.
119+
*/
120+
size_t Size(std::optional<Network> net = {}, std::optional<bool> in_new = {}) const;
115121

116122
/**
117123
* Attempt to add one or more addresses to addrman's new table.

‎src/addrman_impl.h‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ class AddrManImpl
110110
template <typename Stream>
111111
void Unserialize(Stream& s_) EXCLUSIVE_LOCKS_REQUIRED(!cs);
112112

113-
size_t size() const EXCLUSIVE_LOCKS_REQUIRED(!cs);
113+
size_t Size(std::optional<Network> net, std::optional<bool> in_new) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
114114

115115
bool Add(const std::vector<CAddress>& vAddr, const CNetAddr& source, int64_t nTimePenalty)
116116
EXCLUSIVE_LOCKS_REQUIRED(!cs);
@@ -216,6 +216,14 @@ class AddrManImpl
216216
/** Reference to the netgroup manager. netgroupman must be constructed before addrman and destructed after. */
217217
const NetGroupManager& m_netgroupman;
218218

219+
struct NewTriedCount {
220+
size_t n_new;
221+
size_t n_tried;
222+
};
223+
224+
/** Number of entries in addrman per network and new/tried table. */
225+
std::unordered_map<Network, NewTriedCount> m_network_counts GUARDED_BY(cs);
226+
219227
//! Find an entry.
220228
AddrInfo* Find(const CService& addr, int* pnId = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs);
221229

@@ -260,6 +268,8 @@ class AddrManImpl
260268

261269
std::optional<AddressPosition> FindAddressEntry_(const CAddress& addr) EXCLUSIVE_LOCKS_REQUIRED(cs);
262270

271+
size_t Size_(std::optional<Network> net, std::optional<bool> in_new) const EXCLUSIVE_LOCKS_REQUIRED(cs);
272+
263273
//! Consistency check, taking into account m_consistency_check_ratio.
264274
//! Will std::abort if an inconsistency is detected.
265275
void Check() const EXCLUSIVE_LOCKS_REQUIRED(cs);

‎src/net.cpp‎

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2404,7 +2404,7 @@ void CConnman::ThreadDNSAddressSeed()
24042404
if (gArgs.GetBoolArg("-forcednsseed", DEFAULT_FORCEDNSSEED)) {
24052405
// When -forcednsseed is provided, query all.
24062406
seeds_right_now = seeds.size();
2407-
} else if (addrman.size() == 0) {
2407+
} else if (addrman.Size() == 0) {
24082408
// If we have no known peers, query all.
24092409
// This will occur on the first run, or if peers.dat has been
24102410
// deleted.
@@ -2423,13 +2423,13 @@ void CConnman::ThreadDNSAddressSeed()
24232423
// * If we continue having problems, eventually query all the
24242424
// DNS seeds, and if that fails too, also try the fixed seeds.
24252425
// (done in ThreadOpenConnections)
2426-
const std::chrono::seconds seeds_wait_time = (addrman.size() >= DNSSEEDS_DELAY_PEER_THRESHOLD ? DNSSEEDS_DELAY_MANY_PEERS : DNSSEEDS_DELAY_FEW_PEERS);
2426+
const std::chrono::seconds seeds_wait_time = (addrman.Size() >= DNSSEEDS_DELAY_PEER_THRESHOLD ? DNSSEEDS_DELAY_MANY_PEERS : DNSSEEDS_DELAY_FEW_PEERS);
24272427

24282428
for (const std::string& seed : seeds) {
24292429
if (seeds_right_now == 0) {
24302430
seeds_right_now += DNSSEEDS_TO_QUERY_AT_ONCE;
24312431

2432-
if (addrman.size() > 0) {
2432+
if (addrman.Size() > 0) {
24332433
LogPrintf("Waiting %d seconds before querying DNS seeds.\n", seeds_wait_time.count());
24342434
std::chrono::seconds to_wait = seeds_wait_time;
24352435
while (to_wait.count() > 0) {
@@ -2513,7 +2513,7 @@ void CConnman::DumpAddresses()
25132513
DumpPeerAddresses(::gArgs, addrman);
25142514

25152515
LogPrint(BCLog::NET, "Flushed %d addresses to peers.dat %dms\n",
2516-
addrman.size(), GetTimeMillis() - nStart);
2516+
addrman.Size(), GetTimeMillis() - nStart);
25172517
}
25182518

25192519
void CConnman::ProcessAddrFetch()
@@ -2583,6 +2583,19 @@ int CConnman::GetExtraBlockRelayCount() const
25832583
return std::max(block_relay_peers - m_max_outbound_block_relay, 0);
25842584
}
25852585

2586+
std::unordered_set<Network> CConnman::GetReachableEmptyNetworks() const
2587+
{
2588+
std::unordered_set<Network> networks{};
2589+
for (int n = 0; n < NET_MAX; n++) {
2590+
enum Network net = (enum Network)n;
2591+
if (net == NET_UNROUTABLE || net == NET_INTERNAL) continue;
2592+
if (IsReachable(net) && addrman.Size(net, std::nullopt) == 0) {
2593+
networks.insert(net);
2594+
}
2595+
}
2596+
return networks;
2597+
}
2598+
25862599
void CConnman::ThreadOpenConnections(const std::vector<std::string> connect, CDeterministicMNManager& dmnman)
25872600
{
25882601
AssertLockNotHeld(m_unused_i2p_sessions_mutex);
@@ -2631,7 +2644,8 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect, CDe
26312644
if (interruptNet)
26322645
return;
26332646

2634-
if (add_fixed_seeds && addrman.size() == 0) {
2647+
const std::unordered_set<Network> fixed_seed_networks{GetReachableEmptyNetworks()};
2648+
if (add_fixed_seeds && !fixed_seed_networks.empty()) {
26352649
// When the node starts with an empty peers.dat, there are a few other sources of peers before
26362650
// we fallback on to fixed seeds: -dnsseed, -seednode, -addnode
26372651
// 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
26402654
// It is cheapest to check if enough time has passed first.
26412655
if (GetTime<std::chrono::seconds>() > start + std::chrono::minutes{1}) {
26422656
add_fixed_seeds_now = true;
2643-
LogPrintf("Adding fixed seeds as 60 seconds have passed and addrman is empty\n");
2657+
LogPrintf("Adding fixed seeds as 60 seconds have passed and addrman is empty for at least one reachable network\n");
26442658
}
26452659

26462660
// Checking !dnsseed is cheaper before locking 2 mutexes.
@@ -2657,14 +2671,12 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect, CDe
26572671
// We will not make outgoing connections to peers that are unreachable
26582672
// (e.g. because of -onlynet configuration).
26592673
// Therefore, we do not add them to addrman in the first place.
2660-
// Note that if you change -onlynet setting from one network to another,
2661-
// peers.dat will contain only peers of unreachable networks and
2662-
// manual intervention will be needed (either delete peers.dat after
2663-
// configuration change or manually add some reachable peer using addnode),
2664-
// see <https://github.com/bitcoin/bitcoin/issues/26035> for details.
2674+
// In case previously unreachable networks become reachable
2675+
// (e.g. in case of -onlynet changes by the user), fixed seeds will
2676+
// be loaded only for networks for which we have no addressses.
26652677
seed_addrs.erase(std::remove_if(seed_addrs.begin(), seed_addrs.end(),
2666-
[](const CAddress& addr) { return !IsReachable(addr); }),
2667-
seed_addrs.end());
2678+
[&fixed_seed_networks](const CAddress& addr) { return fixed_seed_networks.count(addr.GetNetwork()) == 0; }),
2679+
seed_addrs.end());
26682680
CNetAddr local;
26692681
local.SetInternal("fixedseeds");
26702682
addrman.Add(seed_addrs, local);

‎src/net.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
#include <optional>
4646
#include <queue>
4747
#include <thread>
48+
#include <unordered_set>
4849
#include <vector>
4950

5051
class CConnman;
@@ -1476,6 +1477,12 @@ friend class CNode;
14761477
void RecordBytesRecv(uint64_t bytes);
14771478
void RecordBytesSent(uint64_t bytes) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex);
14781479

1480+
/**
1481+
Return reachable networks for which we have no addresses in addrman and therefore
1482+
may require loading fixed seeds.
1483+
*/
1484+
std::unordered_set<Network> GetReachableEmptyNetworks() const;
1485+
14791486
/**
14801487
* Return vector of current BLOCK_RELAY peers.
14811488
*/

0 commit comments

Comments
 (0)