diff options
-rw-r--r-- | doc/policy/mempool-replacements.md | 3 | ||||
-rw-r--r-- | src/Makefile.am | 4 | ||||
-rw-r--r-- | src/policy/rbf.cpp | 4 | ||||
-rw-r--r-- | src/policy/rbf.h | 2 | ||||
-rw-r--r-- | src/policy/v3_policy.cpp | 220 | ||||
-rw-r--r-- | src/policy/v3_policy.h | 83 | ||||
-rw-r--r-- | src/rpc/mempool.cpp | 4 | ||||
-rw-r--r-- | src/test/fuzz/package_eval.cpp | 35 | ||||
-rw-r--r-- | src/test/fuzz/tx_pool.cpp | 7 | ||||
-rw-r--r-- | src/test/rbf_tests.cpp | 2 | ||||
-rw-r--r-- | src/test/transaction_tests.cpp | 2 | ||||
-rw-r--r-- | src/test/txvalidation_tests.cpp | 289 | ||||
-rw-r--r-- | src/test/util/txmempool.cpp | 26 | ||||
-rw-r--r-- | src/test/util/txmempool.h | 10 | ||||
-rw-r--r-- | src/txmempool.cpp | 4 | ||||
-rw-r--r-- | src/txmempool.h | 2 | ||||
-rw-r--r-- | src/validation.cpp | 31 | ||||
-rwxr-xr-x | test/functional/feature_bip68_sequence.py | 6 | ||||
-rwxr-xr-x | test/functional/mempool_accept_v3.py | 418 | ||||
-rwxr-xr-x | test/functional/mempool_package_limits.py | 2 | ||||
-rwxr-xr-x | test/functional/mempool_sigoplimit.py | 3 | ||||
-rw-r--r-- | test/functional/test_framework/wallet.py | 20 | ||||
-rwxr-xr-x | test/functional/test_runner.py | 1 |
23 files changed, 1136 insertions, 42 deletions
diff --git a/doc/policy/mempool-replacements.md b/doc/policy/mempool-replacements.md index b3c4239b73..96ab4112e2 100644 --- a/doc/policy/mempool-replacements.md +++ b/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 BIP125 replaceability if any of its inputs have an nSequence number less than (0xffffffff - 1). + A transaction also signals replaceibility if its nVersion field is set to 3. *Rationale*: See [BIP125 explanation](https://github.com/bitcoin/bips/blob/master/bip-0125.mediawiki#motivation). 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. diff --git a/test/functional/feature_bip68_sequence.py b/test/functional/feature_bip68_sequence.py index 894afffc79..8768d4040d 100755 --- a/test/functional/feature_bip68_sequence.py +++ b/test/functional/feature_bip68_sequence.py @@ -408,10 +408,8 @@ class BIP68Test(BitcoinTestFramework): # Use self.nodes[1] to test that version 2 transactions are standard. def test_version2_relay(self): mini_wallet = MiniWallet(self.nodes[1]) - mini_wallet.rescan_utxos() - tx = mini_wallet.create_self_transfer()["tx"] - tx.nVersion = 2 - mini_wallet.sendrawtransaction(from_node=self.nodes[1], tx_hex=tx.serialize().hex()) + mini_wallet.send_self_transfer(from_node=self.nodes[1], version=2) + if __name__ == '__main__': BIP68Test().main() diff --git a/test/functional/mempool_accept_v3.py b/test/functional/mempool_accept_v3.py new file mode 100755 index 0000000000..ca599a9993 --- /dev/null +++ b/test/functional/mempool_accept_v3.py @@ -0,0 +1,418 @@ +#!/usr/bin/env python3 +# Copyright (c) 2024 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +from test_framework.messages import ( + MAX_BIP125_RBF_SEQUENCE, +) +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, + assert_greater_than, + assert_greater_than_or_equal, + assert_raises_rpc_error, +) +from test_framework.wallet import ( + DEFAULT_FEE, + MiniWallet, +) + +def cleanup(extra_args=None): + def decorator(func): + def wrapper(self): + try: + if extra_args is not None: + self.restart_node(0, extra_args=extra_args) + func(self) + finally: + # Clear mempool again after test + self.generate(self.nodes[0], 1) + if extra_args is not None: + self.restart_node(0) + return wrapper + return decorator + +class MempoolAcceptV3(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + self.extra_args = [["-acceptnonstdtxn=1"]] + self.setup_clean_chain = True + + def check_mempool(self, txids): + """Assert exact contents of the node's mempool (by txid).""" + mempool_contents = self.nodes[0].getrawmempool() + assert_equal(len(txids), len(mempool_contents)) + assert all([txid in txids for txid in mempool_contents]) + + @cleanup(extra_args=["-datacarriersize=1000", "-acceptnonstdtxn=1"]) + def test_v3_acceptance(self): + node = self.nodes[0] + self.log.info("Test a child of a v3 transaction cannot be more than 1000vB") + tx_v3_parent_normal = self.wallet.send_self_transfer(from_node=node, version=3) + self.check_mempool([tx_v3_parent_normal["txid"]]) + tx_v3_child_heavy = self.wallet.create_self_transfer( + utxo_to_spend=tx_v3_parent_normal["new_utxo"], + target_weight=4004, + version=3 + ) + assert_greater_than_or_equal(tx_v3_child_heavy["tx"].get_vsize(), 1000) + expected_error_child_heavy = f"v3-rule-violation, v3 child tx {tx_v3_child_heavy['txid']} (wtxid={tx_v3_child_heavy['wtxid']}) is too big" + assert_raises_rpc_error(-26, expected_error_child_heavy, node.sendrawtransaction, tx_v3_child_heavy["hex"]) + self.check_mempool([tx_v3_parent_normal["txid"]]) + # tx has no descendants + assert_equal(node.getmempoolentry(tx_v3_parent_normal["txid"])["descendantcount"], 1) + + self.log.info("Test that, during replacements, only the new transaction counts for v3 descendant limit") + tx_v3_child_almost_heavy = self.wallet.send_self_transfer( + from_node=node, + fee_rate=DEFAULT_FEE, + utxo_to_spend=tx_v3_parent_normal["new_utxo"], + target_weight=3987, + version=3 + ) + assert_greater_than_or_equal(1000, tx_v3_child_almost_heavy["tx"].get_vsize()) + self.check_mempool([tx_v3_parent_normal["txid"], tx_v3_child_almost_heavy["txid"]]) + assert_equal(node.getmempoolentry(tx_v3_parent_normal["txid"])["descendantcount"], 2) + tx_v3_child_almost_heavy_rbf = self.wallet.send_self_transfer( + from_node=node, + fee_rate=DEFAULT_FEE * 2, + utxo_to_spend=tx_v3_parent_normal["new_utxo"], + target_weight=3500, + version=3 + ) + assert_greater_than_or_equal(tx_v3_child_almost_heavy["tx"].get_vsize() + tx_v3_child_almost_heavy_rbf["tx"].get_vsize(), 1000) + self.check_mempool([tx_v3_parent_normal["txid"], tx_v3_child_almost_heavy_rbf["txid"]]) + assert_equal(node.getmempoolentry(tx_v3_parent_normal["txid"])["descendantcount"], 2) + + @cleanup(extra_args=["-acceptnonstdtxn=1"]) + def test_v3_replacement(self): + node = self.nodes[0] + self.log.info("Test v3 transactions may be replaced by v3 transactions") + utxo_v3_bip125 = self.wallet.get_utxo() + tx_v3_bip125 = self.wallet.send_self_transfer( + from_node=node, + fee_rate=DEFAULT_FEE, + utxo_to_spend=utxo_v3_bip125, + sequence=MAX_BIP125_RBF_SEQUENCE, + version=3 + ) + self.check_mempool([tx_v3_bip125["txid"]]) + + tx_v3_bip125_rbf = self.wallet.send_self_transfer( + from_node=node, + fee_rate=DEFAULT_FEE * 2, + utxo_to_spend=utxo_v3_bip125, + version=3 + ) + self.check_mempool([tx_v3_bip125_rbf["txid"]]) + + self.log.info("Test v3 transactions may be replaced by V2 transactions") + tx_v3_bip125_rbf_v2 = self.wallet.send_self_transfer( + from_node=node, + fee_rate=DEFAULT_FEE * 3, + utxo_to_spend=utxo_v3_bip125, + version=2 + ) + self.check_mempool([tx_v3_bip125_rbf_v2["txid"]]) + + self.log.info("Test that replacements cannot cause violation of inherited v3") + utxo_v3_parent = self.wallet.get_utxo() + tx_v3_parent = self.wallet.send_self_transfer( + from_node=node, + fee_rate=DEFAULT_FEE, + utxo_to_spend=utxo_v3_parent, + version=3 + ) + tx_v3_child = self.wallet.send_self_transfer( + from_node=node, + fee_rate=DEFAULT_FEE, + utxo_to_spend=tx_v3_parent["new_utxo"], + version=3 + ) + self.check_mempool([tx_v3_bip125_rbf_v2["txid"], tx_v3_parent["txid"], tx_v3_child["txid"]]) + + tx_v3_child_rbf_v2 = self.wallet.create_self_transfer( + fee_rate=DEFAULT_FEE * 2, + utxo_to_spend=tx_v3_parent["new_utxo"], + version=2 + ) + expected_error_v2_v3 = f"v3-rule-violation, non-v3 tx {tx_v3_child_rbf_v2['txid']} (wtxid={tx_v3_child_rbf_v2['wtxid']}) cannot spend from v3 tx {tx_v3_parent['txid']} (wtxid={tx_v3_parent['wtxid']})" + assert_raises_rpc_error(-26, expected_error_v2_v3, node.sendrawtransaction, tx_v3_child_rbf_v2["hex"]) + self.check_mempool([tx_v3_bip125_rbf_v2["txid"], tx_v3_parent["txid"], tx_v3_child["txid"]]) + + + @cleanup(extra_args=["-acceptnonstdtxn=1"]) + def test_v3_bip125(self): + node = self.nodes[0] + self.log.info("Test v3 transactions that don't signal BIP125 are replaceable") + assert_equal(node.getmempoolinfo()["fullrbf"], False) + utxo_v3_no_bip125 = self.wallet.get_utxo() + tx_v3_no_bip125 = self.wallet.send_self_transfer( + from_node=node, + fee_rate=DEFAULT_FEE, + utxo_to_spend=utxo_v3_no_bip125, + sequence=MAX_BIP125_RBF_SEQUENCE + 1, + version=3 + ) + + self.check_mempool([tx_v3_no_bip125["txid"]]) + assert not node.getmempoolentry(tx_v3_no_bip125["txid"])["bip125-replaceable"] + tx_v3_no_bip125_rbf = self.wallet.send_self_transfer( + from_node=node, + fee_rate=DEFAULT_FEE * 2, + utxo_to_spend=utxo_v3_no_bip125, + version=3 + ) + self.check_mempool([tx_v3_no_bip125_rbf["txid"]]) + + @cleanup(extra_args=["-datacarriersize=40000", "-acceptnonstdtxn=1"]) + def test_v3_reorg(self): + node = self.nodes[0] + self.log.info("Test that, during a reorg, v3 rules are not enforced") + tx_v2_block = self.wallet.send_self_transfer(from_node=node, version=2) + tx_v3_block = self.wallet.send_self_transfer(from_node=node, version=3) + tx_v3_block2 = self.wallet.send_self_transfer(from_node=node, version=3) + self.check_mempool([tx_v3_block["txid"], tx_v2_block["txid"], tx_v3_block2["txid"]]) + + block = self.generate(node, 1) + self.check_mempool([]) + tx_v2_from_v3 = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=tx_v3_block["new_utxo"], version=2) + tx_v3_from_v2 = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=tx_v2_block["new_utxo"], version=3) + tx_v3_child_large = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=tx_v3_block2["new_utxo"], target_weight=5000, version=3) + assert_greater_than(node.getmempoolentry(tx_v3_child_large["txid"])["vsize"], 1000) + self.check_mempool([tx_v2_from_v3["txid"], tx_v3_from_v2["txid"], tx_v3_child_large["txid"]]) + node.invalidateblock(block[0]) + self.check_mempool([tx_v3_block["txid"], tx_v2_block["txid"], tx_v3_block2["txid"], tx_v2_from_v3["txid"], tx_v3_from_v2["txid"], tx_v3_child_large["txid"]]) + # This is needed because generate() will create the exact same block again. + node.reconsiderblock(block[0]) + + + @cleanup(extra_args=["-limitdescendantsize=10", "-datacarriersize=40000", "-acceptnonstdtxn=1"]) + def test_nondefault_package_limits(self): + """ + Max standard tx size + v3 rules imply the ancestor/descendant rules (at their default + values), but those checks must not be skipped. Ensure both sets of checks are done by + changing the ancestor/descendant limit configurations. + """ + node = self.nodes[0] + self.log.info("Test that a decreased limitdescendantsize also applies to v3 child") + tx_v3_parent_large1 = self.wallet.send_self_transfer(from_node=node, target_weight=99900, version=3) + tx_v3_child_large1 = self.wallet.create_self_transfer(utxo_to_spend=tx_v3_parent_large1["new_utxo"], version=3) + # Child is within v3 limits, but parent's descendant limit is exceeded + assert_greater_than(1000, tx_v3_child_large1["tx"].get_vsize()) + assert_raises_rpc_error(-26, f"too-long-mempool-chain, exceeds descendant size limit for tx {tx_v3_parent_large1['txid']}", node.sendrawtransaction, tx_v3_child_large1["hex"]) + self.check_mempool([tx_v3_parent_large1["txid"]]) + assert_equal(node.getmempoolentry(tx_v3_parent_large1["txid"])["descendantcount"], 1) + self.generate(node, 1) + + self.log.info("Test that a decreased limitancestorsize also applies to v3 parent") + self.restart_node(0, extra_args=["-limitancestorsize=10", "-datacarriersize=40000", "-acceptnonstdtxn=1"]) + tx_v3_parent_large2 = self.wallet.send_self_transfer(from_node=node, target_weight=99900, version=3) + tx_v3_child_large2 = self.wallet.create_self_transfer(utxo_to_spend=tx_v3_parent_large2["new_utxo"], version=3) + # Child is within v3 limits + assert_greater_than_or_equal(1000, tx_v3_child_large2["tx"].get_vsize()) + assert_raises_rpc_error(-26, f"too-long-mempool-chain, exceeds ancestor size limit", node.sendrawtransaction, tx_v3_child_large2["hex"]) + self.check_mempool([tx_v3_parent_large2["txid"]]) + + @cleanup(extra_args=["-datacarriersize=1000", "-acceptnonstdtxn=1"]) + def test_v3_ancestors_package(self): + self.log.info("Test that v3 ancestor limits are checked within the package") + node = self.nodes[0] + tx_v3_parent_normal = self.wallet.create_self_transfer( + fee_rate=0, + target_weight=4004, + version=3 + ) + tx_v3_parent_2_normal = self.wallet.create_self_transfer( + fee_rate=0, + target_weight=4004, + version=3 + ) + tx_v3_child_multiparent = self.wallet.create_self_transfer_multi( + utxos_to_spend=[tx_v3_parent_normal["new_utxo"], tx_v3_parent_2_normal["new_utxo"]], + fee_per_output=10000, + version=3 + ) + tx_v3_child_heavy = self.wallet.create_self_transfer_multi( + utxos_to_spend=[tx_v3_parent_normal["new_utxo"]], + target_weight=4004, + fee_per_output=10000, + version=3 + ) + + self.check_mempool([]) + result = node.submitpackage([tx_v3_parent_normal["hex"], tx_v3_parent_2_normal["hex"], tx_v3_child_multiparent["hex"]]) + assert_equal(result['package_msg'], f"v3-violation, tx {tx_v3_child_multiparent['txid']} (wtxid={tx_v3_child_multiparent['wtxid']}) would have too many ancestors") + self.check_mempool([]) + + self.check_mempool([]) + result = node.submitpackage([tx_v3_parent_normal["hex"], tx_v3_child_heavy["hex"]]) + # tx_v3_child_heavy is heavy based on weight, not sigops. + assert_equal(result['package_msg'], f"v3-violation, v3 child tx {tx_v3_child_heavy['txid']} (wtxid={tx_v3_child_heavy['wtxid']}) is too big: {tx_v3_child_heavy['tx'].get_vsize()} > 1000 virtual bytes") + self.check_mempool([]) + + tx_v3_parent = self.wallet.create_self_transfer(version=3) + tx_v3_child = self.wallet.create_self_transfer(utxo_to_spend=tx_v3_parent["new_utxo"], version=3) + tx_v3_grandchild = self.wallet.create_self_transfer(utxo_to_spend=tx_v3_child["new_utxo"], version=3) + result = node.testmempoolaccept([tx_v3_parent["hex"], tx_v3_child["hex"], tx_v3_grandchild["hex"]]) + assert all([txresult["package-error"] == f"v3-violation, tx {tx_v3_grandchild['txid']} (wtxid={tx_v3_grandchild['wtxid']}) would have too many ancestors" for txresult in result]) + + @cleanup(extra_args=["-acceptnonstdtxn=1"]) + def test_v3_ancestors_package_and_mempool(self): + """ + A v3 transaction in a package cannot have 2 v3 parents. + Test that if we have a transaction graph A -> B -> C, where A, B, C are + all v3 transactions, that we cannot use submitpackage to get the + transactions all into the mempool. + + Verify, in particular, that if A is already in the mempool, then + submitpackage(B, C) will fail. + """ + node = self.nodes[0] + self.log.info("Test that v3 ancestor limits include transactions within the package and all in-mempool ancestors") + # This is our transaction "A": + tx_in_mempool = self.wallet.send_self_transfer(from_node=node, version=3) + + # Verify that A is in the mempool + self.check_mempool([tx_in_mempool["txid"]]) + + # tx_0fee_parent is our transaction "B"; just create it. + tx_0fee_parent = self.wallet.create_self_transfer(utxo_to_spend=tx_in_mempool["new_utxo"], fee=0, fee_rate=0, version=3) + + # tx_child_violator is our transaction "C"; create it: + tx_child_violator = self.wallet.create_self_transfer_multi(utxos_to_spend=[tx_0fee_parent["new_utxo"]], version=3) + + # submitpackage(B, C) should fail + result = node.submitpackage([tx_0fee_parent["hex"], tx_child_violator["hex"]]) + assert_equal(result['package_msg'], f"v3-violation, tx {tx_child_violator['txid']} (wtxid={tx_child_violator['wtxid']}) would have too many ancestors") + self.check_mempool([tx_in_mempool["txid"]]) + + @cleanup(extra_args=["-acceptnonstdtxn=1"]) + def test_mempool_sibling(self): + self.log.info("Test that v3 transaction cannot have mempool siblings") + node = self.nodes[0] + # Add a parent + child to mempool + tx_mempool_parent = self.wallet.send_self_transfer_multi( + from_node=node, + utxos_to_spend=[self.wallet.get_utxo()], + num_outputs=2, + version=3 + ) + tx_mempool_sibling = self.wallet.send_self_transfer( + from_node=node, + utxo_to_spend=tx_mempool_parent["new_utxos"][0], + version=3 + ) + self.check_mempool([tx_mempool_parent["txid"], tx_mempool_sibling["txid"]]) + + tx_has_mempool_sibling = self.wallet.create_self_transfer( + utxo_to_spend=tx_mempool_parent["new_utxos"][1], + version=3 + ) + expected_error_mempool_sibling = f"v3-rule-violation, tx {tx_mempool_parent['txid']} (wtxid={tx_mempool_parent['wtxid']}) would exceed descendant count limit" + assert_raises_rpc_error(-26, expected_error_mempool_sibling, node.sendrawtransaction, tx_has_mempool_sibling["hex"]) + + tx_has_mempool_uncle = self.wallet.create_self_transfer(utxo_to_spend=tx_has_mempool_sibling["new_utxo"], version=3) + + # Also fails with another non-related transaction via testmempoolaccept + tx_unrelated = self.wallet.create_self_transfer(version=3) + result_test_unrelated = node.testmempoolaccept([tx_has_mempool_sibling["hex"], tx_unrelated["hex"]]) + assert_equal(result_test_unrelated[0]["reject-reason"], "v3-rule-violation") + + result_test_1p1c = node.testmempoolaccept([tx_has_mempool_sibling["hex"], tx_has_mempool_uncle["hex"]]) + assert_equal(result_test_1p1c[0]["reject-reason"], "v3-rule-violation") + + # Also fails with a child via submitpackage + result_submitpackage = node.submitpackage([tx_has_mempool_sibling["hex"], tx_has_mempool_uncle["hex"]]) + assert_equal(result_submitpackage["tx-results"][tx_has_mempool_sibling['wtxid']]['error'], expected_error_mempool_sibling) + + + @cleanup(extra_args=["-datacarriersize=1000", "-acceptnonstdtxn=1"]) + def test_v3_package_inheritance(self): + self.log.info("Test that v3 inheritance is checked within package") + node = self.nodes[0] + tx_v3_parent = self.wallet.create_self_transfer( + fee_rate=0, + target_weight=4004, + version=3 + ) + tx_v2_child = self.wallet.create_self_transfer_multi( + utxos_to_spend=[tx_v3_parent["new_utxo"]], + fee_per_output=10000, + version=2 + ) + self.check_mempool([]) + result = node.submitpackage([tx_v3_parent["hex"], tx_v2_child["hex"]]) + assert_equal(result['package_msg'], f"v3-violation, non-v3 tx {tx_v2_child['txid']} (wtxid={tx_v2_child['wtxid']}) cannot spend from v3 tx {tx_v3_parent['txid']} (wtxid={tx_v3_parent['wtxid']})") + self.check_mempool([]) + + @cleanup(extra_args=["-acceptnonstdtxn=1"]) + def test_v3_in_testmempoolaccept(self): + node = self.nodes[0] + + self.log.info("Test that v3 inheritance is accurately assessed in testmempoolaccept") + tx_v2 = self.wallet.create_self_transfer(version=2) + tx_v2_from_v2 = self.wallet.create_self_transfer(utxo_to_spend=tx_v2["new_utxo"], version=2) + tx_v3_from_v2 = self.wallet.create_self_transfer(utxo_to_spend=tx_v2["new_utxo"], version=3) + tx_v3 = self.wallet.create_self_transfer(version=3) + tx_v2_from_v3 = self.wallet.create_self_transfer(utxo_to_spend=tx_v3["new_utxo"], version=2) + tx_v3_from_v3 = self.wallet.create_self_transfer(utxo_to_spend=tx_v3["new_utxo"], version=3) + + # testmempoolaccept paths don't require child-with-parents topology. Ensure that topology + # assumptions aren't made in inheritance checks. + test_accept_v2_and_v3 = node.testmempoolaccept([tx_v2["hex"], tx_v3["hex"]]) + assert all([result["allowed"] for result in test_accept_v2_and_v3]) + + test_accept_v3_from_v2 = node.testmempoolaccept([tx_v2["hex"], tx_v3_from_v2["hex"]]) + expected_error_v3_from_v2 = f"v3-violation, v3 tx {tx_v3_from_v2['txid']} (wtxid={tx_v3_from_v2['wtxid']}) cannot spend from non-v3 tx {tx_v2['txid']} (wtxid={tx_v2['wtxid']})" + assert all([result["package-error"] == expected_error_v3_from_v2 for result in test_accept_v3_from_v2]) + + test_accept_v2_from_v3 = node.testmempoolaccept([tx_v3["hex"], tx_v2_from_v3["hex"]]) + expected_error_v2_from_v3 = f"v3-violation, non-v3 tx {tx_v2_from_v3['txid']} (wtxid={tx_v2_from_v3['wtxid']}) cannot spend from v3 tx {tx_v3['txid']} (wtxid={tx_v3['wtxid']})" + assert all([result["package-error"] == expected_error_v2_from_v3 for result in test_accept_v2_from_v3]) + + test_accept_pairs = node.testmempoolaccept([tx_v2["hex"], tx_v3["hex"], tx_v2_from_v2["hex"], tx_v3_from_v3["hex"]]) + assert all([result["allowed"] for result in test_accept_pairs]) + + self.log.info("Test that descendant violations are caught in testmempoolaccept") + tx_v3_independent = self.wallet.create_self_transfer(version=3) + tx_v3_parent = self.wallet.create_self_transfer_multi(num_outputs=2, version=3) + tx_v3_child_1 = self.wallet.create_self_transfer(utxo_to_spend=tx_v3_parent["new_utxos"][0], version=3) + tx_v3_child_2 = self.wallet.create_self_transfer(utxo_to_spend=tx_v3_parent["new_utxos"][1], version=3) + test_accept_2children = node.testmempoolaccept([tx_v3_parent["hex"], tx_v3_child_1["hex"], tx_v3_child_2["hex"]]) + expected_error_2children = f"v3-violation, tx {tx_v3_parent['txid']} (wtxid={tx_v3_parent['wtxid']}) would exceed descendant count limit" + assert all([result["package-error"] == expected_error_2children for result in test_accept_2children]) + + # Extra v3 transaction does not get incorrectly marked as extra descendant + test_accept_1child_with_exra = node.testmempoolaccept([tx_v3_parent["hex"], tx_v3_child_1["hex"], tx_v3_independent["hex"]]) + assert all([result["allowed"] for result in test_accept_1child_with_exra]) + + # Extra v3 transaction does not make us ignore the extra descendant + test_accept_2children_with_exra = node.testmempoolaccept([tx_v3_parent["hex"], tx_v3_child_1["hex"], tx_v3_child_2["hex"], tx_v3_independent["hex"]]) + expected_error_extra = f"v3-violation, tx {tx_v3_parent['txid']} (wtxid={tx_v3_parent['wtxid']}) would exceed descendant count limit" + assert all([result["package-error"] == expected_error_extra for result in test_accept_2children_with_exra]) + # Same result if the parent is already in mempool + node.sendrawtransaction(tx_v3_parent["hex"]) + test_accept_2children_with_in_mempool_parent = node.testmempoolaccept([tx_v3_child_1["hex"], tx_v3_child_2["hex"]]) + assert all([result["package-error"] == expected_error_extra for result in test_accept_2children_with_in_mempool_parent]) + + def run_test(self): + self.log.info("Generate blocks to create UTXOs") + node = self.nodes[0] + self.wallet = MiniWallet(node) + self.generate(self.wallet, 110) + self.test_v3_acceptance() + self.test_v3_replacement() + self.test_v3_bip125() + self.test_v3_reorg() + self.test_nondefault_package_limits() + self.test_v3_ancestors_package() + self.test_v3_ancestors_package_and_mempool() + self.test_mempool_sibling() + self.test_v3_package_inheritance() + self.test_v3_in_testmempoolaccept() + + +if __name__ == "__main__": + MempoolAcceptV3().main() diff --git a/test/functional/mempool_package_limits.py b/test/functional/mempool_package_limits.py index 81451bf2a5..2a64597511 100755 --- a/test/functional/mempool_package_limits.py +++ b/test/functional/mempool_package_limits.py @@ -29,7 +29,7 @@ def check_package_limits(func): testres_error_expected = node.testmempoolaccept(rawtxs=package_hex) assert_equal(len(testres_error_expected), len(package_hex)) for txres in testres_error_expected: - assert_equal(txres["package-error"], "package-mempool-limits") + assert "package-mempool-limits" in txres["package-error"] # Clear mempool and check that the package passes now self.generate(node, 1) diff --git a/test/functional/mempool_sigoplimit.py b/test/functional/mempool_sigoplimit.py index 2e7850fb40..384423e5f5 100755 --- a/test/functional/mempool_sigoplimit.py +++ b/test/functional/mempool_sigoplimit.py @@ -165,7 +165,8 @@ class BytesPerSigOpTest(BitcoinTestFramework): # But together, it's exceeding limits in the *package* context. If sigops adjusted vsize wasn't being checked # here, it would get further in validation and give too-long-mempool-chain error instead. packet_test = self.nodes[0].testmempoolaccept([tx_parent.serialize().hex(), tx_child.serialize().hex()]) - assert_equal([x["package-error"] for x in packet_test], ["package-mempool-limits", "package-mempool-limits"]) + expected_package_error = f"package-mempool-limits, package size {2*20*5000} exceeds ancestor size limit [limit: 101000]" + assert_equal([x["package-error"] for x in packet_test], [expected_package_error] * 2) # When we actually try to submit, the parent makes it into the mempool, but the child would exceed ancestor vsize limits res = self.nodes[0].submitpackage([tx_parent.serialize().hex(), tx_child.serialize().hex()]) diff --git a/test/functional/test_framework/wallet.py b/test/functional/test_framework/wallet.py index 53c8e1b0cc..470ed08ed4 100644 --- a/test/functional/test_framework/wallet.py +++ b/test/functional/test_framework/wallet.py @@ -286,11 +286,12 @@ class MiniWallet: utxos_to_spend: Optional[list[dict]] = None, num_outputs=1, amount_per_output=0, + version=2, locktime=0, sequence=0, fee_per_output=1000, target_weight=0, - confirmed_only=False + confirmed_only=False, ): """ Create and return a transaction that spends the given UTXOs and creates a @@ -313,6 +314,7 @@ class MiniWallet: tx = CTransaction() tx.vin = [CTxIn(COutPoint(int(utxo_to_spend['txid'], 16), utxo_to_spend['vout']), nSequence=seq) for utxo_to_spend, seq in zip(utxos_to_spend, sequence)] tx.vout = [CTxOut(amount_per_output, bytearray(self._scriptPubKey)) for _ in range(num_outputs)] + tx.nVersion = version tx.nLockTime = locktime self.sign_tx(tx) @@ -337,14 +339,15 @@ class MiniWallet: "tx": tx, } - def create_self_transfer(self, *, + def create_self_transfer( + self, + *, fee_rate=Decimal("0.003"), fee=Decimal("0"), utxo_to_spend=None, - locktime=0, - sequence=0, target_weight=0, - confirmed_only=False + confirmed_only=False, + **kwargs, ): """Create and return a tx with the specified fee. If fee is 0, use fee_rate, where the resulting fee may be exact or at most one satoshi higher than needed.""" utxo_to_spend = utxo_to_spend or self.get_utxo(confirmed_only=confirmed_only) @@ -360,7 +363,12 @@ class MiniWallet: send_value = utxo_to_spend["value"] - (fee or (fee_rate * vsize / 1000)) # create tx - tx = self.create_self_transfer_multi(utxos_to_spend=[utxo_to_spend], locktime=locktime, sequence=sequence, amount_per_output=int(COIN * send_value), target_weight=target_weight) + tx = self.create_self_transfer_multi( + utxos_to_spend=[utxo_to_spend], + amount_per_output=int(COIN * send_value), + target_weight=target_weight, + **kwargs, + ) if not target_weight: assert_equal(tx["tx"].get_vsize(), vsize) tx["new_utxo"] = tx.pop("new_utxos")[0] diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index d037ccf6dd..e438a60edc 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -266,6 +266,7 @@ BASE_SCRIPTS = [ 'p2p_v2_encrypted.py', 'p2p_v2_earlykeyresponse.py', 'example_test.py', + 'mempool_accept_v3.py', 'wallet_txn_doublespend.py --legacy-wallet', 'wallet_multisig_descriptor_psbt.py --descriptors', 'wallet_txn_doublespend.py --descriptors', |