Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net: Continuous ASMap health check #27581

Merged
merged 4 commits into from Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/addrman.cpp
Expand Up @@ -800,7 +800,7 @@ int AddrManImpl::GetEntry(bool use_tried, size_t bucket, size_t position) const
return -1;
}

std::vector<CAddress> AddrManImpl::GetAddr_(size_t max_addresses, size_t max_pct, std::optional<Network> network) const
std::vector<CAddress> AddrManImpl::GetAddr_(size_t max_addresses, size_t max_pct, std::optional<Network> network, const bool filtered) const
{
AssertLockHeld(cs);

Expand Down Expand Up @@ -830,7 +830,7 @@ std::vector<CAddress> AddrManImpl::GetAddr_(size_t max_addresses, size_t max_pct
if (network != std::nullopt && ai.GetNetClass() != network) continue;

// Filter for quality
if (ai.IsTerrible(now)) continue;
if (ai.IsTerrible(now) && filtered) continue;

addresses.push_back(ai);
}
Expand Down Expand Up @@ -1214,11 +1214,11 @@ std::pair<CAddress, NodeSeconds> AddrManImpl::Select(bool new_only, std::optiona
return addrRet;
}

std::vector<CAddress> AddrManImpl::GetAddr(size_t max_addresses, size_t max_pct, std::optional<Network> network) const
std::vector<CAddress> AddrManImpl::GetAddr(size_t max_addresses, size_t max_pct, std::optional<Network> network, const bool filtered) const
{
LOCK(cs);
Check();
auto addresses = GetAddr_(max_addresses, max_pct, network);
auto addresses = GetAddr_(max_addresses, max_pct, network, filtered);
Check();
return addresses;
}
Expand Down Expand Up @@ -1317,9 +1317,9 @@ std::pair<CAddress, NodeSeconds> AddrMan::Select(bool new_only, std::optional<Ne
return m_impl->Select(new_only, network);
}

std::vector<CAddress> AddrMan::GetAddr(size_t max_addresses, size_t max_pct, std::optional<Network> network) const
std::vector<CAddress> AddrMan::GetAddr(size_t max_addresses, size_t max_pct, std::optional<Network> network, const bool filtered) const
{
return m_impl->GetAddr(max_addresses, max_pct, network);
return m_impl->GetAddr(max_addresses, max_pct, network, filtered);
}

std::vector<std::pair<AddrInfo, AddressPosition>> AddrMan::GetEntries(bool use_tried) const
Expand Down
3 changes: 2 additions & 1 deletion src/addrman.h
Expand Up @@ -164,10 +164,11 @@ class AddrMan
* @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).
* @param[in] filtered Select only addresses that are considered good quality (false = all).
*
* @return A vector of randomly selected addresses from vRandom.
*/
std::vector<CAddress> GetAddr(size_t max_addresses, size_t max_pct, std::optional<Network> network) const;
std::vector<CAddress> GetAddr(size_t max_addresses, size_t max_pct, std::optional<Network> network, const bool filtered = true) const;

/**
* Returns an information-location pair for all addresses in the selected addrman table.
Expand Down
4 changes: 2 additions & 2 deletions src/addrman_impl.h
Expand Up @@ -129,7 +129,7 @@ class AddrManImpl
std::pair<CAddress, NodeSeconds> Select(bool new_only, std::optional<Network> network) const
EXCLUSIVE_LOCKS_REQUIRED(!cs);

std::vector<CAddress> GetAddr(size_t max_addresses, size_t max_pct, std::optional<Network> network) const
std::vector<CAddress> GetAddr(size_t max_addresses, size_t max_pct, std::optional<Network> network, const bool filtered = true) const
EXCLUSIVE_LOCKS_REQUIRED(!cs);

std::vector<std::pair<AddrInfo, AddressPosition>> GetEntries(bool from_tried) const
Expand Down Expand Up @@ -261,7 +261,7 @@ class AddrManImpl
* */
int GetEntry(bool use_tried, size_t bucket, size_t position) const EXCLUSIVE_LOCKS_REQUIRED(cs);

std::vector<CAddress> GetAddr_(size_t max_addresses, size_t max_pct, std::optional<Network> network) const EXCLUSIVE_LOCKS_REQUIRED(cs);
std::vector<CAddress> GetAddr_(size_t max_addresses, size_t max_pct, std::optional<Network> network, const bool filtered = true) const EXCLUSIVE_LOCKS_REQUIRED(cs);

std::vector<std::pair<AddrInfo, AddressPosition>> GetEntries_(bool from_tried) const EXCLUSIVE_LOCKS_REQUIRED(cs);

Expand Down
23 changes: 21 additions & 2 deletions src/net.cpp
Expand Up @@ -3305,6 +3305,12 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions)
// Dump network addresses
scheduler.scheduleEvery([this] { DumpAddresses(); }, DUMP_PEERS_INTERVAL);

// Run the ASMap Health check once and then schedule it to run every 24h.
if (m_netgroupman.UsingASMap()) {
ASMapHealthCheck();
scheduler.scheduleEvery([this] { ASMapHealthCheck(); }, ASMAP_HEALTH_CHECK_INTERVAL);
}

return true;
}

Expand Down Expand Up @@ -3410,9 +3416,9 @@ CConnman::~CConnman()
Stop();
}

std::vector<CAddress> CConnman::GetAddresses(size_t max_addresses, size_t max_pct, std::optional<Network> network) const
std::vector<CAddress> CConnman::GetAddresses(size_t max_addresses, size_t max_pct, std::optional<Network> network, const bool filtered) const
{
std::vector<CAddress> addresses = addrman.GetAddr(max_addresses, max_pct, network);
std::vector<CAddress> addresses = addrman.GetAddr(max_addresses, max_pct, network, filtered);
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 Expand Up @@ -3853,6 +3859,19 @@ void CConnman::PerformReconnections()
}
}

void CConnman::ASMapHealthCheck()
{
const std::vector<CAddress> v4_addrs{GetAddresses(/*max_addresses=*/ 0, /*max_pct=*/ 0, Network::NET_IPV4, /*filtered=*/ false)};
const std::vector<CAddress> v6_addrs{GetAddresses(/*max_addresses=*/ 0, /*max_pct=*/ 0, Network::NET_IPV6, /*filtered=*/ false)};
std::vector<CNetAddr> clearnet_addrs;
clearnet_addrs.reserve(v4_addrs.size() + v6_addrs.size());
std::transform(v4_addrs.begin(), v4_addrs.end(), std::back_inserter(clearnet_addrs),
[](const CAddress& addr) { return static_cast<CNetAddr>(addr); });
std::transform(v6_addrs.begin(), v6_addrs.end(), std::back_inserter(clearnet_addrs),
[](const CAddress& addr) { return static_cast<CNetAddr>(addr); });
m_netgroupman.ASMapHealthCheck(clearnet_addrs);
}

// Dump binary message to file, with timestamp.
static void CaptureMessageToFile(const CAddress& addr,
const std::string& msg_type,
Expand Down
6 changes: 5 additions & 1 deletion src/net.h
Expand Up @@ -87,6 +87,8 @@ static const bool DEFAULT_BLOCKSONLY = false;
static const int64_t DEFAULT_PEER_CONNECT_TIMEOUT = 60;
/** Number of file descriptors required for message capture **/
static const int NUM_FDS_MESSAGE_CAPTURE = 1;
/** Interval for ASMap Health Check **/
static constexpr std::chrono::hours ASMAP_HEALTH_CHECK_INTERVAL{24};

static constexpr bool DEFAULT_FORCEDNSSEED{false};
static constexpr bool DEFAULT_DNSSEED{true};
Expand Down Expand Up @@ -1138,6 +1140,7 @@ class CConnman
void SetNetworkActive(bool active);
void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant&& grant_outbound, const char* strDest, ConnectionType conn_type, bool use_v2transport) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex);
bool CheckIncomingNonce(uint64_t nonce);
void ASMapHealthCheck();

// alias for thread safety annotations only, not defined
RecursiveMutex& GetNodesMutex() const LOCK_RETURNED(m_nodes_mutex);
Expand Down Expand Up @@ -1172,8 +1175,9 @@ class CConnman
* @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).
* @param[in] filtered Select only addresses that are considered high quality (false = all).
*/
std::vector<CAddress> GetAddresses(size_t max_addresses, size_t max_pct, std::optional<Network> network) const;
std::vector<CAddress> GetAddresses(size_t max_addresses, size_t max_pct, std::optional<Network> network, const bool filtered = true) const;
/**
* Cache is used to minimize topology leaks, so it should
* be used for all non-trusted calls, for example, p2p.
Expand Down
21 changes: 21 additions & 0 deletions src/netgroup.cpp
Expand Up @@ -5,6 +5,7 @@
#include <netgroup.h>

#include <hash.h>
#include <logging.h>
#include <util/asmap.h>

uint256 NetGroupManager::GetAsmapChecksum() const
Expand Down Expand Up @@ -109,3 +110,23 @@ uint32_t NetGroupManager::GetMappedAS(const CNetAddr& address) const
uint32_t mapped_as = Interpret(m_asmap, ip_bits);
return mapped_as;
}

void NetGroupManager::ASMapHealthCheck(const std::vector<CNetAddr>& clearnet_addrs) const {
std::set<uint32_t> clearnet_asns{};
int unmapped_count{0};

for (const auto& addr : clearnet_addrs) {
uint32_t asn = GetMappedAS(addr);
if (asn == 0) {
++unmapped_count;
continue;
}
clearnet_asns.insert(asn);
}

LogPrintf("ASMap Health Check: %i clearnet peers are mapped to %i ASNs with %i peers being unmapped\n", clearnet_addrs.size(), clearnet_asns.size(), unmapped_count);
}

bool NetGroupManager::UsingASMap() const {
return m_asmap.size() > 0;
}
10 changes: 10 additions & 0 deletions src/netgroup.h
Expand Up @@ -41,6 +41,16 @@ class NetGroupManager {
*/
uint32_t GetMappedAS(const CNetAddr& address) const;

/**
* Analyze and log current health of ASMap based buckets.
*/
void ASMapHealthCheck(const std::vector<CNetAddr>& clearnet_addrs) const;

/**
* Indicates whether ASMap is being used for clearnet bucketing.
*/
bool UsingASMap() const;

private:
/** Compressed IP->ASN mapping, loaded from a file when a node starts.
*
Expand Down
18 changes: 18 additions & 0 deletions src/test/addrman_tests.cpp
Expand Up @@ -429,6 +429,24 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr)
BOOST_CHECK_EQUAL(addrman->Size(), 2006U);
}

BOOST_AUTO_TEST_CASE(getaddr_unfiltered)
{
auto addrman = std::make_unique<AddrMan>(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node));

// Set time on this addr so isTerrible = false
CAddress addr1 = CAddress(ResolveService("250.250.2.1", 8333), NODE_NONE);
addr1.nTime = Now<NodeSeconds>();
// Not setting time so this addr should be isTerrible = true
CAddress addr2 = CAddress(ResolveService("250.251.2.2", 9999), NODE_NONE);

CNetAddr source = ResolveIP("250.1.2.1");
BOOST_CHECK(addrman->Add({addr1, addr2}, source));

// Filtered GetAddr should only return addr1
BOOST_CHECK_EQUAL(addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt).size(), 1U);
// Unfiltered GetAddr should return addr1 and addr2
BOOST_CHECK_EQUAL(addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt, /*filtered=*/false).size(), 2U);
}

BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket_legacy)
{
Expand Down
3 changes: 2 additions & 1 deletion src/test/fuzz/addrman.cpp
Expand Up @@ -286,7 +286,8 @@ FUZZ_TARGET(addrman, .init = initialize_addrman)
(void)const_addr_man.GetAddr(
/*max_addresses=*/fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 4096),
/*max_pct=*/fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 4096),
network);
network,
/*filtered=*/fuzzed_data_provider.ConsumeBool());
(void)const_addr_man.Select(fuzzed_data_provider.ConsumeBool(), network);
std::optional<bool> in_new;
if (fuzzed_data_provider.ConsumeBool()) {
Expand Down
3 changes: 2 additions & 1 deletion src/test/fuzz/connman.cpp
Expand Up @@ -88,7 +88,8 @@ FUZZ_TARGET(connman, .init = initialize_connman)
(void)connman.GetAddresses(
/*max_addresses=*/fuzzed_data_provider.ConsumeIntegral<size_t>(),
/*max_pct=*/fuzzed_data_provider.ConsumeIntegral<size_t>(),
/*network=*/std::nullopt);
/*network=*/std::nullopt,
/*filtered=*/fuzzed_data_provider.ConsumeBool());
},
[&] {
(void)connman.GetAddresses(
Expand Down
9 changes: 9 additions & 0 deletions test/functional/feature_asmap.py
Expand Up @@ -111,6 +111,14 @@ def test_empty_asmap(self):
self.node.assert_start_raises_init_error(extra_args=['-asmap'], expected_msg=msg)
os.remove(self.default_asmap)

def test_asmap_health_check(self):
self.log.info('Test bitcoind -asmap logs ASMap Health Check with basic stats')
shutil.copyfile(self.asmap_raw, self.default_asmap)
msg = "ASMap Health Check: 2 clearnet peers are mapped to 1 ASNs with 0 peers being unmapped"
with self.node.assert_debug_log(expected_msgs=[msg]):
self.start_node(0, extra_args=['-asmap'])
os.remove(self.default_asmap)

def run_test(self):
self.node = self.nodes[0]
self.datadir = self.node.chain_path
Expand All @@ -124,6 +132,7 @@ def run_test(self):
self.test_asmap_interaction_with_addrman_containing_entries()
self.test_default_asmap_with_missing_file()
self.test_empty_asmap()
self.test_asmap_health_check()


if __name__ == '__main__':
Expand Down