aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorAva Chow <github@achow101.com>2024-02-09 23:29:39 -0500
committerAva Chow <github@achow101.com>2024-02-09 23:37:57 -0500
commit7143d4388407ab3d12005e55a02d5e8f334e4dc9 (patch)
treea377c5653b546f1eee5d4c02f074760e8a076bab /src
parent1d334d830f9423caef0a230dc62e4458a56d3bbe (diff)
parent29029df5c700e6940c712028303761d91ae15847 (diff)
downloadbitcoin-7143d4388407ab3d12005e55a02d5e8f334e4dc9.tar.xz
Merge bitcoin/bitcoin#28948: v3 transaction policy for anti-pinning
29029df5c700e6940c712028303761d91ae15847 [doc] v3 signaling in mempool-replacements.md (glozow) e643ea795e4b6fea4a6bbb3d72870ee6a4c836b1 [fuzz] v3 transactions and sigop-adjusted vsize (glozow) 1fd16b5c62f54c7f4c60122acd65d852f63d1e8b [functional test] v3 transaction submission (glozow) 27c8786ba918a42c860e6a50eaee9fdf56d7c646 test framework: Add and use option for tx-version in MiniWallet methods (MarcoFalke) 9a1fea55b29fe025355b06b45e3d77d192acc635 [policy/validation] allow v3 transactions with certain restrictions (glozow) eb8d5a2e7d939dd3ee683486e98702079e0dfcc0 [policy] add v3 policy rules (glozow) 9a29d470fbb62bbb27d517efeafe46ff03c25f54 [rpc] return full string for package_msg and package-error (glozow) 158623b8e0726dff7eae4288138f1710e727db9c [refactor] change Workspace::m_conflicts and adjacent funcs/structs to use Txid (glozow) Pull request description: See #27463 for overall package relay tracking. Delving Bitcoin discussion thread: https://delvingbitcoin.org/t/v3-transaction-policy-for-anti-pinning/340 Delving Bitcoin discussion for LN usage: https://delvingbitcoin.org/t/lightning-transactions-with-v3-and-ephemeral-anchors/418 Rationale: - There are various pinning problems with RBF and our general ancestor/descendant limits. These policies help mitigate many pinning attacks and make package RBF feasible (see #28984 which implements package RBF on top of this). I would focus the most here on Rule 3 pinning. [1][2] - Switching to a cluster-based mempool (see #27677 and #28676) requires the removal of CPFP carve out, which applications depend on. V3 + package RBF + ephemeral anchors + 1-parent-1-child package relay provides an intermediate solution. V3 policy is for "Priority Transactions." [3][4] It allows users to opt in to more restrictive topological limits for shared transactions, in exchange for the more robust fee-bumping abilities that offers. Even though we don't have cluster limits, we are able to treat these transactions as having as having a maximum cluster size of 2. Immediate benefits: - You can presign a transaction with 0 fees (not just 1sat/vB!) and add a fee-bump later. - Rule 3 pinning is reduced by a significant amount, since the attacker can only attach a maximum of 1000vB to your shared transaction. This also enables some other cool things (again see #27463 for overall roadmap): - Ephemeral Anchors - Package RBF for these 1-parent-1-child packages. That means e.g. a commitment tx + child can replace another commitment tx using the child's fees. - We can transition to a "single anchor" universe without worrying about package limit pinning. So current users of CPFP carve out would have something else to use. - We can switch to a cluster-based mempool [5] (#27677 #28676), which removes CPFP carve out [6]. [1]: Original mailing list post and discussion about RBF pinning problems https://gist.github.com/glozow/25d9662c52453bd08b4b4b1d3783b9ff, https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-January/019817.html [2]: A FAQ is "we need this for cluster mempool, but is this still necessary afterwards?" There are some pinning issues that are fixed here and not fully fixed in cluster mempool, so we will still want this or something similar afterward. [3]: Mailing list post for v3 https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-September/020937.html [4]: Original PR #25038 also contains a lot of the discussion [5]: https://delvingbitcoin.org/t/an-overview-of-the-cluster-mempool-proposal/393/7 [6]: https://delvingbitcoin.org/t/an-overview-of-the-cluster-mempool-proposal/393#the-cpfp-carveout-rule-can-no-longer-be-supported-12 ACKs for top commit: sdaftuar: ACK 29029df5c700e6940c712028303761d91ae15847 achow101: ACK 29029df5c700e6940c712028303761d91ae15847 instagibbs: ACK 29029df5c700e6940c712028303761d91ae15847 modulo that Tree-SHA512: 9664b078890cfdca2a146439f8835c9d9ab483f43b30af8c7cd6962f09aa557fb1ce7689d5e130a2ec142235dbc8f21213881baa75241c5881660f9008d68450
Diffstat (limited to 'src')
-rw-r--r--src/Makefile.am4
-rw-r--r--src/policy/rbf.cpp4
-rw-r--r--src/policy/rbf.h2
-rw-r--r--src/policy/v3_policy.cpp220
-rw-r--r--src/policy/v3_policy.h83
-rw-r--r--src/rpc/mempool.cpp4
-rw-r--r--src/test/fuzz/package_eval.cpp35
-rw-r--r--src/test/fuzz/tx_pool.cpp7
-rw-r--r--src/test/rbf_tests.cpp2
-rw-r--r--src/test/transaction_tests.cpp2
-rw-r--r--src/test/txvalidation_tests.cpp289
-rw-r--r--src/test/util/txmempool.cpp26
-rw-r--r--src/test/util/txmempool.h10
-rw-r--r--src/txmempool.cpp4
-rw-r--r--src/txmempool.h2
-rw-r--r--src/validation.cpp31
16 files changed, 696 insertions, 29 deletions
diff --git a/src/Makefile.am b/src/Makefile.am
index e452b42533..3e8870c828 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -242,6 +242,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 \
@@ -441,6 +442,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 \
@@ -702,6 +704,7 @@ libbitcoin_common_a_SOURCES = \
netbase.cpp \
net_permissions.cpp \
outputtype.cpp \
+ policy/v3_policy.cpp \
policy/feerate.cpp \
policy/policy.cpp \
protocol.cpp \
@@ -960,6 +963,7 @@ 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 \
diff --git a/src/policy/rbf.cpp b/src/policy/rbf.cpp
index 76ab2b1a96..f0830d8f22 100644
--- a/src/policy/rbf.cpp
+++ b/src/policy/rbf.cpp
@@ -115,11 +115,11 @@ std::optional<std::string> HasNoNewUnconfirmed(const CTransaction& tx,
}
std::optional<std::string> EntriesAndTxidsDisjoint(const CTxMemPool::setEntries& ancestors,
- const std::set<uint256>& direct_conflicts,
+ const std::set<Txid>& direct_conflicts,
const uint256& txid)
{
for (CTxMemPool::txiter ancestorIt : ancestors) {
- const uint256& hashAncestor = ancestorIt->GetTx().GetHash();
+ const Txid& hashAncestor = ancestorIt->GetTx().GetHash();
if (direct_conflicts.count(hashAncestor)) {
return strprintf("%s spends conflicting transaction %s",
txid.ToString(),
diff --git a/src/policy/rbf.h b/src/policy/rbf.h
index fff9828482..5a33ed64a3 100644
--- a/src/policy/rbf.h
+++ b/src/policy/rbf.h
@@ -80,7 +80,7 @@ std::optional<std::string> HasNoNewUnconfirmed(const CTransaction& tx, const CTx
* @returns error message if the sets intersect, std::nullopt if they are disjoint.
*/
std::optional<std::string> EntriesAndTxidsDisjoint(const CTxMemPool::setEntries& ancestors,
- const std::set<uint256>& direct_conflicts,
+ const std::set<Txid>& direct_conflicts,
const uint256& txid);
/** Check that the feerate of the replacement transaction(s) is higher than the feerate of each
diff --git a/src/policy/v3_policy.cpp b/src/policy/v3_policy.cpp
new file mode 100644
index 0000000000..158881aeb9
--- /dev/null
+++ b/src/policy/v3_policy.cpp
@@ -0,0 +1,220 @@
+// 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 <util/check.h>
+
+#include <algorithm>
+#include <numeric>
+#include <vector>
+
+/** Helper for PackageV3Checks: 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)
+{
+ std::vector<size_t> in_package_parents;
+
+ std::set<Txid> possible_parents;
+ for (auto &input : ptx->vin) {
+ possible_parents.insert(input.prevout.hash);
+ }
+
+ for (size_t i{0}; i < package.size(); ++i) {
+ const auto& tx = package.at(i);
+ // We assume the package is sorted, so that we don't need to continue
+ // looking past the transaction itself.
+ if (&(*tx) == &(*ptx)) break;
+ if (possible_parents.count(tx->GetHash())) {
+ in_package_parents.push_back(i);
+ }
+ }
+ return in_package_parents;
+}
+
+/** Helper for PackageV3Checks, 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;
+ /** nVersion used to check inheritance of v3 and non-v3 */
+ decltype(CTransaction::nVersion) m_version;
+ /** If parent is in mempool, whether it has any descendants in mempool. */
+ bool m_has_mempool_descendant;
+
+ ParentInfo() = delete;
+ ParentInfo(const Txid& txid, const Wtxid& wtxid, decltype(CTransaction::nVersion) version, bool has_mempool_descendant) :
+ m_txid{txid}, m_wtxid{wtxid}, m_version{version},
+ m_has_mempool_descendant{has_mempool_descendant}
+ {}
+};
+
+std::optional<std::string> PackageV3Checks(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);
+
+ const auto in_package_parents{FindInPackageParents(package, ptx)};
+
+ // Now we have all ancestors, so we can start checking v3 rules.
+ if (ptx->nVersion == 3) {
+ if (mempool_ancestors.size() + in_package_parents.size() + 1 > V3_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",
+ ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(),
+ vsize, V3_CHILD_MAX_VSIZE);
+ }
+
+ const auto parent_info = [&] {
+ if (mempool_ancestors.size() > 0) {
+ // There's a parent in the mempool.
+ auto& mempool_parent = *mempool_ancestors.begin();
+ Assume(mempool_parent->GetCountWithDescendants() == 1);
+ return ParentInfo{mempool_parent->GetTx().GetHash(),
+ mempool_parent->GetTx().GetWitnessHash(),
+ mempool_parent->GetTx().nVersion,
+ /*has_mempool_descendant=*/mempool_parent->GetCountWithDescendants() > 1};
+ } else {
+ // Ancestor must be in the package. Find it.
+ auto& parent_index = in_package_parents.front();
+ auto& package_parent = package.at(parent_index);
+ return ParentInfo{package_parent->GetHash(),
+ package_parent->GetWitnessHash(),
+ package_parent->nVersion,
+ /*has_mempool_descendant=*/false};
+ }
+ }();
+
+ // If there is a parent, it must have the right version.
+ if (parent_info.m_version != 3) {
+ return strprintf("v3 tx %s (wtxid=%s) cannot spend from non-v3 tx %s (wtxid=%s)",
+ ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(),
+ parent_info.m_txid.ToString(), parent_info.m_wtxid.ToString());
+ }
+
+ for (const auto& package_tx : package) {
+ // Skip same tx.
+ if (&(*package_tx) == &(*ptx)) continue;
+
+ 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
+ // are within the same package.
+ if (input.prevout.hash == parent_info.m_txid) {
+ return strprintf("tx %s (wtxid=%s) would exceed descendant count limit",
+ parent_info.m_txid.ToString(),
+ parent_info.m_wtxid.ToString());
+ }
+
+ // This tx can't have both a parent and an in-package child.
+ if (input.prevout.hash == ptx->GetHash()) {
+ return strprintf("tx %s (wtxid=%s) would have too many ancestors",
+ package_tx->GetHash().ToString(), package_tx->GetWitnessHash().ToString());
+ }
+ }
+ }
+
+ // It shouldn't be possible to have any mempool siblings at this point. SingleV3Checks
+ // catches mempool siblings. Also, if the package consists of connected transactions,
+ // any tx having a mempool ancestor would mean the package exceeds ancestor limits.
+ if (!Assume(!parent_info.m_has_mempool_descendant)) {
+ return strprintf("tx %u would exceed descendant count limit", parent_info.m_wtxid.ToString());
+ }
+ }
+ } else {
+ // Non-v3 transactions cannot have v3 parents.
+ for (auto it : mempool_ancestors) {
+ if (it->GetTx().nVersion == 3) {
+ return strprintf("non-v3 tx %s (wtxid=%s) cannot spend from v3 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)->nVersion == 3) {
+ return strprintf("non-v3 tx %s (wtxid=%s) cannot spend from v3 tx %s (wtxid=%s)",
+ ptx->GetHash().ToString(),
+ ptx->GetWitnessHash().ToString(),
+ package.at(index)->GetHash().ToString(),
+ package.at(index)->GetWitnessHash().ToString());
+ }
+ }
+ }
+ return std::nullopt;
+}
+
+std::optional<std::string> SingleV3Checks(const CTransactionRef& ptx,
+ const CTxMemPool::setEntries& mempool_ancestors,
+ const std::set<Txid>& direct_conflicts,
+ int64_t vsize)
+{
+ // Check v3 and non-v3 inheritance.
+ for (const auto& entry : mempool_ancestors) {
+ if (ptx->nVersion != 3 && entry->GetTx().nVersion == 3) {
+ return strprintf("non-v3 tx %s (wtxid=%s) cannot spend from v3 tx %s (wtxid=%s)",
+ ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(),
+ entry->GetSharedTx()->GetHash().ToString(), entry->GetSharedTx()->GetWitnessHash().ToString());
+ } else if (ptx->nVersion == 3 && entry->GetTx().nVersion != 3) {
+ return strprintf("v3 tx %s (wtxid=%s) cannot spend from non-v3 tx %s (wtxid=%s)",
+ ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(),
+ entry->GetSharedTx()->GetHash().ToString(), entry->GetSharedTx()->GetWitnessHash().ToString());
+ }
+ }
+
+ // 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);
+
+ // The rest of the rules only apply to transactions with nVersion=3.
+ if (ptx->nVersion != 3) return std::nullopt;
+
+ // Check that V3_ANCESTOR_LIMIT would not be violated, including both in-package and in-mempool.
+ if (mempool_ancestors.size() + 1 > V3_ANCESTOR_LIMIT) {
+ return strprintf("tx %s (wtxid=%s) would have too many ancestors",
+ ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString());
+ }
+
+ // 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 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);
+ }
+
+ // Check the descendant counts of in-mempool ancestors.
+ const auto& parent_entry = *mempool_ancestors.begin();
+ // If there are any ancestors, this is the only child allowed. The parent cannot have any
+ // other descendants. We handle the possibility of multiple children as that case is
+ // 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.
+ 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 (wtxid=%s) would exceed descendant count limit",
+ parent_entry->GetSharedTx()->GetHash().ToString(),
+ parent_entry->GetSharedTx()->GetWitnessHash().ToString());
+ }
+ }
+ return std::nullopt;
+}
diff --git a/src/policy/v3_policy.h b/src/policy/v3_policy.h
new file mode 100644
index 0000000000..9e871915e5
--- /dev/null
+++ b/src/policy/v3_policy.h
@@ -0,0 +1,83 @@
+// 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_V3_POLICY_H
+#define BITCOIN_POLICY_V3_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 transactions with nVersion=3 ("v3 transactions") which help make
+// RBF abilities more robust.
+
+// v3 only allows 1 parent and 1 child when unconfirmed.
+/** Maximum number of transactions including an unconfirmed tx and its descendants. */
+static constexpr unsigned int V3_DESCENDANT_LIMIT{2};
+/** Maximum number of transactions including a V3 tx and all its mempool ancestors. */
+static constexpr unsigned int V3_ANCESTOR_LIMIT{2};
+
+/** Maximum sigop-adjusted virtual size of a tx which spends from an unconfirmed v3 transaction. */
+static constexpr int64_t V3_CHILD_MAX_VSIZE{1000};
+// These limits are within the default ancestor/descendant limits.
+static_assert(V3_CHILD_MAX_VSIZE + MAX_STANDARD_TX_WEIGHT / WITNESS_SCALE_FACTOR <= DEFAULT_ANCESTOR_SIZE_LIMIT_KVB * 1000);
+static_assert(V3_CHILD_MAX_VSIZE + MAX_STANDARD_TX_WEIGHT / WITNESS_SCALE_FACTOR <= DEFAULT_DESCENDANT_SIZE_LIMIT_KVB * 1000);
+
+/** Must be called for every transaction, even if not v3. Not strictly necessary for transactions
+ * accepted through AcceptMultipleTransactions.
+ *
+ * Checks the following rules:
+ * 1. A v3 tx must only have v3 unconfirmed ancestors.
+ * 2. A non-v3 tx must only have non-v3 unconfirmed ancestors.
+ * 3. A v3's ancestor set, including itself, must be within V3_ANCESTOR_LIMIT.
+ * 4. A v3's descendant set, including itself, must be within V3_DESCENDANT_LIMIT.
+ * 5. If a v3 tx has any unconfirmed ancestors, the tx's sigop-adjusted vsize must be within
+ * V3_CHILD_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 debug string if an error occurs, std::nullopt otherwise.
+ */
+std::optional<std::string> SingleV3Checks(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 v3.
+ *
+ * For each transaction in a package:
+ * If it's not a v3 transaction, verify it has no direct v3 parents in the mempool or the package.
+
+ * If it is a v3 transaction, verify that any direct parents in the mempool or the package are v3.
+ * 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 v3 violations in the package exist, this test will fail for one of them:
+ * - if a v3 transaction T has a parent in the mempool and a child in the package, then PV3C(T) will fail
+ * - if a v3 transaction T has a parent in the package and a child in the package, then PV3C(T) will fail
+ * - if a v3 transaction T and a v3 (sibling) transaction U have some parent in the mempool,
+ * then PV3C(T) and PV3C(U) will fail
+ * - if a v3 transaction T and a v3 (sibling) transaction U have some parent in the package,
+ * then PV3C(T) and PV3C(U) will fail
+ * - if a v3 transaction T has a parent P and a grandparent G in the package, then
+ * PV3C(P) will fail (though PV3C(G) and PV3C(T) might succeed).
+ *
+ * @returns debug string if an error occurs, std::nullopt otherwise.
+ * */
+std::optional<std::string> PackageV3Checks(const CTransactionRef& ptx, int64_t vsize,
+ const Package& package,
+ const CTxMemPool::setEntries& mempool_ancestors);
+
+#endif // BITCOIN_POLICY_V3_POLICY_H
diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp
index 04d2e68939..c1753a1f6e 100644
--- a/src/rpc/mempool.cpp
+++ b/src/rpc/mempool.cpp
@@ -199,7 +199,7 @@ static RPCHelpMan testmempoolaccept()
result_inner.pushKV("txid", tx->GetHash().GetHex());
result_inner.pushKV("wtxid", tx->GetWitnessHash().GetHex());
if (package_result.m_state.GetResult() == PackageValidationResult::PCKG_POLICY) {
- result_inner.pushKV("package-error", package_result.m_state.GetRejectReason());
+ result_inner.pushKV("package-error", package_result.m_state.ToString());
}
auto it = package_result.m_tx_results.find(tx->GetWitnessHash());
if (exit_early || it == package_result.m_tx_results.end()) {
@@ -907,7 +907,7 @@ static RPCHelpMan submitpackage()
case PackageValidationResult::PCKG_TX:
{
// Package-wide error we want to return, but we also want to return individual responses
- package_msg = package_result.m_state.GetRejectReason();
+ package_msg = package_result.m_state.ToString();
CHECK_NONFATAL(package_result.m_tx_results.size() == txns.size() ||
package_result.m_tx_results.empty());
break;
diff --git a/src/test/fuzz/package_eval.cpp b/src/test/fuzz/package_eval.cpp
index 5a08d0ff44..9e658e0ced 100644
--- a/src/test/fuzz/package_eval.cpp
+++ b/src/test/fuzz/package_eval.cpp
@@ -6,6 +6,7 @@
#include <node/context.h>
#include <node/mempool_args.h>
#include <node/miner.h>
+#include <policy/v3_policy.h>
#include <test/fuzz/FuzzedDataProvider.h>
#include <test/fuzz/fuzz.h>
#include <test/fuzz/util.h>
@@ -119,7 +120,8 @@ CTxMemPool MakeMempool(FuzzedDataProvider& fuzzed_data_provider, const NodeConte
mempool_opts.limits.descendant_size_vbytes = fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(0, 202) * 1'000;
mempool_opts.max_size_bytes = fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(0, 200) * 1'000'000;
mempool_opts.expiry = std::chrono::hours{fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(0, 999)};
- nBytesPerSigOp = fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(1, 999);
+ // Only interested in 2 cases: sigop cost 0 or when single legacy sigop cost is >> 1KvB
+ nBytesPerSigOp = fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(0, 1) * 10'000;
mempool_opts.check_ratio = 1;
mempool_opts.require_standard = fuzzed_data_provider.ConsumeBool();
@@ -171,11 +173,11 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool)
// Create transaction to add to the mempool
const CTransactionRef tx = [&] {
CMutableTransaction tx_mut;
- tx_mut.nVersion = CTransaction::CURRENT_VERSION;
+ tx_mut.nVersion = fuzzed_data_provider.ConsumeBool() ? 3 : CTransaction::CURRENT_VERSION;
tx_mut.nLockTime = fuzzed_data_provider.ConsumeBool() ? 0 : fuzzed_data_provider.ConsumeIntegral<uint32_t>();
// Last tx will sweep all outpoints in package
const auto num_in = last_tx ? package_outpoints.size() : fuzzed_data_provider.ConsumeIntegralInRange<int>(1, mempool_outpoints.size());
- const auto num_out = fuzzed_data_provider.ConsumeIntegralInRange<int>(1, mempool_outpoints.size() * 2);
+ auto num_out = fuzzed_data_provider.ConsumeIntegralInRange<int>(1, mempool_outpoints.size() * 2);
auto& outpoints = last_tx ? package_outpoints : mempool_outpoints;
@@ -211,17 +213,24 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool)
tx_mut.vin.push_back(tx_mut.vin.back());
}
- // Refer to a non-existant input
+ // Refer to a non-existent input
if (fuzzed_data_provider.ConsumeBool()) {
tx_mut.vin.emplace_back();
}
+ // Make a p2pk output to make sigops adjusted vsize to violate v3, potentially, which is never spent
+ if (last_tx && amount_in > 1000 && fuzzed_data_provider.ConsumeBool()) {
+ tx_mut.vout.emplace_back(1000, CScript() << std::vector<unsigned char>(33, 0x02) << OP_CHECKSIG);
+ // Don't add any other outputs.
+ num_out = 1;
+ amount_in -= 1000;
+ }
+
const auto amount_fee = fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(0, amount_in);
const auto amount_out = (amount_in - amount_fee) / num_out;
for (int i = 0; i < num_out; ++i) {
tx_mut.vout.emplace_back(amount_out, P2WSH_EMPTY);
}
- // TODO vary transaction sizes to catch size-related issues
auto tx = MakeTransactionRef(tx_mut);
// Restore previously removed outpoints, except in-package outpoints
if (!last_tx) {
@@ -261,7 +270,6 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool)
std::set<CTransactionRef> added;
auto txr = std::make_shared<TransactionsDelta>(added);
RegisterSharedValidationInterface(txr);
- const bool bypass_limits = fuzzed_data_provider.ConsumeBool();
// When there are multiple transactions in the package, we call ProcessNewPackage(txs, test_accept=false)
// and AcceptToMemoryPool(txs.back(), test_accept=true). When there is only 1 transaction, we might flip it
@@ -271,17 +279,20 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool)
const auto result_package = WITH_LOCK(::cs_main,
return ProcessNewPackage(chainstate, tx_pool, txs, /*test_accept=*/single_submit));
- const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, txs.back(), GetTime(), bypass_limits, /*test_accept=*/!single_submit));
- const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID;
+ // Always set bypass_limits to false because it is not supported in ProcessNewPackage and
+ // can be a source of divergence.
+ const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, txs.back(), GetTime(),
+ /*bypass_limits=*/false, /*test_accept=*/!single_submit));
+ const bool passed = res.m_result_type == MempoolAcceptResult::ResultType::VALID;
SyncWithValidationInterfaceQueue();
UnregisterSharedValidationInterface(txr);
// There is only 1 transaction in the package. We did a test-package-accept and a ATMP
if (single_submit) {
- Assert(accepted != added.empty());
- Assert(accepted == res.m_state.IsValid());
- if (accepted) {
+ Assert(passed != added.empty());
+ Assert(passed == res.m_state.IsValid());
+ if (passed) {
Assert(added.size() == 1);
Assert(txs.back() == *added.begin());
}
@@ -295,6 +306,8 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool)
// This is empty if it fails early checks, or "full" if transactions are looked at deeper
Assert(result_package.m_tx_results.size() == txs.size() || result_package.m_tx_results.empty());
}
+
+ CheckMempoolV3Invariants(tx_pool);
}
UnregisterSharedValidationInterface(outpoints_updater);
diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp
index 8c0b0d7029..b44b528b6f 100644
--- a/src/test/fuzz/tx_pool.cpp
+++ b/src/test/fuzz/tx_pool.cpp
@@ -6,6 +6,7 @@
#include <node/context.h>
#include <node/mempool_args.h>
#include <node/miner.h>
+#include <policy/v3_policy.h>
#include <test/fuzz/FuzzedDataProvider.h>
#include <test/fuzz/fuzz.h>
#include <test/fuzz/util.h>
@@ -227,7 +228,7 @@ FUZZ_TARGET(tx_pool_standard, .init = initialize_tx_pool)
// Create transaction to add to the mempool
const CTransactionRef tx = [&] {
CMutableTransaction tx_mut;
- tx_mut.nVersion = CTransaction::CURRENT_VERSION;
+ tx_mut.nVersion = fuzzed_data_provider.ConsumeBool() ? 3 : CTransaction::CURRENT_VERSION;
tx_mut.nLockTime = fuzzed_data_provider.ConsumeBool() ? 0 : fuzzed_data_provider.ConsumeIntegral<uint32_t>();
const auto num_in = fuzzed_data_provider.ConsumeIntegralInRange<int>(1, outpoints_rbf.size());
const auto num_out = fuzzed_data_provider.ConsumeIntegralInRange<int>(1, outpoints_rbf.size() * 2);
@@ -313,6 +314,7 @@ FUZZ_TARGET(tx_pool_standard, .init = initialize_tx_pool)
if (accepted) {
Assert(added.size() == 1); // For now, no package acceptance
Assert(tx == *added.begin());
+ CheckMempoolV3Invariants(tx_pool);
} else {
// Do not consider rejected transaction removed
removed.erase(tx);
@@ -405,6 +407,9 @@ FUZZ_TARGET(tx_pool, .init = initialize_tx_pool)
const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID;
if (accepted) {
txids.push_back(tx->GetHash());
+ // Only check fees if accepted and not bypass_limits, otherwise it's not guaranteed that
+ // trimming has happened for this tx and previous iterations.
+ CheckMempoolV3Invariants(tx_pool);
}
}
Finish(fuzzed_data_provider, tx_pool, chainstate);
diff --git a/src/test/rbf_tests.cpp b/src/test/rbf_tests.cpp
index fb6a3614c0..e6c135eed9 100644
--- a/src/test/rbf_tests.cpp
+++ b/src/test/rbf_tests.cpp
@@ -135,8 +135,6 @@ BOOST_FIXTURE_TEST_CASE(rbf_helper_functions, TestChain100Setup)
// Tests for EntriesAndTxidsDisjoint
BOOST_CHECK(EntriesAndTxidsDisjoint(empty_set, {tx1->GetHash()}, unused_txid) == std::nullopt);
BOOST_CHECK(EntriesAndTxidsDisjoint(set_12_normal, {tx3->GetHash()}, unused_txid) == std::nullopt);
- // EntriesAndTxidsDisjoint uses txids, not wtxids.
- BOOST_CHECK(EntriesAndTxidsDisjoint({entry2}, {tx2->GetWitnessHash()}, unused_txid) == std::nullopt);
BOOST_CHECK(EntriesAndTxidsDisjoint({entry2}, {tx2->GetHash()}, unused_txid).has_value());
BOOST_CHECK(EntriesAndTxidsDisjoint(set_12_normal, {tx1->GetHash()}, unused_txid).has_value());
BOOST_CHECK(EntriesAndTxidsDisjoint(set_12_normal, {tx2->GetHash()}, unused_txid).has_value());
diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp
index d1cb2531aa..e6cf64611e 100644
--- a/src/test/transaction_tests.cpp
+++ b/src/test/transaction_tests.cpp
@@ -786,7 +786,7 @@ BOOST_AUTO_TEST_CASE(test_IsStandard)
t.nVersion = 0;
CheckIsNotStandard(t, "version");
- t.nVersion = 3;
+ t.nVersion = TX_MAX_STANDARD_VERSION + 1;
CheckIsNotStandard(t, "version");
// Allowed nVersion
diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp
index ecf0889711..98d5e892f9 100644
--- a/src/test/txvalidation_tests.cpp
+++ b/src/test/txvalidation_tests.cpp
@@ -4,11 +4,14 @@
#include <consensus/validation.h>
#include <key_io.h>
+#include <policy/v3_policy.h>
#include <policy/packages.h>
#include <policy/policy.h>
#include <primitives/transaction.h>
+#include <random.h>
#include <script/script.h>
#include <test/util/setup_common.h>
+#include <test/util/txmempool.h>
#include <validation.h>
#include <boost/test/unit_test.hpp>
@@ -48,4 +51,290 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_reject_coinbase, TestChain100Setup)
BOOST_CHECK_EQUAL(result.m_state.GetRejectReason(), "coinbase");
BOOST_CHECK(result.m_state.GetResult() == TxValidationResult::TX_CONSENSUS);
}
+
+// Generate a number of random, nonexistent outpoints.
+static inline std::vector<COutPoint> random_outpoints(size_t num_outpoints) {
+ std::vector<COutPoint> outpoints;
+ for (size_t i{0}; i < num_outpoints; ++i) {
+ outpoints.emplace_back(Txid::FromUint256(GetRandHash()), 0);
+ }
+ return outpoints;
+}
+
+static inline std::vector<CPubKey> random_keys(size_t num_keys) {
+ std::vector<CPubKey> keys;
+ keys.reserve(num_keys);
+ for (size_t i{0}; i < num_keys; ++i) {
+ CKey key;
+ key.MakeNewKey(true);
+ keys.emplace_back(key.GetPubKey());
+ }
+ return keys;
+}
+
+// Creates a placeholder tx (not valid) with 25 outputs. Specify the nVersion and the inputs.
+static inline CTransactionRef make_tx(const std::vector<COutPoint>& inputs, int32_t version)
+{
+ CMutableTransaction mtx = CMutableTransaction{};
+ mtx.nVersion = version;
+ mtx.vin.resize(inputs.size());
+ mtx.vout.resize(25);
+ for (size_t i{0}; i < inputs.size(); ++i) {
+ mtx.vin[i].prevout = inputs[i];
+ }
+ for (auto i{0}; i < 25; ++i) {
+ mtx.vout[i].scriptPubKey = CScript() << OP_TRUE;
+ mtx.vout[i].nValue = 10000;
+ }
+ return MakeTransactionRef(mtx);
+}
+
+BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
+{
+ // Test V3 policy helper functions
+ CTxMemPool& pool = *Assert(m_node.mempool);
+ LOCK2(cs_main, pool.cs);
+ TestMemPoolEntryHelper entry;
+ std::set<Txid> empty_conflicts_set;
+ CTxMemPool::setEntries empty_ancestors;
+
+ auto mempool_tx_v3 = make_tx(random_outpoints(1), /*version=*/3);
+ pool.addUnchecked(entry.FromTx(mempool_tx_v3));
+ auto mempool_tx_v2 = make_tx(random_outpoints(1), /*version=*/2);
+ pool.addUnchecked(entry.FromTx(mempool_tx_v2));
+ // Default values.
+ CTxMemPool::Limits m_limits{};
+
+ // Cannot spend from an unconfirmed v3 transaction unless this tx is also v3.
+ {
+ // mempool_tx_v3
+ // ^
+ // tx_v2_from_v3
+ auto tx_v2_from_v3 = make_tx({COutPoint{mempool_tx_v3->GetHash(), 0}}, /*version=*/2);
+ auto ancestors_v2_from_v3{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v2_from_v3), m_limits)};
+ const auto expected_error_str{strprintf("non-v3 tx %s (wtxid=%s) cannot spend from v3 tx %s (wtxid=%s)",
+ tx_v2_from_v3->GetHash().ToString(), tx_v2_from_v3->GetWitnessHash().ToString(),
+ mempool_tx_v3->GetHash().ToString(), mempool_tx_v3->GetWitnessHash().ToString())};
+ BOOST_CHECK(*SingleV3Checks(tx_v2_from_v3, *ancestors_v2_from_v3, empty_conflicts_set, GetVirtualTransactionSize(*tx_v2_from_v3)) == expected_error_str);
+
+ Package package_v3_v2{mempool_tx_v3, tx_v2_from_v3};
+ BOOST_CHECK_EQUAL(*PackageV3Checks(tx_v2_from_v3, GetVirtualTransactionSize(*tx_v2_from_v3), package_v3_v2, empty_ancestors), expected_error_str);
+
+ // mempool_tx_v3 mempool_tx_v2
+ // ^ ^
+ // tx_v2_from_v2_and_v3
+ auto tx_v2_from_v2_and_v3 = make_tx({COutPoint{mempool_tx_v3->GetHash(), 0}, COutPoint{mempool_tx_v2->GetHash(), 0}}, /*version=*/2);
+ auto ancestors_v2_from_both{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v2_from_v2_and_v3), m_limits)};
+ const auto expected_error_str_2{strprintf("non-v3 tx %s (wtxid=%s) cannot spend from v3 tx %s (wtxid=%s)",
+ tx_v2_from_v2_and_v3->GetHash().ToString(), tx_v2_from_v2_and_v3->GetWitnessHash().ToString(),
+ mempool_tx_v3->GetHash().ToString(), mempool_tx_v3->GetWitnessHash().ToString())};
+ BOOST_CHECK(*SingleV3Checks(tx_v2_from_v2_and_v3, *ancestors_v2_from_both, empty_conflicts_set, GetVirtualTransactionSize(*tx_v2_from_v2_and_v3))
+ == expected_error_str_2);
+
+ Package package_v3_v2_v2{mempool_tx_v3, mempool_tx_v2, tx_v2_from_v2_and_v3};
+ BOOST_CHECK_EQUAL(*PackageV3Checks(tx_v2_from_v2_and_v3, GetVirtualTransactionSize(*tx_v2_from_v2_and_v3), package_v3_v2_v2, empty_ancestors), expected_error_str_2);
+ }
+
+ // V3 cannot spend from an unconfirmed non-v3 transaction.
+ {
+ // mempool_tx_v2
+ // ^
+ // tx_v3_from_v2
+ auto tx_v3_from_v2 = make_tx({COutPoint{mempool_tx_v2->GetHash(), 0}}, /*version=*/3);
+ auto ancestors_v3_from_v2{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_from_v2), m_limits)};
+ const auto expected_error_str{strprintf("v3 tx %s (wtxid=%s) cannot spend from non-v3 tx %s (wtxid=%s)",
+ tx_v3_from_v2->GetHash().ToString(), tx_v3_from_v2->GetWitnessHash().ToString(),
+ mempool_tx_v2->GetHash().ToString(), mempool_tx_v2->GetWitnessHash().ToString())};
+ BOOST_CHECK(*SingleV3Checks(tx_v3_from_v2, *ancestors_v3_from_v2, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_from_v2)) == expected_error_str);
+
+ Package package_v2_v3{mempool_tx_v2, tx_v3_from_v2};
+ BOOST_CHECK_EQUAL(*PackageV3Checks(tx_v3_from_v2, GetVirtualTransactionSize(*tx_v3_from_v2), package_v2_v3, empty_ancestors), expected_error_str);
+
+ // mempool_tx_v3 mempool_tx_v2
+ // ^ ^
+ // tx_v3_from_v2_and_v3
+ auto tx_v3_from_v2_and_v3 = make_tx({COutPoint{mempool_tx_v3->GetHash(), 0}, COutPoint{mempool_tx_v2->GetHash(), 0}}, /*version=*/3);
+ auto ancestors_v3_from_both{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_from_v2_and_v3), m_limits)};
+ const auto expected_error_str_2{strprintf("v3 tx %s (wtxid=%s) cannot spend from non-v3 tx %s (wtxid=%s)",
+ tx_v3_from_v2_and_v3->GetHash().ToString(), tx_v3_from_v2_and_v3->GetWitnessHash().ToString(),
+ mempool_tx_v2->GetHash().ToString(), mempool_tx_v2->GetWitnessHash().ToString())};
+ BOOST_CHECK(*SingleV3Checks(tx_v3_from_v2_and_v3, *ancestors_v3_from_both, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_from_v2_and_v3))
+ == expected_error_str_2);
+
+ // tx_v3_from_v2_and_v3 also violates V3_ANCESTOR_LIMIT.
+ const auto expected_error_str_3{strprintf("tx %s (wtxid=%s) would have too many ancestors",
+ tx_v3_from_v2_and_v3->GetHash().ToString(), tx_v3_from_v2_and_v3->GetWitnessHash().ToString())};
+ Package package_v3_v2_v3{mempool_tx_v3, mempool_tx_v2, tx_v3_from_v2_and_v3};
+ BOOST_CHECK_EQUAL(*PackageV3Checks(tx_v3_from_v2_and_v3, GetVirtualTransactionSize(*tx_v3_from_v2_and_v3), package_v3_v2_v3, empty_ancestors), expected_error_str_3);
+ }
+ // V3 from V3 is ok, and non-V3 from non-V3 is ok.
+ {
+ // mempool_tx_v3
+ // ^
+ // tx_v3_from_v3
+ auto tx_v3_from_v3 = make_tx({COutPoint{mempool_tx_v3->GetHash(), 0}}, /*version=*/3);
+ auto ancestors_v3{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_from_v3), m_limits)};
+ BOOST_CHECK(SingleV3Checks(tx_v3_from_v3, *ancestors_v3, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_from_v3))
+ == std::nullopt);
+
+ Package package_v3_v3{mempool_tx_v3, tx_v3_from_v3};
+ BOOST_CHECK(PackageV3Checks(tx_v3_from_v3, GetVirtualTransactionSize(*tx_v3_from_v3), package_v3_v3, empty_ancestors) == std::nullopt);
+
+ // mempool_tx_v2
+ // ^
+ // tx_v2_from_v2
+ auto tx_v2_from_v2 = make_tx({COutPoint{mempool_tx_v2->GetHash(), 0}}, /*version=*/2);
+ auto ancestors_v2{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v2_from_v2), m_limits)};
+ BOOST_CHECK(SingleV3Checks(tx_v2_from_v2, *ancestors_v2, empty_conflicts_set, GetVirtualTransactionSize(*tx_v2_from_v2))
+ == std::nullopt);
+
+ Package package_v2_v2{mempool_tx_v2, tx_v2_from_v2};
+ BOOST_CHECK(PackageV3Checks(tx_v2_from_v2, GetVirtualTransactionSize(*tx_v2_from_v2), package_v2_v2, empty_ancestors) == std::nullopt);
+ }
+
+ // Tx spending v3 cannot have too many mempool ancestors
+ // Configuration where the tx has multiple direct parents.
+ {
+ Package package_multi_parents;
+ std::vector<COutPoint> mempool_outpoints;
+ mempool_outpoints.emplace_back(mempool_tx_v3->GetHash(), 0);
+ package_multi_parents.emplace_back(mempool_tx_v3);
+ for (size_t i{0}; i < 2; ++i) {
+ auto mempool_tx = make_tx(random_outpoints(i + 1), /*version=*/3);
+ pool.addUnchecked(entry.FromTx(mempool_tx));
+ mempool_outpoints.emplace_back(mempool_tx->GetHash(), 0);
+ package_multi_parents.emplace_back(mempool_tx);
+ }
+ auto tx_v3_multi_parent = make_tx(mempool_outpoints, /*version=*/3);
+ package_multi_parents.emplace_back(tx_v3_multi_parent);
+ auto ancestors{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_multi_parent), m_limits)};
+ BOOST_CHECK_EQUAL(ancestors->size(), 3);
+ const auto expected_error_str{strprintf("tx %s (wtxid=%s) would have too many ancestors",
+ tx_v3_multi_parent->GetHash().ToString(), tx_v3_multi_parent->GetWitnessHash().ToString())};
+ BOOST_CHECK_EQUAL(*SingleV3Checks(tx_v3_multi_parent, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_multi_parent)),
+ expected_error_str);
+
+ BOOST_CHECK_EQUAL(*PackageV3Checks(tx_v3_multi_parent, GetVirtualTransactionSize(*tx_v3_multi_parent), package_multi_parents, empty_ancestors),
+ expected_error_str);
+ }
+
+ // Configuration where the tx is in a multi-generation chain.
+ {
+ Package package_multi_gen;
+ CTransactionRef middle_tx;
+ auto last_outpoint{random_outpoints(1)[0]};
+ for (size_t i{0}; i < 2; ++i) {
+ auto mempool_tx = make_tx({last_outpoint}, /*version=*/3);
+ pool.addUnchecked(entry.FromTx(mempool_tx));
+ last_outpoint = COutPoint{mempool_tx->GetHash(), 0};
+ package_multi_gen.emplace_back(mempool_tx);
+ if (i == 1) middle_tx = mempool_tx;
+ }
+ auto tx_v3_multi_gen = make_tx({last_outpoint}, /*version=*/3);
+ package_multi_gen.emplace_back(tx_v3_multi_gen);
+ auto ancestors{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_multi_gen), m_limits)};
+ const auto expected_error_str{strprintf("tx %s (wtxid=%s) would have too many ancestors",
+ tx_v3_multi_gen->GetHash().ToString(), tx_v3_multi_gen->GetWitnessHash().ToString())};
+ BOOST_CHECK_EQUAL(*SingleV3Checks(tx_v3_multi_gen, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_multi_gen)),
+ expected_error_str);
+
+ // Middle tx is what triggers a failure for the grandchild:
+ BOOST_CHECK_EQUAL(*PackageV3Checks(middle_tx, GetVirtualTransactionSize(*middle_tx), package_multi_gen, empty_ancestors), expected_error_str);
+ BOOST_CHECK(PackageV3Checks(tx_v3_multi_gen, GetVirtualTransactionSize(*tx_v3_multi_gen), package_multi_gen, empty_ancestors) == std::nullopt);
+ }
+
+ // Tx spending v3 cannot be too large in virtual size.
+ auto many_inputs{random_outpoints(100)};
+ many_inputs.emplace_back(mempool_tx_v3->GetHash(), 0);
+ {
+ auto tx_v3_child_big = make_tx(many_inputs, /*version=*/3);
+ const auto vsize{GetVirtualTransactionSize(*tx_v3_child_big)};
+ auto ancestors{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_child_big), m_limits)};
+ const auto expected_error_str{strprintf("v3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
+ tx_v3_child_big->GetHash().ToString(), tx_v3_child_big->GetWitnessHash().ToString(), vsize, V3_CHILD_MAX_VSIZE)};
+ BOOST_CHECK_EQUAL(*SingleV3Checks(tx_v3_child_big, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_child_big)),
+ expected_error_str);
+
+ Package package_child_big{mempool_tx_v3, tx_v3_child_big};
+ BOOST_CHECK_EQUAL(*PackageV3Checks(tx_v3_child_big, GetVirtualTransactionSize(*tx_v3_child_big), package_child_big, empty_ancestors),
+ expected_error_str);
+ }
+
+ // Tx spending v3 cannot have too many sigops.
+ // This child has 10 P2WSH multisig inputs.
+ auto multisig_outpoints{random_outpoints(10)};
+ multisig_outpoints.emplace_back(mempool_tx_v3->GetHash(), 0);
+ auto keys{random_keys(2)};
+ CScript script_multisig;
+ script_multisig << OP_1;
+ for (const auto& key : keys) {
+ script_multisig << ToByteVector(key);
+ }
+ script_multisig << OP_2 << OP_CHECKMULTISIG;
+ {
+ CMutableTransaction mtx_many_sigops = CMutableTransaction{};
+ mtx_many_sigops.nVersion = 3;
+ for (const auto& outpoint : multisig_outpoints) {
+ mtx_many_sigops.vin.emplace_back(outpoint);
+ mtx_many_sigops.vin.back().scriptWitness.stack.emplace_back(script_multisig.begin(), script_multisig.end());
+ }
+ mtx_many_sigops.vout.resize(1);
+ mtx_many_sigops.vout.back().scriptPubKey = CScript() << OP_TRUE;
+ mtx_many_sigops.vout.back().nValue = 10000;
+ auto tx_many_sigops{MakeTransactionRef(mtx_many_sigops)};
+
+ auto ancestors{pool.CalculateMemPoolAncestors(entry.FromTx(tx_many_sigops), m_limits)};
+ // legacy uses fAccurate = false, and the maximum number of multisig keys is used
+ const int64_t total_sigops{static_cast<int64_t>(tx_many_sigops->vin.size()) * static_cast<int64_t>(script_multisig.GetSigOpCount(/*fAccurate=*/false))};
+ BOOST_CHECK_EQUAL(total_sigops, tx_many_sigops->vin.size() * MAX_PUBKEYS_PER_MULTISIG);
+ const int64_t bip141_vsize{GetVirtualTransactionSize(*tx_many_sigops)};
+ // Weight limit is not reached...
+ BOOST_CHECK(SingleV3Checks(tx_many_sigops, *ancestors, empty_conflicts_set, bip141_vsize) == std::nullopt);
+ // ...but sigop limit is.
+ const auto expected_error_str{strprintf("v3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
+ tx_many_sigops->GetHash().ToString(), tx_many_sigops->GetWitnessHash().ToString(),
+ total_sigops * DEFAULT_BYTES_PER_SIGOP / WITNESS_SCALE_FACTOR, V3_CHILD_MAX_VSIZE)};
+ BOOST_CHECK_EQUAL(*SingleV3Checks(tx_many_sigops, *ancestors, empty_conflicts_set,
+ GetVirtualTransactionSize(*tx_many_sigops, /*nSigOpCost=*/total_sigops, /*bytes_per_sigop=*/ DEFAULT_BYTES_PER_SIGOP)),
+ expected_error_str);
+
+ Package package_child_sigops{mempool_tx_v3, tx_many_sigops};
+ BOOST_CHECK_EQUAL(*PackageV3Checks(tx_many_sigops, total_sigops * DEFAULT_BYTES_PER_SIGOP / WITNESS_SCALE_FACTOR, package_child_sigops, empty_ancestors),
+ expected_error_str);
+ }
+
+ // Parent + child with v3 in the mempool. Child is allowed as long as it is under V3_CHILD_MAX_VSIZE.
+ auto tx_mempool_v3_child = make_tx({COutPoint{mempool_tx_v3->GetHash(), 0}}, /*version=*/3);
+ {
+ BOOST_CHECK(GetTransactionWeight(*tx_mempool_v3_child) <= V3_CHILD_MAX_VSIZE * WITNESS_SCALE_FACTOR);
+ auto ancestors{pool.CalculateMemPoolAncestors(entry.FromTx(tx_mempool_v3_child), m_limits)};
+ BOOST_CHECK(SingleV3Checks(tx_mempool_v3_child, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_mempool_v3_child)) == std::nullopt);
+ pool.addUnchecked(entry.FromTx(tx_mempool_v3_child));
+
+ Package package_v3_1p1c{mempool_tx_v3, tx_mempool_v3_child};
+ BOOST_CHECK(PackageV3Checks(tx_mempool_v3_child, GetVirtualTransactionSize(*tx_mempool_v3_child), package_v3_1p1c, empty_ancestors) == std::nullopt);
+ }
+
+ // A v3 transaction cannot have more than 1 descendant.
+ // Configuration where tx has multiple direct children.
+ {
+ auto tx_v3_child2 = make_tx({COutPoint{mempool_tx_v3->GetHash(), 1}}, /*version=*/3);
+ auto ancestors{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_child2), m_limits)};
+ const auto expected_error_str{strprintf("tx %s (wtxid=%s) would exceed descendant count limit",
+ mempool_tx_v3->GetHash().ToString(), mempool_tx_v3->GetWitnessHash().ToString())};
+ BOOST_CHECK_EQUAL(*SingleV3Checks(tx_v3_child2, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_child2)),
+ expected_error_str);
+ // If replacing the child, make sure there is no double-counting.
+ BOOST_CHECK(SingleV3Checks(tx_v3_child2, *ancestors, {tx_mempool_v3_child->GetHash()}, GetVirtualTransactionSize(*tx_v3_child2))
+ == std::nullopt);
+
+ Package package_v3_1p2c{mempool_tx_v3, tx_mempool_v3_child, tx_v3_child2};
+ BOOST_CHECK_EQUAL(*PackageV3Checks(tx_v3_child2, GetVirtualTransactionSize(*tx_v3_child2), package_v3_1p2c, empty_ancestors),
+ expected_error_str);
+ }
+
+ // Configuration where tx has multiple generations of descendants is not tested because that is
+ // equivalent to the tx with multiple generations of ancestors.
+}
+
BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/test/util/txmempool.cpp b/src/test/util/txmempool.cpp
index 379c3c9329..3b4161ddd3 100644
--- a/src/test/util/txmempool.cpp
+++ b/src/test/util/txmempool.cpp
@@ -7,6 +7,7 @@
#include <chainparams.h>
#include <node/context.h>
#include <node/mempool_args.h>
+#include <policy/v3_policy.h>
#include <txmempool.h>
#include <util/check.h>
#include <util/time.h>
@@ -116,3 +117,28 @@ std::optional<std::string> CheckPackageMempoolAcceptResult(const Package& txns,
}
return std::nullopt;
}
+
+void CheckMempoolV3Invariants(const CTxMemPool& tx_pool)
+{
+ LOCK(tx_pool.cs);
+ for (const auto& tx_info : tx_pool.infoAll()) {
+ const auto& entry = *Assert(tx_pool.GetEntry(tx_info.tx->GetHash()));
+ if (tx_info.tx->nVersion == 3) {
+ // Check that special v3 ancestor/descendant limits and rules are always respected
+ Assert(entry.GetCountWithDescendants() <= V3_DESCENDANT_LIMIT);
+ Assert(entry.GetCountWithAncestors() <= V3_ANCESTOR_LIMIT);
+ // If this transaction has at least 1 ancestor, it's a "child" and has restricted weight.
+ if (entry.GetCountWithAncestors() > 1) {
+ Assert(entry.GetTxSize() <= V3_CHILD_MAX_VSIZE);
+ // All v3 transactions must only have v3 unconfirmed parents.
+ const auto& parents = entry.GetMemPoolParentsConst();
+ Assert(parents.begin()->get().GetSharedTx()->nVersion == 3);
+ }
+ } else if (entry.GetCountWithAncestors() > 1) {
+ // All non-v3 transactions must only have non-v3 unconfirmed parents.
+ for (const auto& parent : entry.GetMemPoolParentsConst()) {
+ Assert(parent.get().GetSharedTx()->nVersion != 3);
+ }
+ }
+ }
+}
diff --git a/src/test/util/txmempool.h b/src/test/util/txmempool.h
index a866d1ce74..b3022af7df 100644
--- a/src/test/util/txmempool.h
+++ b/src/test/util/txmempool.h
@@ -46,4 +46,14 @@ std::optional<std::string> CheckPackageMempoolAcceptResult(const Package& txns,
const PackageMempoolAcceptResult& result,
bool expect_valid,
const CTxMemPool* mempool);
+
+/** For every transaction in tx_pool, check v3 invariants:
+ * - a v3 tx's ancestor count must be within V3_ANCESTOR_LIMIT
+ * - a v3 tx's descendant count must be within V3_DESCENDANT_LIMIT
+ * - if a v3 tx has ancestors, its sigop-adjusted vsize must be within V3_CHILD_MAX_VSIZE
+ * - any non-v3 tx must only have non-v3 parents
+ * - any v3 tx must only have v3 parents
+ * */
+void CheckMempoolV3Invariants(const CTxMemPool& tx_pool);
+
#endif // BITCOIN_TEST_UTIL_TXMEMPOOL_H
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 57a86549d9..de340f6b6d 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -448,7 +448,7 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAnces
cachedInnerUsage += entry.DynamicMemoryUsage();
const CTransaction& tx = newit->GetTx();
- std::set<uint256> setParentTransactions;
+ std::set<Txid> setParentTransactions;
for (unsigned int i = 0; i < tx.vin.size(); i++) {
mapNextTx.insert(std::make_pair(&tx.vin[i].prevout, &tx));
setParentTransactions.insert(tx.vin[i].prevout.hash);
@@ -950,7 +950,7 @@ std::optional<CTxMemPool::txiter> CTxMemPool::GetIter(const uint256& txid) const
return std::nullopt;
}
-CTxMemPool::setEntries CTxMemPool::GetIterSet(const std::set<uint256>& hashes) const
+CTxMemPool::setEntries CTxMemPool::GetIterSet(const std::set<Txid>& hashes) const
{
CTxMemPool::setEntries ret;
for (const auto& h : hashes) {
diff --git a/src/txmempool.h b/src/txmempool.h
index ca842632da..b98355c65f 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -521,7 +521,7 @@ public:
/** Translate a set of hashes into a set of pool iterators to avoid repeated lookups.
* Does not require that all of the hashes correspond to actual transactions in the mempool,
* only returns the ones that exist. */
- setEntries GetIterSet(const std::set<uint256>& hashes) const EXCLUSIVE_LOCKS_REQUIRED(cs);
+ setEntries GetIterSet(const std::set<Txid>& hashes) const EXCLUSIVE_LOCKS_REQUIRED(cs);
/** Translate a list of hashes into a list of mempool iterators to avoid repeated lookups.
* The nth element in txids becomes the nth element in the returned vector. If any of the txids
diff --git a/src/validation.cpp b/src/validation.cpp
index caa4ff3115..0552bde665 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -29,6 +29,7 @@
#include <logging/timer.h>
#include <node/blockstorage.h>
#include <node/utxo_snapshot.h>
+#include <policy/v3_policy.h>
#include <policy/policy.h>
#include <policy/rbf.h>
#include <policy/settings.h>
@@ -333,7 +334,9 @@ void Chainstate::MaybeUpdateMempoolForReorg(
// Also updates valid entries' cached LockPoints if needed.
// If false, the tx is still valid and its lockpoints are updated.
// If true, the tx would be invalid in the next block; remove this entry and all of its descendants.
- const auto filter_final_and_mature = [this](CTxMemPool::txiter it)
+ // Note that v3 rules are not applied here, so reorgs may cause violations of v3 inheritance or
+ // topology restrictions.
+ const auto filter_final_and_mature = [&](CTxMemPool::txiter it)
EXCLUSIVE_LOCKS_REQUIRED(m_mempool->cs, ::cs_main) {
AssertLockHeld(m_mempool->cs);
AssertLockHeld(::cs_main);
@@ -583,7 +586,7 @@ private:
struct Workspace {
explicit Workspace(const CTransactionRef& ptx) : m_ptx(ptx), m_hash(ptx->GetHash()) {}
/** Txids of mempool transactions that this transaction directly conflicts with. */
- std::set<uint256> m_conflicts;
+ std::set<Txid> m_conflicts;
/** Iterators to mempool entries that this transaction directly conflicts with. */
CTxMemPool::setEntries m_iters_conflicting;
/** Iterators to all mempool entries that would be replaced by this transaction, including
@@ -761,9 +764,12 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
// check all unconfirmed ancestors; otherwise an opt-in ancestor
// might be replaced, causing removal of this descendant.
//
- // If replaceability signaling is ignored due to node setting,
- // replacement is always allowed.
- if (!m_pool.m_full_rbf && !SignalsOptInRBF(*ptxConflicting)) {
+ // All V3 transactions are considered replaceable.
+ //
+ // Replaceability signaling of the original transactions may be
+ // ignored due to node setting.
+ const bool allow_rbf{m_pool.m_full_rbf || SignalsOptInRBF(*ptxConflicting) || ptxConflicting->nVersion == 3};
+ if (!allow_rbf) {
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "txn-mempool-conflict");
}
@@ -865,7 +871,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
// while a tx could be package CPFP'd when entering the mempool, we do not have a DoS-resistant
// method of ensuring the tx remains bumped. For example, the fee-bumping child could disappear
// due to a replacement.
- if (!bypass_limits && ws.m_modified_fees < m_pool.m_min_relay_feerate.GetFee(ws.m_vsize)) {
+ // The only exception is v3 transactions.
+ if (!bypass_limits && ws.m_ptx->nVersion != 3 && ws.m_modified_fees < m_pool.m_min_relay_feerate.GetFee(ws.m_vsize)) {
// Even though this is a fee-related failure, this result is TX_MEMPOOL_POLICY, not
// TX_RECONSIDERABLE, because it cannot be bypassed using package validation.
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "min relay fee not met",
@@ -947,6 +954,9 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
}
ws.m_ancestors = *ancestors;
+ if (const auto err_string{SingleV3Checks(ws.m_ptx, ws.m_ancestors, ws.m_conflicts, ws.m_vsize)}) {
+ return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "v3-rule-violation", *err_string);
+ }
// A transaction that spends outputs that would be replaced by it is invalid. Now
// that we have the set of all ancestors we can detect this
@@ -1307,6 +1317,15 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::
m_viewmempool.PackageAddTransaction(ws.m_ptx);
}
+ // At this point we have all in-mempool ancestors, and we know every transaction's vsize.
+ // Run the v3 checks on the package.
+ for (Workspace& ws : workspaces) {
+ if (auto err{PackageV3Checks(ws.m_ptx, ws.m_vsize, txns, ws.m_ancestors)}) {
+ package_state.Invalid(PackageValidationResult::PCKG_POLICY, "v3-violation", err.value());
+ return PackageMempoolAcceptResult(package_state, {});
+ }
+ }
+
// Transactions must meet two minimum feerates: the mempool minimum fee and min relay fee.
// For transactions consisting of exactly one child and its parents, it suffices to use the
// package feerate (total modified fees / total virtual size) to check this requirement.