Add BIP 338: Disable transaction relay message

sdaftuar commented at 4:43 pm on January 6, 2021: member

This message, valid between version/verack for peers with version >= 70017, would allow establishing at the time of connection that no transactions will be announced/requested between those peers.

p2p: Add disabletx message

This message, valid between version/verack for peers with version >= 70017,
would allow establishing at the time of connection that no transactions will be
announced/requested between those peers.

5f18700477

sdaftuar commented at 4:44 pm on January 6, 2021: member

I sent an email to the bitcoin-dev list about this today (https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2021-January/018340.html) – would like to request a BIP number be assigned.

jonatack commented at 3:52 pm on January 12, 2021: contributor

Concept ACK

Add link to implementation

Also change the phrasing to more clearly indicate when block-relay-only peering
was deployed.

794f20a131

sdaftuar commented at 6:22 pm on January 26, 2021: member

Anything else to be done here?

MarcoFalke commented at 6:30 pm on January 26, 2021: member

@luke-jr

MarcoFalke cross-referenced this on Jan 30, 2021 from issue gui: ensure inbound block relay peers have relevant services by jonatack

sdaftuar commented at 2:48 pm on February 1, 2021: member

ping @luke-jr

luke-jr commented at 10:57 pm on February 3, 2021: member

Can you add something to Motivation that actually explains why this is useful?

luke-jr added the label New BIP on Feb 3, 2021

sdaftuar commented at 2:03 pm on February 4, 2021: member

@luke-jr Happy to edit if you have specific feedback about language I can include. Can I get a BIP number in the meantime so that I have a way to refer to this work elsewhere?

in bip-disable-tx.mediawiki:48 in 794f20a131 outdated

43+addresses; with that knowledge a node would likely choose other peers to
44+receive announced addresses instead.
45+
46+This proposal adds a new, optional message that a node can send a peer when
47+initiating a connection to that peer, to indicate that connection should not be
48+used for transaction-relay for the connection's lifetime. In addition, without

jonatack commented at 6:49 pm on February 5, 2021:

0used for transaction relay for the connection's lifetime. In addition, without

sdaftuar commented at 6:46 pm on February 11, 2021:

Done.

in bip-disable-tx.mediawiki:51 in 794f20a131 outdated

46+This proposal adds a new, optional message that a node can send a peer when
47+initiating a connection to that peer, to indicate that connection should not be
48+used for transaction-relay for the connection's lifetime. In addition, without
49+a current mechanism to negotiate whether addresses should be relayed on a
50+connection, this BIP suggests that address messages not be sent on links where
51+tx-relay has been disabled.

jonatack commented at 6:50 pm on February 5, 2021:

0transaction relay has been disabled.

sdaftuar commented at 6:46 pm on February 11, 2021:

Done.

in bip-disable-tx.mediawiki:58 in 794f20a131 outdated

53+==Specification==
54+
55+# A new disabletx message is added, which is defined as an empty message where pchCommand == "disabletx".
56+# The protocol version of nodes implementing this BIP must be set to 70017 or higher.
57+# If a node sets the transaction relay field in the version message to a peer to false, then the disabletx message MAY also be sent in response to a version message from that peer if the peer's protocol version is >= 70017. If sent, the disabletx message MUST be sent prior to sending a verack.
58+# A node that has sent or received a disabletx message to/from a peer MUST NOT send any of these messages to the peer:

jonatack commented at 9:09 pm on February 5, 2021:

Should disconnection (as opposed to ignoring said messages) be explicitly specified here for the MUST NOT messages (or is that implied?)

Disconnection is mentioned line 68 below, “Nodes MAY decide to not remain connected…”

sdaftuar commented at 6:43 pm on February 11, 2021:

I think how software handles a peer that violates a “MUST NOT” behavior is totally up to the implementation and disconnecting is an implied acceptable outcome.

Calling out that nodes might disconnect a peer further down was just to make it clear that even nodes that faithfully implement the BIP might disconnect each other for a legitimate reason (ie that a node is looking for a full relay connection, but only could establish a block-relay-only connection).

jonatack commented at 10:19 pm on February 5, 2021: contributor

Approach ACK

sdaftuar commented at 6:00 pm on February 9, 2021: member

@luke-jr I’m happy to include some of the explanation that @sipa provided on IRC the other day in this draft – I genuinely don’t know what is confusing about this proposal but I’m happy to iterate on the reasoning in the motivation until it makes sense to everyone. However, even before this is merged to the repo it would be helpful for me to have a BIP number assigned, so that I can continue work on the implementation and having that reviewed, and not be held up for the trivial fact that there is no BIP number.

0562 2021-02-04T21:59:49  <sipa> sdaftuar, luke-jr: the second paragraph of sdaftuar's ML post about it (https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2021-January/018340.html) is probably a good start
1563 2021-02-04T22:00:39  <luke-jr> ah
2564 2021-02-04T22:01:07  <luke-jr> so the point is to allow N*M light connections where being unsure means only N could be accepted?
3565 2021-02-04T22:01:24  <luke-jr> (though IIRC our limits are due to FD set sizes and open fd limits currently?)
4566 2021-02-04T22:03:56  <sipa> luke-jr: my understanding is this: we have a default inbound connection limit (125) which is based on the observation that every "fully fledged" connection has a significant impact on processing costs and bandwidth (among other things), so we can't just raise that without impacting worst-case performance of average nodes
5569 2021-02-04T22:04:58  <sipa> if we instead knew for block-only connections (on the incoming side) that they'd *never* need transactions, the worst part is reduced, and perhaps it becomes acceptable to have more incoming connections per peer, given that some percentage of them are reserved for non-block-relaying connections
6570 2021-02-04T22:05:19  <sipa> right now, the block-only nature of a connection is only known on the outbound side
7571 2021-02-04T22:05:21  <luke-jr> I see

Add more language in hope of BIP number assignment 55a31eb8ee

in bip-disable-tx.mediawiki:64 in 55a31eb8ee outdated

59+sending disabletx) from full-relay peers, potentially allowing for an increase
60+in the number of block-relay-only connections that can be made on the network.
61+
62+==Specification==
63+
64+# A new disabletx message is added, which is defined as an empty message where pchCommand == "disabletx".

MarcoFalke commented at 8:56 am on February 12, 2021:

0# A new disabletx message is added, which is defined as an empty message with message type "disabletx".

unrelated style nit: I know previous bips like to reference Bitcoin Core internal implementation details, such as variable naming. Though, I think it would be good if bips avoided implementation-specific details where possible to avoid confusion when the internals change. Just saying “message type” might even be clearer and the audience that is interested in Bitcoin Core internals, can still follow the link in the Implementation section.

MarcoFalke approved

MarcoFalke commented at 8:59 am on February 12, 2021: member

ACK 55a31eb8ee304984534e6de8a9ef18691defa983 I’ve reviewed the BIP and concluded that it can be merged according to BIP 2

(feel free to ignore the unrelated style nit)

Note that tx messages are never allowed on disabletx links 332b9a4854

in bip-disable-tx.mediawiki:69 in 55a31eb8ee outdated

64+# A new disabletx message is added, which is defined as an empty message where pchCommand == "disabletx".
65+# The protocol version of nodes implementing this BIP must be set to 70017 or higher.
66+# If a node sets the transaction relay field in the version message to a peer to false, then the disabletx message MAY also be sent in response to a version message from that peer if the peer's protocol version is >= 70017. If sent, the disabletx message MUST be sent prior to sending a verack.
67+# A node that has sent or received a disabletx message to/from a peer MUST NOT send any of these messages to the peer:
68+## inv messages for transactions
69+## getdata messages for transactions

MarcoFalke commented at 10:37 am on February 12, 2021:

I see the list only includes inv and getdata for (w)txs. Is it intentional to not disallow message of type tx?

sdaftuar commented at 1:17 pm on February 12, 2021:

No that is an oversight, thanks for catching this. (In my mind, unrequested transactions should generally not be allowed, so I didn’t think to specify that here, though of course I should!)

in bip-disable-tx.mediawiki:45 in 332b9a4854 outdated

40+The low-bandwidth / minimal-resource nature of these connections is currently
41+known only by the initiator of the connection; this is because the transaction
42+relay field in the version message is not a permanent setting for the lifetime
43+of the connection.  Consequently, a node receiving an inbound connection with
44+transaction relay disabled cannot distinguish between a peer that will never
45+enable transaction relay (as described in BIP 37) and one that will.  Moreover,

jnewbery commented at 1:28 pm on February 12, 2021:

I think this part of the motivation needs expansion/clarification. When I first read this, it wasn’t clear to me that the “cannot distinguish between …” part is not the case for the vast majority of connections, since a peer can only restart tx relay if bloom filters are allowed on that connection. Bloom filters are both disabled by default in Bitcoin Core, and discouraged for use on public connections due to DoS and privacy issues. See https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2021-January/018347.html for more details.

sdaftuar commented at 1:30 pm on February 12, 2021:

This BIP is drafted to be compatible with BIP 37; I can add language to clarify that.

MarcoFalke commented at 1:41 pm on February 12, 2021: member

re-ACK 332b9a4854963e8afa8ab89682b9686b79956b95 only change is explicit mention of tx message type behaviour

Mention compatibility with bip 37 31f61e7175

MarcoFalke commented at 8:08 pm on February 12, 2021: member

re-ACK 31f61e71759c8c205294b2065a8383a91c60b436 only change is adding a sentence

luke-jr renamed this:
~~p2p: Add disabletx message~~
Add BIP 338: Disable transaction relay message
on Feb 12, 2021

luke-jr merged this on Feb 12, 2021

luke-jr closed this on Feb 12, 2021

Add BIP 338: Disable transaction relay message #1052