Skip to content

Commit

Permalink
Merge bitcoin#26199: p2p: Don't self-advertise during version processing
Browse files Browse the repository at this point in the history
956c670 refactor, doc: Improve SetupAddressRelay call in version processing (Martin Zumsande)
3c43d9d p2p: Don't self-advertise during VERSION processing (Gleb Naumenko)

Pull request description:

  This picks up the last commit from bitcoin#19843.

  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()` 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`.

ACKs for top commit:
  stratospher:
    ACK  956c670
  naumenkogs:
    ACK 956c670
  amitiuttarwar:
    reACK 956c670

Tree-SHA512: 933d40615289f055c022170dde7bad0ac0a1d4be377538bfe9ba64375cfeb03bcd803901591f0739ac4850c880e8475a68fd1ab0330800030ab7f19e38c00274
  • Loading branch information
MarcoFalke committed Dec 12, 2022
2 parents 1ea0279 + 956c670 commit 6061eb6
Showing 1 changed file with 14 additions and 32 deletions.
46 changes: 14 additions & 32 deletions src/net_processing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3303,39 +3303,20 @@ 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
// important to help us connect to the network.
//
// Attempt to initialize address relay for outbound peers and use result
// to decide whether to send GETADDR, so that we don't send it to
// inbound or outbound block-relay-only peers.
bool send_getaddr{false};
if (!pfrom.IsInboundConn()) {
send_getaddr = SetupAddressRelay(pfrom, *peer);
}
if (send_getaddr) {
// 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, 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
Expand Down Expand Up @@ -5342,8 +5323,9 @@ bool PeerManagerImpl::SetupAddressRelay(const CNode& node, Peer& peer)
if (node.IsBlockOnlyConn()) return false;

if (!peer.m_addr_relay_enabled.exchange(true)) {
// First addr message we have received from the peer, initialize
// m_addr_known
// During version message processing (non-block-relay-only outbound peers)
// or on first addr-related message we have received (inbound peers), initialize
// m_addr_known.
peer.m_addr_known = std::make_unique<CRollingBloomFilter>(5000, 0.001);
}

Expand Down

0 comments on commit 6061eb6

Please sign in to comment.