BIP 331: Ancestor Package Relay #1382

pull glozow wants to merge 1 commits into bitcoin:master from glozow:2022-10-package-relay changing 9 files +437 −0
  1. glozow commented at 9:40 pm on October 13, 2022: member
  2. luke-jr renamed this:
    Ancestor Package Relay
    BIP 331: Ancestor Package Relay
    on Oct 21, 2022
  3. luke-jr commented at 12:37 pm on October 21, 2022: member
    Assigned BIP # 331
  4. glozow force-pushed on Oct 24, 2022
  5. glozow commented at 1:33 pm on October 24, 2022: member
    Updated for number
  6. glozow force-pushed on Oct 24, 2022
  7. in bip-0331.mediawiki:142 in 112f8cc859 outdated
    137+[[File:./bip-package-relay/tx_scenarios.png|800px]]
    138+
    139+'''No Package Information Round:''' Instead of having a package information round, just use the
    140+child's wtxid to refer to the package and always send the entire package together. This would make
    141+it very likely for honest nodes to redownload duplicate transactions.
    142+[[File:./bip-package-relay/no_package_info.png|900px]]
    


    ajtowns commented at 1:25 am on November 3, 2022:
    Need to update paths to images now that bip number is assigned

    glozow commented at 3:45 pm on November 3, 2022:
    Doh, thanks!
  8. glozow force-pushed on Nov 3, 2022
  9. glozow force-pushed on Nov 4, 2022
  10. naumenkogs commented at 10:16 am on December 23, 2022: member

    I’m not a big mempool expert, but this seems like a good direction.


    1. There are obvious typos and misalignments you’re probably aware of, i assume we shouldn’t merge them?

    2.

    Package Information Only

    This example seems confusing: it violates the following rule A node MUST NOT send a "ancpkginfo" message that has not been requested by the recipient. Upon receipt of an unsolicited "ancpkginfo", a node may disconnect the sender. It will likely work if you expand the example to 3 transactions?

    1. Unclear what to do when transactions are replaced in the middle of this process. Should that be covered? Probably will become clearer when you prototype.

    2. Same about timestamping concerns i guess.

    3. As a style comment, I’d suggest doing some compression of the text :)

    4. Is Package Erlay possible? 6a) I actually made some attempt to handle parent/child within Erlay, do you think we should exercise the overlap between the two better? E.g., for those packages flowing through the network (not the IBD case, but fee-bump or something) Erlay might do the batching you need. 6b) One might also think about “what if we reconcile packages”? Probably bad idea, because the sets (packages) are too small? My point is that this should not be confused with what you describing.

    5. MSG_ANCPKGINFO and getpkgtxns have some redundancy in the data… The latter could refer to the hash of the former + the indexes, for example? Not sure savings worth the complexity (how often this will be used? every lightning closure?) but maybe worth making it into q/a :)

  11. glozow commented at 11:41 am on January 9, 2023: member

    Thanks for the review @naumenkogs! yes, I think it’s best to talk specifics with the actual implementation but I’ll throw some quick answers for now:

    Unclear what to do when transactions are replaced in the middle of this process. Should that be covered? Probably will become clearer when you prototype.

    One should treat this situation the same way we do for transactions that get replaced in between inv and getdata (I don’t see any reason it should be different). Currently, we send “notfound”. If in the future we decide to relay recently-replaced transactions, then package relay would include recently-replaced transactions as well. Though I agree this seems more like an implementation question than a spec question. In the spec, I would leave it as “the receiver may respond with notfound.”

    I actually made some attempt to handle parent/child within Erlay, do you think we should exercise the overlap between the two better? E.g., for those packages flowing through the network (not the IBD case, but fee-bump or something) Erlay might do the batching you need.

    Yes it’s great that Erlay orders them correctly. Note below-fee-filter transactions to the reconciliation bucket so not every case is handled. I plan to have a rebase-on-top-of-Erlay version so we can exercise the two together.

    One might also think about “what if we reconcile packages”? Probably bad idea, because the sets (packages) are too small? My point is that this should not be confused with what you describing.

    Mostly the “Is Package Erlay possible” section was to clarify that they aren’t mutually exclusive. Reconciling packages is I think something that may come up when we get to sender-initiated package relay. I’m thinking, after reconciliation, it may be useful to send a package inv instead of individual tx invs if there are fee-bumps… But for now, since packages must be requested by the receiver, we don’t need to think about that yet.

    MSG_ANCPKGINFO and getpkgtxns have some redundancy in the data… The latter could refer to the hash of the former + the indexes, for example?

    Yeah that’s an interesting idea, we could do a bitfield of 4 bytes (should cover any package since max count is 25). However this means you can only request getpkgtxns from somebody who sent an ancpkginfo (currently this is not a requirement). It also requires the sender to keep track of (allocate memory for, remember to expire) what they announced so they know what “the ith transaction from this package” means. Thoughts?

  12. naumenkogs commented at 1:49 pm on January 9, 2023: member

    Though I agree this seems more like an implementation question than a spec question. In the spec, I would leave it as “the receiver may respond with notfound.”

    The thing with current notfound is that it’s atomic I guess — makes it easy to consider every announcement independent and our code treats it as such (in a poor way but there are also no expectations). With packages, I imagine a CPFP transaction gets replaced and dropped from the package due. A package recipient would then do what, drop the entire package, or consider it invalid?

    A requestor will get this package which doesn’t pass the fees, and drop it on the floor? Or should it re-request the package? At the moment, I’m not interested in the final answer — but interested in making sure we don’t miss such questions.

    Reconciling packages is I think something that may come up when we get to sender-initiated package relay. I’m thinking, after reconciliation, it may be useful to send a package inv instead of individual tx invs if there are fee-bumps… But for now, since packages must be requested by the receiver, we don’t need to think about that yet.

    Not sure this anything to do with who initiates packages. Let’s think about the current proposal. Imagine, upon the announcement of C, the receiver sends getdata(MSG_ANCPKGINFO, C), along with some compressed data saying that he already has 1 parent and 1 child of C (by just looking at the wtxid). Then it would save us announcing these 2 wtxids in the reverse direction.

    Yeah that’s an interesting idea, we could do a bitfield of 4 bytes (should cover any package since max count is 25). However this means you can only request getpkgtxns from somebody who sent an ancpkginfo (currently this is not a requirement). It also requires the sender to keep track of (allocate memory for, remember to expire) what they announced so they know what “the ith transaction from this package” means. Thoughts?

    I don’t think memory allocation and stuff is a show-stopper, but it might be too much complexity if the gain is low. We should measure how much savings it allows… I think saving 28 byte per announced tx might be worthy. As for the “same peer”, do you ever expect packages to be requested from a different peer from the one announced them?

  13. glozow force-pushed on Feb 24, 2023
  14. in bip-0331.mediawiki:93 in 2f65ad1658 outdated
    88+changing its txid; a node cannot use txid to deduplicate transactions it has already downloaded
    89+or validated. Ideally, two nodes that both support BIP339 wtxid-based transaction relay shouldn't
    90+ever need to use txid-based transaction relay.
    91+
    92+A single use case of txid-based relay remains: handling "orphan" transactions that spend
    93+output(s) from a transaction they are unaware of.  Orphans are common for new nodes that have just
    


    jnewbery commented at 2:20 pm on March 22, 2023:
    0output(s) from a transaction the receiving node is unaware of.  Orphans are common for new nodes that have just
    
  15. in bip-0331.mediawiki:100 in 2f65ad1658 outdated
     95+
     96+Nodes may handle orphans by storing them in a cache and requesting any missing parent(s) by txid
     97+(prevouts specify txid, not wtxid). These parents may end up being orphans as well, if they also
     98+spend unconfirmed inputs that the node is unaware of. This method of handling orphans is problematic
     99+for two reasons: it requires nodes to allocate memory for unvalidated data received on the p2p
    100+network and txid-based relay between two wtxid-relay peers.
    


    jnewbery commented at 2:20 pm on March 22, 2023:
    0network and it relies on txid-based relay between two wtxid-relay peers.
    
  16. in bip-0331.mediawiki:113 in 2f65ad1658 outdated
    108+
    109+A transaction's '''ancestors''' include, recursively, its parents, the parents of its parents, etc.
    110+A transaction's '''descendants''' include, recursively, its children, the children of its children,
    111+etc. A transaction's parent is its ancestor, but an ancestor is not necessarily a parent.
    112+
    113+A '''package''' is an ordered list of transactions, representable by a connected Directed Acyclic
    


    jnewbery commented at 2:21 pm on March 22, 2023:
    “an ordered list”. Is there a well-defined ordering? What is it?
  17. in bip-0331.mediawiki:136 in 2f65ad1658 outdated
    131+For example, consider the following scenarios (where the mempool minimum feerate is 3sat/vB and all
    132+transactions are the same size):
    133+
    134+* Package {A, B} where A pays 0 satoshis and B pays 8000 satoshis in fees.
    135+* Package {C, D} where C pays 0 satoshis and D pays 1200 satoshis in fees.
    136+* Package {E, F, G, H, J} that pays 4000, 8000, 0, 2000, and 4000 satoshis in fees, respectively.
    


    jnewbery commented at 2:21 pm on March 22, 2023:
    0* Package {E, F, G, H, J} that pay 4000, 8000, 0, 2000, and 4000 satoshis in fees, respectively.
    
  18. in bip-0331.mediawiki:142 in 2f65ad1658 outdated
    129+network bandwidth, avoid downloading a transaction more than once, avoid downloading transactions
    130+that are eventually rejected, and minimize storage allocated for not-yet-validated transactions.
    131+For example, consider the following scenarios (where the mempool minimum feerate is 3sat/vB and all
    132+transactions are the same size):
    133+
    134+* Package {A, B} where A pays 0 satoshis and B pays 8000 satoshis in fees.
    


    jnewbery commented at 2:22 pm on March 22, 2023:
    Why not make the transactions’ virtual size 1kvB to make the math super easy?
  19. in bip-0331.mediawiki:143 in 2f65ad1658 outdated
    133+
    134+* Package {A, B} where A pays 0 satoshis and B pays 8000 satoshis in fees.
    135+* Package {C, D} where C pays 0 satoshis and D pays 1200 satoshis in fees.
    136+* Package {E, F, G, H, J} that pays 4000, 8000, 0, 2000, and 4000 satoshis in fees, respectively.
    137+[[File:./bip-0331/tx_scenarios.png|800px]]
    138+
    


    jnewbery commented at 2:23 pm on March 22, 2023:
    0< br/>
    

    The line below (“No Package Information Round…”) wraps strangely on certain resolution displays. The “No Package” appears to the right of the image and then there’s a new line for “Information Round…”. I think you can force a line break so that the text appears on a new line.

  20. in bip-0331.mediawiki:152 in 2f65ad1658 outdated
    138+
    139+'''No Package Information Round:''' Instead of having a package information round, just use the
    140+child's wtxid to refer to the package and always send the entire package together. This would make
    141+it very likely for honest nodes to redownload duplicate transactions.
    142+[[File:./bip-0331/no_package_info.png|900px]]
    143+
    


    jnewbery commented at 2:24 pm on March 22, 2023:

    As above, force a newline here:

    0<br />
    
  21. in bip-0331.mediawiki:164 in 2f65ad1658 outdated
    159+Too-low-feerate transactions (i.e. below the node's minimum mempool feerate) with high-feerate
    160+descendants can also be relayed this way. If the peers are using BIP133 fee filters and a
    161+low-feerate transaction is below the node's fee filer, the sender will not announce it. The
    162+high-feerate transaction is seen and handled as an orphan, the transactions are validated as a
    163+package, and so the protocol naturally works for this use case.<ref>'''Does this mean BIP133 is
    164+required for package relay to work?'''No. If the low-feerate transaction was sent because it wasn't
    


    jnewbery commented at 2:24 pm on March 22, 2023:
    0required for package relay to work?''' No. If the low-feerate transaction was sent because it wasn't
    
  22. in bip-0331.mediawiki:274 in 2f65ad1658 outdated
    269+
    270+# The "pkgtxns" message has the structure defined above, with pchCommand == "pkgtxns".
    271+
    272+# A "pkgtxns" message should contain the transaction data requested using "getpkgtxns".
    273+
    274+# Upon reeipt of a "pkgtxns" message, the node should validate the transactions together as a package rather than individually.
    


    jnewbery commented at 2:24 pm on March 22, 2023:
    0# Upon receipt of a "pkgtxns" message, the node should validate the transactions together as a package rather than individually.
    
  23. in bip-0331.mediawiki:336 in 2f65ad1658 outdated
    331+# This inv type should only be used if both peers agreed to send version 0 packages. If a "getdata" message with type MSG_ANCPKGINFO is received from a peer with which ancestor package relay was not negotiated, the sender may be disconnected.
    332+
    333+====MSG_PKGTXNS====
    334+
    335+# A new inv type (MSG_PKGTXNS == 0x7) is added, for use only in "notfound" messages pertaining to
    336+package transactions.
    


    jnewbery commented at 2:26 pm on March 22, 2023:
    0# A new inv type (MSG_PKGTXNS == 0x7) is added, for use only in "notfound" messages pertaining to package transactions.
    
  24. in bip-0331.mediawiki:225 in 2f65ad1658 outdated
    203+
    204+===Combined Hash===
    205+
    206+A "combined hash" serves as a unique "package id" for some list of unique transactions.
    207+
    208+The combined hash of a package of transactions is equal to the hash of each transaction's wtxid, concatenated in lexicographical order.
    


    jnewbery commented at 2:31 pm on March 22, 2023:
    Which hash function?
  25. in bip-0331.mediawiki:323 in 2f65ad1658 outdated
    314+
    315+# Upon receipt of a malformed "ancpkginfo" message that does not contain the ancestor package of the transaction (which the node learns after receiving the transaction data and validating them), the sender may be disconnected.
    316+
    317+# A node MUST NOT send a "ancpkginfo" message that has not been requested by the recipient. Upon receipt of an unsolicited "ancpkginfo", a node may disconnect the sender.
    318+
    319+# A "ancpkginfo" message should only be sent if both peers agreed to send version 0 packages. If a "ancpkginfo" message is received from a peer with which ancestor package relay was not negotiated, the sender may be disconnected.
    


    jnewbery commented at 2:35 pm on March 22, 2023:
    It’d be good to explicitly define what a “version 0 package” is in this doc.
  26. glozow force-pushed on Mar 28, 2023
  27. glozow commented at 10:05 am on March 28, 2023: member

    @jnewbery thanks, took all suggestions

    CI seems to be failing on bip-0327 title being too long, so will ignore for now.

  28. kallewoof commented at 1:00 am on March 30, 2023: member
    Sorry about that, the BIP-327 title issue has been fixed, so please rebase.
  29. glozow force-pushed on Mar 30, 2023
  30. in bip-0331.mediawiki:30 in 43be4d7c9e outdated
    25+selecting them for inclusion in blocks
    26+<ref>[https://github.com/bitcoin/bitcoin/pull/7600 Select transactions using feerate-with-ancestors]</ref>.
    27+Incentive-compatible mempool and miner policies help create a fair, fee-based market for block
    28+space. While miners maximize transaction fees in order to earn higher block rewards, non-mining
    29+users participating in transaction relay reap many benefits from employing policies that result in a
    30+mempool with the same contents, including faster compact block relay and more accurate fee
    


    jnewbery commented at 9:05 am on April 3, 2023:
    Perhaps “the same contents” -> “similar contents”. I think that better conveys that there’s a spectrum, where the closer you are to the miners’ mempools, the more benefit you get from compact block relay and fee estimation.
  31. in bip-0331.mediawiki:66 in 43be4d7c9e outdated
    61+</ref>
    62+
    63+These transactions must meet a certain confirmation target to be effective, but their feerates
    64+are negotiated well ahead of broadcast time. If the forecasted feerate was too low and no
    65+fee-bumping options are available, attackers can steal money from their counterparties.  Always
    66+overestimating fees may sidestep this issue temporarily (while mempool traffic is low and
    


    jnewbery commented at 9:08 am on April 3, 2023:
    0overestimating fees may sidestep this issue (at least while mempool traffic is low and
    

    I think the word ’temporarily’ here could be misinterpreted.

  32. in bip-0331.mediawiki:64 in 43be4d7c9e outdated
    59+* [https://github.com/t-bast/lightning-docs/blob/master/pinning-attacks.md T-bast's Pinning Attacks Document]
    60+* [https://gist.github.com/instagibbs/60264606e181451e977e439a49f69fe1 Eltoo Pinning]
    61+</ref>
    62+
    63+These transactions must meet a certain confirmation target to be effective, but their feerates
    64+are negotiated well ahead of broadcast time. If the forecasted feerate was too low and no
    


    jnewbery commented at 9:12 am on April 3, 2023:

    Perhaps prefer “forecast” as the past of “forecast”:

    0are negotiated well ahead of broadcast time. If the forecast feerate was too low and no
    

    (both “forecast” and “forecasted” are acceptable forms for the past participle, but “forecast” seems to be more common)

  33. in bip-0331.mediawiki:67 in 43be4d7c9e outdated
    62+
    63+These transactions must meet a certain confirmation target to be effective, but their feerates
    64+are negotiated well ahead of broadcast time. If the forecasted feerate was too low and no
    65+fee-bumping options are available, attackers can steal money from their counterparties.  Always
    66+overestimating fees may sidestep this issue temporarily (while mempool traffic is low and
    67+predictable), but this solution is not foolproof and wastes users' money. For some attacks,
    


    jnewbery commented at 9:13 am on April 3, 2023:
    Perhaps “foolproof” -> “guaranteed to work”. “foolproof” implies more user error to me, whereas this method may fail due to circumstances outside the user’s control.
  34. in bip-0331.mediawiki:82 in 43be4d7c9e outdated
    77+
    78+Theoretically, developing a safe and incentive-compatible package mempool acceptance policy is
    79+sufficient to solve this issue. Nodes could opportunistically accept packages (e.g. by trying
    80+combinations of transactions rejected from their mempools), but this practice would likely be
    81+inefficient at best and open new Denial of Service attacks at worst.  As such, this proposal
    82+suggests adding new p2p messages enabling nodes to request and share package validation-related
    


    jnewbery commented at 9:16 am on April 3, 2023:

    Does the “related” refer to “package validation”? If yes, I think this is better:

    0suggests adding new p2p messages enabling nodes to request and share package-validation-related
    
  35. in bip-0331.mediawiki:93 in 43be4d7c9e outdated
    88+changing its txid; a node cannot use txid to deduplicate transactions it has already downloaded
    89+or validated. Ideally, two nodes that both support BIP339 wtxid-based transaction relay shouldn't
    90+ever need to use txid-based transaction relay.
    91+
    92+A single use case of txid-based relay remains: handling "orphan" transactions that spend output(s)
    93+from a transaction the receiving node is unaware of.  Orphans are common for new nodes that have
    


    jnewbery commented at 9:16 am on April 3, 2023:
    0from an unconfirmed transaction the receiving node is unaware of.  Orphans are common for new nodes that have
    
  36. in bip-0331.mediawiki:137 in 43be4d7c9e outdated
    132+transactions are 1000vB in size):
    133+
    134+* Package {A, B} where A pays 0 satoshis and B pays 8000 satoshis in fees.
    135+* Package {C, D} where C pays 0 satoshis and D pays 1200 satoshis in fees.
    136+* Package {E, F, G, H, J} that pay 4000, 8000, 0, 2000, and 4000 satoshis in fees, respectively.
    137+[[File:./bip-0331/tx_scenarios.png|800px]]
    


    jnewbery commented at 9:21 am on April 3, 2023:
    You still need to update the values in these bullet points and the image. Also, the image says the virtual size is 1600Wu, but virtual size should be in vB. I’d also remove the “size” from the image.

    willcl-ark commented at 9:42 am on April 4, 2023:
    If it doesn’t clutter it too much it might also be nice to add the package feerates? Although they are simple enough to calculate too…

    glozow commented at 4:39 pm on April 22, 2023:
    Ended up removing this as I don’t think it was adding very much
  37. in bip-0331.mediawiki:138 in 43be4d7c9e outdated
    133+
    134+* Package {A, B} where A pays 0 satoshis and B pays 8000 satoshis in fees.
    135+* Package {C, D} where C pays 0 satoshis and D pays 1200 satoshis in fees.
    136+* Package {E, F, G, H, J} that pay 4000, 8000, 0, 2000, and 4000 satoshis in fees, respectively.
    137+[[File:./bip-0331/tx_scenarios.png|800px]]
    138+< br/>
    


    jnewbery commented at 9:36 am on April 3, 2023:

    Sorry, I got the whitespace wrong in my review comment. It should be:

    0<br />
    

    Same on line 144 below.

  38. in bip-0331.mediawiki:225 in 43be4d7c9e outdated
    205+
    206+===Combined Hash===
    207+
    208+A "combined hash" serves as a unique "package id" for some list of unique transactions.
    209+
    210+The combined hash of a package of transactions is equal to the hash of each transaction's wtxid, concatenated in lexicographical order.
    


    jnewbery commented at 9:40 am on April 3, 2023:
    I think you should include the name of the hash function in this definition.

    stickies-v commented at 4:59 pm on April 3, 2023:
    Should we use a BIP340 tagged hash here? (I’m not sure, thinking out loud). For example, could be relevant for these rolling bloom filters that contain hashes corresponding to both txes and packages?
  39. in bip-0331.mediawiki:72 in 43be4d7c9e outdated
    67+predictable), but this solution is not foolproof and wastes users' money. For some attacks,
    68+the available defenses require nodes to have a bird's-eye view of Bitcoin nodes' mempools,
    69+which is an unreasonable security requirement.
    70+
    71+The best solution is to enable nodes to consider packages of transactions as a unit, e.g. one or
    72+more low-fee parent transactions with a high-fee child, instead of separately. A package-aware
    


    jnewbery commented at 9:41 am on April 3, 2023:
    0more low-fee ancestor transactions with a high-fee descendant, instead of separately. A package-aware
    
  40. in bip-0331.mediawiki:154 in 43be4d7c9e outdated
    149+transaction download fails for some reason, it shouldn't be used as the default because it always
    150+requires storage of unvalidated transactions.
    151+[[File:./bip-0331/package_info_only.png|1200px]]
    152+</ref>
    153+
    154+Upon encountering an orphan, a node may request ancestor package information delineating the wtxids
    


    jnewbery commented at 9:42 am on April 3, 2023:
    0Upon receiving an orphan transaction, a node may request ancestor package information delineating the wtxids
    
  41. in bip-0331.mediawiki:131 in 43be4d7c9e outdated
    119+
    120+==Specification==
    121+
    122+===Intended Protocol Flow===
    123+
    124+Package Relay consists of two phases: a package information round and a transaction data round.
    


    jnewbery commented at 9:52 am on April 3, 2023:
    I’d add a sentence here along the lines of “This package relay protocol satisfies both use cases (orphan transaction handling and high-feerate descendant paying for low-feerate ancestors).” and then have sub-headings for “orphan transaction handling”, “high-feerate descendants” and “feature negotiation” below. Without the sub-headings, it’s a bit difficult to follow the structure of the doc.
  42. in bip-0331.mediawiki:164 in 43be4d7c9e outdated
    159+[[File:./bip-0331/protocol_flow.png|1200px]]
    160+
    161+Too-low-feerate transactions (i.e. below the node's minimum mempool feerate) with high-feerate
    162+descendants can also be relayed this way. If the peers are using BIP133 fee filters and a
    163+low-feerate transaction is below the node's fee filer, the sender will not announce it. The
    164+high-feerate transaction is seen and handled as an orphan, the transactions are validated as a
    


    jnewbery commented at 9:54 am on April 3, 2023:
    0high-feerate transaction ''will'' be sent by the sender, and received and handled as an orphan by the receiver. The transactions are validated as a
    
  43. in bip-0331.mediawiki:165 in 43be4d7c9e outdated
    160+
    161+Too-low-feerate transactions (i.e. below the node's minimum mempool feerate) with high-feerate
    162+descendants can also be relayed this way. If the peers are using BIP133 fee filters and a
    163+low-feerate transaction is below the node's fee filer, the sender will not announce it. The
    164+high-feerate transaction is seen and handled as an orphan, the transactions are validated as a
    165+package, and so the protocol naturally works for this use case.<ref>'''Does this mean BIP133 is
    


    jnewbery commented at 9:55 am on April 3, 2023:
    I suggest you bring this into the main body of this section rather than have it as a footnote.
  44. in bip-0331.mediawiki:265 in 43be4d7c9e outdated
    281+It was [https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-May/020496.html suggested] that
    282+this message is not package-specific and generally useful for downloading transactions as
    283+batches.</ref>
    284+
    285+
    286+====ancpkginfo====
    


    jnewbery commented at 10:00 am on April 3, 2023:
    Would it make sense to put this above the definition of “getpkgtxns”? “ancpkginfo comes first both sequentially in the protocol flow and alphabetically.

    stickies-v commented at 5:51 pm on April 3, 2023:
    Do we want to redefine a package info message for every package version? Could also use a generic pkginfo with version field matching the one in sendpackages?

    glozow commented at 3:51 pm on April 5, 2023:

    Do we want to redefine a package info message for every package version?

    Yes

    Could also use a generic pkginfo with version field matching the one in sendpackages?

    I think it makes more sense to have a separate protocol message for separate things than to add 4-byte overhead on every package info message.

  45. in bip-0331.mediawiki:331 in 43be4d7c9e outdated
    326+
    327+# A new inv type (MSG_ANCPKGINFO == 0x6) is added, for use only in getdata requests pertaining to ancestor packages.
    328+
    329+# As a getdata request type, it indicates that the sender wants a "ancpkginfo" containing all of the unconfirmed ancestors of a transaction, referenced by wtxid.
    330+
    331+# Upon receipt of a "getdata(MSG_ANCPKGINFO)" request for, the node should respond with the "ancpkginfo" corresponding to the transaction's unconfirmed ancestor package, or with NOTFOUND.  The wtxid of the requested transaction should be the last item in the list. The node should not assume that the sender is requesting the transaction data as well.
    


    jnewbery commented at 10:03 am on April 3, 2023:
    0# Upon receipt of a "getdata(MSG_ANCPKGINFO)" request, the node should respond with the "ancpkginfo" corresponding to the transaction's unconfirmed ancestor package, or with NOTFOUND.  The wtxid of the requested transaction should be the last item in the list. The node should not assume that the sender is requesting the transaction data as well.
    

    jnewbery commented at 10:04 am on April 3, 2023:
    0# Upon receipt of a "getdata(MSG_ANCPKGINFO)" request for, the node should respond with an "ancpkginfo" message corresponding to the transaction's unconfirmed ancestor package, or with NOTFOUND.  The wtxid of the requested transaction should be the last item in the list. The node should not assume that the sender is requesting the transaction data as well.
    

    jnewbery commented at 10:06 am on April 3, 2023:
    I think “The node should not assume that the sender is requesting the transaction data as well.” is unnecessary, since the description of the protocol above has already specified that the protocol has separate package information and transaction data rounds.
  46. in bip-0331.mediawiki:333 in 43be4d7c9e outdated
    328+
    329+# As a getdata request type, it indicates that the sender wants a "ancpkginfo" containing all of the unconfirmed ancestors of a transaction, referenced by wtxid.
    330+
    331+# Upon receipt of a "getdata(MSG_ANCPKGINFO)" request for, the node should respond with the "ancpkginfo" corresponding to the transaction's unconfirmed ancestor package, or with NOTFOUND.  The wtxid of the requested transaction should be the last item in the list. The node should not assume that the sender is requesting the transaction data as well.
    332+
    333+# The inv type should not be used in announcements, i.e. "inv(MSG_ANCPKGINFO)" should never be sent.
    


    jnewbery commented at 10:08 am on April 3, 2023:
    What should happen if a node receives an inv(MSG_ANCPKGINFO)? Drop/ignore the message or disconnect/punish?
  47. in bip-0331.mediawiki:319 in 43be4d7c9e outdated
    314+downloaded. See [https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-June/020558.html discussion].
    315+</ref>
    316+
    317+# Upon receipt of a "ancpkginfo" message, the node may use it to request the transactions not already in its mempool.
    318+
    319+# Upon receipt of a malformed "ancpkginfo" message that does not contain the ancestor package of the transaction (which the node learns after receiving the transaction data and validating them), the sender may be disconnected.
    


    jnewbery commented at 10:20 am on April 3, 2023:
    This seems to slightly contradict the “Should a peer be punished if they provide incorrect package info” footnote, or am I misunderstanding?

    glozow commented at 4:40 pm on April 22, 2023:
    Fixed, described what malformed means
  48. in bip-0331.mediawiki:335 in 43be4d7c9e outdated
    330+
    331+# Upon receipt of a "getdata(MSG_ANCPKGINFO)" request for, the node should respond with the "ancpkginfo" corresponding to the transaction's unconfirmed ancestor package, or with NOTFOUND.  The wtxid of the requested transaction should be the last item in the list. The node should not assume that the sender is requesting the transaction data as well.
    332+
    333+# The inv type should not be used in announcements, i.e. "inv(MSG_ANCPKGINFO)" should never be sent.
    334+
    335+# This inv type should only be used if both peers agreed to send version 0 packages. If a "getdata" message with type MSG_ANCPKGINFO is received from a peer with which ancestor package relay was not negotiated, the sender may be disconnected.
    


    jnewbery commented at 10:22 am on April 3, 2023:

    Perhaps:

    0# This inv type should only be used if both peers agreed to send version 0 packages. If a "getdata" message with type MSG_ANCPKGINFO is received from a peer with which ancestor package relay was not negotiated, no response should be sent and the sender may be disconnected.
    
  49. in bip-0331.mediawiki:323 in 43be4d7c9e outdated
    318+
    319+# Upon receipt of a malformed "ancpkginfo" message that does not contain the ancestor package of the transaction (which the node learns after receiving the transaction data and validating them), the sender may be disconnected.
    320+
    321+# A node MUST NOT send a "ancpkginfo" message that has not been requested by the recipient. Upon receipt of an unsolicited "ancpkginfo", a node may disconnect the sender.
    322+
    323+# A "ancpkginfo" message should only be sent if both peers agreed to send version 0 packages. If a "ancpkginfo" message is received from a peer with which ancestor package relay was not negotiated, the sender may be disconnected.
    


    jnewbery commented at 10:23 am on April 3, 2023:
    This seems slightly redundant with the point above. If package relay has not been negotiated, then the peer shouldn’t have sent a getdata(MSG_ANCPKGINFO)
  50. in bip-0331.mediawiki:259 in 43be4d7c9e outdated
    254+
    255+# The "getpkgtxns" message has the structure defined above, with pchCommand == "getpkgtxns".
    256+
    257+# A "getpkgtxns" message should be used to request all or some of the transactions previously announced in a "ancpkginfo" message, specified by witness transaction id. This message is intended to allow nodes to avoid downloading and storing transactions that cannot be validated immediately.
    258+
    259+# Upon receipt of a "getpkgtxns" message, a node must respond with either a "pkgtxns" containing the requested transactions or "notfound" messages indicating one or more of the transactions is unavailable.<ref>'''Why isn't package relay negotiation required for getpkgtxns?'''
    


    jnewbery commented at 10:26 am on April 3, 2023:
    If I read this bullet point and footnote literally, it says that all nodes must respond to getpkgtxns messages, even if they haven’t negotiated package relay. That seems wrong. It means that all existing nodes on the network would be in violation of this rule.

    glozow commented at 3:47 pm on April 5, 2023:
    Yes my bad, a node should respond.
  51. in bip-0331.mediawiki:191 in 43be4d7c9e outdated
    186+right, I propose we hold off on sender-initiated for now, deploy receiver-initiated package relay,
    187+observe its usage and figure out where we can save a round trip, and then introduce a
    188+well-researched sender-initiated package relay protocol.</ref>
    189+
    190+Ancestor package relay is negotiated between two peers during the version handshake using a
    191+"sendpackages" message containing version=1. It requires both peers to support wtxid-based relay
    


    jnewbery commented at 10:27 am on April 3, 2023:
    Should this be version=0?
  52. in bip-0331.mediawiki:258 in 43be4d7c9e outdated
    238+Yes, this is allowed in order to reduce the number of negotiation steps. This means nodes can
    239+announce features without first checking what the other peer has sent, and then apply logic at the
    240+end. See [https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-May/020510.html discussion].
    241+</ref>
    242+
    243+# Version=0 packages correspond to ancestor packages, i.e. "ancpkginfo" and "MSG_ANCPKGINFO".
    


    jnewbery commented at 10:32 am on April 3, 2023:
    I think it’d be better to pull this definition out and make it more explicit. It’d also be useful to distinguish between the parts of this proposal that are specific to version 0 (MSG_ANCPKGINFO and ancpkginfo) and the parts that are generic to any kind of package (sendpackages, getpkgtxns, pkgtxns and MSG_PKGTXNS)

    glozow commented at 4:38 pm on April 22, 2023:
    Done
  53. in bip-0331.mediawiki:353 in 43be4d7c9e outdated
    348+Older clients remain fully compatible and interoperable after this change. Clients implementing this
    349+protocol will only attempt to send and request packages if agreed upon during the version handshake.
    350+<ref>'''Will package relay cause non-package relay nodes to waste bandwidth on low-feerate transactions?'''
    351+If a node supports package relay, it may accept low-feerate transactions (e.g. paying zero fees)
    352+into its mempool, but non-package relay nodes would most likely reject them. To mitigate bandwidth
    353+waste, a package relay node should probably not announce descendants of below-fee-filter
    


    jnewbery commented at 10:33 am on April 3, 2023:
    Can we remove “probably” here?
  54. in bip-0331.mediawiki:367 in 43be4d7c9e outdated
    362+
    363+==Implementation==
    364+
    365+Implementation for Bitcoin Core: https://github.com/glozow/bitcoin/tree/2023-01-package-relay
    366+
    367+Caveat: the crux of package relay is a mempool policy that works.
    


    jnewbery commented at 10:40 am on April 3, 2023:

    Caveat/crux/“that works” all feel a bit informal/imprecise. Perhaps:

    0Sample implementation for Bitcoin Core: https://github.com/glozow/bitcoin/tree/2023-01-package-relay. A prerequisite for implementing a safe package relay protocol is to have a mempool acceptance policy that is able to safely validate packages of transactions.
    
  55. in bip-0331.mediawiki:115 in 43be4d7c9e outdated
    110+A transaction's '''descendants''' include, recursively, its children, the children of its children,
    111+etc. A transaction's parent is its ancestor, but an ancestor is not necessarily a parent.
    112+
    113+A '''package''' is a list of transactions, representable by a connected Directed Acyclic
    114+Graph (a directed edge exists between a transaction that spends the output of another transaction).
    115+Typically, a package refers to unconfirmed transactions.
    


    stickies-v commented at 2:29 pm on April 3, 2023:
    0In this proposal, a package is limited to unconfirmed transactions.
    

    willcl-ark commented at 9:35 am on April 4, 2023:
    What happens if my node sends a package including a confirmed ancestor transaction?

    glozow commented at 3:49 pm on April 5, 2023:

    What happens if my node sends a package including a confirmed ancestor transaction?

    The receiver should be able to deal with it. It is not a violation to do so, as you might just have different chainstates.

  56. in bip-0331.mediawiki:169 in 43be4d7c9e outdated
    154+Upon encountering an orphan, a node may request ancestor package information delineating the wtxids
    155+of the transaction's unconfirmed ancestors. This is done without using txid-based relay. The package
    156+information can be used to request transaction data. As these transactions are dependent upon one
    157+another to be valid, the transactions can be requested and sent as a batch.
    158+
    159+[[File:./bip-0331/protocol_flow.png|1200px]]
    


    stickies-v commented at 2:40 pm on April 3, 2023:
    I think it’s supposed to be getdata(MSG_ANCPKGINFO, ...) instead of getdata(ANCPKGINFO, ...)?

    stickies-v commented at 2:46 pm on April 3, 2023:
    Also general comment: perhaps figure annotations would be helpful?

    glozow commented at 4:38 pm on April 22, 2023:
    I omit the MSG because all the inv types have it
  57. in bip-0331.mediawiki:238 in 43be4d7c9e outdated
    218+====sendpackages====
    219+
    220+{|
    221+|  Field Name  ||  Type  ||  Size  ||  Purpose
    222+|-
    223+|version || uint32_t || 4 || Denotes a package version supported by the node.
    


    stickies-v commented at 5:11 pm on April 3, 2023:

    If version is a uint32_t, it seems to indicate that we want to leave room for a significant amount more package versions (and even if we don’t, it’s an infrequently sent message, so no biggie to have this be oversized). In that light, would it make sense to have instead have a versions array<uint32_t>, so in a future with lots of possible package versions we don’t need to flood sendpackages messages, and making it an array now is not a lot of overhead?

    I’d be surprised if we end up with a huge number of package versions so I’m probably exaggerating here, but I think even for a small number of versions it makes sense, since the expectation is that we’ll have multiple versions?


    stickies-v commented at 5:39 pm on April 3, 2023:

    It’s a shame we can’t version getdata messages, because then we could have a generic MSG_PKGINFO (and MSG_PKGTXNS INV type, and use the same version that we announce in sendpackages. That would avoid having to create a new INV type for every package version.

    I think we can make things more consistent by having the version(s) field in sendpackages correspond to the INV type, though? I.e. instead of version=0 we’d use version=0x6 for ancestor packages?


    jnewbery commented at 10:22 am on April 21, 2023:

    I agree that having space for 4294967295 package versions is probably more than we’ll ever need.

    An alternative would be to treat this as a bit field, in the way that nServices flags are signalled in the version message. This BIP would define version (1 << 0) and future versions would be defined as (1 << n). When we receive a sendpackages message, we XOR the received versions with the received versions from previous sendpackages messages.


    glozow commented at 4:06 pm on April 21, 2023:
    Yes I think a bitfield is best :+1: 32 package versions seems plenty, and this makes the implementation cleaner as well.
  58. in bip-0331.mediawiki:258 in 43be4d7c9e outdated
    231+# Upon receipt of a "sendpackages" message with a version that is not supported, a node must treat the peer as if it never received the message.
    232+
    233+# The "sendpackages" message MUST be sent before sending a "verack" message. If a "sendpackages" message is received afer "verack", the sender may be disconnected.
    234+
    235+# A node that sends "sendpackages" MUST also send "wtxidrelay". Upon receipt of a "verack", if the sender has sent a "sendpackages" but not "wtxidrelay", the sender may be disconnected.
    236+
    


    stickies-v commented at 5:13 pm on April 3, 2023:
    This is inconsistent with Erlay behaviour. I think I prefer the “may be disconnected” approach, but just flagging that since ideally they’re aligned?

    glozow commented at 4:37 pm on April 22, 2023:
    Removed
  59. in bip-0331.mediawiki:356 in 43be4d7c9e outdated
    334+
    335+# This inv type should only be used if both peers agreed to send version 0 packages. If a "getdata" message with type MSG_ANCPKGINFO is received from a peer with which ancestor package relay was not negotiated, the sender may be disconnected.
    336+
    337+====MSG_PKGTXNS====
    338+
    339+# A new inv type (MSG_PKGTXNS == 0x7) is added, for use only in "notfound" messages pertaining to package transactions.
    


    stickies-v commented at 5:44 pm on April 3, 2023:
    supernit: I know it really doesn’t matter, and yet it feels so much cleaner if the version-less MSG_PKGTXNS has the lower number (0x6) so all the package-specific invs can increase from there? (i.e. MSG_ANCPKGINFO (0x7))

    glozow commented at 4:37 pm on April 22, 2023:
    Done, I like it better too :)
  60. in bip-0331.mediawiki:34 in 43be4d7c9e outdated
    29+users participating in transaction relay reap many benefits from employing policies that result in a
    30+mempool with the same contents, including faster compact block relay and more accurate fee
    31+estimation. Additionally, users may take advantage of mempool and miner policy to bump the priority
    32+of their transactions by attaching high-fee descendants (Child Pays for Parent or CPFP).
    33+
    34+Only considering transactions one at a time for submission to the mempool creates a limitation in
    


    willcl-ark commented at 9:25 am on April 4, 2023:

    You could consider re-wording this to:

    “Individually considering transactions for submission to the mempool creates a limitation…”

  61. in bip-0331.mediawiki:42 in 43be4d7c9e outdated
    37+transaction's descendants when considering which of two conflicting transactions to keep (Replace by
    38+Fee or RBF).
    39+
    40+When a user's transaction does not meet a mempool's minimum feerate and they cannot create a
    41+replacement transaction directly, their transaction will simply be rejected by this mempool. They
    42+also cannot attach a descendant to pay for replacing a conflicting transaction.
    


    willcl-ark commented at 9:30 am on April 4, 2023:
    I think it’s alluded to, but perhaps it’s worth clarifying that they also currently cannot submit the high-fee descendant first, followed by the low-fee ancestor?
  62. in bip-0331.mediawiki:163 in 43be4d7c9e outdated
    158+
    159+[[File:./bip-0331/protocol_flow.png|1200px]]
    160+
    161+Too-low-feerate transactions (i.e. below the node's minimum mempool feerate) with high-feerate
    162+descendants can also be relayed this way. If the peers are using BIP133 fee filters and a
    163+low-feerate transaction is below the node's fee filer, the sender will not announce it. The
    


    willcl-ark commented at 9:49 am on April 4, 2023:
    0low-feerate transaction is below the node's fee filter, the sender will not announce it. The
    
  63. in bip-0331.mediawiki:203 in 43be4d7c9e outdated
    183+to design the sender-initiated protocol carefully, and inform the design decisions using data
    184+collected from the mainnet p2p network. However, there is no historical transaction data to use
    185+because the goal is to enable currently-rejected transactions to propagate. In order to get this
    186+right, I propose we hold off on sender-initiated for now, deploy receiver-initiated package relay,
    187+observe its usage and figure out where we can save a round trip, and then introduce a
    188+well-researched sender-initiated package relay protocol.</ref>
    


    willcl-ark commented at 9:57 am on April 4, 2023:

    I thought that a decent middle-ground could be to apply something like the following:

    • If you receive a package initially, you proactively re-broadcast it as a package to your peers.
    • If you recieve an orphan and end up requesting a package, you re-broadcast the orphan and not the package.

    This way you still retain some bandwidth minimisation, whilst also being somewhat proactive about broadcasting packages and reducing the # of roundtrips. Also it places the onus on the (original) sender to be broadcasting as a package if they want it relayed (as quickly as possible) as a package to the network.

    But in thinking it through more, this could permit malicious nodes to start sending many large packages around unsolicited wasting the bandwidth of our P2P network…

    Therefore I agree that reciever-initiated makes the most sense here.

  64. in bip-0331.mediawiki:223 in 43be4d7c9e outdated
    203+
    204+[[File:./bip-0331/version_negotiation.png|600px]]
    205+
    206+===Combined Hash===
    207+
    208+A "combined hash" serves as a unique "package id" for some list of unique transactions.
    


    willcl-ark commented at 10:24 am on April 4, 2023:

    What exactly is this combined hash for – is it only used in the inv(MSG_PKGTXNS) message?

    My understanding is that we are referring to packages by the child wtxid?


    stickies-v commented at 2:04 pm on April 4, 2023:
    I think one use case to unambiguously determine if we’ve already validated (/rejected) a certain (exact) package as per this comment

    glozow commented at 3:54 pm on April 5, 2023:

    Yes, the combined hash is only used in notfound(MSG_PKGTXNS).

    We cannot use the child wtxid because the child wtxid is not included in the getpkgtxns request.

  65. glozow commented at 3:55 pm on April 5, 2023: member
    Thanks @jnewbery @stickies-v @willcl-ark for the feedback. I’m working on incorporating the suggestions, apologies for the delay.
  66. glozow force-pushed on Apr 6, 2023
  67. glozow force-pushed on Apr 22, 2023
  68. glozow commented at 4:37 pm on April 22, 2023: member

    Changed the version field in “sendpackages” to versions, now interpreted as a bitfield, and updated the negotiation procedure accordingly. (https://github.com/bitcoin/bips/pull/1382#discussion_r1173605835) Swapped the inv values for MSG_ANCPKGINFO and MSG_PKGTXNS (https://github.com/bitcoin/bips/pull/1382#discussion_r1156267093)

    Neither of these are implemented in the branch I had linked before, so I’ve removed that link. Will update when I’ve done that.

    I’ve restructured the BIP to make the difference between “generic” (can be used in future versions of package relay) vs “ancestor info” (first defined version) parts more clear (https://github.com/bitcoin/bips/pull/1382#discussion_r1155782599). Also updated the diagrams and took wording suggestions.

  69. fjahr commented at 4:07 pm on May 1, 2023: contributor
    Maybe it’s obvious or I am missing something but I couldn’t see the answer in the doc: How would a node decide between sending Inv(PKGINFO, C) or Inv(WTX, C) in the Generic Package Relay example graphic? Is it always sending PKGINFO if the peer wants packages and C is part of a package?
  70. instagibbs commented at 2:57 pm on May 2, 2023: member
    @fjahr “The inv type should not be used in announcements” it’s a type used for getdata only IIUC
  71. fjahr commented at 3:53 pm on May 2, 2023: contributor

    @fjahr “The inv type should not be used in announcements” it’s a type used for getdata only IIUC

    Hmm, ok, first insight: the inconsistencies in the message naming can be confusing. I guess it gets obvious when someone goes deeper but I did a pretty superficial pass and then tried to find an answer to this question and I guess I stumbled over that it’s PKGINFO in the graphics but ANCPKGINFO in the text.

    It seems that the name has changed before which makes researching past discussions on the mailing list harder. Maybe that could be noted somewhere (the artist formerly known as PKGINFO, also formerly know as MSG_PCKG1 etc.). Not sure it’s worth the hassle but in this case it would have helped me.

    In an old ML post I found:

    0''Q: Under what circumstances should a sender announce a
    1child-with-unconfirmed-parents package?''
    2
    3A child-with-unconfirmed-parents package for a transaction should be
    4announced when it meets the peer's fee filter but one or more of its
    5parents don't; a "inv(MSG_PCKG1)" instead of "inv(WTX)" should be sent
    6for the child. Each of the parents which meet the peer's fee filter
    7should still be announced normally.
    

    (from https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-May/020493.html)

    But this is now changed it seems so maybe the graphics can be updated in this regard as well and the inv messages removed?

  72. glozow commented at 10:34 am on May 3, 2023: member

    Yes, PKGINFO is not specified anywhere; think of it as an “abstract class.” The purpose of that diagram is to show how this protocol might be extended or upgraded in the future, i.e. we can define a new type of PKGINFO message to be used in invs and/or getdatas, and continue using the same getpkgtxns and pkgtxns message for transaction download. I want to separate those 2 concepts: exchanging package information and exchanging transaction data.

    For example, CHUNKPKGINFO could contain a chunk (with cluster-based mempool), perhaps specifying the chunk fee and size as well. Originally, I proposed a PCKGINFO1 type which was a child and its parents to communicate a fee-bumping relationship.

    The reason there is an “inv(PKGINFO)” there is to communicate that, while inv(ANCPKGINFO) is not allowed, some other kind of inv(PKGINFO) can be allowed in the future for a sender-initiated protocol.

    I will update the diagram to make this more clear, thanks for the feedback and sorry for the confusion!

  73. glozow force-pushed on May 3, 2023
  74. glozow force-pushed on May 3, 2023
  75. joostjager commented at 2:13 pm on May 5, 2023: none
    Linking https://github.com/nostr-protocol/nips/pull/476, nostr-based package relay.
  76. in bip-0331.mediawiki:232 in 7118e550cf outdated
    233+historical data is available, as one of the primary goals of this protocol is to enable
    234+currently-rejected transactions to propagate. After deploying receiver-initiated package relay, we
    235+can observe its usage and then introduce a sender-initiated package relay protocol informed by data
    236+collected from the p2p network.</ref>
    237+
    238+===Combined Hash===
    


    fjahr commented at 1:41 pm on May 12, 2023:
    I think this could be moved into the definitions section.

    glozow commented at 7:51 pm on May 20, 2023:
    Unsure… somebody would need to use this to implement MSG_PKGTXNS. That seems like it should be “specification” ?

    glozow commented at 11:14 am on May 31, 2023:
    Kept this as is for now

    ajtowns commented at 6:53 am on June 10, 2023:
    It would be good to have the motivation (ie “to provide a meaningful but short notfound response to getpkgtxns”) mentioned before the definition
  77. in bip-0331.mediawiki:263 in 7118e550cf outdated
    258+|-
    259+|}
    260+
    261+# The "sendpackages" message has the structure defined above, with pchCommand == "sendpackages".
    262+
    263+# During version handshake, nodes should one "sendpackages" message indicating they support package relay, with the versions field indicating which versions of they support.
    


    fjahr commented at 1:49 pm on May 12, 2023:
    should one => should send one of they => they

    glozow commented at 11:15 am on May 31, 2023:
    Thanks, fixed
  78. in bip-0331.mediawiki:129 in 7118e550cf outdated
    122+ancestors.
    123+
    124+In a '''topologically sorted''' package, each parent appears somewhere in the list before its child.
    125+
    126+==Specification==
    127+
    


    fjahr commented at 2:16 pm on May 12, 2023:
    Consider adding something here like “This BIP defines both Generic Package Relay, the unconditional part of the protocol that all nodes opt-in to when they indicate support for package relay, as well as Ancestor Package Relay, the first optional part of the protocol that nodes may opt-in to during the version handshake. The version handshake itself is considered part of Generic Package Relay.”

    glozow commented at 11:11 am on May 31, 2023:

    Thanks, yeah, it’s weird the way drew a line between generic/ancestor package relay!

    Had an offline conversation with someone about the “generic” thing. I decided to get rid of that and instead add the information in an “Extensibility” section. At the end of the day, I’m just suggesting what a future package relay upgrade could look like and we are not locked into using the format I outlined.

  79. in bip-0331.mediawiki:183 in 7118e550cf outdated
    178+supporting version n, it should be easy to announce support for version n-1, but not version n,
    179+packages.</ref>
    180+
    181+[[File:./bip-0331/version_negotiation.png|400px]]
    182+
    183+===Ancestor Package Information===
    


    fjahr commented at 2:28 pm on May 12, 2023:
    Was it ever considered to name Ancestor Package Relay together with the sendpackages version number it uses? It’s possible that there is a different way of exchanging ancestors in the future and then that might be confusing if we are not referring to it with v0 ancestor package relay and v1 ancestor package relay. It’s also much shorter to reference ;)

    glozow commented at 11:16 am on May 31, 2023:
    Heh, I think in the future if we have 2, we could probably say PKG_RELAY_ANC (the name of the version bit) to refer to this one?
  80. in bip-0331.mediawiki:161 in 7118e550cf outdated
    159+
    160+This protocol can be extended to include more types of package information in the future, while
    161+continuing to use the same messages for transaction data download.
    162+[[File:./bip-0331/generic_package_relay.png|600px]]
    163+
    164+Package relay is negotiated between two peers during the version handshake using a "sendpackages"
    


    fjahr commented at 2:40 pm on May 12, 2023:
    Consider moving the version stuff at the beginning of the Generic Package Relay section or even giving it it’s own section. It feels more natural and aligns with the chronology and the order of the messages in “New Messages”.

    glozow commented at 11:13 am on May 31, 2023:
    :+1: I’ve gotten rid of Generic and hopefully the flow is now more natural since there’s only 1 section in “New Messages”
  81. in bip-0331.mediawiki:285 in 7118e550cf outdated
    280+|txns||List of wtxids||txns_length * 32|| The wtxids of each transaction in the package.
    281+|}
    282+
    283+# The "getpkgtxns" message has the structure defined above, with pchCommand == "getpkgtxns".
    284+
    285+# A "getpkgtxns" message should be used to request some list of transactions specified by witness transaction id. It indicates that the node wants to receiver either all the specified transactions or none of them. This messages is intended to allow nodes to avoid downloading and storing transactions that cannot be validated without each other. The list of transactions does not need to correspond to a previously-received pkginfo message.
    


    fjahr commented at 2:41 pm on May 12, 2023:
    receiver => receive This messages => This message
  82. in bip-0331.mediawiki:265 in 7118e550cf outdated
    260+
    261+# The "sendpackages" message has the structure defined above, with pchCommand == "sendpackages".
    262+
    263+# During version handshake, nodes should one "sendpackages" message indicating they support package relay, with the versions field indicating which versions of they support.
    264+
    265+# The "sendpackages" message MUST be sent before sending a "verack" message. If a "sendpackages" message is received afer "verack", the sender may be disconnected.
    


    fjahr commented at 2:42 pm on May 12, 2023:
    afer => after
  83. in bip-0331.mediawiki:287 in 7118e550cf outdated
    282+
    283+# The "getpkgtxns" message has the structure defined above, with pchCommand == "getpkgtxns".
    284+
    285+# A "getpkgtxns" message should be used to request some list of transactions specified by witness transaction id. It indicates that the node wants to receiver either all the specified transactions or none of them. This messages is intended to allow nodes to avoid downloading and storing transactions that cannot be validated without each other. The list of transactions does not need to correspond to a previously-received pkginfo message.
    286+
    287+# Upon receipt of a "getpkgtxns" message, a node should respond with either a "pkgtxns" containing all of the requested transactions in the same order specified in the "getpkgtxns" request or one "notfound" message of type MSG_PKGTXNS and combined hash indicating one or more of the transactions is unavailable.<ref>'''Why isn't package relay negotiation required for getpkgtxns?'''
    


    fjahr commented at 8:03 pm on May 12, 2023:
    1. Just checking: so if a notfound message is sent as a response, there is never also a pgtxns sent, even if some of the txs are available, right?
    2. This will be the combined hash of the all the requested wtxids, not all the wtxids that are not available, right? I think this could be a little more clear.

    glozow commented at 11:17 am on May 31, 2023:
    Yep! tried to clarify this
  84. in bip-0331.mediawiki:308 in 7118e550cf outdated
    303+
    304+# A "pkgtxns" message should contain the transaction data requested using "getpkgtxns".
    305+
    306+# Upon receipt of a "pkgtxns" message, the node should validate the transactions together as a package rather than individually.
    307+
    308+# A "pkgtxns" message should only be sent to a peer that requested the package using "getpkgtxns".  If a node receives an unsolicited package, the sender may be disconnected.<ref>'''Why isn't package relay negotiation required for pkgtxns?'''
    


    fjahr commented at 8:06 pm on May 12, 2023:
    What if a node sends more transactions than were requested? Might that also be a reason to disconnect?

    glozow commented at 10:41 am on May 31, 2023:
    Yes, reason to disconnect. I think that is the same thing as an unsolicited package.
  85. in bip-0331.mediawiki:177 in 7118e550cf outdated
    180+
    181+[[File:./bip-0331/version_negotiation.png|400px]]
    182+
    183+===Ancestor Package Information===
    184+
    185+Ancestor Package Relay allows nodes to send and request a transaction's list of unconfirmed
    


    fjahr commented at 8:43 pm on May 12, 2023:
    “Ancestor Package Relay” and “Ancestor Package Information” is used interchangeably in the proposal. The reason is probably that this only contains an *info message. Nonetheless, I think it would be better if this could be made consistent. I don’t have a preference since both are correct, it may even be “Ancestor Package Information Relay”

    glozow commented at 11:14 am on May 31, 2023:
    Changed all the proper noun instances to Ancestor Package Relay
  86. in bip-0331.mediawiki:267 in 7118e550cf outdated
    262+
    263+# During version handshake, nodes should one "sendpackages" message indicating they support package relay, with the versions field indicating which versions of they support.
    264+
    265+# The "sendpackages" message MUST be sent before sending a "verack" message. If a "sendpackages" message is received afer "verack", the sender may be disconnected.
    266+
    267+# Upon successful connection ("verack" sent by both peers), a node may relay packages with the peer if they did not set "fRelay" to false in the "version" message, both peers sent "wtxidrelay", and both peers sent "sendpackages" for matching version(s). Peers should relay packages corresponding to versions that both sent "sendpackages" for.<ref>'''Is it ok to send "sendpackages" to a peer that specified fRelay=false in their "version" message?'''
    


    fjahr commented at 9:15 pm on May 12, 2023:
    Should versions == 0 be allowed?

    instagibbs commented at 5:54 pm on May 30, 2023:
    0# Upon successful connection ("verack" sent by both peers), a node may relay packages with the peer if they did not set "fRelay" to false in the "version" message, both peers sent "wtxidrelay", and both peers sent "sendpackages" for matching version bit(s)). Peers should relay packages corresponding to versions that both sent "sendpackages" for.<ref>'''Is it ok to send "sendpackages" to a peer that specified fRelay=false in their "version" message?'''
    

    glozow commented at 11:15 am on May 31, 2023:
    Yes, though ignored. I’ve added a note about this now

    glozow commented at 11:15 am on May 31, 2023:
    Thanks, taken
  87. in bip-0331.mediawiki:181 in 7118e550cf outdated
    184+
    185+Ancestor Package Relay allows nodes to send and request a transaction's list of unconfirmed
    186+ancestors.  Nodes indicate support for Ancestor Package Relay by adding the
    187+<code>PKG_RELAY_ANCPKG = (1 << 0)</code> bit in their "sendpackages" messages during version handshake.
    188+
    189+===Intended Protocol Flow===
    


    fjahr commented at 2:15 pm on May 14, 2023:
    While this section does talk about protocol flow it does so in the context of specific examples. I would probably call this section “Practical application of the Protocol with examples” or something like that, and move it out of and right after the specification section. I.e. have it between Specification and Compatibility. But I don’t feel too strongly about this.

    glozow commented at 10:59 am on July 4, 2023:
    I’ve changed this to “Protocol Flow Examples” but kept them in the same place. I’m thinking they might be helpful earlier on as a quick way to learn how the dialogue looks. Lmk if you disagree though!
  88. fjahr commented at 2:23 pm on May 14, 2023: contributor
    In my opinion, the proposed protocol is sound and the BIP provides almost all the necessary information. I am asking for a little more clarification in some places but most of my suggestions are intended to make to document easier to parse for first-time readers. While the information is there it is often ordered differently than I would expect. Maybe others’ brains work differently but for me, this would have made a significant difference when I started studying this.
  89. instagibbs commented at 5:49 pm on May 30, 2023: member
    bip-0331/package_cpfp_flow.png <— looks like receiver is asking for A even though they already have it
  90. glozow force-pushed on May 31, 2023
  91. glozow commented at 11:19 am on May 31, 2023: member

    bip-0331/package_cpfp_flow.png <— looks like receiver is asking for A even though they already have it

    Thanks for catching!!! Fixed

  92. fjahr commented at 8:14 pm on June 5, 2023: contributor

    ACK, thanks for the updates, I find it an easier read now and didn’t see any other issues in my latest pass.

    One more question: this protocol seems pretty convenient for fingerprinting nodes by giving specific ancestors to different connections and then observing which tx the connections request to complete the package. I don’t think this is a huge problem since there are several other options that allow fingerprinting. But I am curious if this has been discussed somewhere in the past or if there are already ideas for mitigations in the implementation (which I haven’t fully reviewed yet).

  93. instagibbs commented at 8:24 pm on June 5, 2023: member
    @fjahr can you think of a fundamentally new vectors vs the back and forth orphan fetching? Package fetching seems to be more of a optimization on that. Higher resolution fingerprinting, if it’s more reliable I guess :)
  94. fjahr commented at 8:47 pm on June 5, 2023: contributor

    @fjahr can you think of a fundamentally new vectors vs the back and forth orphan fetching? Package fetching seems to be more of a optimization on that. Higher resolution fingerprinting, if it’s more reliable I guess :)

    I don’t think it’s fundamentally new, that’s why I don’t think it’s a big issue. But I do think it makes the attack more convenient to scale.

    What I had in mind: 1. Make an unconfirmed tx chain A through G. 2. Give different connections different combinations of the txs B through F. This should be 5! = 120 unique fingerprints. 3. Send G and respond to the getdata(ANCPKGINFO, G) to see which tx of B through F the connection is aware of.

  95. glozow commented at 10:29 am on June 6, 2023: member

    @fjahr fingerprinting is a good consideration but neither new nor made easier. An even more convenient approach: make invalid transactions (as many as you want for free), each with different wtxids, and send one to each node. Nodes will cache their unique rejections and not send a getdata on announcement.

    El El lun, 5 jun 2023 a las 21:47, Fabian Jahr @.***> escribió:

    @fjahr https://github.com/fjahr can you think of a fundamentally new vectors vs the back and forth orphan fetching? Package fetching seems to be more of a optimization on that. Higher resolution fingerprinting, if it’s more reliable I guess :)

    I don’t think it’s fundamentally new, that’s why I don’t think it’s a big issue. But I do think it makes the attack more convenient to scale.

    What I had in mind: 1. Make an unconfirmed tx chain A through G. 2. Give different connections different combinations of the txs B through F. This should be 5! = 120 unique fingerprints. 3. Send G and respond to the getdata(ANCPKGINFO, G) to see which tx of B through F the connection is aware of.

    — Reply to this email directly, view it on GitHub https://github.com/bitcoin/bips/pull/1382#issuecomment-1577450727, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGAEGGKBQPRGH6NQJYKJNG3XJZAVLANCNFSM6AAAAAAREVY6QA . You are receiving this because you authored the thread.Message ID: @.***>

  96. in bip-0331.mediawiki:51 in 7060589918 outdated
    45+This limitation harms users' ability to fee-bump their transactions. Further, it presents a security
    46+issue in contracting protocols which rely on presigned, time-sensitive transactions<ref>'''Examples of time-sensitive pre-signed transactions in L2 protocols.'''
    47+* [https://github.com/lightning/bolts/blob/master/03-transactions.md#htlc-timeout-and-htlc-success-transactions HTCL-Timeout in LN Penalty]
    48+* [https://github.com/revault/practical-revault/blob/master/transactions.md#cancel_tx Unvault Cancel in Revault]
    49+* [https://github.com/discreetlogcontracts/dlcspecs/blob/master/Transactions.md#refund-transaction Refund Transaction in Discreet Log Contracts]
    50+* [https://gist.github.com/instagibbs/60264606e181451e977e439a49f69fe1 Updates in Eltoo]
    


    ariard commented at 1:03 am on June 7, 2023:

    There is one more L2 protocol with time-sensitive pre-signed transactions deployed today in production (at least by Eclair and CLN) that benefits from package relay: https://github.com/ElementsProject/peerswap/blob/master/docs/peer-protocol.md#peer-protocol-for-peerswap-swaps

    The maker can pin the off-chain commitment transaction until the claim_by_csv path becomes final with the CSV expiration, and therefore spend both the incoming HTLC output and the opening transaction output.


    glozow commented at 11:06 am on August 3, 2023:
    Added
  97. in bip-0331.mediawiki:145 in 7060589918 outdated
    137+that are eventually rejected, and minimize storage allocated for not-yet-validated transactions.
    138+
    139+<br />
    140+
    141+'''No Package Information Round:''' One proposal is to just use the child's wtxid to refer to the
    142+package and always send the entire package together, skipping the package information round.
    


    ariard commented at 1:21 am on June 7, 2023:

    And I think such design would suffer from transaction censorship vector if you have a parent with multiple childs of unequal feerate ?

    Let’s say you have LN commitment transaction with anchor output support. Both anchors are present as both counterparties have balances. If your counterparty has broadcast its commitment with its own child, you can attach your to_remote child downgrading the feerate of the whole package.

    Of course, from the sender-side, you could re-announce the package for each known child, though it sounds a bandwidth waste as you should do ancestor feerate to propagate the parent first.

    I need to look on the PR to see how package are assembled on the sender-side.


    glozow commented at 8:03 am on June 7, 2023:

    Note that this comment is on an alternative design considered but not proposed.

    And I think such design would suffer from transaction censorship vector if you have a parent with multiple childs of unequal feerate ?

    you can attach your to_remote child downgrading the feerate of the whole package.

    Sure, an additional child may exist. But it is not included in the package because it’s not part of the actual cpfp child’s ancestor set. Its low fees should not be considered in combination with transactions that don’t rely on it.

    As for potential censorship, that is solved by caching “reconsiderable” rejections separately (see m_recent_rejects_reconsiderable in PR). You can relay a separate package that is the ancestor set of the additional child, and its rejection should not affect whether another overlapping ancestor package is considered/accepted.


    ariard commented at 0:51 am on June 16, 2023:

    As for potential censorship, that is solved by caching “reconsiderable” rejections separately (see m_recent_rejects_reconsiderable in PR). You can relay a separate package that is the ancestor set of the additional child, and its rejection should not affect whether another overlapping ancestor package is considered/accepted.

    Yes sounds correct to process each cpfp’s ancestor set separately.

  98. in bip-0331.mediawiki:165 in 7060589918 outdated
    160+multiple versions of packages. Package relay requires both peers to support wtxid-based relay
    161+because package transactions are referenced by their wtxids.
    162+<ref>'''Why do we need multiple versions? Why can't we just support arbitrary packages?'''
    163+Attempting to support arbitrary packages in mempool validation may result in very complex logic, new
    164+Denial of Service attack vectors, and policy limitations that could be leveraged to censor
    165+transactions (aka "pinning attacks"). This protocol is extensible to support up to 32 types of
    


    ariard commented at 1:38 am on June 7, 2023:

    It sounds that type of cheap bandwidth saving that might reveal as a downside a decade from now ? And it’s one time message per-link.

    Let’s hypothetize you will have half a dozen of time-sensitive second-layers deployed few years from now (lightning, vaults, statechains, submarine swaps, time-locked wallets, discreet log contracts). Among those second-layers, they might need different variants of packages for each of their protocol versions (e.g for Lightning taproot channels, eltoo channels, demux channels for jamming) and for each protocol phase (dual-funding, splicing).

    And this becomes worst if you have to combine package size characteristics (e.g N-parent for 1-child) with other future package information like feerare or relationships between transactions.

    Using an extensible feature flags (e.g TLV) like Lightning is doing for most of its peer messages has been discussed so far in the context of BIP 331 ?


    glozow commented at 12:23 pm on June 8, 2023:

    It sounds that type of cheap bandwidth saving that might reveal as a downside a decade from now ? And it’s one time message per-link.

    Let’s hypothetize you will have half a dozen of time-sensitive second-layers deployed few years from now (lightning, vaults, statechains, submarine swaps, time-locked wallets, discreet log contracts). Among those second-layers, they might need different variants of packages for each of their protocol versions (e.g for Lightning taproot channels, eltoo channels, demux channels for jamming) and for each protocol phase (dual-funding, splicing).

    Er, what exactly is the “cheap bandwidth saving?” Are you suggesting to increase the field size?

    I’ve removed the mention of 32 versions to leave room for other ways to extend. For now, I don’t think we should over-engineer the versions field for a hypothetical super-complex future that may or may not need dozens of types of packages. I think 32 is plenty, and the worst case scenario is defining a new message and sending another few bytes once per connection.


    ariard commented at 0:54 am on June 16, 2023:

    Er, what exactly is the “cheap bandwidth saving?” Are you suggesting to increase the field size?

    Yes increasing the filed size to a u64 or u128 to have more bits available for mechanism.

    I think 32 is plenty, and the worst case scenario is defining a new message and sending another few bytes once per connection.

    Sure we can always define a new message, or an extension of it at the price of more p2p code surface and complexity (and note all the bikeshedding with fRelay). Sounds package relay will get more complex not less, though happy to take the bet and wait when this hypothetical super-complex future does happen :)


    glozow commented at 10:57 am on July 4, 2023:

    I’ve changed this to u64.

    happy to take the bet and wait when this hypothetical super-complex future does happen :)

    haha :handshake: If this super-complex future happens and we run out of package types, I’ll send you sats via each protocol

  99. in bip-0331.mediawiki:222 in 7060589918 outdated
    215+<ref>'''Why no sender-initiated protocol?''' Sender-initiated package
    216+relay can, theoretically, save a round trip by notifying the receiver ahead of time that they will
    217+probably need to request and validate a group of transactions together in order for them to be
    218+accepted. As with any proactive communication, there is a chance that the receiver already knows
    219+this information, so this network bandwidth may be wasted. Shortened latency is less significant
    220+than wasted bandwidth.
    


    ariard commented at 2:13 am on June 7, 2023:

    I think this assumption is dependent on the bumping patterns of package broadcast. In the sense that if the package is re-broadcast as it is (and there is no mempool fluctuations), there is a complete waste of bandwidth as both parent and child are known.

    If the package is rebroadcast with the CPFP bumped (probably the future usage for LN where the parent commitment transaction stays an invariant), there is only a need to re-announce the child to the peer nodes.

    And note bandwidth assumptions we’re making on our side can be falsified in changes in the re-broadcast logic of LN implementations, like it has been done recently.

    I think a realistic simulation model would encompass at least few dimensions

    • the peer-to-peer announcement format
    • the mempool fluctuations and evictions of package component
    • the rebroadcast frequency of applications (and if there is a malleation of CFPP at each rebroadcast)

    More dimensions to think about ?


    instagibbs commented at 2:40 pm on June 7, 2023:
    I think this is where practical experience is going to help. Receiver initiated imo helps out in the sense of it fits somewhat more neatly in the current orphan/feefilter paradigm?

    glozow commented at 12:08 pm on June 8, 2023:

    Yeah so sender-initiated is not being proposed here. The general philosophy is that since everything here is receiver-initiated in certain situations, and we are careful to not waste bandwidth in honest cases in the implementation, hopefully we are not blowing up bandwidth.

    If we can do even better by doing sender-initiated in some cases in the future, that’d be nice. I don’t think any simulation can be realistic, as we have 0 data about package relay usage until we actually deploy it and people use it to relay transactions that they couldn’t before. But these are very helpful considerations for measuring things, thanks will keep in mind.


    ariard commented at 1:00 am on June 16, 2023:

    Receiver initiated imo helps out in the sense of it fits somewhat more neatly in the current orphan/feefilter paradigm?

    Correct the update frequency sounds another dimension to think about to evaluate bandwidth consumption.

    Yeah so sender-initiated is not being proposed here. The general philosophy is that since everything here is receiver-initiated in certain situations, and we are careful to not waste bandwidth in honest cases in the implementation, hopefully we are not blowing up bandwidth.

    Note here I’m not thinking to evaluate future hypothetical sender-initiated though the performance of this current receiver-initiated version, especially the burden than rebroadcast frequency of Lightning node might throw on the base layer. Yeah not a blocker right now to deploy, I might start to do simulation by myself once I’m good reviewing the code, really interested to have a clearer picture, and tweak back in consequence Lightning backend.

  100. ariard commented at 2:14 am on June 7, 2023: member
    Review at “Combined Hash” excluded.
  101. in bip-0331.mediawiki:389 in 7060589918 outdated
    384+continuing to use the same messages for transaction data download. One would define a new package
    385+information message, allocate its corresponding inv type, and its bit in the versions field of
    386+"sendpackages".  A future version of package relay may allow a sender-initiated dialogue if it is
    387+specified that the "*PKGINFO" inv type can be used in an "inv" message.
    388+<br />
    389+[[File:./bip-0331/generic_package_relay.png|600px]]
    


    instagibbs commented at 2:51 pm on June 7, 2023:
    Actually not totally clear what this image is showing. why is there a notfound followed by packagetxns?

    glozow commented at 12:05 pm on June 8, 2023:
    Changed it and added some more descriptions. I had wanted to show multiple possible dialogues but it just looks confusing.
  102. glozow force-pushed on Jun 8, 2023
  103. glozow force-pushed on Jun 8, 2023
  104. in bip-0331.mediawiki:20 in 0a46f7d4cb outdated
    15+Peer-to-peer protocol messages enabling nodes to request and relay the unconfirmed ancestor package
    16+of a given transaction, and to request and relay transactions in batches.
    17+
    18+==Motivation==
    19+
    20+(1) Help incentive-compatible transactions propagate.
    


    ajtowns commented at 4:46 am on June 10, 2023:

    Use a subheading? ===Help incen...=== ?

    “incentive-compatible” seems a bit euphemistic – wouldn’t it be better just to say “help ensure the transactions paying the most fees propogate” ?

  105. in bip-0331.mediawiki:86 in 0a46f7d4cb outdated
    81+combinations of transactions rejected from their mempools), but this practice would likely be
    82+inefficient at best and open new Denial of Service attacks at worst.  As such, this proposal
    83+suggests adding new p2p messages enabling nodes to request and share package-validation-related
    84+information with one another, resulting in a more efficient and reliable way to propagate packages.
    85+
    86+(2) Eliminate the need for txid-based transaction relay and make orphan handling more robust.
    


    ajtowns commented at 5:07 am on June 10, 2023:

    You still “need” txid-based relay for backwards compatibility.

    Arguably, txid-based relay would be more efficient than the proposed protocol: if you see an orphan Z, whose parent Y does not spend any in-mempool txs, then this protocol would make three round trips:

    • GETDATA wtx(Z); TZ Z
    • GETDATA andpkginfo(Z); ANCPKGINFO Z, Y
    • GETPKGTXNS wtx(Y); PKGTXNS Y

    but you could alternatively do two round trips:

    • GETDATA wtx(Z); TZ Z
    • GETDATA andpkginfo(Z), tx(Y); ANCPKGINFO Z, Y; TX Y

    That approach would not reduce round-trips when receiving orphans that do have an in-mempool grandparent that the receiver does not already know about (but it wouldn’t increase the number of roundtrips either), and would also require storing both the child and parents while waiting for any grandparents.


    glozow commented at 10:55 am on July 4, 2023:
    I suppose it’s true that this protocol is not going to have fewer round trips in every possible case of orphans, but we wouldn’t have a way of detecting those cases on the receiver end.
  106. in bip-0331.mediawiki:128 in 0a46f7d4cb outdated
    123+
    124+In a '''topologically sorted''' package, each parent appears somewhere in the list before its child.
    125+
    126+==Specification==
    127+
    128+Ancestor Package Relay includes two parts: package information and transaction data download.
    


    ajtowns commented at 6:15 am on June 10, 2023:
    “package information round and package transaction download round” might read better – as it is my first read associated “download” with both the “information” and “data”. (Being synonyms, “information” and “data” aren’t great ways of describing two different things, either…)
  107. in bip-0331.mediawiki:134 in 0a46f7d4cb outdated
    126+==Specification==
    127+
    128+Ancestor Package Relay includes two parts: package information and transaction data download.
    129+A package information round is used to help a receiver learn what transactions are in a package and
    130+whether they want to download them. The transaction data round is used to help a node download
    131+multiple transactions in one message instead of as separate messages.
    


    ajtowns commented at 6:19 am on June 10, 2023:
    The information round doesn’t really help you learn “whether you want to download the transactions” – you know that already, either because you already have the txs, or you’ve remembered that you’ve downloaded them previously and rejected them.

    instagibbs commented at 1:15 pm on June 12, 2023:
    In future package types where you might get additional information, e.g., package feerates or similar this may make more sense to say.

    glozow commented at 8:27 pm on June 13, 2023:

    I don’t think it’s that simple - we have cases of

    • I downloaded this tx and rejected it for too low feerate, but I’m okay with trying again because it’s with a package of other transactions I haven’t seen before
    • I haven’t seen this tx before but there’s another tx in the package that I know is consensus-invalid so I’m not going to download any of them

    ajtowns commented at 2:45 am on June 14, 2023:

    In future package types where you might get additional information, e.g., package feerates or similar this may make more sense to say.

    Okay, “the information round described here doesn’t really help you learn…”

    I haven’t seen this tx before but there’s another tx in the package that I know is consensus-invalid so I’m not going to download any of them

    In that case, (on the implementation side) would we need different reject caches, one for “consensus invalid”, one for “policy invalid”? Not sure that would be worth the hassle (honest peers sending consensus invalid stuff should be extremely rare; attackers could just send novel txs each time avoiding the cache)


    glozow commented at 12:48 pm on June 14, 2023:

    In that case, (on the implementation side) would we need different reject caches, one for “consensus invalid”, one for “policy invalid”?

    Yes. Two rejection caches is necessary, see https://github.com/glozow/bitcoin/pull/8#issuecomment-1453791776. But not policy vs consensus, just ones that are eligible for reconsideration in a package (i.e. too low feerate or missing ephemeral anchor spend) vs ones that are not. I said “consensus invalid” to provide a clear example, but I just mean “something we don’t need to validate again, even if it’s in a package.” If something was larger than max standard size or has too many ancestors, trying it in a package won’t make it valid.


    ajtowns commented at 1:49 pm on June 15, 2023:
    I guess as long as they’re global caches and not per-peer it doesn’t matter much anyway.

    glozow commented at 10:49 am on July 4, 2023:
    I’m going to resolve this for now, but let me know if you still feel that I should change this. Right now the text is “The package information round is used to help a receiver learn what transactions are in a package and decide whether they want to download them.”
  108. in bip-0331.mediawiki:164 in 0a46f7d4cb outdated
    156+</ref>
    157+
    158+Package relay is negotiated between two peers during the version handshake using a "sendpackages"
    159+message. The versions field within "sendpackages" is interpreted as a bitfield; peers may relay
    160+multiple versions of packages. Package relay requires both peers to support wtxid-based relay
    161+because package transactions are referenced by their wtxids.
    


    ajtowns commented at 6:43 am on June 10, 2023:

    You could break ancpkginfo (“ancestor package information”) and pkgtxns (“package tx data relay”) into two separate features, with the idea that maybe someday we’ll have something better than ancestor package relay and want to deprecate the ancpkginfo part, while continuing to have the pkgtxns part.

    If so, you could consider sending sendpackages 3 instead of sendpackages 1 – with bit 0 indicating support of the pkgtxns part, and bit 1 indicating support of the ancpkginfo part.


    glozow commented at 11:38 am on June 14, 2023:
    Ah good idea.
  109. in bip-0331.mediawiki:172 in 0a46f7d4cb outdated
    167+different types of packages and/or contain more information than a list of wtxids, e.g. feerate or
    168+relationships between transactions.</ref>
    169+<ref>'''Why send one message per version instead of sending a single message for the highest
    170+supported version?''' It should also be possible to support some subset of the existing package
    171+types. For example, if a node's mempool policy doesn't support or a node implementation stops
    172+supporting version n, it should be easy to announce support for version n-1, but not version n,
    


    ajtowns commented at 6:45 am on June 10, 2023:
    “doesn’t support” seems sufficient and clearer?
  110. in bip-0331.mediawiki:308 in 0a46f7d4cb outdated
    303+
    304+====MSG_ANCPKGINFO====
    305+
    306+# A new inv type (MSG_ANCPKGINFO == 0x7) is added, for use only in getdata requests pertaining to ancestor packages.
    307+
    308+# As a getdata request type, it indicates that the sender wants a "ancpkginfo" containing all of the unconfirmed ancestors of a transaction, referenced by wtxid.
    


    ajtowns commented at 7:25 am on June 10, 2023:
    an “ancpkginfo”
  111. in bip-0331.mediawiki:310 in 0a46f7d4cb outdated
    305+
    306+# A new inv type (MSG_ANCPKGINFO == 0x7) is added, for use only in getdata requests pertaining to ancestor packages.
    307+
    308+# As a getdata request type, it indicates that the sender wants a "ancpkginfo" containing all of the unconfirmed ancestors of a transaction, referenced by wtxid.
    309+
    310+# Upon receipt of a "getdata(MSG_ANCPKGINFO)" request, the node should respond with an "ancpkginfo" message corresponding to the transaction's unconfirmed ancestor package, or with "notfound".  The wtxid of the requested transaction should be the last item in the list.
    


    ajtowns commented at 7:30 am on June 10, 2023:

    “The ancestor package should be topologically sorted, and therefore the requested transaction should be the last item in the list.” ?

    Alternatively, if we’re using the last item in the list to determine which request this was in response to, perhaps change “should” to “must” ?

    Maybe be explicit about whether a node that receive a ancpkginfo set that’s not topologically sorted (which they determine either because the requested tx isn’t last, or they figure out after downloading the tx contents) should potentially disconnect in response? (I assume they shouldn’t)

    Maybe note that topological sort can be achieved by sorting by mempool acceptance order, if you always accept parents prior to accepting any of their children?


    glozow commented at 11:01 am on July 4, 2023:
    Updated to clarify that the last transaction must be the requested one, the rest can be in any order (nodes should not disconnect or punish), but sender “should” sort topologically for the receiver’s convenience, and that can be achieved through sorting by acceptance order.
  112. in bip-0331.mediawiki:312 in 0a46f7d4cb outdated
    307+
    308+# As a getdata request type, it indicates that the sender wants a "ancpkginfo" containing all of the unconfirmed ancestors of a transaction, referenced by wtxid.
    309+
    310+# Upon receipt of a "getdata(MSG_ANCPKGINFO)" request, the node should respond with an "ancpkginfo" message corresponding to the transaction's unconfirmed ancestor package, or with "notfound".  The wtxid of the requested transaction should be the last item in the list.
    311+
    312+# The inv type should not be used in announcements, i.e. "inv(MSG_ANCPKGINFO)" must never be sent.  If a "inv(MSG_ANCPKGINFO)" is received, the sender may be disconnected.
    


    ajtowns commented at 7:35 am on June 10, 2023:

    an “inv()”

    Maybe better to phrase it as “the MSG_ANCPKGINFO inv type must only be used in getdata message; it must not be used with the inv message” or similar?

  113. in bip-0331.mediawiki:151 in 0a46f7d4cb outdated
    146+
    147+[[File:./bip-0331/no_package_info.png|600px]]
    148+<br />
    149+
    150+'''Package Information Only:''' Just having package information gives enough information for the
    151+receiver to accept the packages. Omit the "getpkgtxns" and "pkgtxns" messages and just download the
    


    ajtowns commented at 7:44 am on June 10, 2023:
    change “Omit the” to “that is, rather than using getpkgtxns and pkgtxns, send getdata and receive each tx individually” ?
  114. in bip-0331.mediawiki:351 in 0a46f7d4cb outdated
    346+
    347+# A "pkgtxns" message should contain the transaction data requested using "getpkgtxns".
    348+
    349+# Upon receipt of a "pkgtxns" message, the node should validate the transactions together as a package rather than individually.
    350+
    351+# A "pkgtxns" message should only be sent to a peer that requested the package using "getpkgtxns".  If a node receives an unsolicited package, the sender may be disconnected.<ref>'''Why isn't package relay negotiation required for pkgtxns?'''
    


    ajtowns commented at 7:49 am on June 10, 2023:

    Maybe also mention that a node might simply ignore an unsolicited package, or may attempt to validate it, or may accept a subset of transactions from the package that it can validate, and may disconnect only if validation fails, or only if validation fails due the package being consensus invalid?

    I assume what core would do is only disconnect if one or more of the txs fails validation due to TX_CONSENSUS.


    instagibbs commented at 1:29 pm on June 12, 2023:
    leaving it undefined seems ok, since it’s just a bucket of “mays” at this point?

    ajtowns commented at 2:50 am on June 14, 2023:
    The way I would read the above is that most of those “mays” would be violating the previous paragraph: “the node should validate the transactions together as a package rather than individually”.

    glozow commented at 1:26 pm on June 14, 2023:
    Do I understand correctly that you’re saying we might want to validate an unsolicited package and not disconnect the sender? Why would we do this? It doesn’t seem like a good idea to spend resources validating transactions if we didn’t ask for them.

    instagibbs commented at 1:40 pm on June 14, 2023:

    “the node should validate the transactions together as a package rather than individually”.

    hm good point. I’m not actually sure what this phrase means? Even the implementation for Core tries transactions individually and in subpackages.

    Why would we do this? It doesn’t seem like a good idea to spend resources validating transactions if we didn’t ask for them.

    It’s kind of an implementation detail, in that it doesn’t effect the underlying protocol(?)


    glozow commented at 1:45 pm on June 14, 2023:
    Ah good point, I suppose I can just delete this bullet. Doesn’t really matter for protocol what the node does with the transactions.
  115. in bip-0331.mediawiki:330 in 0a46f7d4cb outdated
    325+
    326+# The "getpkgtxns" message has the structure defined above, with pchCommand == "getpkgtxns".
    327+
    328+# A "getpkgtxns" message should be used to request some list of transactions specified by witness transaction id. It indicates that the node wants to receive either all the specified transactions or none of them. This message is intended to allow nodes to avoid downloading and storing transactions that cannot be validated without each other. The list of transactions does not need to correspond to a previously-received pkginfo message.
    329+
    330+# Upon receipt of a "getpkgtxns" message, a node should respond with either a "pkgtxns" containing all of the requested transactions in the same order specified in the "getpkgtxns" request or one "notfound" message of type MSG_PKGTXNS and combined hash of all of the wtxids in the "getpkgtxns" request (only one "notfound" message and nothing else), indicating one or more of the transactions is unavailable.<ref>'''Why isn't package relay negotiation required for getpkgtxns?'''
    


    ajtowns commented at 7:54 am on June 10, 2023:
    What if you received a getpkgtxns for wtxids that weren’t in an ancpkginfo that you set? Should you always accept and respond if you have the txs and they all fit into a single message? Can you say “I’ll never accept a tx with more than 20 ancestors in my mempool, so I’ll never send more than 21 wtxids in an ancpkginfo, so I’ll never send more than 21 txs in response to a getpkgtxns?

    glozow commented at 8:43 pm on June 13, 2023:

    What if you received a getpkgtxns for wtxids that weren’t in an ancpkginfo that you set?

    That’s fine imo - we’ll also respond to getdata(tx) for transactions we didn’t announce under certain conditions.

    Can you say “I’ll never accept a tx with more than 20 ancestors in my mempool, so I’ll never send more than 21 wtxids in an ancpkginfo, so I’ll never send more than 21 txs in response to a getpkgtxns?

    I think it’s better for ancpkginfo and getpkgtxns logic to be independent, as the idea is to be able to reuse getpkgtxns for other package info types in the future. One might imagine a future where we have cluster package info (or something), and even though ancestor limit is 21, there is a cluster with 22 transactions.


    ajtowns commented at 2:58 am on June 14, 2023:
    I agree in principle, but I think there probably should be some explicit limit for what you can put in getpkgtxns. With 65 byte txs being standard, you could fit ~60k txs in a pkgtxns response; is there anything beyond “whoops, your response would exceed MAX_PROTOCOL_MESSAGE_LENGTH so you get a notfound instead” that we could use to trim nonsense requests early? There’s a big difference between 21 and 60k!

    glozow commented at 1:34 pm on June 14, 2023:
    Ah good point. Perhaps we limit to, say, 50? 100? (Arbitrarily picked, 25 just seems a little bit too limiting)

    instagibbs commented at 1:48 pm on June 14, 2023:

    If we pick an explicit limit, and want to re-use this message for later info fetching types, we should pick something “stupidly” high to not interfere with known ancestor package/cluster sizes being considered

    e.g., 500?


    glozow commented at 4:29 pm on June 20, 2023:
    Marking as resolved based on #1382 (review)
  116. in bip-0331.mediawiki:377 in 0a46f7d4cb outdated
    372+non-package relay peers.
    373+</ref>
    374+<ref>'''Is Package Erlay possible?'''
    375+A client using BIP330 reconciliation-based transaction relay (Erlay) is able to use package relay
    376+without interference. After reconciliation, any transactions with unconfirmed ancestors may be
    377+relayed using ancestor package relay.
    


    ajtowns commented at 8:10 am on June 10, 2023:
    We already have the transaction at that point, so maybe better to phrase it as “any transaction with unconfirmed ancestors may have those ancestors resolved using ancestor package relay” ?
  117. in bip-0331.mediawiki:349 in 0a46f7d4cb outdated
    344+
    345+# The "pkgtxns" message has the structure defined above, with pchCommand == "pkgtxns".
    346+
    347+# A "pkgtxns" message should contain the transaction data requested using "getpkgtxns".
    348+
    349+# Upon receipt of a "pkgtxns" message, the node should validate the transactions together as a package rather than individually.
    


    instagibbs commented at 1:53 pm on June 14, 2023:

    #1382 (review)

    As per discussion, I’m not sure what this sentence actually means. It’s up to the node implementation to decide how to handle these, including trying individual validation first, then subpackage subsets…


    glozow commented at 1:55 pm on June 14, 2023:
    Deleted
  118. glozow force-pushed on Jun 14, 2023
  119. glozow commented at 2:00 pm on June 14, 2023: member

    Updated for suggestions from @ajtowns, thanks!

    Major changes (still need to update implementation to reflect these):

  120. in bip-0331.mediawiki:334 in d07535cc3d outdated
    329+
    330+# A "getpkgtxns" message should be used to request some list of transactions specified by witness transaction id. It indicates that the node wants to receive either all the specified transactions or none of them. This message is intended to allow nodes to avoid downloading and storing transactions that cannot be validated without each other. The list of transactions does not need to correspond to a previously-received pkginfo message.
    331+
    332+# Upon receipt of a "getpkgtxns" message, a node should respond with either a "pkgtxns" containing all of the requested transactions in the same order specified in the "getpkgtxns" request or one "notfound" message of type MSG_PKGTXNS and combined hash of all of the wtxids in the "getpkgtxns" request (only one "notfound" message and nothing else), indicating one or more of the transactions is unavailable.
    333+
    334+# A "getpkgtxns" message must contain at most 100 wtxids. Upon receipt of a "getpkgtxns" message with more than 100 wtxids, a node may ignore the message (to avoid calculating the combined hash) and disconnect the sender.
    


    instagibbs commented at 3:09 pm on June 14, 2023:

    Sorry for re-painting this shed, but I think this number is a bit low and means we may need to revisit sooner rather than later, for instance if we transition to cluster mempools and have large chunks we want to fetch?

    Can debate the number with more information later I suppose


    glozow commented at 3:40 pm on June 14, 2023:
    No worries. I picked 100 arbitrarily, but I don’t imagine we’d be picking a cluster limit higher than that (just a guess, maybe somebody else has more insight). Note that this max is also the number of wtxids we may potentially need to sort + hash together to send a “notfound” response.

    ajtowns commented at 1:42 pm on June 15, 2023:
    I’d be pretty cautious about trying to request large chunks via getpkgtxns – if any tx has been bumped, you’ll just get a notfound with no indication of what part might be okay to retry.

    glozow commented at 4:46 pm on June 15, 2023:
    Inclined to keep this at 100. Agree it doesn’t make much sense to request a ton of transactions anyway, given the response is all or nothing.

    instagibbs commented at 4:07 pm on June 20, 2023:
    sounds like clusters are unlikely to go this high anyways, so let’s keep it at 100 for this and the other reasons above

  121. in bip-0331.mediawiki:274 in d07535cc3d outdated
    269+|txns||List of wtxids||txns_length * 32|| The wtxids of each transaction in the package.
    270+|}
    271+
    272+# The "ancpkginfo" message has the structure defined above, with pchCommand == "ancpkginfo".
    273+
    274+# The "txns" field should contain a list of wtxids which constitute the ancestor package of the last wtxid. The sender should - but is not required to - sort these wtxids in topological order. The topological sort can be achieved by sorting the transactions by mempool acceptance order (if parents are always accepted before children). Nodes should not disconnect or punish a peer who provides a list not sorted in topological order.<ref>'''Why not include feerate information to help the receiver decide whether these transactions are worth downloading?'''
    


    ariard commented at 0:00 am on June 16, 2023:
    Side-note, I think the feerate cannot be trusted under BIP118 ANYPREVOUTANYSCRIPT, where the spent amount are not committed in the digest and transaction child A can be attached to parent X or parent Z of different amounts (of course there is the issue to re-bind the ANYPREVOUTANYSCRIPT on a parent by scriptpubkey only…). So I think this avoid to introduce a dependency in light of potential future consensus changes.

    glozow commented at 10:53 am on July 4, 2023:
    Useful extra context, though I didn’t add any of this to the text.
  122. in bip-0331.mediawiki:271 in d07535cc3d outdated
    264+{|
    265+|  Field Name  ||  Type  ||  Size  ||   Purpose
    266+|-
    267+|txns_length||CompactSize||1 or 3 bytes|| The number of transactions provided.
    268+|-
    269+|txns||List of wtxids||txns_length * 32|| The wtxids of each transaction in the package.
    


    ariard commented at 0:22 am on June 16, 2023:

    There is no limit on the number of announced ancestors ?

    From my understanding, there is a bandwidth asymmetry in this design. A receiver can request an unbounded number of parents from the sender by sending a single GETDATA(wtixd), where the child transaction is deliberately spending a large number of in-mempool unconfirmed ancestors. As the ancestors are not linked without the child, I think the number can be beyond in-mempool ancestors limits.

    Those ancestors might have been previously announced by an inbound connection controlled by the adversary for a bandwidth cost of O(n), however the target node might have to spend a bandwidth cost of O(n*m), where m is the number of transaction-relay connections controlled by the adversary, if I’m correct ?


    glozow commented at 1:40 pm on June 19, 2023:

    A receiver can request an unbounded number of parents from the sender by sending a single GETDATA(wtixd), where the child transaction is deliberately spending a large number of in-mempool unconfirmed ancestors.

    This doesn’t make sense to me. The max size of an ancpkginfo is the maximum number of ancestors a tx could have. For default bitcoin core, that’s 25.

    I don’t think we should add a limit like 25 to the protocol spec because it’s a policy value and configurable. In the implementation, sender side is bounded by what your ancestor package limits are. On the receiver side, if you’re reading an ancpkginfo and see it has >25 items in the list (i.e. higher than your ancestor limits), you can just drop it on the floor. This is more helpful than getting a notfound (e.g. if you tell them ahead of time that you only want lists up to 10 wtxids long).

  123. in bip-0331.mediawiki:302 in d07535cc3d outdated
    297+
    298+# Upon receipt of a "ancpkginfo" message, the node may use it to request the transactions it does not already have (e.g. using "getpkgtxns" or "tx").
    299+
    300+# Upon receipt of a malformed "ancpkginfo" message, the sender may be disconnected. An "ancpkginfo" message is malformed if it contains duplicate wtxids or conflicting transactions (spending the same inputs). The receiver may learn that a package info was malformed after downloading the transactions.
    301+
    302+# A node MUST NOT send a "ancpkginfo" message that has not been requested by the recipient. Upon receipt of an unsolicited "ancpkginfo", a node may disconnect the sender.
    


    ariard commented at 0:25 am on June 16, 2023:
    If the wtxid of the requested transaction is missing in the list, and therefore the “ancpkginfo” is uncomplete, should the peer be disconnected (as there is no way to dissociate it’s a unsolicited package) ?

    glozow commented at 1:40 pm on June 19, 2023:
    Yes, that is an unsolicited ancpkginfo.

    glozow commented at 10:51 am on July 4, 2023:
    Added an extra sentence about the fact that the last transaction is required to be the transaction we’re talking about.
  124. in bip-0331.mediawiki:330 in d07535cc3d outdated
    325+|txns||List of wtxids||txns_length * 32|| The wtxids of each transaction in the package.
    326+|}
    327+
    328+# The "getpkgtxns" message has the structure defined above, with pchCommand == "getpkgtxns".
    329+
    330+# A "getpkgtxns" message should be used to request some list of transactions specified by witness transaction id. It indicates that the node wants to receive either all the specified transactions or none of them. This message is intended to allow nodes to avoid downloading and storing transactions that cannot be validated without each other. The list of transactions does not need to correspond to a previously-received pkginfo message.
    


    ariard commented at 0:28 am on June 16, 2023:
    “previsously-received “ancpkginfo” message” for more clarity i think

    glozow commented at 10:51 am on July 4, 2023:
    Done
  125. in bip-0331.mediawiki:332 in d07535cc3d outdated
    327+
    328+# The "getpkgtxns" message has the structure defined above, with pchCommand == "getpkgtxns".
    329+
    330+# A "getpkgtxns" message should be used to request some list of transactions specified by witness transaction id. It indicates that the node wants to receive either all the specified transactions or none of them. This message is intended to allow nodes to avoid downloading and storing transactions that cannot be validated without each other. The list of transactions does not need to correspond to a previously-received pkginfo message.
    331+
    332+# Upon receipt of a "getpkgtxns" message, a node should respond with either a "pkgtxns" containing all of the requested transactions in the same order specified in the "getpkgtxns" request or one "notfound" message of type MSG_PKGTXNS and combined hash of all of the wtxids in the "getpkgtxns" request (only one "notfound" message and nothing else), indicating one or more of the transactions is unavailable.
    


    ariard commented at 0:35 am on June 16, 2023:

    Here I think you’re restraining some partial package download, where none of your peers have all the ancestors for a known child transaction however the unions of all the available transactions announced by our peers give you such complete package.

    I think that’s okay and the behavior of “notfound(MSG_PKGTXND)” can be changed by bumping the version bits.


    glozow commented at 1:45 pm on June 19, 2023:

    where none of your peers have all the ancestors for a known child transaction however the unions of all the available transactions announced by our peers give you such complete package.

    We should be able to hope that at least one of the peers was able to keep the list of wtxids they told us about in their ancpkginfo message. If they evicted some ancestors, they also evicted the child we’re trying to resolve with them.

    Also, this definition of how the “getpkgtxns” message works does not preclude having logic for retrieving packages from multiple peers in your implementation.

  126. in bip-0331.mediawiki:352 in d07535cc3d outdated
    347+
    348+# The "pkgtxns" message has the structure defined above, with pchCommand == "pkgtxns".
    349+
    350+# A "pkgtxns" message should contain the transaction data requested using "getpkgtxns".
    351+
    352+# A "pkgtxns" message should only be sent to a peer that requested the package using "getpkgtxns". If a node receives an unsolicited package, it may choose to validate the transactions or not, and the sender may be disconnected.
    


    ariard commented at 0:39 am on June 16, 2023:
    I think here the receiver behavior can be more liberal and unsolicited “pkgtxns” shouldn’t be treated as a disconnection. A sender implementing some “prediction algorithm”, in the sense knowing the parent is going to be rejected on its own (e.g due to feerate under feefilter) might optimize by sending a full-package, without opt-in from your peer. This behavior could be legit as it’s RTT/bandwidth consumed on its side.

    glozow commented at 1:50 pm on June 19, 2023:
    I can see a “prediction algorithm” for proactive, sender-initiated announcements being appropriate and more effective. But I don’t see why it would make sense to do this for full transaction data. Can you provide a specific reason why this would be needed?
  127. in bip-0331.mediawiki:362 in d07535cc3d outdated
    357+
    358+# A new inv type (MSG_PKGTXNS == 0x6) is added, for use only in "notfound" messages pertaining to package transactions.
    359+
    360+# As a notfound type, it indicates that the sender is unable to send all the transactions requested in a prior "getpkgtxns" message. The hash used is equal to the combined hash (defined above) of the wtxids in the getpkgtxns request.
    361+
    362+# This inv type should only be used in "notfound" messages, i.e. "inv(MSG_PKGTXNS)" and "getdata(MSG_PKGTXNS)" must never be sent.
    


    ariard commented at 0:42 am on June 16, 2023:
    Can precise peer will be disconnected.

    glozow commented at 10:51 am on July 4, 2023:
    Added
  128. ariard commented at 0:46 am on June 16, 2023: member
    See the note on maybe asymmetry around “ancpkginfo"
  129. glozow force-pushed on Jul 4, 2023
  130. glozow commented at 11:02 am on July 4, 2023: member
    I think I’ve addressed all comments now.
  131. in bip-0331.mediawiki:334 in c2da2cffd6 outdated
    329+
    330+# A "getpkgtxns" message should be used to request some list of transactions specified by witness transaction id. It indicates that the node wants to receive either all the specified transactions or none of them. This message is intended to allow nodes to avoid downloading and storing transactions that cannot be validated without each other. The list of transactions does not need to correspond to a previously-received ancpkginfo message.
    331+
    332+# Upon receipt of a "getpkgtxns" message, a node should respond with either a "pkgtxns" containing all of the requested transactions in the same order specified in the "getpkgtxns" request or one "notfound" message of type MSG_PKGTXNS and combined hash of all of the wtxids in the "getpkgtxns" request (only one "notfound" message and nothing else), indicating one or more of the transactions is unavailable.
    333+
    334+# A "getpkgtxns" message must contain at most 100 wtxids. Upon receipt of a "getpkgtxns" message with more than 100 wtxids, a node may ignore the message (to avoid calculating the combined hash) and disconnect the sender.
    


    ariard commented at 3:39 am on July 7, 2023:
    Can precise GETPKGTXNS must not be empty, enforced as a disconnection condition (Misbehaving(*peer, 100, …)) on GETPKGTXNS reception (if i’m on the right commit).

    glozow commented at 9:48 am on July 24, 2023:
    I don’t think it’s implemented this way? if (num_txns == 0) return; means it’s ignored.

    ariard commented at 2:41 am on August 2, 2023:
    Yeah would have to check back the code here.
  132. in bip-0331.mediawiki:35 in c2da2cffd6 outdated
    30+mempool with similar contents, including faster compact block relay and more accurate fee
    31+estimation. Additionally, users may take advantage of mempool and miner policy to bump the priority
    32+of their transactions by attaching high-fee descendants (Child Pays for Parent or CPFP).
    33+
    34+Only individually considering transactions for submission to the mempool creates a limitation in
    35+the node's ability to determine which transactions have the highest feerates, since it cannot take
    


    instagibbs commented at 7:55 pm on August 2, 2023:
    0the node's ability to determine which transactions to include in the mempool, since it cannot take
    

    glozow commented at 10:59 am on August 3, 2023:
    done
  133. in bip-0331.mediawiki:41 in c2da2cffd6 outdated
    36+into account descendants until all the transactions are in the mempool. Similarly, it cannot use a
    37+transaction's descendants when considering which of two conflicting transactions to keep (Replace by
    38+Fee or RBF).
    39+
    40+When a user's transaction does not meet a mempool's minimum feerate and they cannot create a
    41+replacement transaction directly, their transaction will simply be rejected by this mempool. They
    


    instagibbs commented at 7:56 pm on August 2, 2023:
    0replacement transaction directly, their transaction will simply be rejected by this mempool or evicted if already included. They
    

    glozow commented at 10:59 am on August 3, 2023:
    added
  134. in bip-0331.mediawiki:46 in c2da2cffd6 outdated
    41+replacement transaction directly, their transaction will simply be rejected by this mempool. They
    42+also cannot attach a descendant to pay for replacing a conflicting transaction; it would be rejected
    43+for spending inputs that do not exist.
    44+
    45+This limitation harms users' ability to fee-bump their transactions. Further, it presents a security
    46+issue in contracting protocols which rely on presigned, time-sensitive transactions<ref>'''Examples of time-sensitive pre-signed transactions in L2 protocols.'''
    


    instagibbs commented at 7:59 pm on August 2, 2023:
    0This limitation harms users' ability to fee-bump their transactions. Further, it presents security and complexity
    1issues in contracting protocols which rely on presigned, time-sensitive transactions<ref>'''Examples of time-sensitive pre-signed transactions in L2 protocols.'''
    

    inability to consider package feerates also creates complexity in these protocols, I think important to note


    glozow commented at 10:58 am on August 3, 2023:
    Added
  135. in bip-0331.mediawiki:72 in c2da2cffd6 outdated
    67+overestimating fees may sidestep this issue (but only while mempool traffic is low and
    68+predictable), but this solution is not guaranteed to work and wastes users' money. For some attacks,
    69+the available defenses require nodes to have a bird's-eye view of Bitcoin nodes' mempools, which is
    70+an unreasonable security requirement.
    71+
    72+The best solution is to enable nodes to consider packages of transactions as a unit, e.g. one or
    


    instagibbs commented at 8:01 pm on August 2, 2023:
    0Part of the solution is to enable nodes to consider packages of transactions as a unit, e.g. one or
    

    not everything is fixed yet :)


    glozow commented at 10:56 am on August 3, 2023:
    heh. changed
  136. in bip-0331.mediawiki:106 in c2da2cffd6 outdated
    101+(prevouts specify txid, not wtxid). These parents may end up being orphans as well, if they also
    102+spend unconfirmed inputs that the node is unaware of. This method of handling orphans is problematic
    103+for two reasons: it requires nodes to allocate memory for unvalidated data received on the p2p
    104+network and it relies on txid-based relay between two wtxid-relay peers.
    105+
    106+This proposal makes orphan-fetching more efficient and no longer require txid-based relay.
    


    instagibbs commented at 8:06 pm on August 2, 2023:
    0This proposal makes orphan-resolution more efficient and no longer require txid-based relay.
    

    we don’t really “fetch orphans”, at least intentionally


    glozow commented at 10:56 am on August 3, 2023:
    True, changed
  137. in bip-0331.mediawiki:312 in c2da2cffd6 outdated
    307+
    308+# A new inv type (MSG_ANCPKGINFO == 0x7) is added, for use only in getdata requests pertaining to ancestor packages.
    309+
    310+# As a getdata request type, it indicates that the sender wants an "ancpkginfo" containing all of the unconfirmed ancestors of a transaction, referenced by wtxid.
    311+
    312+# Upon receipt of a "getdata(MSG_ANCPKGINFO)" request, the node should respond with an "ancpkginfo" message corresponding to the transaction's unconfirmed ancestor package, or with "notfound".  The wtxid of the requested transaction must be the last item in the list, as the last item is used to determine which transaction the "ancpkginfo" pertains to.
    


    instagibbs commented at 8:52 pm on August 2, 2023:
    0# Upon receipt of a "getdata(MSG_ANCPKGINFO)" request, the node should respond with an "ancpkginfo" message corresponding to the transaction's unconfirmed ancestor package, or with "notfound".  The wtxid of the requested transaction must be the last item in the "ancpkginfo" response list, as the last item is used to determine which transaction the "ancpkginfo" pertains to.
    

    glozow commented at 10:51 am on August 3, 2023:
    Added
  138. in bip-0331.mediawiki:360 in c2da2cffd6 outdated
    355+
    356+====MSG_PKGTXNS====
    357+
    358+# A new inv type (MSG_PKGTXNS == 0x6) is added, for use only in "notfound" messages pertaining to package transactions.
    359+
    360+# As a notfound type, it indicates that the sender is unable to send all the transactions requested in a prior "getpkgtxns" message. The hash used is equal to the combined hash (defined above) of the wtxids in the getpkgtxns request.
    


    instagibbs commented at 9:03 pm on August 2, 2023:
    0# As a notfound type, it indicates that the sender is unable to send all the transactions requested in a prior "getpkgtxns" message. The hash used is equal to the combined hash of the wtxids in the getpkgtxns request.
    

    glozow commented at 10:50 am on August 3, 2023:
    Done
  139. in bip-0331.mediawiki:275 in c2da2cffd6 outdated
    270+|}
    271+
    272+# The "ancpkginfo" message has the structure defined above, with pchCommand == "ancpkginfo".
    273+
    274+# The "txns" field should contain a list of wtxids which constitute the ancestor package of the last wtxid. For the receiver's convenience, the sender should - but is not required to - sort the wtxids in topological order. The topological sort can be achieved by sorting the transactions by mempool acceptance order (if parents are always accepted before children). Apart from the last wtxid which is used to learn which transaction the message corresponds to, there is no enforced ordering. Nodes should not disconnect or punish a peer who provides a list not sorted in topological order.<ref>'''Why not include feerate information to help the receiver decide whether these transactions are worth downloading?'''
    275+Peers are not trusted to provide correct information, so a reported low feerate should not stop a
    


    instagibbs commented at 9:13 pm on August 2, 2023:
    I’m not sure this first half is very persuasive, a node can lie and waste our bandwidth regardless. I think the latter half of the answer is the most sensible.

    glozow commented at 10:53 am on August 3, 2023:
    Deleted the first sentence.
  140. in bip-0331.mediawiki:236 in c2da2cffd6 outdated
    230+===Combined Hash===
    231+
    232+A "combined hash" serves as a unique "package id" for some list of transactions and helps provide a
    233+meaningful but short "notfound" response to "getpkgtxns."
    234+
    235+The combined hash of a package of transactions is equal to the double-sha256 hash of each transaction's
    


    instagibbs commented at 9:18 pm on August 2, 2023:
    we could just make this single-sha2…

    glozow commented at 9:40 am on August 7, 2023:
    Changed
  141. instagibbs approved
  142. instagibbs commented at 9:21 pm on August 2, 2023: member

    ACK

    nits only

  143. glozow force-pushed on Aug 3, 2023
  144. glozow commented at 11:07 am on August 3, 2023: member
    Addressed the text-related comments
  145. glozow force-pushed on Aug 7, 2023
  146. glozow commented at 9:41 am on August 7, 2023: member
    Changed combined hash to be single instead of double sha256
  147. instagibbs commented at 2:51 pm on August 8, 2023: member

    re-ACK https://github.com/bitcoin/bips/pull/1382/commits/02ec218c7857ef60914e9a3d383b68caf987f70b

    suggested textual additions and change to single-sha2 for combined hash

  148. glozow commented at 11:44 am on January 17, 2024: member
    @luke-jr @kallewoof is anything else needed here?
  149. Kampe approved
  150. glozow commented at 10:10 am on April 11, 2024: member
    @luke-jr pinging again, can we get this merged?
  151. in bip-0331.mediawiki:353 in 02ec218c78 outdated
    348+
    349+# A "pkgtxns" message should contain the transaction data requested using "getpkgtxns".
    350+
    351+# A "pkgtxns" message should only be sent to a peer that requested the package using "getpkgtxns". If a node receives an unsolicited package, it may choose to validate the transactions or not, and the sender may be disconnected.
    352+
    353+# This message must only be used if both peers both set <code>PKG_RELAY_PKGTXNS</code> in their "sendpackages" message. If a "getpkgtxns" message is received from a peer with which this type of package relay was not negotiated, no response should be sent and the sender may be disconnected.
    


    sr-gi commented at 5:38 pm on April 16, 2024:
    0# This message must only be used if both peers both set <code>PKG_RELAY_PKGTXNS</code> in their "sendpackages" message. If a "sendpackages" message is received from a peer with which this type of package relay was not negotiated, no response should be sent and the sender may be disconnected.
    

    glozow commented at 10:50 am on April 17, 2024:
    I think “getpkgtxns” is correct actually?

    sr-gi commented at 11:11 am on April 17, 2024:

    Is it? This is part of the “pkgtxns” section, so it would make sense to be referring to the message that is being talked about. That is, at least, how this note is handled in every other message section that includes it throughout the doc.

    Also notice that, as is, this is the exact same text as in point 5 of “getpkgtxns”


    glozow commented at 4:00 pm on April 24, 2024:
    Ah we were both wrong. I’ve changed it to “pkgtxns”, thanks
  152. in bip-0331.mediawiki:358 in 02ec218c78 outdated
    352+
    353+# This message must only be used if both peers both set <code>PKG_RELAY_PKGTXNS</code> in their "sendpackages" message. If a "getpkgtxns" message is received from a peer with which this type of package relay was not negotiated, no response should be sent and the sender may be disconnected.
    354+
    355+====MSG_PKGTXNS====
    356+
    357+# A new inv type (MSG_PKGTXNS == 0x6) is added, for use only in "notfound" messages pertaining to package transactions.
    


    sr-gi commented at 5:45 pm on April 16, 2024:

    nit: here, “notfound” is quoted, but it is not in the next line. In contrast, when referring to MSG_ANCPKGINFO == 0x7, getdata is not quoted in any of the cases.

    It may be good to make this consistent


    glozow commented at 10:58 am on April 17, 2024:
    Added quotes for all notfounds. thanks!
  153. in bip-0331.mediawiki:303 in 02ec218c78 outdated
    298+
    299+# Upon receipt of a malformed "ancpkginfo" message, the sender may be disconnected. An "ancpkginfo" message is malformed if it contains duplicate wtxids or conflicting transactions (spending the same inputs). The receiver may learn that a package info was malformed after downloading the transactions.
    300+
    301+# A node MUST NOT send a "ancpkginfo" message that has not been requested by the recipient. Upon receipt of an unsolicited "ancpkginfo", a node may disconnect the sender.
    302+
    303+# This message must only be used if both peers both set <code>PKG_RELAY_ANC</code> in their "sendpackages" message. If an "ancpkginfo" message is received from a peer with which this type of package relay was not negotiated, no response should be sent and the sender may be disconnected.
    


    sr-gi commented at 5:54 pm on April 16, 2024:

    “Both peers both set” sounds like both of the peers set two distinct things: “Both peers both set this and that”, but here it is referring to both peers setting a single thing each (PKG_RELAY_ANC in this case).

    Shouldn’t this be: “Both peers set”? If so, this applies to multiple places in the doc


    glozow commented at 10:49 am on April 17, 2024:
    Oh yes, this is a typo - I have an extra ‘both’. Thanks!

    sr-gi commented at 11:07 am on April 17, 2024:
    Note that this is not the only place in the doc where “Both peers both set” appears
  154. sr-gi commented at 6:09 pm on April 16, 2024: member
    A couple of comments/nits I found when going over this
  155. glozow force-pushed on Apr 17, 2024
  156. jonatack added the label New BIP on Apr 23, 2024
  157. glozow force-pushed on Apr 24, 2024
  158. specify BIP331 Ancestor Package Relay 632f143fad
  159. glozow force-pushed on Apr 24, 2024
  160. glozow commented at 4:06 pm on April 24, 2024: member
    pinging again, if any BIP editors are available to have a look?
  161. jonatack merged this on Apr 24, 2024
  162. jonatack closed this on Apr 24, 2024

  163. jonatack commented at 6:10 pm on April 24, 2024: contributor
    ACK 632f143fadaed8c91bba12d096e62feb2a308054
  164. glozow deleted the branch on May 22, 2024

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bips. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-28 02:10 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me