highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-01-20 15:19:07 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#32994: p2p: rename GetAddresses -> GetAddressesUnsafe

Browse Source 
1cb2399703 doc: clarify the GetAddresses/GetAddressesUnsafe documentation (Daniela Brozzoni)
e5a7dfd79f p2p: rename GetAddresses -> GetAddressesUnsafe (Daniela Brozzoni)

Pull request description:

  Rename GetAddresses to GetAddressesUnsafe to make it clearer that this function should only be used in trusted contexts. This helps avoid accidental privacy leaks by preventing the uncached version from being used in non-trusted scenarios, like P2P.

  Additionally, better reflect in the documentation that the two methods should be used in different contexts.
  Also update the outdated "call the function without a parameter" phrasing in the cached version. This wording was accurate when the cache was introduced in #18991, but became outdated after later commits (f26502e9fc, 81b00f8780) added parameters to each
  function, and the previous commit changed the function naming completely.

ACKs for top commit:
  stickies-v:
    re-ACK 1cb2399703
  l0rinc:
    ACK 1cb2399703
  luisschwab:
    ACK 1cb2399703
  brunoerg:
    ACK 1cb2399703
  theStack:
    Code-review ACK 1cb2399703
  mzumsande:
    Code review ACK 1cb2399703

Tree-SHA512: 02c05d88436abcdfabad994f47ec5144e9ba47668667a2c1818f57bf8710727505faf8426fd0672c63de14fcf20b96f17cea2acc39fe3c1f56abbc2b1a9e9c23
This commit is contained in:
merge-script
2025-07-25 16:15:50 +01:00
parent 633d8ea17b 1cb2399703
commit fc162299f0
5 changed files with 24 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
src/net.cpp
View File

@@ -3496,7 +3496,7 @@ CConnman::~CConnman()
Stop();
}
std::vector<CAddress> CConnman::GetAddresses(size_t max_addresses, size_t max_pct, std::optional<Network> network, const bool filtered) const
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) {
@@ -3521,7 +3521,7 @@ std::vector<CAddress> CConnman::GetAddresses(CNode& requestor, size_t max_addres
auto r = m_addr_response_caches.emplace(cache_id, CachedAddrResponse{});
CachedAddrResponse& cache_entry = r.first->second;
if (cache_entry.m_cache_entry_expiration < current_time) { // If emplace() added new one it has expiration 0.
cache_entry.m_addrs_response_cache = GetAddresses(max_addresses, max_pct, /*network=*/std::nullopt);
cache_entry.m_addrs_response_cache = GetAddressesUnsafe(max_addresses, max_pct, /*network=*/std::nullopt);
// Choosing a proper cache lifetime is a trade-off between the privacy leak minimization
// and the usefulness of ADDR responses to honest users.
//
@@ -3964,8 +3964,8 @@ 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)};
const std::vector<CAddress> v4_addrs{GetAddressesUnsafe(/*max_addresses=*/0, /*max_pct=*/0, Network::NET_IPV4, /*filtered=*/false)};
const std::vector<CAddress> v6_addrs{GetAddressesUnsafe(/*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),

23
src/net.h
View File

@@ -1169,19 +1169,30 @@ public:
// Addrman functions
/**
* Return all or many randomly selected addresses, optionally by network.
* Return randomly selected addresses. This function does not use the address response cache and
* should only be used in trusted contexts.
*
* An untrusted caller (e.g. from p2p) should instead use @ref GetAddresses to use the cache.
*
* @param[in] max_addresses Maximum number of addresses to return (0 = all).
* @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 high quality (false = all).
*/
std::vector<CAddress> GetAddresses(size_t max_addresses, size_t max_pct, std::optional<Network> network, const bool filtered = true) const;
std::vector<CAddress> GetAddressesUnsafe(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.
* A non-malicious call (from RPC or a peer with addr permission) should
* call the function without a parameter to avoid using the cache.
* Return addresses from the per-requestor cache. If no cache entry exists, it is populated with
* randomly selected addresses. This function can be used in untrusted contexts.
*
* A trusted caller (e.g. from RPC or a peer with addr permission) can use
* @ref GetAddressesUnsafe to avoid using the cache.
*
* @param[in] requestor The requesting peer. Used to key the cache to prevent privacy leaks.
* @param[in] max_addresses Maximum number of addresses to return (0 = all). Ignored when cache
* already contains an entry for requestor.
* @param[in] max_pct Maximum percentage of addresses to return (0 = all). Value must be
* from 0 to 100. Ignored when cache already contains an entry for
* requestor.
*/
std::vector<CAddress> GetAddresses(CNode& requestor, size_t max_addresses, size_t max_pct);

2
src/net_processing.cpp
View File

@@ -4715,7 +4715,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
peer->m_addrs_to_send.clear();
std::vector<CAddress> vAddr;
if (pfrom.HasPermission(NetPermissionFlags::Addr)) {
vAddr = m_connman.GetAddresses(MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND, /*network=*/std::nullopt);
vAddr = m_connman.GetAddressesUnsafe(MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND, /*network=*/std::nullopt);
} else {
vAddr = m_connman.GetAddresses(pfrom, MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND);
}

2
src/rpc/net.cpp
View File

@@ -953,7 +953,7 @@ static RPCHelpMan getnodeaddresses()
}
// returns a shuffled list of CAddress
const std::vector<CAddress> vAddr{connman.GetAddresses(count, /*max_pct=*/0, network)};
const std::vector<CAddress> vAddr{connman.GetAddressesUnsafe(count, /*max_pct=*/0, network)};
UniValue ret(UniValue::VARR);
for (const CAddress& addr : vAddr) {

2
src/test/fuzz/connman.cpp
View File

@@ -113,7 +113,7 @@ FUZZ_TARGET(connman, .init = initialize_connman)
auto max_addresses = fuzzed_data_provider.ConsumeIntegral<size_t>();
auto max_pct = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 100);
auto filtered = fuzzed_data_provider.ConsumeBool();
(void)connman.GetAddresses(max_addresses, max_pct, /*network=*/std::nullopt, filtered);
(void)connman.GetAddressesUnsafe(max_addresses, max_pct, /*network=*/std::nullopt, filtered);
},
[&] {
auto max_addresses = fuzzed_data_provider.ConsumeIntegral<size_t>();