Skip to content

Commit

Permalink
[net] Don't PushAddress(self) on VERSION
Browse files Browse the repository at this point in the history
Previously, we would prepare to self-announce to a new peer while
parsing a VERSION message from that peer. This is useless,
because we do something very similar in AdvertiseLocal(),
although there are a couple differences:
1) AdvertiseLocal() does this for all peers, not just outbound
2) AdvertiseLocal() always asks the peer to advertise based on what
they think we are AND what we think we are (assuming it's routable),
while PushAddress(self) on VERSION always does one of the two.

(1) and the fact that AdvertiseLocal() is called right before
actually sending out ADDR message with our address makes it fully
encompassing PushAddress(self) on VERSION.

Per (2), AdvertiseLocal() seems like a better version of
PushAddress(self) on VERSION.

Thus, it's fine to drop PushAddress(self) on VERSION.
  • Loading branch information
naumenkogs committed Dec 18, 2020
1 parent 10fc62f commit 99f6768
Showing 1 changed file with 4 additions and 22 deletions.
26 changes: 4 additions & 22 deletions src/net_processing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2386,32 +2386,14 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat
}

if (!pfrom.IsInboundConn() && !pfrom.IsBlockOnlyConn()) {
// For outbound peers, we try to relay our address (so that other
// nodes can try to find us more quickly, as we have no guarantee
// that an outbound peer is even aware of how to reach us) and do a
// one-time address fetch (to help populate/update our addrman). If
// we're starting up for the first time, our addrman may be pretty
// empty and no one will know who we are, so these mechanisms are
// For outbound peers, we do a one-time address fetch
// (to help populate/update our addrman).
// If we're starting up for the first time, our addrman may be pretty
// empty and no one will know who we are, so this mechanism is
// important to help us connect to the network.
//
// We skip this for block-relay-only peers to avoid potentially leaking
// information about our block-relay-only connections via address relay.
if (fListen && !::ChainstateActive().IsInitialBlockDownload() && pfrom.fSuccessfullyConnected)
{
CAddress addr = GetLocalAddress(pfrom.addr, pfrom.GetLocalServices());
FastRandomContext insecure_rand;
if (addr.IsRoutable())
{
LogPrint(BCLog::NET, "ProcessMessages: advertising address %s\n", addr.ToString());
pfrom.PushAddress(addr, insecure_rand);
} else if (IsPeerAddrLocalGood(pfrom)) {
addr.SetIP(addrMe);
LogPrint(BCLog::NET, "ProcessMessages: advertising address %s\n", addr.ToString());
pfrom.PushAddress(addr, insecure_rand);
}
}

// Get recent addresses
m_connman.PushMessage(&pfrom, CNetMsgMaker(greatest_common_version).Make(NetMsgType::GETADDR));
pfrom.fGetAddr = true;
}
Expand Down

0 comments on commit 99f6768

Please sign in to comment.