Skip to content

Commit f26502e

Browse files
jnewberyjnewbery
committed
[addrman] Specify max addresses and pct when calling GetAddresses()
CAddrMan.GetAddr() would previously limit the number and percentage of addresses returned (to ADDRMAN_GETADDR_MAX (1000) and ADDRMAN_GETADDR_MAX_PCT (23) respectively). Instead, make it the callers responsibility to specify the maximum addresses and percentage they want returned. For net_processing, the maximums are MAX_ADDR_TO_SEND (1000) and MAX_PCT_ADDR_TO_SEND (23). For rpc/net, the maximum is specified by the client.
1 parent 3c93623 commit f26502e

File tree

8 files changed

+35
-38
lines changed

8 files changed

+35
-38
lines changed

‎src/addrman.cpp‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -479,11 +479,15 @@ int CAddrMan::Check_()
479479
}
480480
#endif
481481

482-
void CAddrMan::GetAddr_(std::vector<CAddress>& vAddr)
482+
void CAddrMan::GetAddr_(std::vector<CAddress>& vAddr, size_t max_addresses, size_t max_pct)
483483
{
484-
unsigned int nNodes = ADDRMAN_GETADDR_MAX_PCT * vRandom.size() / 100;
485-
if (nNodes > ADDRMAN_GETADDR_MAX)
486-
nNodes = ADDRMAN_GETADDR_MAX;
484+
size_t nNodes = vRandom.size();
485+
if (max_pct != 0) {
486+
nNodes = max_pct * nNodes / 100;
487+
}
488+
if (max_addresses != 0) {
489+
nNodes = std::min(nNodes, max_addresses);
490+
}
487491

488492
// gather a list of random nodes, skipping those of low quality
489493
for (unsigned int n = 0; n < vRandom.size(); n++) {

‎src/addrman.h‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -153,12 +153,6 @@ class CAddrInfo : public CAddress
153153
//! how recent a successful connection should be before we allow an address to be evicted from tried
154154
#define ADDRMAN_REPLACEMENT_HOURS 4
155155

156-
//! the maximum percentage of nodes to return in a getaddr call
157-
#define ADDRMAN_GETADDR_MAX_PCT 23
158-
159-
//! the maximum number of nodes to return in a getaddr call
160-
#define ADDRMAN_GETADDR_MAX 1000
161-
162156
//! Convenience
163157
#define ADDRMAN_TRIED_BUCKET_COUNT (1 << ADDRMAN_TRIED_BUCKET_COUNT_LOG2)
164158
#define ADDRMAN_NEW_BUCKET_COUNT (1 << ADDRMAN_NEW_BUCKET_COUNT_LOG2)
@@ -261,7 +255,7 @@ friend class CAddrManTest;
261255
#endif
262256

263257
//! Select several addresses at once.
264-
void GetAddr_(std::vector<CAddress> &vAddr) EXCLUSIVE_LOCKS_REQUIRED(cs);
258+
void GetAddr_(std::vector<CAddress> &vAddr, size_t max_addresses, size_t max_pct) EXCLUSIVE_LOCKS_REQUIRED(cs);
265259

266260
//! Mark an entry as currently-connected-to.
267261
void Connected_(const CService &addr, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs);
@@ -638,13 +632,13 @@ friend class CAddrManTest;
638632
}
639633

640634
//! Return a bunch of addresses, selected at random.
641-
std::vector<CAddress> GetAddr()
635+
std::vector<CAddress> GetAddr(size_t max_addresses, size_t max_pct)
642636
{
643637
Check();
644638
std::vector<CAddress> vAddr;
645639
{
646640
LOCK(cs);
647-
GetAddr_(vAddr);
641+
GetAddr_(vAddr, max_addresses, max_pct);
648642
}
649643
Check();
650644
return vAddr;

‎src/bench/addrman.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ static void AddrManGetAddr(benchmark::Bench& bench)
9898
FillAddrMan(addrman);
9999

100100
bench.run([&] {
101-
const auto& addresses = addrman.GetAddr();
101+
const auto& addresses = addrman.GetAddr(2500, 23);
102102
assert(addresses.size() > 0);
103103
});
104104
}

‎src/net.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2528,9 +2528,9 @@ void CConnman::AddNewAddresses(const std::vector<CAddress>& vAddr, const CAddres
25282528
addrman.Add(vAddr, addrFrom, nTimePenalty);
25292529
}
25302530

2531-
std::vector<CAddress> CConnman::GetAddresses()
2531+
std::vector<CAddress> CConnman::GetAddresses(size_t max_addresses, size_t max_pct)
25322532
{
2533-
std::vector<CAddress> addresses = addrman.GetAddr();
2533+
std::vector<CAddress> addresses = addrman.GetAddr(max_addresses, max_pct);
25342534
if (m_banman) {
25352535
addresses.erase(std::remove_if(addresses.begin(), addresses.end(),
25362536
[this](const CAddress& addr){return m_banman->IsDiscouraged(addr) || m_banman->IsBanned(addr);}),
@@ -2539,12 +2539,12 @@ std::vector<CAddress> CConnman::GetAddresses()
25392539
return addresses;
25402540
}
25412541

2542-
std::vector<CAddress> CConnman::GetAddresses(Network requestor_network)
2542+
std::vector<CAddress> CConnman::GetAddresses(Network requestor_network, size_t max_addresses, size_t max_pct)
25432543
{
25442544
const auto current_time = GetTime<std::chrono::microseconds>();
25452545
if (m_addr_response_caches.find(requestor_network) == m_addr_response_caches.end() ||
25462546
m_addr_response_caches[requestor_network].m_update_addr_response < current_time) {
2547-
m_addr_response_caches[requestor_network].m_addrs_response_cache = GetAddresses();
2547+
m_addr_response_caches[requestor_network].m_addrs_response_cache = GetAddresses(max_addresses, max_pct);
25482548
m_addr_response_caches[requestor_network].m_update_addr_response = current_time + std::chrono::hours(21) + GetRandMillis(std::chrono::hours(6));
25492549
}
25502550
return m_addr_response_caches[requestor_network].m_addrs_response_cache;

‎src/net.h‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,8 @@ static const bool DEFAULT_WHITELISTFORCERELAY = false;
5151
static const int TIMEOUT_INTERVAL = 20 * 60;
5252
/** Run the feeler connection loop once every 2 minutes or 120 seconds. **/
5353
static const int FEELER_INTERVAL = 120;
54-
/** The maximum number of new addresses to accumulate before announcing. */
55-
static const unsigned int MAX_ADDR_TO_SEND = 1000;
56-
// TODO: remove ADDRMAN_GETADDR_MAX and let the caller specify this limit with MAX_ADDR_TO_SEND.
57-
static_assert(MAX_ADDR_TO_SEND == ADDRMAN_GETADDR_MAX,
58-
"Max allowed ADDR message size should be equal to the max number of records returned from AddrMan.");
54+
/** The maximum number of addresses from our addrman to return in response to a getaddr message. */
55+
static constexpr size_t MAX_ADDR_TO_SEND = 1000;
5956
/** Maximum length of incoming protocol messages (no message over 4 MB is currently acceptable). */
6057
static const unsigned int MAX_PROTOCOL_MESSAGE_LENGTH = 4 * 1000 * 1000;
6158
/** Maximum length of the user agent string in `version` message */
@@ -254,14 +251,14 @@ class CConnman
254251
void SetServices(const CService &addr, ServiceFlags nServices);
255252
void MarkAddressGood(const CAddress& addr);
256253
void AddNewAddresses(const std::vector<CAddress>& vAddr, const CAddress& addrFrom, int64_t nTimePenalty = 0);
257-
std::vector<CAddress> GetAddresses();
254+
std::vector<CAddress> GetAddresses(size_t max_addresses, size_t max_pct);
258255
/**
259256
* Cache is used to minimize topology leaks, so it should
260257
* be used for all non-trusted calls, for example, p2p.
261258
* A non-malicious call (from RPC or a peer with addr permission) should
262259
* call the function without a parameter to avoid using the cache.
263260
*/
264-
std::vector<CAddress> GetAddresses(Network requestor_network);
261+
std::vector<CAddress> GetAddresses(Network requestor_network, size_t max_addresses, size_t max_pct);
265262

266263
// This allows temporarily exceeding m_max_outbound_full_relay, with the goal of finding
267264
// a peer that is better than all our current peers.

‎src/net_processing.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,8 @@ static constexpr unsigned int MAX_FEEFILTER_CHANGE_DELAY = 5 * 60;
143143
static constexpr uint32_t MAX_GETCFILTERS_SIZE = 1000;
144144
/** Maximum number of cf hashes that may be requested with one getcfheaders. See BIP 157. */
145145
static constexpr uint32_t MAX_GETCFHEADERS_SIZE = 2000;
146+
/** the maximum percentage of addresses from our addrman to return in response to a getaddr message. */
147+
static constexpr size_t MAX_PCT_ADDR_TO_SEND = 23;
146148

147149
struct COrphanTx {
148150
// When modifying, adapt the copy of this definition in tests/DoS_tests.
@@ -3476,9 +3478,9 @@ void ProcessMessage(
34763478
pfrom.vAddrToSend.clear();
34773479
std::vector<CAddress> vAddr;
34783480
if (pfrom.HasPermission(PF_ADDR)) {
3479-
vAddr = connman.GetAddresses();
3481+
vAddr = connman.GetAddresses(MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND);
34803482
} else {
3481-
vAddr = connman.GetAddresses(pfrom.addr.GetNetwork());
3483+
vAddr = connman.GetAddresses(pfrom.addr.GetNetwork(), MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND);
34823484
}
34833485
FastRandomContext insecure_rand;
34843486
for (const CAddress &addr : vAddr) {

‎src/rpc/net.cpp‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -727,7 +727,7 @@ static UniValue getnodeaddresses(const JSONRPCRequest& request)
727727
RPCHelpMan{"getnodeaddresses",
728728
"\nReturn known addresses which can potentially be used to find new nodes in the network\n",
729729
{
730-
{"count", RPCArg::Type::NUM, /* default */ "1", "How many addresses to return. Limited to the smaller of " + ToString(ADDRMAN_GETADDR_MAX) + " or " + ToString(ADDRMAN_GETADDR_MAX_PCT) + "% of all known addresses."},
730+
{"count", RPCArg::Type::NUM, /* default */ "1", "The maximum number of addresses to return. Specify 0 to return all known addresses."},
731731
},
732732
RPCResult{
733733
RPCResult::Type::ARR, "", "",
@@ -754,18 +754,16 @@ static UniValue getnodeaddresses(const JSONRPCRequest& request)
754754
int count = 1;
755755
if (!request.params[0].isNull()) {
756756
count = request.params[0].get_int();
757-
if (count <= 0) {
757+
if (count < 0) {
758758
throw JSONRPCError(RPC_INVALID_PARAMETER, "Address count out of range");
759759
}
760760
}
761761
// returns a shuffled list of CAddress
762-
std::vector<CAddress> vAddr = node.connman->GetAddresses();
762+
std::vector<CAddress> vAddr = node.connman->GetAddresses(count, /* max_pct */ 0);
763763
UniValue ret(UniValue::VARR);
764764

765-
int address_return_count = std::min<int>(count, vAddr.size());
766-
for (int i = 0; i < address_return_count; ++i) {
765+
for (const CAddress& addr : vAddr) {
767766
UniValue obj(UniValue::VOBJ);
768-
const CAddress& addr = vAddr[i];
769767
obj.pushKV("time", (int)addr.nTime);
770768
obj.pushKV("services", (uint64_t)addr.nServices);
771769
obj.pushKV("address", addr.ToStringIP());

‎src/test/addrman_tests.cpp‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr)
392392
// Test: Sanity check, GetAddr should never return anything if addrman
393393
// is empty.
394394
BOOST_CHECK_EQUAL(addrman.size(), 0U);
395-
std::vector<CAddress> vAddr1 = addrman.GetAddr();
395+
std::vector<CAddress> vAddr1 = addrman.GetAddr(/* max_addresses */ 0, /* max_pct */0);
396396
BOOST_CHECK_EQUAL(vAddr1.size(), 0U);
397397

398398
CAddress addr1 = CAddress(ResolveService("250.250.2.1", 8333), NODE_NONE);
@@ -415,13 +415,15 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr)
415415
BOOST_CHECK(addrman.Add(addr4, source2));
416416
BOOST_CHECK(addrman.Add(addr5, source1));
417417

418-
// GetAddr returns 23% of addresses, 23% of 5 is 1 rounded down.
419-
BOOST_CHECK_EQUAL(addrman.GetAddr().size(), 1U);
418+
BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 0, /* max_pct */ 0).size(), 5U);
419+
// Net processing asks for 23% of addresses. 23% of 5 is 1 rounded down.
420+
BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23).size(), 1U);
420421

421422
// Test: Ensure GetAddr works with new and tried addresses.
422423
addrman.Good(CAddress(addr1, NODE_NONE));
423424
addrman.Good(CAddress(addr2, NODE_NONE));
424-
BOOST_CHECK_EQUAL(addrman.GetAddr().size(), 1U);
425+
BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 0, /* max_pct */ 0).size(), 5U);
426+
BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23).size(), 1U);
425427

426428
// Test: Ensure GetAddr still returns 23% when addrman has many addrs.
427429
for (unsigned int i = 1; i < (8 * 256); i++) {
@@ -436,7 +438,7 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr)
436438
if (i % 8 == 0)
437439
addrman.Good(addr);
438440
}
439-
std::vector<CAddress> vAddr = addrman.GetAddr();
441+
std::vector<CAddress> vAddr = addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23);
440442

441443
size_t percent23 = (addrman.size() * 23) / 100;
442444
BOOST_CHECK_EQUAL(vAddr.size(), percent23);

0 commit comments

Comments
 (0)