From 99f67685a223148693c400292ef8fec9de8d01d3 Mon Sep 17 00:00:00 2001 From: Gleb Naumenko Date: Wed, 9 Dec 2020 14:38:21 +0200 Subject: [PATCH] [net] Don't PushAddress(self) on VERSION 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. --- src/net_processing.cpp | 26 ++++---------------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index ba000a1f3900f..b261deb70c962 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -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; }