Merge bitcoin#30272: doc: use TRUC instead of v3 and add release note

926b8e3 [doc] add release note for TRUC (glozow)
19a9b90 use version=3 instead of v3 in debug strings (glozow)
881fac8 scripted-diff: change names from V3 to TRUC (glozow)
a573dd2 [doc] replace mentions of v3 with TRUC (glozow)
089b575 rename mempool_accept_v3.py to mempool_truc.py (glozow)
f543852 rename policy/v3_policy.* to policy/truc_policy.* (glozow)

Pull request description:

  Adds a release note for TRUC policy which will be live in v28.0.

  For clarity, replaces mentions of "v3" with "TRUC" in most places. Suggested in
  - bitcoin#29496 (comment)
  - bitcoin#29496 (comment)

  I changed error strings from "v3-violation" to "TRUC-violation" but left v3 in the debug strings because I think it might be clearer for somebody who is debugging. Similarly, I left some variables unchanged because I think they're more descriptive this way, e.g. `tx_v3_from_v2_and_v3`. I'm happy to debate places that should or shouldn't be documented differently in this PR, whatever is clearest to everyone.

ACKs for top commit:
  instagibbs:
    reACK bitcoin@926b8e3
  achow101:
    ACK 926b8e3
  ismaelsadeeq:
    Code review ACK 926b8e3

Tree-SHA512: 16c88add0a29dc6d1236c4d45f34a17b850f6727b231953cbd52eb9f7268d1d802563eadfc8b7928c94ed3d7a615275dd103e57e81439ebf3ba2b12efa1e42af

Author: achow101

doc/release-notes-29496.md

@@ -0,0 +1,11 @@
+Mempool Policy Changes
+----------------------
+
+- Transactions with version number set to 3 are now treated as standard on all networks (#29496),
+  subject to Opt-in Topologically Restricted Until Confirmation (TRUC) Transactions policy as
+  described in [BIP 431](https://github.com/bitcoin/bips/blob/master/bip-0431.mediawiki).  The
+  policy includes limits on spending unconfirmed outputs (#28948), eviction of a previous descendant
+  if a more incentive-compatible one is submitted (#29306), and a maximum transaction size of 10,000vB
+  (#29873). These restrictions simplify the assessment of incentive compatibility of accepting or
+  replacing TRUC transactions, thus ensuring any replacements are more profitable for the node and
+  making fee-bumping more reliable.

src/Makefile.am

@@ -244,14 +244,14 @@ BITCOIN_CORE_H = \
   node/warnings.h \
   noui.h \
   outputtype.h \
-  policy/v3_policy.h \
   policy/feerate.h \
   policy/fees.h \
   policy/fees_args.h \
   policy/packages.h \
   policy/policy.h \
   policy/rbf.h \
   policy/settings.h \
+  policy/truc_policy.h \
   pow.h \
   protocol.h \
   psbt.h \
@@ -448,12 +448,12 @@ libbitcoin_node_a_SOURCES = \
   node/validation_cache_args.cpp \
   node/warnings.cpp \
   noui.cpp \
-  policy/v3_policy.cpp \
   policy/fees.cpp \
   policy/fees_args.cpp \
   policy/packages.cpp \
   policy/rbf.cpp \
   policy/settings.cpp \
+  policy/truc_policy.cpp \
   pow.cpp \
   rest.cpp \
   rpc/blockchain.cpp \
@@ -708,9 +708,9 @@ libbitcoin_common_a_SOURCES = \
   netbase.cpp \
   net_permissions.cpp \
   outputtype.cpp \
-  policy/v3_policy.cpp \
   policy/feerate.cpp \
   policy/policy.cpp \
+  policy/truc_policy.cpp \
   protocol.cpp \
   psbt.cpp \
   rpc/external_signer.cpp \
@@ -955,12 +955,12 @@ libbitcoinkernel_la_SOURCES = \
   node/blockstorage.cpp \
   node/chainstate.cpp \
   node/utxo_snapshot.cpp \
-  policy/v3_policy.cpp \
   policy/feerate.cpp \
   policy/packages.cpp \
   policy/policy.cpp \
   policy/rbf.cpp \
   policy/settings.cpp \
+  policy/truc_policy.cpp \
   pow.cpp \
   primitives/block.cpp \
   primitives/transaction.cpp \

src/policy/v3_policy.cpp renamed to src/policy/truc_policy.cpp

@@ -2,7 +2,7 @@
 // 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 <policy/truc_policy.h>
 
 #include <coins.h>
 #include <consensus/amount.h>
@@ -14,7 +14,7 @@
 #include <numeric>
 #include <vector>
 
-/** Helper for PackageV3Checks: Returns a vector containing the indices of transactions (within
+/** Helper for PackageTRUCChecks: Returns a vector containing the indices of transactions (within
  * package) that are direct parents of ptx. */
 std::vector<size_t> FindInPackageParents(const Package& package, const CTransactionRef& ptx)
 {
@@ -37,13 +37,13 @@ std::vector<size_t> FindInPackageParents(const Package& package, const CTransact
     return in_package_parents;
 }
 
-/** Helper for PackageV3Checks, storing info for a mempool or package parent. */
+/** Helper for PackageTRUCChecks, storing info for a mempool or package parent. */
 struct ParentInfo {
     /** Txid used to identify this parent by prevout */
     const Txid& m_txid;
     /** Wtxid used for debug string */
     const Wtxid& m_wtxid;
-    /** version used to check inheritance of v3 and non-v3 */
+    /** version used to check inheritance of TRUC and non-TRUC */
     decltype(CTransaction::version) m_version;
     /** If parent is in mempool, whether it has any descendants in mempool. */
     bool m_has_mempool_descendant;
@@ -55,36 +55,36 @@ struct ParentInfo {
     {}
 };
 
-std::optional<std::string> PackageV3Checks(const CTransactionRef& ptx, int64_t vsize,
+std::optional<std::string> PackageTRUCChecks(const CTransactionRef& ptx, int64_t vsize,
                                            const Package& package,
                                            const CTxMemPool::setEntries& mempool_ancestors)
 {
     // This function is specialized for these limits, and must be reimplemented if they ever change.
-    static_assert(V3_ANCESTOR_LIMIT == 2);
-    static_assert(V3_DESCENDANT_LIMIT == 2);
+    static_assert(TRUC_ANCESTOR_LIMIT == 2);
+    static_assert(TRUC_DESCENDANT_LIMIT == 2);
 
     const auto in_package_parents{FindInPackageParents(package, ptx)};
 
-    // Now we have all ancestors, so we can start checking v3 rules.
+    // Now we have all ancestors, so we can start checking TRUC rules.
     if (ptx->version == TRUC_VERSION) {
-        // SingleV3Checks should have checked this already.
-        if (!Assume(vsize <= V3_MAX_VSIZE)) {
-            return strprintf("v3 tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
-                             ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(), vsize, V3_MAX_VSIZE);
+        // SingleTRUCChecks should have checked this already.
+        if (!Assume(vsize <= TRUC_MAX_VSIZE)) {
+            return strprintf("version=3 tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
+                             ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(), vsize, TRUC_MAX_VSIZE);
         }
 
-        if (mempool_ancestors.size() + in_package_parents.size() + 1 > V3_ANCESTOR_LIMIT) {
+        if (mempool_ancestors.size() + in_package_parents.size() + 1 > TRUC_ANCESTOR_LIMIT) {
             return strprintf("tx %s (wtxid=%s) would have too many ancestors",
                              ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString());
         }
 
         const bool has_parent{mempool_ancestors.size() + in_package_parents.size() > 0};
         if (has_parent) {
-            // A v3 child cannot be too large.
-            if (vsize > V3_CHILD_MAX_VSIZE) {
-                return strprintf("v3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
+            // A TRUC child cannot be too large.
+            if (vsize > TRUC_CHILD_MAX_VSIZE) {
+                return strprintf("version=3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
                                  ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(),
-                                 vsize, V3_CHILD_MAX_VSIZE);
+                                 vsize, TRUC_CHILD_MAX_VSIZE);
             }
 
             // Exactly 1 parent exists, either in mempool or package. Find it.
@@ -107,7 +107,7 @@ std::optional<std::string> PackageV3Checks(const CTransactionRef& ptx, int64_t v
 
             // If there is a parent, it must have the right version.
             if (parent_info.m_version != TRUC_VERSION) {
-                return strprintf("v3 tx %s (wtxid=%s) cannot spend from non-v3 tx %s (wtxid=%s)",
+                return strprintf("version=3 tx %s (wtxid=%s) cannot spend from non-version=3 tx %s (wtxid=%s)",
                                  ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(),
                                  parent_info.m_txid.ToString(), parent_info.m_wtxid.ToString());
             }
@@ -118,7 +118,7 @@ std::optional<std::string> PackageV3Checks(const CTransactionRef& ptx, int64_t v
 
                 for (auto& input : package_tx->vin) {
                     // Fail if we find another tx with the same parent. We don't check whether the
-                    // sibling is to-be-replaced (done in SingleV3Checks) because these transactions
+                    // sibling is to-be-replaced (done in SingleTRUCChecks) because these transactions
                     // are within the same package.
                     if (input.prevout.hash == parent_info.m_txid) {
                         return strprintf("tx %s (wtxid=%s) would exceed descendant count limit",
@@ -140,17 +140,17 @@ std::optional<std::string> PackageV3Checks(const CTransactionRef& ptx, int64_t v
             }
         }
     } else {
-        // Non-v3 transactions cannot have v3 parents.
+        // Non-TRUC transactions cannot have TRUC parents.
         for (auto it : mempool_ancestors) {
             if (it->GetTx().version == TRUC_VERSION) {
-                return strprintf("non-v3 tx %s (wtxid=%s) cannot spend from v3 tx %s (wtxid=%s)",
+                return strprintf("non-version=3 tx %s (wtxid=%s) cannot spend from version=3 tx %s (wtxid=%s)",
                                  ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(),
                                  it->GetSharedTx()->GetHash().ToString(), it->GetSharedTx()->GetWitnessHash().ToString());
             }
         }
         for (const auto& index: in_package_parents) {
             if (package.at(index)->version == TRUC_VERSION) {
-                return strprintf("non-v3 tx %s (wtxid=%s) cannot spend from v3 tx %s (wtxid=%s)",
+                return strprintf("non-version=3 tx %s (wtxid=%s) cannot spend from version=3 tx %s (wtxid=%s)",
                                  ptx->GetHash().ToString(),
                                  ptx->GetWitnessHash().ToString(),
                                  package.at(index)->GetHash().ToString(),
@@ -161,52 +161,52 @@ std::optional<std::string> PackageV3Checks(const CTransactionRef& ptx, int64_t v
     return std::nullopt;
 }
 
-std::optional<std::pair<std::string, CTransactionRef>> SingleV3Checks(const CTransactionRef& ptx,
+std::optional<std::pair<std::string, CTransactionRef>> SingleTRUCChecks(const CTransactionRef& ptx,
                                           const CTxMemPool::setEntries& mempool_ancestors,
                                           const std::set<Txid>& direct_conflicts,
                                           int64_t vsize)
 {
-    // Check v3 and non-v3 inheritance.
+    // Check TRUC and non-TRUC inheritance.
     for (const auto& entry : mempool_ancestors) {
         if (ptx->version != TRUC_VERSION && entry->GetTx().version == TRUC_VERSION) {
-            return std::make_pair(strprintf("non-v3 tx %s (wtxid=%s) cannot spend from v3 tx %s (wtxid=%s)",
+            return std::make_pair(strprintf("non-version=3 tx %s (wtxid=%s) cannot spend from version=3 tx %s (wtxid=%s)",
                              ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(),
                              entry->GetSharedTx()->GetHash().ToString(), entry->GetSharedTx()->GetWitnessHash().ToString()),
                 nullptr);
         } else if (ptx->version == TRUC_VERSION && entry->GetTx().version != TRUC_VERSION) {
-            return std::make_pair(strprintf("v3 tx %s (wtxid=%s) cannot spend from non-v3 tx %s (wtxid=%s)",
+            return std::make_pair(strprintf("version=3 tx %s (wtxid=%s) cannot spend from non-version=3 tx %s (wtxid=%s)",
                              ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(),
                              entry->GetSharedTx()->GetHash().ToString(), entry->GetSharedTx()->GetWitnessHash().ToString()),
                 nullptr);
         }
     }
 
     // This function is specialized for these limits, and must be reimplemented if they ever change.
-    static_assert(V3_ANCESTOR_LIMIT == 2);
-    static_assert(V3_DESCENDANT_LIMIT == 2);
+    static_assert(TRUC_ANCESTOR_LIMIT == 2);
+    static_assert(TRUC_DESCENDANT_LIMIT == 2);
 
     // The rest of the rules only apply to transactions with version=3.
     if (ptx->version != TRUC_VERSION) return std::nullopt;
 
-    if (vsize > V3_MAX_VSIZE) {
-        return std::make_pair(strprintf("v3 tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
-                         ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(), vsize, V3_MAX_VSIZE),
+    if (vsize > TRUC_MAX_VSIZE) {
+        return std::make_pair(strprintf("version=3 tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
+                         ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(), vsize, TRUC_MAX_VSIZE),
             nullptr);
     }
 
-    // Check that V3_ANCESTOR_LIMIT would not be violated.
-    if (mempool_ancestors.size() + 1 > V3_ANCESTOR_LIMIT) {
+    // Check that TRUC_ANCESTOR_LIMIT would not be violated.
+    if (mempool_ancestors.size() + 1 > TRUC_ANCESTOR_LIMIT) {
         return std::make_pair(strprintf("tx %s (wtxid=%s) would have too many ancestors",
                          ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString()),
             nullptr);
     }
 
     // Remaining checks only pertain to transactions with unconfirmed ancestors.
     if (mempool_ancestors.size() > 0) {
-        // If this transaction spends V3 parents, it cannot be too large.
-        if (vsize > V3_CHILD_MAX_VSIZE) {
-            return std::make_pair(strprintf("v3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
-                             ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(), vsize, V3_CHILD_MAX_VSIZE),
+        // If this transaction spends TRUC parents, it cannot be too large.
+        if (vsize > TRUC_CHILD_MAX_VSIZE) {
+            return std::make_pair(strprintf("version=3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
+                             ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(), vsize, TRUC_CHILD_MAX_VSIZE),
                 nullptr);
         }
 
@@ -217,14 +217,14 @@ std::optional<std::pair<std::string, CTransactionRef>> SingleV3Checks(const CTra
         // possible through a reorg.
         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.
+        // any descendant of the TRUC transaction is a direct child, which makes sense because a
+        // TRUC 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) {
-            // Allow sibling eviction for v3 transaction: if another child already exists, even if
-            // we don't conflict inputs with it, consider evicting it under RBF rules. We rely on v3 rules
+        if (parent_entry->GetCountWithDescendants() + 1 > TRUC_DESCENDANT_LIMIT && !child_will_be_replaced) {
+            // Allow sibling eviction for TRUC transaction: if another child already exists, even if
+            // we don't conflict inputs with it, consider evicting it under RBF rules. We rely on TRUC rules
             // only permitting 1 descendant, as otherwise we would need to have logic for deciding
             // which descendant to evict. Skip if this isn't true, e.g. if the transaction has
             // multiple children or the sibling also has descendants due to a reorg.

src/policy/truc_policy.h

@@ -0,0 +1,94 @@
+// 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.
+
+#ifndef BITCOIN_POLICY_TRUC_POLICY_H
+#define BITCOIN_POLICY_TRUC_POLICY_H
+
+#include <consensus/amount.h>
+#include <policy/packages.h>
+#include <policy/policy.h>
+#include <primitives/transaction.h>
+#include <txmempool.h>
+#include <util/result.h>
+
+#include <set>
+#include <string>
+
+// This module enforces rules for BIP 431 TRUC transactions which help make
+// RBF abilities more robust. A transaction with version=3 is treated as TRUC.
+static constexpr decltype(CTransaction::version) TRUC_VERSION{3};
+
+// TRUC only allows 1 parent and 1 child when unconfirmed. This translates to a descendant set size
+// of 2 and ancestor set size of 2.
+/** Maximum number of transactions including an unconfirmed tx and its descendants. */
+static constexpr unsigned int TRUC_DESCENDANT_LIMIT{2};
+/** Maximum number of transactions including a TRUC tx and all its mempool ancestors. */
+static constexpr unsigned int TRUC_ANCESTOR_LIMIT{2};
+
+/** Maximum sigop-adjusted virtual size of all v3 transactions. */
+static constexpr int64_t TRUC_MAX_VSIZE{10000};
+/** Maximum sigop-adjusted virtual size of a tx which spends from an unconfirmed TRUC transaction. */
+static constexpr int64_t TRUC_CHILD_MAX_VSIZE{1000};
+// These limits are within the default ancestor/descendant limits.
+static_assert(TRUC_MAX_VSIZE + TRUC_CHILD_MAX_VSIZE <= DEFAULT_ANCESTOR_SIZE_LIMIT_KVB * 1000);
+static_assert(TRUC_MAX_VSIZE + TRUC_CHILD_MAX_VSIZE <= DEFAULT_DESCENDANT_SIZE_LIMIT_KVB * 1000);
+
+/** Must be called for every transaction, even if not TRUC. Not strictly necessary for transactions
+ * accepted through AcceptMultipleTransactions.
+ *
+ * Checks the following rules:
+ * 1. A TRUC tx must only have TRUC unconfirmed ancestors.
+ * 2. A non-TRUC tx must only have non-TRUC unconfirmed ancestors.
+ * 3. A TRUC's ancestor set, including itself, must be within TRUC_ANCESTOR_LIMIT.
+ * 4. A TRUC's descendant set, including itself, must be within TRUC_DESCENDANT_LIMIT.
+ * 5. If a TRUC tx has any unconfirmed ancestors, the tx's sigop-adjusted vsize must be within
+ * TRUC_CHILD_MAX_VSIZE.
+ * 6. A TRUC tx must be within TRUC_MAX_VSIZE.
+ *
+ *
+ * @param[in]   mempool_ancestors       The in-mempool ancestors of ptx.
+ * @param[in]   direct_conflicts        In-mempool transactions this tx conflicts with. These conflicts
+ *                                      are used to more accurately calculate the resulting descendant
+ *                                      count of in-mempool ancestors.
+ * @param[in]   vsize                   The sigop-adjusted virtual size of ptx.
+ *
+ * @returns 3 possibilities:
+ * - std::nullopt if all TRUC checks were applied successfully
+ * - debug string + pointer to a mempool sibling if this transaction would be the second child in a
+ *   1-parent-1-child cluster; the caller may consider evicting the specified sibling or return an
+ *   error with the debug string.
+ * - debug string + nullptr if this transaction violates some TRUC rule and sibling eviction is not
+ *   applicable.
+ */
+std::optional<std::pair<std::string, CTransactionRef>> SingleTRUCChecks(const CTransactionRef& ptx,
+                                          const CTxMemPool::setEntries& mempool_ancestors,
+                                          const std::set<Txid>& direct_conflicts,
+                                          int64_t vsize);
+
+/** Must be called for every transaction that is submitted within a package, even if not TRUC.
+ *
+ * For each transaction in a package:
+ * If it's not a TRUC transaction, verify it has no direct TRUC parents in the mempool or the package.
+
+ * If it is a TRUC transaction, verify that any direct parents in the mempool or the package are TRUC.
+ * If such a parent exists, verify that parent has no other children in the package or the mempool,
+ * and that the transaction itself has no children in the package.
+ *
+ * If any TRUC violations in the package exist, this test will fail for one of them:
+ * - if a TRUC transaction T has a parent in the mempool and a child in the package, then PTRUCC(T) will fail
+ * - if a TRUC transaction T has a parent in the package and a child in the package, then PTRUCC(T) will fail
+ * - if a TRUC transaction T and a TRUC (sibling) transaction U have some parent in the mempool,
+ *   then PTRUCC(T) and PTRUCC(U) will fail
+ * - if a TRUC transaction T and a TRUC (sibling) transaction U have some parent in the package,
+ *   then PTRUCC(T) and PTRUCC(U) will fail
+ * - if a TRUC transaction T has a parent P and a grandparent G in the package, then
+ *   PTRUCC(P) will fail (though PTRUCC(G) and PTRUCC(T) might succeed).
+ *
+ * @returns debug string if an error occurs, std::nullopt otherwise.
+ * */
+std::optional<std::string> PackageTRUCChecks(const CTransactionRef& ptx, int64_t vsize,
+                                           const Package& package,
+                                           const CTxMemPool::setEntries& mempool_ancestors);
+
+#endif // BITCOIN_POLICY_TRUC_POLICY_H