From 3c43d9db1e0f84279b08a93afd730caeece061a9 Mon Sep 17 00:00:00 2001 From: Gleb Naumenko Date: Wed, 9 Dec 2020 14:38:21 +0200 Subject: [PATCH] p2p: Don't self-advertise during VERSION processing Previously, we would prepare to self-announce to a new peer while parsing a VERSION message from that peer. This is redundant, because we do something very similar in MaybeSendAddr(), which is called from SendMessages() after the version handshake is finished. There are a couple of differences: 1) MaybeSendAddr() self-advertises to all peers we do address relay with, not just outbound ones. 2) GetLocalAddrForPeer() called from MaybeSendAddr() makes a probabilistic decision to either advertise what they think we are or what we think we are, while PushAddress(self) on VERSION deterministically only does the former if the address from the latter is unroutable. 3) During VERSION processing, we haven't received a potential sendaddrv2 message from our peer yet, so self-advertisements with addresses from addrV2-only networks would always be dropped in PushAddress(). Since it's confusing to have two slightly different mechanisms for self-advertising, and the one in MaybeSendAddr() is better, remove the one in VERSION. Co-authored-by: Martin Zumsande --- src/net_processing.cpp | 31 ++++--------------------------- 1 file changed, 4 insertions(+), 27 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index eca6263392c..1adcb3695b2 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3274,39 +3274,16 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, m_num_preferred_download_peers += state->fPreferredDownload; } - // Self advertisement & GETADDR logic if (!pfrom.IsInboundConn() && SetupAddressRelay(pfrom, *peer)) { - // 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. We want to avoid // potentially leaking addr information and we do not want to // indicate to the peer that we will participate in addr relay. - if (fListen && !m_chainman.ActiveChainstate().IsInitialBlockDownload()) - { - CAddress addr{GetLocalAddress(pfrom.addr), peer->m_our_services, Now()}; - FastRandomContext insecure_rand; - if (addr.IsRoutable()) - { - LogPrint(BCLog::NET, "ProcessMessages: advertising address %s\n", addr.ToString()); - PushAddress(*peer, addr, insecure_rand); - } else if (IsPeerAddrLocalGood(&pfrom)) { - // Override just the address with whatever the peer sees us as. - // Leave the port in addr as it was returned by GetLocalAddress() - // above, as this is an outbound connection and the peer cannot - // observe our listening port. - addr.SetIP(addrMe); - LogPrint(BCLog::NET, "ProcessMessages: advertising address %s\n", addr.ToString()); - PushAddress(*peer, addr, insecure_rand); - } - } - - // Get recent addresses m_connman.PushMessage(&pfrom, CNetMsgMaker(greatest_common_version).Make(NetMsgType::GETADDR)); peer->m_getaddr_sent = true; // When requesting a getaddr, accept an additional MAX_ADDR_TO_SEND addresses in response