diff options
| -rw-r--r-- | src/addrman.cpp | 12 | ||||
| -rw-r--r-- | src/addrman.h | 3 | ||||
| -rw-r--r-- | src/addrman_impl.h | 4 | ||||
| -rw-r--r-- | src/net.cpp | 23 | ||||
| -rw-r--r-- | src/net.h | 6 | ||||
| -rw-r--r-- | src/netgroup.cpp | 21 | ||||
| -rw-r--r-- | src/netgroup.h | 10 | ||||
| -rw-r--r-- | src/test/addrman_tests.cpp | 18 | ||||
| -rw-r--r-- | src/test/fuzz/addrman.cpp | 3 | ||||
| -rw-r--r-- | src/test/fuzz/connman.cpp | 3 | ||||
| -rwxr-xr-x | test/functional/feature_asmap.py | 9 |
11 files changed, 98 insertions, 14 deletions
diff --git a/src/addrman.cpp b/src/addrman.cpp index 5a11526471..a8206de6ee 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -802,7 +802,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); @@ -832,7 +832,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); } @@ -1216,11 +1216,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; } @@ -1319,9 +1319,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 diff --git a/src/addrman.h b/src/addrman.h index 67dc7604a4..ef9c766eff 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -164,10 +164,11 @@ public: * @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. diff --git a/src/addrman_impl.h b/src/addrman_impl.h index 512f085a21..867c894d01 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -129,7 +129,7 @@ public: 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 @@ -261,7 +261,7 @@ private: * */ 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); diff --git a/src/net.cpp b/src/net.cpp index dc76fdfb44..102d81579f 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -3278,6 +3278,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; } @@ -3383,9 +3389,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);}), @@ -3840,6 +3846,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, @@ -88,6 +88,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}; @@ -1112,6 +1114,7 @@ public: 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); @@ -1146,8 +1149,9 @@ public: * @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. diff --git a/src/netgroup.cpp b/src/netgroup.cpp index a03927b152..0ae229b3f3 100644 --- a/src/netgroup.cpp +++ b/src/netgroup.cpp @@ -5,6 +5,7 @@ #include <netgroup.h> #include <hash.h> +#include <logging.h> #include <util/asmap.h> uint256 NetGroupManager::GetAsmapChecksum() const @@ -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; +} diff --git a/src/netgroup.h b/src/netgroup.h index 2dd63ec66b..5aa6ef7742 100644 --- a/src/netgroup.h +++ b/src/netgroup.h @@ -41,6 +41,16 @@ public: */ 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. * diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index bfefc3ff97..5bd4f271cd 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -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) { diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index 1b11ff6fdf..ece396aadf 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -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()) { diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp index 551e1c526d..24f91abd25 100644 --- a/src/test/fuzz/connman.cpp +++ b/src/test/fuzz/connman.cpp @@ -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( diff --git a/test/functional/feature_asmap.py b/test/functional/feature_asmap.py index 9cff8042a8..ae483fe449 100755 --- a/test/functional/feature_asmap.py +++ b/test/functional/feature_asmap.py @@ -111,6 +111,14 @@ class AsmapTest(BitcoinTestFramework): 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 @@ -124,6 +132,7 @@ class AsmapTest(BitcoinTestFramework): 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__': |