Skip to content

Commit

Permalink
p2p: allow CAddrMan::GetAddr() by network, add doxygen
Browse files Browse the repository at this point in the history
  • Loading branch information
jonatack committed May 19, 2021
1 parent c38981e commit a49f3dd
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 13 deletions.
14 changes: 10 additions & 4 deletions src/addrman.h
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ friend class CAddrManTest;
* @param[in] max_pct Maximum percentage of addresses to return (0 = all).
* @param[in] network Select only addresses of this network (nullopt = all).
*/
void GetAddr_(std::vector<CAddress>& vAddr, size_t max_addresses, size_t max_pct, std::optional<Network> network = std::nullopt) EXCLUSIVE_LOCKS_REQUIRED(cs);
void GetAddr_(std::vector<CAddress>& vAddr, size_t max_addresses, size_t max_pct, std::optional<Network> network) EXCLUSIVE_LOCKS_REQUIRED(cs);

/** We have successfully connected to this peer. Calling this function
* updates the CAddress's nTime, which is used in our IsTerrible()
Expand Down Expand Up @@ -723,14 +723,20 @@ friend class CAddrManTest;
return addrRet;
}

//! Return a bunch of addresses, selected at random.
std::vector<CAddress> GetAddr(size_t max_addresses, size_t max_pct)
/**
* Return all or many randomly selected addresses, optionally by network.
*
* @param[in] max_addresses Maximum number of addresses to return (0 = all).
* @param[in] max_pct Maximum percentage of addresses to return (0 = all).
* @param[in] network Select only addresses of this network (nullopt = all).
*/
std::vector<CAddress> GetAddr(size_t max_addresses, size_t max_pct, std::optional<Network> network)
{
Check();
std::vector<CAddress> vAddr;
{
LOCK(cs);
GetAddr_(vAddr, max_addresses, max_pct);
GetAddr_(vAddr, max_addresses, max_pct, network);
}
Check();
return vAddr;
Expand Down
3 changes: 2 additions & 1 deletion src/bench/addrman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <random.h>
#include <util/time.h>

#include <optional>
#include <vector>

/* A "source" is a source address from which we have received a bunch of other addresses. */
Expand Down Expand Up @@ -98,7 +99,7 @@ static void AddrManGetAddr(benchmark::Bench& bench)
FillAddrMan(addrman);

bench.run([&] {
const auto& addresses = addrman.GetAddr(2500, 23);
const auto& addresses = addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23, /* network */ std::nullopt);
assert(addresses.size() > 0);
});
}
Expand Down
2 changes: 1 addition & 1 deletion src/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2671,7 +2671,7 @@ CConnman::~CConnman()

std::vector<CAddress> CConnman::GetAddresses(size_t max_addresses, size_t max_pct) const
{
std::vector<CAddress> addresses = addrman.GetAddr(max_addresses, max_pct);
std::vector<CAddress> addresses = addrman.GetAddr(max_addresses, max_pct, /* network */ std::nullopt);
if (m_banman) {
addresses.erase(std::remove_if(addresses.begin(), addresses.end(),
[this](const CAddress& addr){return m_banman->IsDiscouraged(addr) || m_banman->IsBanned(addr);}),
Expand Down
13 changes: 7 additions & 6 deletions src/test/addrman_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

#include <boost/test/unit_test.hpp>

#include <optional>
#include <string>

class CAddrManTest : public CAddrMan
Expand Down Expand Up @@ -392,7 +393,7 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr)
// Test: Sanity check, GetAddr should never return anything if addrman
// is empty.
BOOST_CHECK_EQUAL(addrman.size(), 0U);
std::vector<CAddress> vAddr1 = addrman.GetAddr(/* max_addresses */ 0, /* max_pct */0);
std::vector<CAddress> vAddr1 = addrman.GetAddr(/* max_addresses */ 0, /* max_pct */ 0, /* network */ std::nullopt);
BOOST_CHECK_EQUAL(vAddr1.size(), 0U);

CAddress addr1 = CAddress(ResolveService("250.250.2.1", 8333), NODE_NONE);
Expand All @@ -415,15 +416,15 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr)
BOOST_CHECK(addrman.Add(addr4, source2));
BOOST_CHECK(addrman.Add(addr5, source1));

BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 0, /* max_pct */ 0).size(), 5U);
BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 0, /* max_pct */ 0, /* network */ std::nullopt).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);
BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23, /* network */ std::nullopt).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(/* max_addresses */ 0, /* max_pct */ 0).size(), 5U);
BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23).size(), 1U);
BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 0, /* max_pct */ 0, /* network */ std::nullopt).size(), 5U);
BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23, /* network */ std::nullopt).size(), 1U);

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

size_t percent23 = (addrman.size() * 23) / 100;
BOOST_CHECK_EQUAL(vAddr.size(), percent23);
Expand Down
5 changes: 4 additions & 1 deletion src/test/fuzz/addrman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,10 @@ FUZZ_TARGET_INIT(addrman, initialize_addrman)
(void)addr_man.Select(fuzzed_data_provider.ConsumeBool());
},
[&] {
(void)addr_man.GetAddr(fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 4096), fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 4096));
(void)addr_man.GetAddr(
/* max_addresses */ fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 4096),
/* max_pct */ fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 4096),
/* network */ std::nullopt);
},
[&] {
const std::optional<CAddress> opt_address = ConsumeDeserializable<CAddress>(fuzzed_data_provider);
Expand Down

0 comments on commit a49f3dd

Please sign in to comment.