From: "Eric Voskuil" <eric@voskuil.org>
To: "'Anthony Towns'" <aj@erisian.com.au>
Date: Wed, 5 Oct 2022 13:43:10 -0700
Message-ID: <03ca01d8d8fb$1558ed50$400ac7f0$@voskuil.org>
MIME-Version: 1.0
Content-Type: text/plain;
	charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Thread-Index: AQGlvvms9os83PkuShznljIluyzkPw==
Content-Language: en-us
Cc: 'Bitcoin Protocol Discussion' <bitcoin-dev@lists.linuxfoundation.org>
Subject: Re: [bitcoin-dev] Packaged Transaction Relay
Precedence: list

>> [Regarding bandwidth waste: I've pointed out in years past that
>> breaking the Bitcoin versioning scheme creates a requirement that any
>> unknown message type be considered valid. Up until a =
recently-deployed
>> protocol change, it had always been possible to validate messages by
>> type. I noticed recently that validating nodes have been dropping =
peers
>> at an increasing rate (a consequence of that deployment). Despite =
being
>> an undocumented compatibility break, it is now unfortunately a matter
>> of protocol that a peer must allow its peers to waste its bandwidth =
to
>> remain compatible - something which we should eliminate.]
>=20
> The only message listed as not being preceded by a bumped version =
number
> in:
>=20
> =
https://github.com/libbitcoin/libbitcoin-network/wiki/Protocol-Versioning=


Good find, still a work in progress.

> is addrv2 (though addrv2 is gated on mutual exchange of sendaddrv2, so
> it's presumably the sendaddrv2 message at issue),

addrv2 is listed as the BIP title, the message that would cause the =
break is sendaddrv2 (quoted text).

> however since [0]
> sendaddrv2 messages are only sent to nodes advertising version 70016 =
or
> later (same as wtxidrelay).

I don=E2=80=99t see this constraint in BIP155. Do you mean that addrv2 =
support was released in Core at the same time as wtxidrelay, or that it =
is an undocumented version constraint implemented in Core?

> ADDRV2 was introduced May 20 2020 after the
> 0.20 branch, and SENDADDRV2 gating was merged Dec 9 2020 and included
> from 0.21.0rc3 onwards.

To clarify, there was no Core release of addrv2 without sendaddrv2 apart =
from 0.21 release candidates?

> [0] https://github.com/bitcoin/bitcoin/pull/20564
>=20
> I'm only seeing "bytesrecv_per_msg.*other*" entries for nodes =
advertising
> a version of 0.17 and 0.18,

> which I presume is due to REJECT messages (for taproot txs, perhaps?).

Ideally you should not be seeing reject messages as protocol =
=E2=80=9Cother=E2=80=9D, as these are valid messages as of protocol =
version 70002, and they are excluded by negotiated version before that. =
While there is no requirement to send them (BIP61 only defines a new =
message type), they remain defined messages until removed by a future =
protocol version.

> Otherwise, I don't think there are any unexpected
> messages you should be receiving when advertising version 70015 or =
lower.

Yet nodes with an advertised protocol version of 70013 are receiving =
sendaddrv2. I've removed the IP address from the log extract below.

17:53:45.022347 DEBUG [network] Peer [x.x.x.x:8333] protocol version =
(70016) user agent: /Satoshi:0.21.0()/
17:53:45.022377 DEBUG [network] Negotiated protocol version (70013) for =
[x.x.x.x.135:8333]
17:53:45.022767 INFO [network] Connected outbound channel =
[x.x.x.x.135:8333]
17:53:45.022913 DEBUG [node] Ask [x.x.x.x:8333] for headers after =
[00000000000000000002e8c1c59fc86f721ba3a3294d2b1165597ddb910058e6]
17:53:45.023184 WARNING [network] Invalid sendaddrv2 payload from =
[x.x.x.x:8333] object does not exist
17:53:45.023317 DEBUG [network] Stop protocol version on [x.x.x.x:8333] =
object does not exist
17:53:45.023359 DEBUG [network] Outbound channel stopped [x.x.x.x:8333] =
success

To my knowledge the only other time we've seen consistent invalid =
message traffic on the network was during the work on BIP150 =
(withdrawn), at which point BIP150 nodes were being deployed on mainnet. =
I made comments here on the issue at the time, which as I recall were =
generally rejected in favor of forcing nodes to allow all invalid =
traffic. In any case BIP150 was withdrawn and BIP324 proposed, which =
fixes this particular issue (using a service bit).

Some argued at the time that allowance for invalid messages was a =
longstanding requirement in the protocol. I knew that this was not the =
case (except for BIP37, break documented in BIP60) because libbitcoin =
validates all messages, which led me to eventually document it. Recently =
I updated and posted that documentation (the github wiki link you =
found). This was a consequence of reviewing the Generic Package Relay =
proposal, which is also incompatible. In doing so I noticed this issue =
with BIP155 and BIP330 as well. This led us to check the logs for peer =
disconnects as a result of invalid messages, at which point the above =
was found to be an increasingly common occurrence.

Best,
e

> Cheers,
> aj