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 <[email protected]>

Author: naumenkogs

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<NodeSeconds>()};
-                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