Merge #21187: Net processing: Only call PushAddress() from net_processing

3e68efa615 [net] Move checks from GetLocalAddrForPeer to caller (John Newbery)
d21d2b264c [net] Change AdvertiseLocal to GetLocalAddrForPeer (John Newbery)

Pull request description:

  This is the first part of #21186. It slightly disentangles addr handling in net/net_processing by making it explicit that net_processing is responsible for pushing addr records into `vAddrToSend`.

ACKs for top commit:
  MarcoFalke:
    re-ACK 3e68efa615 🍅

Tree-SHA512: 9af50c41f5a977e2e277f24a589db38e2980b353401def5e74b108ac5f493d9b5d6b1b8bf15323a4d66321495f04bc271450fcef7aa7d1c095f051a4f8e9b15f
This commit is contained in:
MarcoFalke 2021-02-19 12:58:30 +01:00
commit 09eb46c943
No known key found for this signature in database
GPG key ID: D2EA4850E7528B25
4 changed files with 31 additions and 27 deletions

View file

@ -201,11 +201,8 @@ bool IsPeerAddrLocalGood(CNode *pnode)
IsReachable(addrLocal.GetNetwork()); IsReachable(addrLocal.GetNetwork());
} }
// pushes our own address to a peer Optional<CAddress> GetLocalAddrForPeer(CNode *pnode)
void AdvertiseLocal(CNode *pnode)
{ {
if (fListen && pnode->fSuccessfullyConnected)
{
CAddress addrLocal = GetLocalAddress(&pnode->addr, pnode->GetLocalServices()); CAddress addrLocal = GetLocalAddress(&pnode->addr, pnode->GetLocalServices());
if (gArgs.GetBoolArg("-addrmantest", false)) { if (gArgs.GetBoolArg("-addrmantest", false)) {
// use IPv4 loopback during addrmantest // use IPv4 loopback during addrmantest
@ -222,10 +219,11 @@ void AdvertiseLocal(CNode *pnode)
} }
if (addrLocal.IsRoutable() || gArgs.GetBoolArg("-addrmantest", false)) if (addrLocal.IsRoutable() || gArgs.GetBoolArg("-addrmantest", false))
{ {
LogPrint(BCLog::NET, "AdvertiseLocal: advertising address %s\n", addrLocal.ToString()); LogPrint(BCLog::NET, "Advertising address %s to peer=%d\n", addrLocal.ToString(), pnode->GetId());
pnode->PushAddress(addrLocal, rng); return addrLocal;
}
} }
// Address is unroutable. Don't advertise.
return nullopt;
} }
// learn a new local address // learn a new local address

View file

@ -197,7 +197,8 @@ enum
}; };
bool IsPeerAddrLocalGood(CNode *pnode); bool IsPeerAddrLocalGood(CNode *pnode);
void AdvertiseLocal(CNode *pnode); /** Returns a local address that we should advertise to this peer */
Optional<CAddress> GetLocalAddrForPeer(CNode *pnode);
/** /**
* Mark a network as reachable or unreachable (no automatic connects to it) * Mark a network as reachable or unreachable (no automatic connects to it)

View file

@ -4416,7 +4416,9 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
// Address refresh broadcast // Address refresh broadcast
auto current_time = GetTime<std::chrono::microseconds>(); auto current_time = GetTime<std::chrono::microseconds>();
if (pto->RelayAddrsWithConn() && !::ChainstateActive().IsInitialBlockDownload() && pto->m_next_local_addr_send < current_time) { if (fListen && pto->RelayAddrsWithConn() &&
!::ChainstateActive().IsInitialBlockDownload() &&
pto->m_next_local_addr_send < current_time) {
// If we've sent before, clear the bloom filter for the peer, so that our // If we've sent before, clear the bloom filter for the peer, so that our
// self-announcement will actually go out. // self-announcement will actually go out.
// This might be unnecessary if the bloom filter has already rolled // This might be unnecessary if the bloom filter has already rolled
@ -4426,7 +4428,10 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
if (pto->m_next_local_addr_send != 0us) { if (pto->m_next_local_addr_send != 0us) {
pto->m_addr_known->reset(); pto->m_addr_known->reset();
} }
AdvertiseLocal(pto); if (Optional<CAddress> local_addr = GetLocalAddrForPeer(pto)) {
FastRandomContext insecure_rand;
pto->PushAddress(*local_addr, insecure_rand);
}
pto->m_next_local_addr_send = PoissonNextSend(current_time, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL); pto->m_next_local_addr_send = PoissonNextSend(current_time, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL);
} }

View file

@ -691,7 +691,7 @@ BOOST_AUTO_TEST_CASE(ipv4_peer_with_ipv6_addrMe_test)
pnode->SetAddrLocal(addrLocal); pnode->SetAddrLocal(addrLocal);
// before patch, this causes undefined behavior detectable with clang's -fsanitize=memory // before patch, this causes undefined behavior detectable with clang's -fsanitize=memory
AdvertiseLocal(&*pnode); GetLocalAddrForPeer(&*pnode);
// suppress no-checks-run warning; if this test fails, it's by triggering a sanitizer // suppress no-checks-run warning; if this test fails, it's by triggering a sanitizer
BOOST_CHECK(1); BOOST_CHECK(1);