Package Relay and child-with-unconfirmed-parents + tx-with-unconfirmed-ancestors Packages #1324

glozow · 2022-05-17T15:26:17Z

Please leave concept/approach feedback on the mailing list thread so that people can follow the discussion in one place. Feel free to leave a review comment if you find typos or incorrect wording.

sdaftuar

Thanks for working on this! Just some initial questions to try to make sure I'm understanding this proposal right (will do my best to save broader feedback for the mailing list!).

sdaftuar · 2022-05-17T16:40:56Z

bip-v1-packages.mediawiki

+
+# '''Sorted topologically.''' For every transaction t in the package, if any of t's parents are present in the package, the parent must appear somewhere in the list before t. In other words, the transactions must be sorted in ascending order of number of ancestors present in the package.
+
+# '''Only 1 child with unconfirmed parents.''' The package must consist of one transaction and its unconfirmed parents. There must not be any other transactions in the package. Other dependency relationships may exist within the package (e.g. one parent may spend the output of another parent) provided that topological order is respected.


One thing I overlooked when I first read this over -- by "unconfirmed parents" you mean direct parents, ie transactions which contain an outpoint being spent in the child transaction, is that right? (I was trying to figure out exactly what you meant by the 2-generation limit further down, and then I realized I might have misunderstood the language here.)

I'd suggest defining this language more explicitly somehow, since I think "unconfirmed parent" can read a lot like "unconfirmed ancestor" (and at any rate I don't think these words have meanings well defined across the developer community).

Also, if a GETDATA(PCKG1, wtxid) comes in for a transaction that has unconfirmed ancestors outside the set of unconfirmed direct parents, how is software supposed to respond?

by "unconfirmed parents" you mean direct parents, ie transactions which contain an outpoint being spent in the child transaction, is that right?

Yes, exactly.

I'd suggest defining this language more explicitly somehow, since I think "unconfirmed parent" can read a lot like "unconfirmed ancestor" (and at any rate I don't think these words have meanings well defined across the developer community).

Ah, noted. How about something like this:
"Given any two transactions Tx0 and Tx1 where Tx1 spends an output of Tx0, Tx0 is a parent of Tx1 and Tx1 is a child of Tx0. A transaction's ancestors include, recursively, its parents, the parents of its parents, etc. A transaction's descendants include, recursively, its children, the children of its children, etc."

Also, if a GETDATA(PCKG1, wtxid) comes in for a transaction that has unconfirmed ancestors outside the set of unconfirmed direct parents, how is software supposed to respond?

It should send only the direct parents, and no other ancestors.

bip-v1-packages.mediawiki

bip-package-relay.mediawiki

bip-v1-packages.mediawiki

sdaftuar · 2022-05-17T16:53:57Z

bip-v1-packages.mediawiki

+
+# A new inv type (MSG_PCKG1 == 0x6) is added, for use in inv messages and getdata requests pertaining to version 1 packages.
+
+# As an inv type, it indicates that both transaction data and version 1 package information are available for the transaction. The transaction is referenced by its wtxid. As a getdata request type, it indicates that the sender wants package information for the transaction.


Does using MSG_PCKG1 as an inv-type imply that the transaction's unconfirmed ancestors are exactly equal to its unconfirmed parents? (See my other questions below)

sdaftuar · 2022-05-17T16:54:52Z

bip-v1-packages.mediawiki

+
+# As an inv type, it indicates that both transaction data and version 1 package information are available for the transaction. The transaction is referenced by its wtxid. As a getdata request type, it indicates that the sender wants package information for the transaction.
+
+# Upon receipt of a "getdata" request for "MSG_PCKG1", the node should respond with the version 1 package corresponding to the requested transaction and its current chain tip, or with NOTFOUND. The node should not assume that the sender is requesting the transaction data as well.


Is it permitted to send a GETDATA(MSG_PCKG1, wtxid) if the peer didn't send an INV(MSG_PCKG1, wtxid)? Ie could software use this message to reconstruct the parents of an orphan transaction?

Yes absolutely! This kind of "half solves" the orphan fetching use case if there's only direct parents missing, but I actually think it would make sense to create another type of package for tx-with-unconfirmed-ancestors:

To answer the likely question of "why wasn't v1 packages tx-with-unconfirmed-ancestors?" I have two primary reasons:

The mempool policy created for child-with-unconfirmed-parents packages is insufficient to ensure that a the fees of a tx-with-ancestors package would be evaluated correctly. I noticed you had some consideration for that in #16401. Probably the best algorithm is to sort topologically, then submit each transaction with its ancestor subset within the package. But it doesn't seem like anyone is asking for more than 2 generations of fee-bumping, and I haven't really figured out what RBF would look like.

It seems like the orphan-processing and fee-bumping use cases can be thought of separately. For instance, fee-bumping should be a sender-initiated "I'm telling you about extra info you need" and orphan-fetching is more appropriate as a receiver-initiated "hey I'm new here, so I don't know the ancestors of this transaction."

instagibbs · 2022-05-17T20:53:39Z

Based on email discussion, I'd request that the spec makes it explicit that you do not request packages from peers when they report a different chaintip blockhash(and disconnect them right after...)

it's possible I missed it, but I keep looking and not seeing it

glozow · 2022-05-17T21:01:12Z

Based on email discussion, I'd request that the spec makes it explicit that you do not request packages from peers when they report a different chaintip blockhash(and disconnect them right after...)

Thanks @instagibbs I will add this. Also, if a peer sends us a "pckginfo1" with a blockhash that's different from ours, we should probably just drop it and re-request when they catch up (I think nowadays we usually have an accurate idea of what our peer's best block is). We're definitely not going to disconnect a peer for sending a "pckginfo1" with a different blockhash from ours.

michaelfolkson

First read through. Looks great thus far. Just some nits (feel free to ignore).

I know this hasn't been done in the past but in the spirit of trying to make BIPs more understandable (and precise when it comes to references to particular attacks) I wonder if a glossary of terms (e.g. pinning attacks and the different varieties of such attacks) could be a good idea (maybe footnotes rather than a glossary, they have been used in the past). I guess the top priority is to clearly go into what attacks/vulns package relay addresses (including on higher layers), what attacks/vulns package relay potentially opens up on the base layer with their introduction and how the design of package relay has minimized the scope of those attacks/vulns on the base layer. My gut feel is that the current draft is still a little light on these details but it was just a first readthrough and it clearly doesn't ignore them.

edit: Was looking at the BIP118 draft for inspiration on what a BIP should cover that is primarily targeted towards higher layer use cases (but with base layer implications). That has a Security section which may be a good idea for this BIP.

michaelfolkson · 2022-05-19T12:37:20Z

bip-package-relay.mediawiki

+
+# Download and validate packages of transactions together.
+
+# Provide information to help peers decide whether to request and/or how validate transactions which are part of a package.


nit: s/how validate/how to validate

bip-package-relay.mediawiki

michaelfolkson · 2022-05-19T12:41:16Z

bip-package-relay.mediawiki

+once, avoid downloading transactions that are eventually rejected, and minimize storage allocated
+for not-yet-validated transactions.
+
+Consider these scenarios specifically transaction relay:


nit: s/specifically transaction relay/specifically for transaction relay

michaelfolkson · 2022-05-19T12:42:42Z

bip-package-relay.mediawiki

+===P2P Message Design===
+
+These p2p messages are added for communication efficiency and, as such, one should measure
+alternative solutions based on the resources used to communicate (not necessarily trustworthy)


nit: s/not necessarily trustworthy/in the presence of possibly malicious peers

michaelfolkson · 2022-05-19T12:51:41Z

bip-package-relay.mediawiki

+
+# The "sendpackages" message has the structure defined above, with pchCommand == "sendpackages".
+
+# During version handshake, nodes should send a "sendpackages" message indicate they support package relay and may request packages.


nits: s/During version handshake/During the version handshake, s/message indicate/ message to indicate

michaelfolkson · 2022-05-19T12:56:39Z

bip-package-relay.mediawiki

+
+''Diagram: A receiver-initiated dialogue.''
+
+Sometimes, no matter what order transactions are received by a node, validating them individually is


nit: s/no matter what order transactions are received by a node/regardless of the order in which transactions are received by a node

bip-package-relay.mediawiki

glozow · 2022-05-20T22:57:09Z

Added tx-with-unconfirmed-ancestors packages as a second type of package dedicated to orphan-fetching.

glozow · 2022-05-24T18:28:54Z

Last push (thanks @ajtowns)

Count and size are implied by the version. Version 1 and Version 2 both have maximum of 25 transactions and 404KWu.
Announce sendpackages based on our own state. It’s ok to send “sendpackages” if they sent fRelay=false.
At verack, require fRelay=true and wtxidrelay if they sent sendpackages, otherwise disconnect.
If we get “getpckgtxns” or “pckgtxns” without having negotiated “sendpackages” ahead of time, ignore, don’t disconnect. Emphasize that the intention is to validate all of the transactions received through “pckgtxns” together.

Thanks for the feedback @michaelfolkson, I'll get to the style/wording comments later when the design is more finalized.

flack · 2022-05-25T17:38:40Z

Sorry for also leaving a style nit, but as someone also mentioned on the mailing list, pkg is a much more common abbreviation than pckg (it's even mentioned on Wikipedia: https://en.wikipedia.org/wiki/PKG), so it might be nice to consider switching to that.

glozow · 2022-06-07T17:47:37Z

Last push:

Renamed s/pckg/pkg everywhere, updated diagrams
Removed fee and weight information from pkginfo1

sdaftuar · 2022-06-08T16:31:28Z

bip-v1-packages.mediawiki

+
+A version 1 or '''child-with-unconfirmed-parents''' package can be defined for any transaction that spends
+unconfirmed inputs. The child can be thought of as the "representative" of the package. This package
+can be uniquely identified by the transaction's wtxid and the current chain tip block hash.


The child's wtxid does not commit to the wtxids of its ancestors (just the txids), so I don't think it's correct to say that a package is uniquely identified by the child transaction's wtxid and the current chain tip. (I tried to bring this point up in the mailing list thread as well, in the context of package de-duplication.)

Yes sorry, I think I should have said the package can be easily reconstructed using the transaction's wtxid and current chain tip block hash.

luke-jr · 2022-07-25T21:33:23Z

bip-package-relay.mediawiki

+  Status: Draft
+  Type: Standards Track
+  Created: 2022-04-14
+  License: PD


Before taking this out of draft, please license it under one of the acceptable licenses

Thanks will do! I'm still working on a few changes based on feedback, and then will take this out of draft.

glozow · 2022-10-13T21:37:08Z

Closing as the updated proposal is going to be significantly different from this one, which has no number assigned, and I think it would be confusing.

sdaftuar reviewed May 17, 2022

View reviewed changes

glozow force-pushed the 2022-04-package-relay branch from dee1711 to 20fe00b Compare May 17, 2022 19:06

glozow force-pushed the 2022-04-package-relay branch from 20fe00b to 66b050a Compare May 17, 2022 21:31

michaelfolkson reviewed May 19, 2022

View reviewed changes

glozow force-pushed the 2022-04-package-relay branch from 66b050a to c833fd6 Compare May 20, 2022 22:55

glozow changed the title ~~Package Relay and child-with-unconfirmed-parents Packages~~ Package Relay and child-with-unconfirmed-parents + tx-with-unconfirmed-ancestors Packages May 20, 2022

glozow force-pushed the 2022-04-package-relay branch from c833fd6 to 5c73c62 Compare May 24, 2022 18:24

glozow added 3 commits June 7, 2022 18:35

specify generic package relay

09cfd6e

specify version 1 child-with-unconfirmed-parents package relay

2c53cdd

specify version 2 transaction-with-unconfirmed-ancestors package relay

9a85fef

glozow force-pushed the 2022-04-package-relay branch from 5c73c62 to 9a85fef Compare June 7, 2022 17:41

sdaftuar reviewed Jun 8, 2022

View reviewed changes

luke-jr reviewed Jul 25, 2022

View reviewed changes

glozow closed this Oct 13, 2022

glozow mentioned this pull request Oct 13, 2022

BIP 331: Ancestor Package Relay #1382

Merged

chinggg mentioned this pull request Dec 6, 2022

fuzz: Modify tx_pool_standard target to test package processing bitcoin/bitcoin#25778

Closed


		# '''Sorted topologically.''' For every transaction t in the package, if any of t's parents are present in the package, the parent must appear somewhere in the list before t. In other words, the transactions must be sorted in ascending order of number of ancestors present in the package.

		# '''Only 1 child with unconfirmed parents.''' The package must consist of one transaction and its unconfirmed parents. There must not be any other transactions in the package. Other dependency relationships may exist within the package (e.g. one parent may spend the output of another parent) provided that topological order is respected.


		# A new inv type (MSG_PCKG1 == 0x6) is added, for use in inv messages and getdata requests pertaining to version 1 packages.

		# As an inv type, it indicates that both transaction data and version 1 package information are available for the transaction. The transaction is referenced by its wtxid. As a getdata request type, it indicates that the sender wants package information for the transaction.


		# As an inv type, it indicates that both transaction data and version 1 package information are available for the transaction. The transaction is referenced by its wtxid. As a getdata request type, it indicates that the sender wants package information for the transaction.

		# Upon receipt of a "getdata" request for "MSG_PCKG1", the node should respond with the version 1 package corresponding to the requested transaction and its current chain tip, or with NOTFOUND. The node should not assume that the sender is requesting the transaction data as well.


		# Download and validate packages of transactions together.

		# Provide information to help peers decide whether to request and/or how validate transactions which are part of a package.


		# The "sendpackages" message has the structure defined above, with pchCommand == "sendpackages".

		# During version handshake, nodes should send a "sendpackages" message indicate they support package relay and may request packages.


		''Diagram: A receiver-initiated dialogue.''

		Sometimes, no matter what order transactions are received by a node, validating them individually is

Package Relay and child-with-unconfirmed-parents + tx-with-unconfirmed-ancestors Packages #1324

Package Relay and child-with-unconfirmed-parents + tx-with-unconfirmed-ancestors Packages #1324

Uh oh!

Conversation

glozow commented May 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sdaftuar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

instagibbs commented May 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

glozow commented May 17, 2022

Uh oh!

michaelfolkson left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

glozow commented May 20, 2022

Uh oh!

glozow commented May 24, 2022

Uh oh!

flack commented May 25, 2022

Uh oh!

glozow commented Jun 7, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

glozow commented Oct 13, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

glozow commented May 17, 2022 •

edited

Loading

instagibbs commented May 17, 2022 •

edited

Loading

michaelfolkson left a comment •

edited

Loading