-
Notifications
You must be signed in to change notification settings - Fork 5.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BIP431: Opt In Topologically Restricted Until Confirmation Transactions For More Robust Fee-bumping #1541
Conversation
a6277c0
to
4bd12d5
Compare
FWIW, concept ACK :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 4bd12d5
4bd12d5
to
af8e903
Compare
Thanks @murchandamus, took all your suggestions (4bd12d5...af8e903) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concept ACK
752a287
to
542f833
Compare
@luke-jr this has been open for a month, would you mind taking a look? |
I think this should document that 1000 vb child limit is experimental and it cannot be relied on by downstream projects. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 63e8a71
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @murchandamus! Accepted your suggestions.
As with #1524, node policy is not a standardizable subject matter in itself - every node decides its own policy. BIP 125 already defined a way for wallets to indicate they prefer RBF-like treatment of their transactions, and I don't see why the same signal can't be used for this. |
Every node deciding its own policy is not mutually exclusive with it being standardizable – each node decides which (if any) standards to follow. Policy standards make just as much sense as e.g. wallet standards or P2P protocol standards. |
BIP 125 defines a set of rules w.r.t transaction replacement. Some of those rules have edge cases that can lead to defects where transactions that should otherwise be replaced, can't be replaced. This class of replacement defects loosely falls under the umbrella of "transaction pinning". The TRUC rules resolve those issues, creating a replacement semantics that better serve off-chain protocols, and address some known pinning vectors. For many off-chain protocols, TRUC will supersede base RBF. As the semantics are distinct, it cannot use the existing sequence fields carved out by BIP 125 (the rules are also incompatible). Instead, it uses transaction v3 (no longer non standard) as a way to signal replacement under a distinct set of rules. |
BIP 125 defines a signalling mechanism, not policy (which is again outside the scope of BIPs) even if it describes a particular policy. The intent of the signal is clear, and it can be implemented in other ways such as the one that seems to be desired here. |
It clearly defines both:
It also defines a precise set of rules for the policy. If an implementation deviates from those rules, then it isn't BIP 125. From the OP, there's a clear need to define a new policy and signalling mechanism, which this document does. |
It may be poorly worded, but that isn't the point. |
(Note that BIP 125's relationship with policy was a matter of discussion back when it was submitted, and it was nearly rejected - the only reason it was accepted in the end, was as a signalling BIP. And even if you want to insist it defines policy, mistakes made in the past would not be a reason to repeat them in the future, and thus would actually work against accepting this proposal) |
a9a9aad
to
32e4e44
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 32e4e44
With the tiny non-blocking nitpick that a few places in the doc use markdown _
for italics (I think) where they should use ''
for mediawiki italics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me and ready to go except for a couple nits (which I don’t think would need to hold up a merge) and a formatting issue:
-
The JEDEC standard specifies that uppercase K (kilo) refers to 1024¹ (and M (mega), G (giga) as 1024² and 1024³ respectively). However, since the numbers in your example are referring to a factor of 1000¹, they should be using lowercase k, the symbol of the metric prefix kilo referring to a factor of 1000¹. Therefore, it would be preferable if the units were updated to read "kvB" instead of "KvB", and "ksats" instead of "Ksats".
-
AFAIU, technical writing standards generally recommend a space between the numerical value and unit symbol.
E.g. using a space is recommended by the SI Standard,
via https://www.nist.gov/pml/special-publication-330/sp-330-section-5
by NIST,
via https://physics.nist.gov/cuu/Units/checklist.html
as well as IEEE:
via https://academia.stackexchange.com/q/54885/8305
My personal preference is to separate the value and symbol with a Narrow No-Break Space (U+202F) as that provides the expected visual offset, renders as a smaller gap than a full length space, but still prevents unfortunate line breaks. My suggested changes already make use of NNBSPs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, I overlooked a couple more values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @carlaKC @murchandamus! I've fixed the italics, spacing, and "k".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-ACK 04d3a06 🤩
(I think this is ready, please lmk if there's anything left to do on my end) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 04d3a06
Re-checked the RBF rules vs description headers.
<br />At least 1 descendant is required to allow CPFP of the presigned transaction. Without package RBF, multiple anchor outputs would be required to allow each counterparty to fee-bump any presigned transaction. With package RBF, since the presigned transactions can replace each other, 1 anchor output is sufficient. | ||
</ref> | ||
|
||
4. A TRUC transaction cannot have a sigop-adjusted virtual size larger than 10,000 vB. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated for bitcoin/bitcoin#29873 👍
|
||
===RBF pinning through absolute fees=== | ||
|
||
Imagine that counterparties Alice and Mallory have transactions (or packages) A and B, respectively, which conflict with each other. Alice broadcasts A and Mallory broadcasts B. RBF rules require the replacement transaction pay a higher absolute fee than the aggregate fees paid by all original transactions ([https://github.com/bitcoin/bitcoin/blob/master/doc/policy/mempool-replacements.md#current-replace-by-fee-policy "Rule 3"]). This means Mallory may increase the fees required to replace B beyond what Alice was planning to pay for A's fees. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With bitcoin/bitcoin#29496 on the table, note to possibly consider updating/appending to https://github.com/bitcoin/bitcoin/blob/master/doc/policy/mempool-replacements.md#current-replace-by-fee-policy with TRUC at some point (and making sure the multiple links to it in this BIP remain valid).
This BIP is missing an explanation of how it is expected to work with future nVersion upgrades. Eg if we add a third consensus critical transaction version, how should that interact with TRUC? Do we just add another version? Obviously this is a serious drawback of mixing up mempool behavior and consensus code. |
It seems to me that BIP 431 only applies to version 3 transactions. So, I’m not sure I understand your concern. If someone e.g. proposed giving meaning to version 4 in say a BIP-v4, it could propose how transactions with version 3 should interact with transactions of version 4 and then whoever implements BIP-v4 would follow those recommendations, while those that don’t implement it, would not. |
TRUC behavior is not desirable for all transactions; it is not a clear upgrade.
Thus, the obvious question is how do we expect a future soft fork version upgrade to deal with this?
For example, consider what would have happened had we tried to introduce TRUC prior to v2 transactions.
This can of course be avoided by not using a version number to signal TRUC behavior. I do not see a reason why we actually need to do that, given how narrow the actual usage of TRUC is likely to be. Triggering it simply on zero fee transactions should be fine.
…On July 8, 2024 4:58:22 PM GMT+02:00, murchandamus ***@***.***> wrote:
I’m not sure I understand your concern. If someone e.g. proposed giving meaning to version 4 in say a _BIP-v4_, it could propose how transactions with version 3 should interact with transactions of version 4 and then whoever implements BIP-v4 would follow those recommendations, while those that don’t implement it, would not.
|
I'd imagine that the author of a proposal that defines a new consensus rule under version=3 would consider what usage exists on the network, particularly if the rules described in this BIP are adopted by the network when that proposal is created. They would probably also want to explain why gating it on version=3 (or any version-related marker at all) is necessary. Of course, coordinating between all the many proposals may not be easy. Are you perhaps making an argument for why we should document proposed uses of version=3 in a central place to avoid creating and deploying conflicting uses? |
It’s not clear to me where you see a problem. This BIP applies to transactions with version 3, not transactions signaling a version of 3 and higher. |
So an obvious example of where TRUC's use of v3 transactions would cause a problem is if we had a situation similar to v2 transactions, where an upgrade happened that had wide applicability. Since TRUC is so limited, we'd need to have two different versions of the consensus upgrade for TRUC and non-TRUC transactions. Obviously, that would be a mess. This gets even worse when you consider that we all know that TRUC only mitigates pinning for a very narrow use-case, and people are already discussing extensions to TRUC with new version bits. Meanwhile, it would be quite easy for TRUC to achieve it's actual main goal of enabling empheral anchor outputs as quick fix by defining TRUC behavior as being enabled for zero-fee transactions. Choosing V3 didn't even do "TRUC behavior" cleanly as a bit: bin(3) = 0b11, two bits set. |
Let’s say we have a hypothetical soft-fork in the future to clean all the pinning and transaction-relay jamming broken mess, nVersion is a 32-bit field, so we can still split it in two 16-bit halves: one set for truc-like policy behavior and one set for consensus validation. Without an example, it’s quite theoretical as there are always other signaling options: the taproot annex, the input nsequence, the nlocktime field... |
I do think that bifurcating things along consensus vs policy is likely a good idea. It would fully orthogonalize the problems. While consensus-invalid implies policy-invalid, I believe these are otherwise independent concerns and we should treat them as such in their signaling/encoding schemes. |
My understanding was that the version field is the place for policy/application signals because it's not expected to be reinterpreted in the future for consensus things. That is why I chose it. |
This is a BIP for Topologically Restricted Until Confirmation (TRUC) Transactions. It's also called "v3 transaction policy" since the marker is
nVersion=3
.A specification is useful for coordination between node impls that want to implement the same policy and applications that want to use it. For those that are not interested in the details of v3 policy, this also serves as a writeup of the specific pinning problems we aim to address. There has been discussion of using this in other protocol design and multiple requests for its documentation to exist in the BIPs repository, so I'm opening a PR here.
Implementation:
Example usage and things built on top:
Discussion and history: