BIP155 follow-ups #20119

pull sipa wants to merge 2 commits into bitcoin:master from sipa:202010_bip155_followup changing 3 files +11 −1

sipa commented at 7:17 AM on October 11, 2020: member
This:
- Documents BIP155 support in doc/bips.md
- Restricts addrv2 relay to IPv4, IPv6, and Tor addresses. Relaying addresses in ranges that no network software has support for seems like a gratuitous spam vector.
fanquake added the label P2P on Oct 11, 2020
hebasto commented at 8:36 AM on October 11, 2020: member

Concept ACK.
hebasto approved
hebasto commented at 8:42 AM on October 11, 2020: member

ACK 7ca0e87278ffc096df28c6779bb2a8d2d10161f9

in doc/bips.md:40 in 7ca0e87278 outdated

  36 | @@ -37,7 +37,8 @@ BIPs that are implemented by Bitcoin Core (up-to-date up to **v0.21.0**):
  37 |  * [`BIP 145`](https://github.com/bitcoin/bips/blob/master/bip-0145.mediawiki): getblocktemplate updates for Segregated Witness as of **v0.13.0** ([PR 8149](https://github.com/bitcoin/bitcoin/pull/8149)).
  38 |  * [`BIP 147`](https://github.com/bitcoin/bips/blob/master/bip-0147.mediawiki): NULLDUMMY softfork as of **v0.13.1** ([PR 8636](https://github.com/bitcoin/bitcoin/pull/8636) and [PR 8937](https://github.com/bitcoin/bitcoin/pull/8937)), *buried* since **v0.19.0** ([PR #16060](https://github.com/bitcoin/bitcoin/pull/16060)).
  39 |  * [`BIP 152`](https://github.com/bitcoin/bips/blob/master/bip-0152.mediawiki): Compact block transfer and related optimizations are used as of **v0.13.0** ([PR 8068](https://github.com/bitcoin/bitcoin/pull/8068)).
  40 | -- [`BIP 158`](https://github.com/bitcoin/bips/blob/master/bip-0158.mediawiki): Compact Block Filters for Light Clients can be indexed as of **v0.19.0** ([PR #14121](https://github.com/bitcoin/bitcoin/pull/14121)).
  41 | +* [`BIP 155`](https://github.com/bitcoin/bips/blob/master/bip-0155.mediawiki): The 'addrv2' and 'sendaddrv2' messages which enable relay of Tor V3 addresses and more are supported as of **v0.21.0** ([PR 19845](https://github.com/bitcoin/bitcoin/pull/19845)).

jonatack commented at 11:14 AM on October 11, 2020:

ISTM should be (or also be) [PR 19954](https://github.com/bitcoin/bitcoin/pull/19954) (or maybe 19031)

sipa commented at 6:31 PM on October 11, 2020:

Fixed, it should have been #19954.

jonatack commented at 11:31 AM on October 11, 2020: member
Concept ACK.

I wondered a little why the additional "should we relay this address?" logic is in RelayAddress() instead in the caller along with the existing logic. (Edit: domain separation and/or more elegant code, I suppose. It's certainly not a blocker.)

src/net_processing.cpp::L2660
```
            bool fReachable = IsReachable(addr);
            if (addr.nTime > nSince && !pfrom.fGetAddr && vAddr.size() <= 10 && addr.IsRoutable())
            {
                // Relay to a limited number of other nodes
                RelayAddress(addr, fReachable, m_connman);
            }
```
Mention BIP155 in doc/bips.md 79f3d9b932
Only relay IPv4, IPv6, Tor addresses 56f9dba015
sipa force-pushed on Oct 11, 2020
jonatack commented at 6:48 PM on October 11, 2020: member

ACK 56f9dba015c592b8925795012e3061a710070a27
hebasto approved
hebasto commented at 6:47 AM on October 12, 2020: member

ACK 56f9dba015c592b8925795012e3061a710070a27, verified both links.
naumenkogs commented at 7:04 AM on October 12, 2020: member

ACK 56f9dba
fanquake merged this on Oct 12, 2020
fanquake closed this on Oct 12, 2020
vasild commented at 8:06 AM on October 12, 2020: member

(post merge) ACK 56f9dba01

This patch achieves the same as the one at the bottom of #19954 (comment) and in addition the patch in this PR will relay I2P addresses if they become reachable in the future without needing an adjustment.

Given that the newly added function IsRelayable() deals with a higher level logic of whether to relay or not, maybe it would be better if it lives somewhere in net_processing.cc, next to RelayAddress() instead of as a method of the lower level CNetAddr class. But it works anyway and can be adjusted further if needed.

Thanks!
jonatack commented at 8:15 AM on October 12, 2020: member

Further BIP155 follow-ups and minor bugfix at #20120.
vasild referenced this in commit ba45f02708 on Jun 10, 2021
luke-jr referenced this in commit 0ffe41289b on Jun 27, 2021
laanwj referenced this in commit a88fa1a555 on Jul 15, 2021
hebasto referenced this in commit cb443c744c on Jul 19, 2021
jarolrod referenced this in commit 20bf2d3301 on Jul 19, 2021
josibake referenced this in commit 74e076b489 on Jul 21, 2021
sidhujag referenced this in commit ae9a90b03d on Jul 23, 2021
janus referenced this in commit 184583b41e on Oct 29, 2021
Fabcien referenced this in commit 5a006987e7 on Nov 10, 2021
DrahtBot locked this on Feb 15, 2022

Contributors

Labels

Linked (view graph)

#22211 net: relay I2P addresses even if not reachable (by us)