addrman, net: filter during selection via AddrPolicy to avoid underfill

Add AddrMan::AddrPolicy, a predicate returning true to exclude an address
during selection. This allows callers to apply custom filtering directly in
AddrMan rather than post-processing results.

Use the new policy in CConnman::GetAddressesUnsafe to exclude banned and
discouraged peers while filling address requests. This fixes underfilled
results observed when banned peers were removed after selection, causing
P2P responses from `GETADDR` to return fewer entries than requested.

Also log the number of filtered entries for debugging and diagnostics.

Author: waketraindev

src/addrman.cpp

@@ -809,7 +809,7 @@ nid_type AddrManImpl::GetEntry(bool use_tried, size_t bucket, size_t position) c
     return -1;
 }
 
-std::vector<CAddress> AddrManImpl::GetAddr_(size_t max_addresses, size_t max_pct, std::optional<Network> network, const bool filtered) const
+std::vector<CAddress> AddrManImpl::GetAddr_(size_t max_addresses, size_t max_pct, std::optional<Network> network, const bool filtered, const AddrMan::AddrPolicy& policy) const
 {
     AssertLockHeld(cs);
     Assume(max_pct <= 100);
@@ -827,6 +827,7 @@ std::vector<CAddress> AddrManImpl::GetAddr_(size_t max_addresses, size_t max_pct
     const auto now{Now<NodeSeconds>()};
     std::vector<CAddress> addresses;
     addresses.reserve(nNodes);
+    size_t filtered_addresses = 0;
     for (unsigned int n = 0; n < vRandom.size(); n++) {
         if (addresses.size() >= nNodes)
             break;
@@ -839,14 +840,26 @@ std::vector<CAddress> AddrManImpl::GetAddr_(size_t max_addresses, size_t max_pct
         const AddrInfo& ai{it->second};
 
         // Filter by network (optional)
-        if (network != std::nullopt && ai.GetNetClass() != network) continue;
+        if (network != std::nullopt && ai.GetNetClass() != network) {
+            ++filtered_addresses;
+            continue;
+        }
 
         // Filter for quality
-        if (ai.IsTerrible(now) && filtered) continue;
+        if (ai.IsTerrible(now) && filtered) {
+            ++filtered_addresses;
+            continue;
+        }
+
+        // Filter by policy
+        if (policy && policy(ai)) {
+            ++filtered_addresses;
+            continue;
+        }
 
         addresses.push_back(ai);
     }
-    LogDebug(BCLog::ADDRMAN, "GetAddr returned %d random addresses\n", addresses.size());
+    LogDebug(BCLog::ADDRMAN, "GetAddr returned %zu random addresses, %zu addresses filtered\n", addresses.size(), filtered_addresses);
     return addresses;
 }
 
@@ -1226,11 +1239,11 @@ std::pair<CAddress, NodeSeconds> AddrManImpl::Select(bool new_only, const std::u
     return addrRet;
 }
 
-std::vector<CAddress> AddrManImpl::GetAddr(size_t max_addresses, size_t max_pct, std::optional<Network> network, const bool filtered) const
+std::vector<CAddress> AddrManImpl::GetAddr(size_t max_addresses, size_t max_pct, std::optional<Network> network, const bool filtered, const AddrMan::AddrPolicy& policy) const
 {
     LOCK(cs);
     Check();
-    auto addresses = GetAddr_(max_addresses, max_pct, network, filtered);
+    auto addresses = GetAddr_(max_addresses, max_pct, network, filtered, policy);
     Check();
     return addresses;
 }
@@ -1329,9 +1342,9 @@ std::pair<CAddress, NodeSeconds> AddrMan::Select(bool new_only, const std::unord
     return m_impl->Select(new_only, networks);
 }
 
-std::vector<CAddress> AddrMan::GetAddr(size_t max_addresses, size_t max_pct, std::optional<Network> network, const bool filtered) const
+std::vector<CAddress> AddrMan::GetAddr(size_t max_addresses, size_t max_pct, std::optional<Network> network, const bool filtered, const AddrPolicy& policy) const
 {
-    return m_impl->GetAddr(max_addresses, max_pct, network, filtered);
+    return m_impl->GetAddr(max_addresses, max_pct, network, filtered, policy);
 }
 
 std::vector<std::pair<AddrInfo, AddressPosition>> AddrMan::GetEntries(bool use_tried) const

src/addrman.h

@@ -18,6 +18,7 @@
 #include <unordered_set>
 #include <utility>
 #include <vector>
+#include <functional>
 
 class InvalidAddrManVersionError : public std::ios_base::failure
 {
@@ -91,6 +92,12 @@ class AddrMan
     const std::unique_ptr<AddrManImpl> m_impl;
 
 public:
+    /** Predicate used to exclude addresses during selection.
+     *  Return true to skip the given address.
+     *  Must be non-blocking and must not acquire locks that can conflict with AddrMan::cs.
+     */
+    using AddrPolicy = std::function<bool(const CAddress&)>;
+
     explicit AddrMan(const NetGroupManager& netgroupman, bool deterministic, int32_t consistency_check_ratio);
 
     ~AddrMan();
@@ -169,10 +176,11 @@ class AddrMan
      * @param[in] max_pct        Maximum percentage of addresses to return (0 = all). Value must be from 0 to 100.
      * @param[in] network        Select only addresses of this network (nullopt = all).
      * @param[in] filtered       Select only addresses that are considered good quality (false = all).
+     * @param[in] policy         Optional predicate to exclude candidates during selection (true = exclude).
      *
      * @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 bool filtered = true) const;
+    std::vector<CAddress> GetAddr(size_t max_addresses, size_t max_pct, std::optional<Network> network, const bool filtered = true, const AddrPolicy& policy = {}) const;
 
     /**
      * Returns an information-location pair for all addresses in the selected addrman table.

src/addrman_impl.h

@@ -135,7 +135,7 @@ class AddrManImpl
     std::pair<CAddress, NodeSeconds> Select(bool new_only, const std::unordered_set<Network>& networks) 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
+    std::vector<CAddress> GetAddr(size_t max_addresses, size_t max_pct, std::optional<Network> network, const bool filtered = true, const AddrMan::AddrPolicy& policy = {}) const
         EXCLUSIVE_LOCKS_REQUIRED(!cs);
 
     std::vector<std::pair<AddrInfo, AddressPosition>> GetEntries(bool from_tried) const
@@ -267,7 +267,7 @@ class AddrManImpl
      * */
     nid_type 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 bool filtered = true) 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 AddrMan::AddrPolicy& policy = {}) const EXCLUSIVE_LOCKS_REQUIRED(cs);
 
     std::vector<std::pair<AddrInfo, AddressPosition>> GetEntries_(bool from_tried) const EXCLUSIVE_LOCKS_REQUIRED(cs);
 

src/net.cpp

@@ -3507,13 +3507,10 @@ CConnman::~CConnman()
 
 std::vector<CAddress> CConnman::GetAddressesUnsafe(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, 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);}),
-                        addresses.end());
-    }
-    return addresses;
+        const AddrMan::AddrPolicy policy = [this](const CAddress& addr) {
+            return m_banman && (m_banman->IsDiscouraged(addr) || m_banman->IsBanned(addr));
+        };
+        return addrman.GetAddr(/*max_addresses=*/max_addresses, /*max_pct=*/max_pct, /*network=*/network, /*filtered=*/filtered, /*policy=*/policy);
 }
 
 std::vector<CAddress> CConnman::GetAddresses(CNode& requestor, size_t max_addresses, size_t max_pct)

src/test/addrman_tests.cpp

@@ -407,6 +407,27 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr)
     // 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, /*network=*/std::nullopt).size(), 1U);
 
+    // Test AddrMan::GetAddr filtering with various AddrPolicy rules.
+    const size_t addedAddresses = addrman->Size();
+    const AddrMan::AddrPolicy policyNoSkip = [](const CAddress&) { return false; };
+    const AddrMan::AddrPolicy policySkipAll = [](const CAddress&) { return true; };
+    const AddrMan::AddrPolicy policyPortPolicy = [](const CAddress& addr) { return addr.GetPort() != 8333; };
+
+    const std::vector<CAddress> vAddrNoPolicy = addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt, /*filtered=*/false);
+    const std::vector<CAddress> vAddrPolicyNoSkip = addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt, /*filtered=*/false, /*policy=*/policyNoSkip);
+    const std::vector<CAddress> vAddrPolicySkipAll = addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt, /*filtered=*/false, /*policy=*/policySkipAll);
+    const std::vector<CAddress> vAddrPolicyPort = addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt, /*filtered=*/false, /*policy=*/policyPortPolicy);
+    const std::set<CAddress> vExpectedAddresses = {addr1, addr3, addr4, addr5};
+
+    BOOST_CHECK_EQUAL(vAddrNoPolicy.size(), addedAddresses);
+    BOOST_CHECK_EQUAL(vAddrPolicyNoSkip.size(), vAddrNoPolicy.size());
+    BOOST_CHECK_EQUAL(vAddrPolicySkipAll.size(), 0);
+
+    // Check that vAddrPolicyPort contains exactly the same items as vExpectedAddresses
+    BOOST_CHECK_EQUAL(vAddrPolicyPort.size(), vExpectedAddresses.size());
+    BOOST_CHECK(std::all_of(vAddrPolicyPort.begin(), vAddrPolicyPort.end(), [&](const CAddress& a){ return vExpectedAddresses.contains(a); }));
+    BOOST_CHECK(std::none_of(vAddrPolicyPort.begin(), vAddrPolicyPort.end(), policyPortPolicy));
+
     // Test: Ensure GetAddr works with new and tried addresses.
     BOOST_CHECK(addrman->Good(CAddress(addr1, NODE_NONE)));
     BOOST_CHECK(addrman->Good(CAddress(addr2, NODE_NONE)));