policy: nVersion=3 and Package RBF

doc/policy/mempool-replacements.md

@@ -11,7 +11,8 @@ their in-mempool descendants (together, "original transactions") if, in addition
 other consensus and policy rules, each of the following conditions are met:
 
 1. The directly conflicting transactions all signal replaceability explicitly. A transaction is
-   signaling replaceability if any of its inputs have an nSequence number less than (0xffffffff - 1).
+   signaling replaceability if any of its inputs have an nSequence number less than (0xffffffff - 1)
+   or if its nVersion field is set to 3. See [version 3 policies](./version3_transactions.md).
 
    *Rationale*: See [BIP125
    explanation](https://github.com/bitcoin/bips/blob/master/bip-0125.mediawiki#motivation).

doc/policy/packages.md

@@ -48,8 +48,13 @@ The following rules are enforced for all packages:
      heavily connected, i.e. some transaction in the package is the ancestor or descendant of all
      the other transactions.
 
-The following rules are only enforced for packages to be submitted to the mempool (not enforced for
-test accepts):
+* [CPFP Carve Out](./mempool-limits.md#CPFP-Carve-Out) is disabled. (#21800)
+
+   - *Rationale*: This carve out cannot be accurately applied when there are multiple transactions'
+     ancestors and descendants being considered at the same time.
+
+The following rules are only enforced for packages to be submitted to the mempool (not
+enforced for test accepts):
 
 * Packages must be child-with-unconfirmed-parents packages. This also means packages must contain at
   least 2 transactions. (#22674)

doc/policy/version3_transactions.md

@@ -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.
+   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.
+
+### "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.
+
+*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.
+
+*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.
+
+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
+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.

src/Makefile.am

@@ -237,6 +237,7 @@ BITCOIN_CORE_H = \
   node/validation_cache_args.h \
   noui.h \
   outputtype.h \
+  policy/v3_policy.h \
   policy/feerate.h \
   policy/fees.h \
   policy/fees_args.h \
@@ -438,6 +439,7 @@ libbitcoin_node_a_SOURCES = \
   node/utxo_snapshot.cpp \
   node/validation_cache_args.cpp \
   noui.cpp \
+  policy/v3_policy.cpp \
   policy/fees.cpp \
   policy/fees_args.cpp \
   policy/packages.cpp \
@@ -694,6 +696,7 @@ libbitcoin_common_a_SOURCES = \
   netbase.cpp \
   net_permissions.cpp \
   outputtype.cpp \
+  policy/v3_policy.cpp \
   policy/feerate.cpp \
   policy/policy.cpp \
   protocol.cpp \
@@ -953,6 +956,7 @@ libbitcoinkernel_la_SOURCES = \
   node/blockstorage.cpp \
   node/chainstate.cpp \
   node/utxo_snapshot.cpp \
+  policy/v3_policy.cpp \
   policy/feerate.cpp \
   policy/fees.cpp \
   policy/packages.cpp \

src/policy/policy.h

@@ -131,7 +131,7 @@ bool IsStandard(const CScript& scriptPubKey, const std::optional<unsigned>& max_
 // Changing the default transaction version requires a two step process: first
 // adapting relay policy by bumping TX_MAX_STANDARD_VERSION, and then later
 // allowing the new transaction version in the wallet/RPC.
-static constexpr decltype(CTransaction::nVersion) TX_MAX_STANDARD_VERSION{2};
+static constexpr decltype(CTransaction::nVersion) TX_MAX_STANDARD_VERSION{3};
 
 /**
 * Check for standard transaction types

src/policy/rbf.cpp

@@ -181,3 +181,49 @@ std::optional<std::string> PaysForRBF(CAmount original_fees,
     }
     return std::nullopt;
 }
+
+std::optional<std::string> CheckMinerScores(CAmount replacement_fees,
+                                            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);
+        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;
+}

src/policy/rbf.h

@@ -106,4 +106,15 @@ std::optional<std::string> PaysForRBF(CAmount original_fees,
                                       CFeeRate relay_fee,
                                       const uint256& txid);
 
+/** Require that replacement transactions are more incentive compatible to mine than the
+ * transactions they are replacing. Currently, this function requires that min(ancestor feerate,
+ * individual feerate) of the replacement transaction(s) be higher than the individual feerates of
+ * all directly conflicting transactions and the ancestor feerates of all original transactions.
+ * */
+std::optional<std::string> CheckMinerScores(CAmount replacement_fees,
+                                            int64_t replacement_vsize,
+                                            const CTxMemPool::setEntries& ancestors,
+                                            const CTxMemPool::setEntries& direct_conflicts,
+                                            const CTxMemPool::setEntries& original_transactions);
+
 #endif // BITCOIN_POLICY_RBF_H

src/policy/v3_policy.cpp

@@ -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;})) {
+        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;
+}