Page MenuHomePhabricator

p2p: Don't self-advertise during VERSION processing
Needs ReviewPublic

Authored by PiRK on Fri, Sep 27, 15:32.

Details

Reviewers
None
Group Reviewers
Restricted Project
Summary

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 <mzumsande@gmail.com>

Co-authored-by: Amiti Uttarwar <amiti@uttarwar.org>

This is a backport of core#26199

Test Plan

ninja all check-all