diff --git a/src/addrman.h b/src/addrman.h --- a/src/addrman.h +++ b/src/addrman.h @@ -172,12 +172,6 @@ //! be evicted from tried #define ADDRMAN_REPLACEMENT_SECONDS (4 * 60 * 60) -//! the maximum percentage of nodes to return in a getaddr call -#define ADDRMAN_GETADDR_MAX_PCT 23 - -//! the maximum number of nodes to return in a getaddr call -#define ADDRMAN_GETADDR_MAX 1000 - //! Convenience #define ADDRMAN_TRIED_BUCKET_COUNT (1 << ADDRMAN_TRIED_BUCKET_COUNT_LOG2) #define ADDRMAN_NEW_BUCKET_COUNT (1 << ADDRMAN_NEW_BUCKET_COUNT_LOG2) @@ -318,7 +312,8 @@ #endif //! Select several addresses at once. - void GetAddr_(std::vector &vAddr) EXCLUSIVE_LOCKS_REQUIRED(cs); + void GetAddr_(std::vector &vAddr, size_t max_addresses, + size_t max_pct) EXCLUSIVE_LOCKS_REQUIRED(cs); //! Mark an entry as currently-connected-to. void Connected_(const CService &addr, int64_t nTime) @@ -746,12 +741,12 @@ } //! Return a bunch of addresses, selected at random. - std::vector GetAddr() { + std::vector GetAddr(size_t max_addresses, size_t max_pct) { Check(); std::vector vAddr; { LOCK(cs); - GetAddr_(vAddr); + GetAddr_(vAddr, max_addresses, max_pct); } Check(); return vAddr; diff --git a/src/addrman.cpp b/src/addrman.cpp --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -568,10 +568,14 @@ } #endif -void CAddrMan::GetAddr_(std::vector &vAddr) { - unsigned int nNodes = ADDRMAN_GETADDR_MAX_PCT * vRandom.size() / 100; - if (nNodes > ADDRMAN_GETADDR_MAX) { - nNodes = ADDRMAN_GETADDR_MAX; +void CAddrMan::GetAddr_(std::vector &vAddr, size_t max_addresses, + size_t max_pct) { + size_t nNodes = vRandom.size(); + if (max_pct != 0) { + nNodes = max_pct * nNodes / 100; + } + if (max_addresses != 0) { + nNodes = std::min(nNodes, max_addresses); } // gather a list of random nodes, skipping those of low quality diff --git a/src/bench/addrman.cpp b/src/bench/addrman.cpp --- a/src/bench/addrman.cpp +++ b/src/bench/addrman.cpp @@ -96,7 +96,7 @@ FillAddrMan(addrman); bench.run([&] { - const auto &addresses = addrman.GetAddr(); + const auto &addresses = addrman.GetAddr(2500, 23); assert(addresses.size() > 0); }); } diff --git a/src/net.h b/src/net.h --- a/src/net.h +++ b/src/net.h @@ -59,13 +59,11 @@ static const int TIMEOUT_INTERVAL = 20 * 60; /** Run the feeler connection loop once every 2 minutes or 120 seconds. **/ static const int FEELER_INTERVAL = 120; -/** The maximum number of new addresses to accumulate before announcing. */ -static const unsigned int MAX_ADDR_TO_SEND = 1000; -// TODO: remove ADDRMAN_GETADDR_MAX and let the caller specify this limit with -// MAX_ADDR_TO_SEND. -static_assert(MAX_ADDR_TO_SEND == ADDRMAN_GETADDR_MAX, - "Max allowed ADDR message size should be equal to the max number " - "of records returned from AddrMan."); +/** + * The maximum number of addresses from our addrman to return in response to + * a getaddr message. + */ +static constexpr size_t MAX_ADDR_TO_SEND = 1000; /** Maximum length of the user agent string in `version` message */ static const unsigned int MAX_SUBVERSION_LENGTH = 256; /** @@ -342,14 +340,15 @@ void MarkAddressGood(const CAddress &addr); void AddNewAddresses(const std::vector &vAddr, const CAddress &addrFrom, int64_t nTimePenalty = 0); - std::vector GetAddresses(); + std::vector GetAddresses(size_t max_addresses, size_t max_pct); /** * Cache is used to minimize topology leaks, so it should * be used for all non-trusted calls, for example, p2p. * A non-malicious call (from RPC or a peer with addr permission) should * call the function without a parameter to avoid using the cache. */ - std::vector GetAddresses(Network requestor_network); + std::vector GetAddresses(Network requestor_network, + size_t max_addresses, size_t max_pct); // This allows temporarily exceeding m_max_outbound_full_relay, with the // goal of finding a peer that is better than all our current peers. diff --git a/src/net.cpp b/src/net.cpp --- a/src/net.cpp +++ b/src/net.cpp @@ -2864,8 +2864,9 @@ addrman.Add(vAddr, addrFrom, nTimePenalty); } -std::vector CConnman::GetAddresses() { - std::vector addresses = addrman.GetAddr(); +std::vector CConnman::GetAddresses(size_t max_addresses, + size_t max_pct) { + std::vector addresses = addrman.GetAddr(max_addresses, max_pct); if (m_banman) { addresses.erase(std::remove_if(addresses.begin(), addresses.end(), [this](const CAddress &addr) { @@ -2878,14 +2879,16 @@ return addresses; } -std::vector CConnman::GetAddresses(Network requestor_network) { +std::vector CConnman::GetAddresses(Network requestor_network, + size_t max_addresses, + size_t max_pct) { const auto current_time = GetTime(); if (m_addr_response_caches.find(requestor_network) == m_addr_response_caches.end() || m_addr_response_caches[requestor_network].m_update_addr_response < current_time) { m_addr_response_caches[requestor_network].m_addrs_response_cache = - GetAddresses(); + GetAddresses(max_addresses, max_pct); m_addr_response_caches[requestor_network].m_update_addr_response = current_time + std::chrono::hours(21) + GetRandMillis(std::chrono::hours(6)); diff --git a/src/net_processing.cpp b/src/net_processing.cpp --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -273,6 +273,11 @@ * BIP 157. */ static constexpr uint32_t MAX_GETCFHEADERS_SIZE = 2000; +/** + * the maximum percentage of addresses from our addrman to return in response + * to a getaddr message. + */ +static constexpr size_t MAX_PCT_ADDR_TO_SEND = 23; /// How many non standard orphan do we consider from a node before ignoring it. static constexpr uint32_t MAX_NON_STANDARD_ORPHAN_PER_NODE = 5; @@ -4496,9 +4501,12 @@ pfrom.vAddrToSend.clear(); std::vector vAddr; if (pfrom.HasPermission(PF_ADDR)) { - vAddr = m_connman.GetAddresses(); + vAddr = + m_connman.GetAddresses(MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND); } else { - vAddr = m_connman.GetAddresses(pfrom.addr.GetNetwork()); + vAddr = + m_connman.GetAddresses(pfrom.addr.GetNetwork(), + MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND); } FastRandomContext insecure_rand; for (const CAddress &addr : vAddr) { diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -924,10 +924,8 @@ "nodes in the network\n", { {"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."}, + "The maximum number of addresses to return. Specify 0 to return " + "all known addresses."}, }, RPCResult{ RPCResult::Type::ARR, @@ -962,19 +960,18 @@ int count = 1; if (!request.params[0].isNull()) { count = request.params[0].get_int(); - if (count <= 0) { + if (count < 0) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Address count out of range"); } } // returns a shuffled list of CAddress - std::vector vAddr = node.connman->GetAddresses(); + std::vector vAddr = + node.connman->GetAddresses(count, /* max_pct */ 0); UniValue ret(UniValue::VARR); - int address_return_count = std::min(count, vAddr.size()); - for (int i = 0; i < address_return_count; ++i) { + for (const CAddress &addr : vAddr) { UniValue obj(UniValue::VOBJ); - const CAddress &addr = vAddr[i]; obj.pushKV("time", int(addr.nTime)); obj.pushKV("services", uint64_t(addr.nServices)); obj.pushKV("address", addr.ToStringIP()); diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -378,7 +378,8 @@ // Test: Sanity check, GetAddr should never return anything if addrman // is empty. BOOST_CHECK_EQUAL(addrman.size(), 0U); - std::vector vAddr1 = addrman.GetAddr(); + std::vector vAddr1 = + addrman.GetAddr(/* max_addresses */ 0, /* max_pct */ 0); BOOST_CHECK_EQUAL(vAddr1.size(), 0U); CAddress addr1 = CAddress(ResolveService("250.250.2.1", 8333), NODE_NONE); @@ -401,13 +402,19 @@ BOOST_CHECK(addrman.Add(addr4, source2)); BOOST_CHECK(addrman.Add(addr5, source1)); - // GetAddr returns 23% of addresses, 23% of 5 is 1 rounded down. - BOOST_CHECK_EQUAL(addrman.GetAddr().size(), 1U); + BOOST_CHECK_EQUAL( + addrman.GetAddr(/* max_addresses */ 0, /* max_pct */ 0).size(), 5U); + // Net processing asks for 23% of addresses. 23% of 5 is 1 rounded down. + BOOST_CHECK_EQUAL( + addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23).size(), 1U); // Test: Ensure GetAddr works with new and tried addresses. addrman.Good(CAddress(addr1, NODE_NONE)); addrman.Good(CAddress(addr2, NODE_NONE)); - BOOST_CHECK_EQUAL(addrman.GetAddr().size(), 1U); + BOOST_CHECK_EQUAL( + addrman.GetAddr(/* max_addresses */ 0, /* max_pct */ 0).size(), 5U); + BOOST_CHECK_EQUAL( + addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23).size(), 1U); // Test: Ensure GetAddr still returns 23% when addrman has many addrs. for (unsigned int i = 1; i < (8 * 256); i++) { @@ -424,7 +431,8 @@ addrman.Good(addr); } } - std::vector vAddr = addrman.GetAddr(); + std::vector vAddr = + addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23); size_t percent23 = (addrman.size() * 23) / 100; BOOST_CHECK_EQUAL(vAddr.size(), percent23);