-
Notifications
You must be signed in to change notification settings - Fork 39.1k
policy: nVersion=3 and Package RBF #25038
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
Changes from all commits
3bca679
18406ec
1b2cd22
c74c826
3ad3ea5
b828ea0
f82c9fd
d6bd8dc
75a1efd
25f528a
0933d4f
d629f29
4785582
678f56d
0bc6375
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| # Transactions with nVersion 3 | ||
|
|
||
| A transaction with its `nVersion` field set to 3 ("V3 transactions") is allowed in mempool and | ||
| transaction relay, with additional policies described below. | ||
|
|
||
| The goal with V3 is to create a policy that is DoS-resistant and makes fee-bumping more robust by | ||
| avoiding specific RBF pinning attacks. Contracting protocols in which transactions are signed by | ||
| untrusted counterparties before broadcast time, e.g. the Lightning Network (LN), may benefit from | ||
| opting in to these policies. | ||
|
|
||
| ## V3 Rationale: RBF Pinning Attacks | ||
|
|
||
| Since contracting transactions are shared between multiple parties and mempool congestion is | ||
| difficult to predict, [RBF policy](./mempool-replacements.md) restrictions may accidentally allow a | ||
| malicious party to "pin" a transaction, making it impossible or difficult to replace. | ||
|
|
||
| ### "Rule 3" Pinning | ||
|
|
||
| Imagine that counterparties Alice (honest) and Mallory (malicious) have conflicting transactions A | ||
| and B, respectively. RBF rules require the replacement transaction pay a higher absolute fee than | ||
| the aggregate fees paid by all original transactions. This means Mallory may increase the fees | ||
| required to replace B by: | ||
|
|
||
| 1. Adding transaction(s) that descend from B and pay a feerate too low to fee-bump B through CPFP. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I don't get this sentence. Nobody wants to fee-bump B, no? Alice would want to replace B, what does it have to do with fee-bumping?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mallory's goal is to pin the transaction in the mempool. That is, increase the fees required to replace B, but not make it confirm. Mallory can of course put 1BTC of fees in the descendant, thus making it extremely expensive to replace B, but then she's also CPFP'd B and it's probably going to be mined very soon. |
||
| For example, assuming the default descendant size limit is 101KvB and B is 1000vB paying a | ||
| feerate of 2sat/vB, adding a 100KvB, 2sat/vB child increases the cost to replace B by 200Ksat. | ||
|
|
||
| 2. Adding a high-fee descendant of B that also spends from a large, low-feerate mempool transaction, | ||
| C. The child may pay a very large fee but not actually be fee-bumping B if its overall ancestor | ||
| feerate is still lower than B's individual feerate. For example, assuming the default ancestor size | ||
| limit is 101KvB, B is 1000vB paying 2sat/vB, and C is 99KvB paying 1sat/vB, adding a 1000vB child of | ||
| B and C increases the cost to replace B by 101Ksat. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we would like to add more rational to the design of the rules, we could explain why a scorched earth approach would solve the Lightning case really imperfectly (discussed recently here: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-November/021176.html). Indeed, Lightning implementation could just blindly replace-by-fee the pinning transactions, paying up to the channel value. This wouldn't work at all against an attacker pinning with one single malicious transaction multiple channel from different parties where the malicious transaction fee is above the value of each channel. E.g channel A 10000 sats, channel B 25000 sats and channel C 8000 sats. If the malicious pinning transactions fee equals 26000 sats, none of the honest party will rationally fee-bump more than their own channel value. Feerate-only replacement rules scales up to the maximum package size and worst historical mempool reduce consideraly the fee-bumping reserves requirement for a LN node, which is a notable advantage I think. |
||
|
|
||
| ### "Rule 5" Pinning | ||
|
|
||
| RBF rules requires that no replacement trigger the removal of more than 100 transactions. This | ||
| number includes the descendants of the conflicted mempool transactions. Mallory can make it more | ||
| difficult to replace transactions by attaching lots of descendants to them. For example, if Alice | ||
| wants to replace 4 transactions and each one has 25 or more descendants, the replacement will be | ||
| rejected regardless of its fees. | ||
|
|
||
| ## Version 3 Rules | ||
|
|
||
| All existing standardness rules and policies apply to V3. The following set of additional | ||
| rules apply to V3 transactions: | ||
|
|
||
| 1. A v3 transaction signals replaceability, even if it does not signal BIP125 replaceability. Other | ||
| conditions apply, see [RBF rules](./mempool-replacements.md) and [Package RBF | ||
| rules][./packages.md#Package-Replace-By-Fee]. | ||
|
|
||
| 2. Any descendant of an unconfirmed V3 transaction must also be V3. | ||
|
glozow marked this conversation as resolved.
Outdated
|
||
|
|
||
| *Rationale*: Combined with Rule 1, this gives us the property of "inherited signaling" when | ||
| descendants of unconfirmed transactions are created. Additionally, checking whether a transaction | ||
| signals replaceability this way does not require mempool traversal, and does not change based on | ||
| what transactions are mined. | ||
|
|
||
| *Note*: A V3 transaction can spend outputs from *confirmed* non-V3 transactions. | ||
|
|
||
| *Note*: This rule is enforced during reorgs. Transactions violating this rule are removed from the | ||
| mempool. | ||
|
|
||
| 3. A V3 transaction's unconfirmed ancestors must all be V3. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you ever discuss what will happen in the 2-block-reorg case? A chain of txs could end up being not-re-added to the mempool (due to rules 2/3), and then something becomes insecure. I think I should bring a concrete example here.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ahhh too bad, the next comment asks the same question. Certainly worth documenting though :)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think a concrete example would help, yeah. For example, when you say "something becomes insecure" it's not really clear to me what you mean - a v3 transaction gets censored? A mempool has a chain of more than 2 v3 transactions? |
||
|
|
||
| *Rationale*: Ensure the ancestor feerate rule does not underestimate a to-be-replaced V3 mempool | ||
| transaction's incentive compatibility. Imagine the original transaction, A, has a child B and | ||
| co-parent C (i.e. B spends from A and C). C also has another child, D. B is one of the original | ||
| transactions and thus its ancestor feerate must be lower than the package's. However, this may be an | ||
| underestimation because D can bump C without B's help. This is resolved if V3 transactions can only | ||
| have V3 ancestors, as then C cannot have another child. | ||
|
|
||
| *Note*: This rule is enforced during reorgs. Transactions violating this rule are removed from the | ||
| mempool. | ||
|
|
||
| 4. A V3 transaction cannot have more than 1 unconfirmed descendant. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question about this and sorry if it's already been addressed: Does this require extra logic for reorg handling? I'm thinking of a chain of v3 TXs: tx1 (confirmed in block 100) -> if block 101 is disconnected, that potentially puts tx2 back in the mempool meaning tx3 is now invalid and should be evicted.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sweet 👍 Couldn't hurt to mention that in a bullet point here I suppose. |
||
|
|
||
| Also, [CPFP Carve Out](./mempool-limits.md#CPFP-Carve-Out) does not apply to V3 transactions. | ||
|
|
||
| *Rationale*: (upper bound) the larger the descendant limit, the more transactions may need to be | ||
| replaced. This is a problematic pinning attack, i.e., a malicious counterparty prevents the | ||
| transaction from being replaced by adding many descendant transactions that aren't fee-bumping. | ||
| See example #1 in ["Rule 3" Pining](#Rule-3-Pinning) section above. | ||
|
|
||
| *Rationale*: (lower bound) at least 1 descendant is required to allow CPFP of the presigned | ||
| transaction. The contract protocol can create presigned transactions paying 0 fees and 1 output for | ||
| attaching a CPFP at broadcast time ("anchor output"). 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. | ||
|
|
||
| 5. A V3 transaction that has an unconfirmed V3 ancestor cannot be larger than 4000Wu. | ||
|
|
||
| *Rationale*: (upper bound) the larger the descendant size limit, the more vbytes may need to be | ||
|
glozow marked this conversation as resolved.
Outdated
|
||
| replaced. With default limits, if the child is e.g. 100,000vB, that might be an additional | ||
| 100,000sats (at 1sat/vbyte) or more, depending on the feerate. Restricting all children to 4000Wu | ||
| (at most 1000vB) bounds the potential fees by at least 1/100. | ||
|
|
||
| *Rationale*: (lower bound) the smaller this limit, the fewer UTXOs a child may use to fund this | ||
| fee-bump. For example, only allowing the V3 child to have 2 inputs would require L2 protocols to | ||
| manage a wallet with high-value UTXOs and make batched fee-bumping impossible. However, as the | ||
| fee-bumping child only needs to fund fees (as opposed to payments), just a few UTXOs should suffice. | ||
|
|
||
| *Rationale*: With a limit of 4000 weight units, depending on the output types, the child can have | ||
| 6-15 UTXOs, which should be enough to fund a fee-bump without requiring a carefully-managed UTXO | ||
| pool. With this descendant limit, the cost to replace a V3 transaction has much lower variance. | ||
|
|
||
| *Rationale*: This makes the rule very easily "tacked on" to existing logic for policy and wallets. | ||
| A transaction may be up to 100KvB on its own (`MAX_STANDARD_TX_WEIGHT`) and 101KvB with descendants | ||
| (`DEFAULT_DESCENDANT_SIZE_LIMIT_KVB`). If an existing V3 transaction in the mempool is 100KvB, its | ||
| descendant cannot be more than 1000vB, even if the policy is 10KvB. | ||
|
|
||
| 6. A V3 transaction cannot have more than 1 unconfirmed ancestor. | ||
|
|
||
| *Rationale*: Prevent the child of an unconfirmed V3 transaction from "bringing in" more unconfirmed | ||
| ancestors. See example #2 in ["Rule 3" Pining](#Rule-3-Pinning) section above. | ||
|
|
||
| 7. An individual V3 transaction is permitted to be below the mempool min relay feerate, assuming it | ||
| is considered within a [package](./packages.md#Package-Feerate) that meets all feerate requirements. | ||
|
|
||
| *Rationale*: This allows for contracting protocols that create presigned transactions | ||
| with 0 fees and bump them at broadcast time. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -181,3 +181,49 @@ std::optional<std::string> PaysForRBF(CAmount original_fees, | |
| } | ||
| return std::nullopt; | ||
| } | ||
|
|
||
| std::optional<std::string> CheckMinerScores(CAmount replacement_fees, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. some related discussion on estimating mining score of the replacement: #26451 (comment)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This incentive deficiency linked above should be a non-issue for CheckMinerScores since V3 package RBF is a restricted topology which disallows an multiple ancestors entirely. The two cases of "adding a new unconfirmed input" during RBF with this PR:
The asymmetry above is annoying and a mild pinning vector. As of this PR, wallet authors can avoid this pinning scenario entirely by not moving fee utxos between different unconfirmed parents, but it would be nice if asymmetry was avoided to make wallet reasoning simpler.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do I understand correctly that you are pointing out that package RBF and single tx RBF have different rules? And suggesting we try to make them more symmetrical by applying something similar in the single tx rules?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally yes. Doesn't have to be done now, it just reduces the utility of new unconfirmed inputs to randomly working sometimes, basically. |
||
| int64_t replacement_vsize, | ||
| const CTxMemPool::setEntries& ancestors, | ||
| const CTxMemPool::setEntries& direct_conflicts, | ||
| const CTxMemPool::setEntries& original_transactions) | ||
| { | ||
| const CFeeRate replacement_individual_feerate(replacement_fees, replacement_vsize); | ||
| // Ancestor feerate is the total modified fees divided by the total size. To get the | ||
| // ancestor feerate, add up all the individual modified fees and sizes. Don't try to use the | ||
| // cached ancestor fees and sizes because entries may have overlapping ancestors. | ||
| for (CTxMemPool::txiter it : ancestors) { | ||
| replacement_fees += it->GetModifiedFee(); | ||
| replacement_vsize += it->GetTxSize(); | ||
| } | ||
| const CFeeRate replacement_ancestor_feerate(replacement_fees, replacement_vsize); | ||
| // A package/transaction's ancestor feerate is not equivalent to the miner score; it may | ||
| // overestimate. Some subset of the ancestors could be included by itself if it has other | ||
| // high-feerate descendants or are themselves higher feerate than this package/transaction. | ||
| // For now, as a conservative estimate, use the minimum between the transaction's individual | ||
| // feerate and ancestor feerate. | ||
| const CFeeRate replacement_miner_score = std::min(replacement_individual_feerate, replacement_ancestor_feerate); | ||
|
glozow marked this conversation as resolved.
Outdated
|
||
| for (const auto& entry : direct_conflicts) { | ||
| const bool conflict_is_v3{entry->GetSharedTx()->nVersion == 3}; | ||
| CFeeRate original_score(entry->GetModifiedFee(), entry->GetTxSize()); | ||
| // If the original transaction is v3, we can calculate the exact miner score and avoid overestimating. | ||
| if (conflict_is_v3) { | ||
| original_score = std::min(original_score, CFeeRate(entry->GetModFeesWithAncestors(), entry->GetSizeWithAncestors())); | ||
| } | ||
| if (replacement_miner_score < original_score) { | ||
| return strprintf("replacement miner score lower than %s miner score of direct conflict; %s < %s", | ||
| conflict_is_v3 ? "calculated" : "estimated", | ||
| replacement_miner_score.ToString(), | ||
| original_score.ToString()); | ||
| } | ||
| } | ||
| for (const auto& entry : original_transactions) { | ||
| const CFeeRate original_ancestor_feerate(entry->GetModFeesWithAncestors(), entry->GetSizeWithAncestors()); | ||
| if (replacement_miner_score < original_ancestor_feerate) { | ||
| return strprintf("replacement miner score lower than ancestor feerate of original tx; %s < %s", | ||
| replacement_miner_score.ToString(), | ||
| original_ancestor_feerate.ToString()); | ||
| } | ||
| } | ||
| return std::nullopt; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| // Copyright (c) 2022 The Bitcoin Core developers | ||
| // Distributed under the MIT software license, see the accompanying | ||
| // file COPYING or http://www.opensource.org/licenses/mit-license.php. | ||
|
|
||
| #include <policy/v3_policy.h> | ||
|
|
||
| #include <coins.h> | ||
| #include <consensus/amount.h> | ||
| #include <logging.h> | ||
| #include <tinyformat.h> | ||
|
|
||
| #include <numeric> | ||
| #include <vector> | ||
|
|
||
| std::optional<std::tuple<uint256, uint256, bool>> CheckV3Inheritance(const Package& package) | ||
| { | ||
| assert(std::all_of(package.cbegin(), package.cend(), [](const auto& tx){return tx != nullptr;})); | ||
| // If all transactions are V3, we can stop here. | ||
| if (std::all_of(package.cbegin(), package.cend(), [](const auto& tx){return tx->nVersion == 3;})) { | ||
| return std::nullopt; | ||
| } | ||
| // If all transactions are non-V3, we can stop here. | ||
| if (std::all_of(package.cbegin(), package.cend(), [](const auto& tx){return tx->nVersion != 3;})) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After this check, we know it's a mixture(?) and therefore illicit, making hitting 000facb#diff-a19c07329799a164e1e3bd89e7cd07f1a8e5e97b88f2b24c9bef17df04479c74R51 impossible and the below loops a only useful for returning an example illicit tuple. Perhaps just grab one v3 example and one non-v3 example and return those to simplify the rest of the code?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, the loops/maps are only used to return a more helpful string to the user. I don't think it's too much work? This function accepts a list of transactions that doesn't necessarily need to be an ancestor packages, so just grabbing a V3 and non-V3 could be 2 transactions that aren't parent-child. |
||
| return std::nullopt; | ||
| } | ||
| // Look for a V3 transaction spending a non-V3 or vice versa. | ||
| std::unordered_map<uint256, uint256, SaltedTxidHasher> v3_txid_to_wtxid; | ||
| std::unordered_map<uint256, uint256, SaltedTxidHasher> non_v3_txid_to_wtxid; | ||
| for (const auto& tx : package) { | ||
| if (tx->nVersion == 3) { | ||
| v3_txid_to_wtxid.emplace(tx->GetHash(), tx->GetWitnessHash()); | ||
| } else { | ||
| non_v3_txid_to_wtxid.emplace(tx->GetHash(), tx->GetWitnessHash()); | ||
| } | ||
| } | ||
| for (const auto& tx : package) { | ||
| if (tx->nVersion == 3) { | ||
| for (const auto& input : tx->vin) { | ||
| if (auto it = non_v3_txid_to_wtxid.find(input.prevout.hash); it != non_v3_txid_to_wtxid.end()) { | ||
| return std::make_tuple(it->second, tx->GetWitnessHash(), true); | ||
| } | ||
| } | ||
| } else { | ||
| for (const auto& input : tx->vin) { | ||
| if (auto it = v3_txid_to_wtxid.find(input.prevout.hash); it != v3_txid_to_wtxid.end()) { | ||
| return std::make_tuple(it->second, tx->GetWitnessHash(), false); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return std::nullopt; | ||
| } | ||
|
|
||
| std::optional<std::string> CheckV3Inheritance(const CTransactionRef& ptx, | ||
| const CTxMemPool::setEntries& ancestors) | ||
| { | ||
| for (const auto& entry : ancestors) { | ||
| if (ptx->nVersion != 3 && entry->GetTx().nVersion == 3) { | ||
| return strprintf("tx that spends from %s must be nVersion=3", | ||
| entry->GetTx().GetWitnessHash().ToString()); | ||
| } else if (ptx->nVersion == 3 && entry->GetTx().nVersion != 3) { | ||
| return strprintf("v3 tx cannot spend from %s which is not nVersion=3", | ||
| entry->GetTx().GetWitnessHash().ToString()); | ||
| } | ||
| } | ||
| return std::nullopt; | ||
| } | ||
|
|
||
| std::optional<std::string> ApplyV3Rules(const CTransactionRef& ptx, | ||
| const CTxMemPool::setEntries& ancestors, | ||
| const std::set<uint256>& direct_conflicts) | ||
| { | ||
| // These rules only apply to transactions with nVersion=3. | ||
| if (ptx->nVersion != 3) return std::nullopt; | ||
|
|
||
| if (ancestors.size() + 1 > V3_ANCESTOR_LIMIT) { | ||
| return strprintf("tx %s would have too many ancestors", ptx->GetWitnessHash().ToString()); | ||
| } | ||
| if (ancestors.empty()) { | ||
| return std::nullopt; | ||
| } else { | ||
| const auto tx_weight{GetTransactionWeight(*ptx)}; | ||
| // If this transaction spends V3 parents, it cannot be too large. | ||
| if (tx_weight > V3_CHILD_MAX_WEIGHT) { | ||
| return strprintf("v3 child tx is too big: %u weight units", tx_weight); | ||
| } | ||
| // Any ancestor of a V3 transaction must also be V3. | ||
| const auto& parent_entry = *ancestors.begin(); | ||
| if (parent_entry->GetTx().nVersion != 3) { | ||
| return strprintf("v3 tx cannot spend from %s which is not nVersion=3", | ||
| parent_entry->GetTx().GetWitnessHash().ToString()); | ||
| } | ||
| // If there are any ancestors, this is the only child allowed. The parent cannot have any | ||
| // other descendants. | ||
| const auto& children = parent_entry->GetMemPoolChildrenConst(); | ||
| // Don't double-count a transaction that is going to be replaced. This logic assumes that | ||
| // any descendant of the V3 transaction is a direct child, which makes sense because a V3 | ||
| // transaction can only have 1 descendant. | ||
| const bool child_will_be_replaced = !children.empty() && | ||
| std::any_of(children.cbegin(), children.cend(), | ||
| [&direct_conflicts](const CTxMemPoolEntry& child){return direct_conflicts.count(child.GetTx().GetHash()) > 0;}); | ||
| if (parent_entry->GetCountWithDescendants() + 1 > V3_DESCENDANT_LIMIT && !child_will_be_replaced) { | ||
| return strprintf("tx %u would exceed descendant count limit", parent_entry->GetTx().GetHash().ToString()); | ||
| } | ||
| } | ||
| return std::nullopt; | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.