BIP155: extra-clarify the semantic of sendaddrv2 #1134

vasild commented at 11:29 AM on June 16, 2021: contributor

BIP155: extra-clarify the semantic of sendaddrv2.

This is also how the code works currently.

There seems to be some misinterpretation of the current wording in BIP155. Clarify that (hopefully).

vasild cross-referenced this on Jun 16, 2021 from issue [p2p] Stop sending SENDADDRV2 message to block-relay-only peers by amitiuttarwar

in bip-0155.mediawiki:134 in b4f30fd717 outdated

 130 | @@ -131,7 +131,7 @@ See the appendices for the address encodings to be used for the various networks
 131 |  
 132 |  ==Signaling support and compatibility==
 133 |  
 134 | -Introduce a new message type <code>sendaddrv2</code>. Sending such a message indicates that a node can understand and prefers to receive <code>addrv2</code> messages instead of <code>addr</code> messages. I.e. "Send me addrv2".
 135 | +Introduce a new message type <code>sendaddrv2</code>. Sending such a message indicates that a node can understand and prefers to receive <code>addrv2</code> messages instead of <code>addr</code> messages. I.e. "Send me addrv2". Sending or not sending this message does not imply preference or dis-preference of receiving unrequested address messages.

jonatack commented at 11:47 AM on June 16, 2021:

s/dis-preference/non-preference/ or "any preference"

Introduce a new message type <code>sendaddrv2</code>. Sending such a message indicates that a node can understand and prefers to receive <code>addrv2</code> messages instead of <code>addr</code> messages. I.e. "Send me addrv2". Sending or not sending this message does not imply any preference with respect to receiving unrequested address messages.

vasild commented at 12:00 PM on June 16, 2021:

Done

jonatack commented at 11:47 AM on June 16, 2021: contributor

Concept and approach ACK

BIP155: extra-clarify the semantic of sendaddrv2 337114b06e

vasild force-pushed on Jun 16, 2021

vasild commented at 11:59 AM on June 16, 2021: contributor

b4f30fd...337114b: address suggestion

laanwj commented at 12:12 PM on June 16, 2021: member

ACK, this was how I imagined it was.

jnewbery commented at 12:42 PM on June 16, 2021: member

NACK. This sentence is confusing and unhelpful to implementers. You could equally (to use your analogy in https://github.com/bitcoin/bitcoin/pull/22245#issuecomment-862284713) update the BIP to say "Sending or not sending this message does not imply any preference with respect to red apples or green apples". It'd be equally true, and equally unhelpful for anyone implementing the spec.

jonatack commented at 12:44 PM on June 16, 2021: contributor

ACK 337114b06eee649b5b2fe75dcbdcd7e5b984bbde

in bip-0155.mediawiki:134 in 337114b06e

 130 | @@ -131,7 +131,7 @@ See the appendices for the address encodings to be used for the various networks
 131 |  
 132 |  ==Signaling support and compatibility==
 133 |  
 134 | -Introduce a new message type <code>sendaddrv2</code>. Sending such a message indicates that a node can understand and prefers to receive <code>addrv2</code> messages instead of <code>addr</code> messages. I.e. "Send me addrv2".
 135 | +Introduce a new message type <code>sendaddrv2</code>. Sending such a message indicates that a node can understand and prefers to receive <code>addrv2</code> messages instead of <code>addr</code> messages. I.e. "Send me addrv2". Sending or not sending this message does not imply any preference with respect to receiving unrequested address messages.

harding commented at 9:23 PM on June 16, 2021:

It's weird to say "prefers to receive" and then contradict that by saying "does not imply any preference", even if those two statements are referring to different aspects of the same thing. I think the change you are suggesting is much more effectively made by deleting words than by adding them.

Introduce a new message type <code>sendaddrv2</code>. Sending such a message indicates that a node can understand <code>addrv2</code> messages.

I personally don't think that change makes sense. We don't care about what a node understands or doesn't understand, we only care about what it wants us to send it (or doesn't want us to send it). I think the BIP would be much more useful if it said:

Introduce a new message type <code>sendaddrv2</code>. Sending such a message indicates that a node wants to receive <code>addrv2</code> messages.  Any node receiving <code>addrv2</code> messages should not also receive <code>addr</code> messages.

~~I don't think that represents a behavior change.~~ I think that provides an opportunity for a useful behavior change on the part of the requesting node, but doesn't represent any behavior change on the part of the receiving node, which is the part where I think we need to be especially careful. The final paragraph of this section still conflicts with the policy described in the OP of https://github.com/bitcoin/bitcoin/pull/21528 , but that final paragraph is to my eyes only a description of how to maintain compatibility with Bitcoin Core's previous un-BIP'd behavior, not a prescription for how all BIP155-conforming nodes should implement addr gossip in the future.

Edit: clarified "behavior change" remarks.

vasild commented at 1:18 PM on June 18, 2021:

What about this:

Sending such a message signals support for the <code>addrv2</code> format - a node can create such messages and understand (parse) them.

Given that some addresses can only be represented in <code>addrv2</code> format and that IPv4 addresses' representation is more compact, a sender, if they choose to send an address message for whatever reason, may find it more suitable to use <code>addrv2</code> instead of <code>addr</code> if both peers have signaled <code>addrv2</code> support.

jonatack commented at 4:23 PM on June 21, 2021:

What about this:

Sending such a message signals support for the addrv2 format - a node can create such messages and understand (parse) them.

Agree -- would omit "Send me addrv2" or replace it with "I support addrv2"

luke-jr merged this on Jul 2, 2021

luke-jr closed this on Jul 2, 2021

vasild deleted the branch on Jul 5, 2021