diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index da0010acc6ab6e..53e01adcb0b5b6 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1198,7 +1198,7 @@ static RPCHelpMan testmempoolaccept() RPCResult{ RPCResult::Type::ARR, "", "The result of the mempool acceptance test for each raw transaction in the input array.\n" "Returns results for each transaction in the same order they were passed in.\n" - "It is possible for transactions to not be fully validated ('allowed' unset) if another transaction failed.\n", + "Transactions that cannot be fully validated due to failures in other transactions will not contain an 'allowed' result.\n", { {RPCResult::Type::OBJ, "", "", { diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 1f125a4c171bc1..149d3362a4e012 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -159,33 +159,17 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector &vHashes } } -bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntries &setAncestors, uint64_t limitAncestorCount, uint64_t limitAncestorSize, uint64_t limitDescendantCount, uint64_t limitDescendantSize, std::string &errString, bool fSearchForParents /* = true */) const -{ - CTxMemPoolEntry::Parents staged_ancestors; - const CTransaction &tx = entry.GetTx(); - - if (fSearchForParents) { - // Get parents of this transaction that are in the mempool - // GetMemPoolParents() is only valid for entries in the mempool, so we - // iterate mapTx to find parents. - for (unsigned int i = 0; i < tx.vin.size(); i++) { - std::optional piter = GetIter(tx.vin[i].prevout.hash); - if (piter) { - staged_ancestors.insert(**piter); - if (staged_ancestors.size() + 1 > limitAncestorCount) { - errString = strprintf("too many unconfirmed parents [limit: %u]", limitAncestorCount); - return false; - } - } - } - } else { - // If we're not searching for parents, we require this to be an - // entry in the mempool already. - txiter it = mapTx.iterator_to(entry); - staged_ancestors = it->GetMemPoolParentsConst(); - } - - size_t totalSizeWithAncestors = entry.GetTxSize(); +bool CTxMemPool::CalculateAncestorsAndCheckLimits(size_t entry_size, + size_t entry_count, + setEntries& setAncestors, + CTxMemPoolEntry::Parents& staged_ancestors, + uint64_t limitAncestorCount, + uint64_t limitAncestorSize, + uint64_t limitDescendantCount, + uint64_t limitDescendantSize, + std::string &errString) const +{ + size_t totalSizeWithAncestors = entry_size; while (!staged_ancestors.empty()) { const CTxMemPoolEntry& stage = staged_ancestors.begin()->get(); @@ -195,10 +179,10 @@ bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntr staged_ancestors.erase(stage); totalSizeWithAncestors += stageit->GetTxSize(); - if (stageit->GetSizeWithDescendants() + entry.GetTxSize() > limitDescendantSize) { + if (stageit->GetSizeWithDescendants() + entry_size > limitDescendantSize) { errString = strprintf("exceeds descendant size limit for tx %s [limit: %u]", stageit->GetTx().GetHash().ToString(), limitDescendantSize); return false; - } else if (stageit->GetCountWithDescendants() + 1 > limitDescendantCount) { + } else if (stageit->GetCountWithDescendants() + entry_count > limitDescendantCount) { errString = strprintf("too many descendants for tx %s [limit: %u]", stageit->GetTx().GetHash().ToString(), limitDescendantCount); return false; } else if (totalSizeWithAncestors > limitAncestorSize) { @@ -214,7 +198,7 @@ bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntr if (setAncestors.count(parent_it) == 0) { staged_ancestors.insert(parent); } - if (staged_ancestors.size() + setAncestors.size() + 1 > limitAncestorCount) { + if (staged_ancestors.size() + setAncestors.size() + entry_count > limitAncestorCount) { errString = strprintf("too many unconfirmed ancestors [limit: %u]", limitAncestorCount); return false; } @@ -224,6 +208,80 @@ bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntr return true; } +bool CTxMemPool::CheckPackageLimits(const Package& package, + uint64_t limitAncestorCount, + uint64_t limitAncestorSize, + uint64_t limitDescendantCount, + uint64_t limitDescendantSize, + std::string &errString) const +{ + CTxMemPoolEntry::Parents staged_ancestors; + size_t total_size = 0; + for (const auto& tx : package) { + total_size += GetVirtualTransactionSize(*tx); + for (const auto& input : tx->vin) { + std::optional piter = GetIter(input.prevout.hash); + if (piter) { + staged_ancestors.insert(**piter); + if (staged_ancestors.size() + package.size() > limitAncestorCount) { + errString = strprintf("too many unconfirmed parents [limit: %u]", limitAncestorCount); + return false; + } + } + } + } + // When multiple transactions are passed in, the ancestors and descendants of all transactions + // considered together must be within limits even if they are not interdependent. This may be + // stricter than the limits for each individual transaction. + setEntries setAncestors; + const auto ret = CalculateAncestorsAndCheckLimits(total_size, package.size(), + setAncestors, staged_ancestors, + limitAncestorCount, limitAncestorSize, + limitDescendantCount, limitDescendantSize, errString); + // It's possible to overestimate the ancestor/descendant totals. + if (!ret) errString.insert(0, "possibly "); + return ret; +} + +bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, + setEntries &setAncestors, + uint64_t limitAncestorCount, + uint64_t limitAncestorSize, + uint64_t limitDescendantCount, + uint64_t limitDescendantSize, + std::string &errString, + bool fSearchForParents /* = true */) const +{ + CTxMemPoolEntry::Parents staged_ancestors; + const CTransaction &tx = entry.GetTx(); + + if (fSearchForParents) { + // Get parents of this transaction that are in the mempool + // GetMemPoolParents() is only valid for entries in the mempool, so we + // iterate mapTx to find parents. + for (unsigned int i = 0; i < tx.vin.size(); i++) { + std::optional piter = GetIter(tx.vin[i].prevout.hash); + if (piter) { + staged_ancestors.insert(**piter); + if (staged_ancestors.size() + 1 > limitAncestorCount) { + errString = strprintf("too many unconfirmed parents [limit: %u]", limitAncestorCount); + return false; + } + } + } + } else { + // If we're not searching for parents, we require this to already be an + // entry in the mempool and use the entry's cached parents. + txiter it = mapTx.iterator_to(entry); + staged_ancestors = it->GetMemPoolParentsConst(); + } + + return CalculateAncestorsAndCheckLimits(entry.GetTxSize(), /* entry_count */ 1, + setAncestors, staged_ancestors, + limitAncestorCount, limitAncestorSize, + limitDescendantCount, limitDescendantSize, errString); +} + void CTxMemPool::UpdateAncestorsOf(bool add, txiter it, setEntries &setAncestors) { CTxMemPoolEntry::Parents parents = it->GetMemPoolParents(); diff --git a/src/txmempool.h b/src/txmempool.h index 12efa08b5147bd..98b70d8547cbf7 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -588,6 +589,25 @@ class CTxMemPool */ std::set m_unbroadcast_txids GUARDED_BY(cs); + + /** + * Helper function to calculate all in-mempool ancestors of staged_ancestors and apply ancestor + * and descendant limits (including staged_ancestors thsemselves, entry_size and entry_count). + * param@[in] entry_size Virtual size to include in the limits. + * param@[in] entry_count How many entries to include in the limits. + * param@[in] staged_ancestors Should contain entries in the mempool. + * param@[out] setAncestors Will be populated with all mempool ancestors. + */ + bool CalculateAncestorsAndCheckLimits(size_t entry_size, + size_t entry_count, + setEntries& setAncestors, + CTxMemPoolEntry::Parents &staged_ancestors, + uint64_t limitAncestorCount, + uint64_t limitAncestorSize, + uint64_t limitDescendantCount, + uint64_t limitDescendantSize, + std::string &errString) const EXCLUSIVE_LOCKS_REQUIRED(cs); + public: indirectmap mapNextTx GUARDED_BY(cs); std::map mapDeltas GUARDED_BY(cs); @@ -715,6 +735,28 @@ class CTxMemPool */ bool CalculateMemPoolAncestors(const CTxMemPoolEntry& entry, setEntries& setAncestors, uint64_t limitAncestorCount, uint64_t limitAncestorSize, uint64_t limitDescendantCount, uint64_t limitDescendantSize, std::string& errString, bool fSearchForParents = true) const EXCLUSIVE_LOCKS_REQUIRED(cs); + /** Calculate all in-mempool ancestors of a set of transactions not already in the mempool and + * check ancestor and descendant limits. Heuristics are used to estimate the ancestor and + * descendant count of all entries if the package were to be added to the mempool. The limits + * are applied to the union of all package transactions. For example, if the package has 3 + * transactions and limitAncestorCount = 25, the union of all 3 sets of ancestors (including the + * transactions themselves) must be <= 22. + * @param[in] package Transaction package being evaluated for acceptance + * to mempool. The transactions need not be direct + * ancestors/descendants of each other. + * @param[in] limitAncestorCount Max number of txns including ancestors. + * @param[in] limitAncestorSize Max virtual size including ancestors. + * @param[in] limitDescendantCount Max number of txns including descendants. + * @param[in] limitDescendantSize Max virtual size including descendants. + * @param[out] errString Populated with error reason if a limit is hit. + */ + bool CheckPackageLimits(const Package& package, + uint64_t limitAncestorCount, + uint64_t limitAncestorSize, + uint64_t limitDescendantCount, + uint64_t limitDescendantSize, + std::string &errString) const EXCLUSIVE_LOCKS_REQUIRED(cs); + /** Populate setDescendants with all in-mempool descendants of hash. * Assumes that setDescendants includes all in-mempool descendants of anything * already in it. */ diff --git a/src/validation.cpp b/src/validation.cpp index 15be0df400ea43..799f916ef22013 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -980,6 +980,19 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: m_viewmempool.PackageAddTransaction(ws.m_ptx); } + // Apply package mempool ancestor/descendant limits. Skip if there is only one transaction, + // because it's unnecessary. Also, CPFP carve out can increase the limit for individual + // transactions, but this exemption is not extended to packages in CheckPackageLimits(). + std::string err_string; + if (txns.size() > 1 && + !m_pool.CheckPackageLimits(txns, m_limit_ancestors, m_limit_ancestor_size, m_limit_descendants, + m_limit_descendant_size, err_string)) { + // All transactions must have individually passed mempool ancestor and descendant limits + // inside of PreChecks(), so this is separate from an individual transaction error. + package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package-mempool-limits", err_string); + return PackageMempoolAcceptResult(package_state, std::move(results)); + } + for (Workspace& ws : workspaces) { PrecomputedTransactionData txdata; if (!PolicyScriptChecks(args, ws, txdata)) { diff --git a/src/validation.h b/src/validation.h index 9b2541fd933a5d..699d8bc4825007 100644 --- a/src/validation.h +++ b/src/validation.h @@ -258,7 +258,8 @@ struct PackageMempoolAcceptResult /** * Map from txid to finished MempoolAcceptResults. The client is responsible * for keeping track of the transaction objects themselves. If a result is not - * present, it means validation was unfinished for that transaction. + * present, it means validation was unfinished for that transaction. If there + * was a package-wide error (see result in m_state), m_tx_results will be empty. */ std::map m_tx_results; @@ -285,7 +286,8 @@ MempoolAcceptResult AcceptToMemoryPool(CChainState& active_chainstate, CTxMemPoo * @param[in] txns Group of transactions which may be independent or contain * parent-child dependencies. The transactions must not conflict * with each other, i.e., must not spend the same inputs. If any -* dependencies exist, parents must appear before children. +* dependencies exist, parents must appear anywhere in the list +* before their children. * @returns a PackageMempoolAcceptResult which includes a MempoolAcceptResult for each transaction. * If a transaction fails, validation will exit early and some results may be missing. */ diff --git a/test/functional/mempool_package_limits.py b/test/functional/mempool_package_limits.py new file mode 100755 index 00000000000000..41ea90a843eb53 --- /dev/null +++ b/test/functional/mempool_package_limits.py @@ -0,0 +1,467 @@ +#!/usr/bin/env python3 +# Copyright (c) 2021 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test logic for limiting mempool and package ancestors/descendants.""" + +from decimal import Decimal + +from test_framework.address import ADDRESS_BCRT1_P2SH_OP_TRUE +from test_framework.test_framework import BitcoinTestFramework +from test_framework.messages import ( + COIN, + CTransaction, + tx_from_hex, +) +from test_framework.util import ( + assert_equal, +) +from test_framework.wallet import ( + bulk_transaction, + create_child_with_parents, + make_chain, +) + +class MempoolPackageLimitsTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + self.setup_clean_chain = True + + def run_test(self): + self.log.info("Generate blocks to create UTXOs") + node = self.nodes[0] + self.privkeys = [node.get_deterministic_priv_key().key] + self.address = node.get_deterministic_priv_key().address + self.coins = [] + # The last 100 coinbase transactions are premature + for b in node.generatetoaddress(200, self.address)[:100]: + coinbase = node.getblock(blockhash=b, verbosity=2)["tx"][0] + self.coins.append({ + "txid": coinbase["txid"], + "amount": coinbase["vout"][0]["value"], + "scriptPubKey": coinbase["vout"][0]["scriptPubKey"], + }) + + self.test_chain_limits() + self.test_desc_count_limits() + self.test_anc_count_limits() + self.test_anc_count_limits_2() + self.test_anc_count_limits_bushy() + + # The node will accept our (nonstandard) extra large OP_RETURN outputs + self.restart_node(0, extra_args=["-acceptnonstdtxn=1"]) + self.test_anc_size_limits() + self.test_desc_size_limits() + + def test_chain_limits_helper(self, mempool_count, package_count): + node = self.nodes[0] + assert_equal(0, node.getmempoolinfo()["size"]) + first_coin = self.coins.pop() + spk = None + txid = first_coin["txid"] + chain_hex = [] + chain_txns = [] + value = first_coin["amount"] + + for i in range(mempool_count + package_count): + (tx, txhex, value, spk) = make_chain(node, self.address, self.privkeys, txid, value, 0, spk) + txid = tx.rehash() + if i < mempool_count: + node.sendrawtransaction(txhex) + assert_equal(node.getrawmempool(verbose=True)[txid]["ancestorcount"], i + 1) + else: + chain_hex.append(txhex) + chain_txns.append(tx) + testres_too_long = node.testmempoolaccept(rawtxs=chain_hex) + for txres in testres_too_long: + assert_equal(txres["package-error"], "package-mempool-limits") + + # Clear mempool and check that the package passes now + node.generate(1) + assert all([res["allowed"] for res in node.testmempoolaccept(rawtxs=chain_hex)]) + + def test_chain_limits(self): + """Create chains from mempool and package transactions that are longer than 25, + but only if both in-mempool and in-package transactions are considered together. + This checks that both mempool and in-package transactions are taken into account when + calculating ancestors/descendant limits. + """ + self.log.info("Check that in-package ancestors count for mempool ancestor limits") + + # 24 transactions in the mempool and 2 in the package. The parent in the package has + # 24 in-mempool ancestors and 1 in-package descendant. The child has 0 direct parents + # in the mempool, but 25 in-mempool and in-package ancestors in total. + self.test_chain_limits_helper(24, 2) + # 2 transactions in the mempool and 24 in the package. + self.test_chain_limits_helper(2, 24) + # 13 transactions in the mempool and 13 in the package. + self.test_chain_limits_helper(13, 13) + + def test_desc_count_limits(self): + """Create an 'A' shaped package with 24 transactions in the mempool and 2 in the package: + M1 + ^ ^ + M2a M2b + . . + . . + . . + M12a ^ + ^ M13b + ^ ^ + Pa Pb + The top ancestor in the package exceeds descendant limits but only if the in-mempool and in-package + descendants are all considered together (24 including in-mempool descendants and 26 including both + package transactions). + """ + node = self.nodes[0] + assert_equal(0, node.getmempoolinfo()["size"]) + self.log.info("Check that in-mempool and in-package descendants are calculated properly in packages") + # Top parent in mempool, M1 + first_coin = self.coins.pop() + parent_value = (first_coin["amount"] - Decimal("0.0002")) / 2 # Deduct reasonable fee and make 2 outputs + inputs = [{"txid": first_coin["txid"], "vout": 0}] + outputs = [{self.address : parent_value}, {ADDRESS_BCRT1_P2SH_OP_TRUE : parent_value}] + rawtx = node.createrawtransaction(inputs, outputs) + + parent_signed = node.signrawtransactionwithkey(hexstring=rawtx, privkeys=self.privkeys) + assert parent_signed["complete"] + parent_tx = tx_from_hex(parent_signed["hex"]) + parent_txid = parent_tx.rehash() + node.sendrawtransaction(parent_signed["hex"]) + + package_hex = [] + + # Chain A + spk = parent_tx.vout[0].scriptPubKey.hex() + value = parent_value + txid = parent_txid + for i in range(12): + (tx, txhex, value, spk) = make_chain(node, self.address, self.privkeys, txid, value, 0, spk) + txid = tx.rehash() + if i < 11: # M2a... M12a + node.sendrawtransaction(txhex) + else: # Pa + package_hex.append(txhex) + + # Chain B + value = parent_value - Decimal("0.0001") + rawtx_b = node.createrawtransaction([{"txid": parent_txid, "vout": 1}], {self.address : value}) + tx_child_b = tx_from_hex(rawtx_b) # M2b + tx_child_b_hex = tx_child_b.serialize().hex() + node.sendrawtransaction(tx_child_b_hex) + spk = tx_child_b.vout[0].scriptPubKey.hex() + txid = tx_child_b.rehash() + for i in range(12): + (tx, txhex, value, spk) = make_chain(node, self.address, self.privkeys, txid, value, 0, spk) + txid = tx.rehash() + if i < 11: # M3b... M13b + node.sendrawtransaction(txhex) + else: # Pb + package_hex.append(txhex) + + assert_equal(24, node.getmempoolinfo()["size"]) + assert_equal(2, len(package_hex)) + testres_too_long = node.testmempoolaccept(rawtxs=package_hex) + for txres in testres_too_long: + assert_equal(txres["package-error"], "package-mempool-limits") + + # Clear mempool and check that the package passes now + node.generate(1) + assert all([res["allowed"] for res in node.testmempoolaccept(rawtxs=package_hex)]) + + def test_anc_count_limits(self): + """Create a 'V' shaped chain with 24 transactions in the mempool and 3 in the package: + M1a M1b + ^ ^ + M2a M2b + . . + . . + . . + M12a M12b + ^ ^ + Pa Pb + ^ ^ + Pc + The lowest descendant, Pc, exceeds ancestor limits, but only if the in-mempool + and in-package ancestors are all considered together. + """ + node = self.nodes[0] + assert_equal(0, node.getmempoolinfo()["size"]) + package_hex = [] + parents_tx = [] + values = [] + scripts = [] + + self.log.info("Check that in-mempool and in-package ancestors are calculated properly in packages") + + # Two chains of 13 transactions each + for _ in range(2): + spk = None + top_coin = self.coins.pop() + txid = top_coin["txid"] + value = top_coin["amount"] + for i in range(13): + (tx, txhex, value, spk) = make_chain(node, self.address, self.privkeys, txid, value, 0, spk) + txid = tx.rehash() + if i < 12: + node.sendrawtransaction(txhex) + else: # Save the 13th transaction for the package + package_hex.append(txhex) + parents_tx.append(tx) + scripts.append(spk) + values.append(value) + + # Child Pc + child_hex = create_child_with_parents(node, self.address, self.privkeys, parents_tx, values, scripts) + package_hex.append(child_hex) + + assert_equal(24, node.getmempoolinfo()["size"]) + assert_equal(3, len(package_hex)) + testres_too_long = node.testmempoolaccept(rawtxs=package_hex) + for txres in testres_too_long: + assert_equal(txres["package-error"], "package-mempool-limits") + + # Clear mempool and check that the package passes now + node.generate(1) + assert all([res["allowed"] for res in node.testmempoolaccept(rawtxs=package_hex)]) + + def test_anc_count_limits_2(self): + """Create a 'Y' shaped chain with 24 transactions in the mempool and 2 in the package: + M1a M1b + ^ ^ + M2a M2b + . . + . . + . . + M12a M12b + ^ ^ + Pc + ^ + Pd + The lowest descendant, Pd, exceeds ancestor limits, but only if the in-mempool + and in-package ancestors are all considered together. + """ + node = self.nodes[0] + assert_equal(0, node.getmempoolinfo()["size"]) + parents_tx = [] + values = [] + scripts = [] + + self.log.info("Check that in-mempool and in-package ancestors are calculated properly in packages") + # Two chains of 12 transactions each + for _ in range(2): + spk = None + top_coin = self.coins.pop() + txid = top_coin["txid"] + value = top_coin["amount"] + for i in range(12): + (tx, txhex, value, spk) = make_chain(node, self.address, self.privkeys, txid, value, 0, spk) + txid = tx.rehash() + value -= Decimal("0.0001") + node.sendrawtransaction(txhex) + if i == 11: + # last 2 transactions will be the parents of Pc + parents_tx.append(tx) + values.append(value) + scripts.append(spk) + + # Child Pc + pc_hex = create_child_with_parents(node, self.address, self.privkeys, parents_tx, values, scripts) + pc_tx = tx_from_hex(pc_hex) + pc_value = sum(values) - Decimal("0.0002") + pc_spk = pc_tx.vout[0].scriptPubKey.hex() + + # Child Pd + (_, pd_hex, _, _) = make_chain(node, self.address, self.privkeys, pc_tx.rehash(), pc_value, 0, pc_spk) + + assert_equal(24, node.getmempoolinfo()["size"]) + testres_too_long = node.testmempoolaccept(rawtxs=[pc_hex, pd_hex]) + for txres in testres_too_long: + assert_equal(txres["package-error"], "package-mempool-limits") + + # Clear mempool and check that the package passes now + node.generate(1) + assert all([res["allowed"] for res in node.testmempoolaccept(rawtxs=[pc_hex, pd_hex])]) + + def test_anc_count_limits_bushy(self): + """Create a tree with 20 transactions in the mempool and 6 in the package: + M1...M4 M5...M8 M9...M12 M13...M16 M17...M20 + ^ ^ ^ ^ ^ (each with 4 parents) + P0 P1 P2 P3 P4 + ^ ^ ^ ^ ^ (5 parents) + PC + Where M(4i+1)...M+(4i+4) are the parents of Pi and P0, P1, P2, P3, and P4 are the parents of PC. + P0... P4 individually only have 4 parents each, and PC has no in-mempool parents. But + combined, PC has 25 in-mempool and in-package parents. + """ + node = self.nodes[0] + assert_equal(0, node.getmempoolinfo()["size"]) + package_hex = [] + parent_txns = [] + parent_values = [] + scripts = [] + for _ in range(5): # Make package transactions P0 ... P4 + gp_tx = [] + gp_values = [] + gp_scripts = [] + for _ in range(4): # Make mempool transactions M(4i+1)...M(4i+4) + parent_coin = self.coins.pop() + value = parent_coin["amount"] + txid = parent_coin["txid"] + (tx, txhex, value, spk) = make_chain(node, self.address, self.privkeys, txid, value) + gp_tx.append(tx) + gp_values.append(value) + gp_scripts.append(spk) + node.sendrawtransaction(txhex) + # Package transaction Pi + pi_hex = create_child_with_parents(node, self.address, self.privkeys, gp_tx, gp_values, gp_scripts) + package_hex.append(pi_hex) + pi_tx = tx_from_hex(pi_hex) + parent_txns.append(pi_tx) + parent_values.append(Decimal(pi_tx.vout[0].nValue) / COIN) + scripts.append(pi_tx.vout[0].scriptPubKey.hex()) + # Package transaction PC + package_hex.append(create_child_with_parents(node, self.address, self.privkeys, parent_txns, parent_values, scripts)) + + assert_equal(20, node.getmempoolinfo()["size"]) + assert_equal(6, len(package_hex)) + testres = node.testmempoolaccept(rawtxs=package_hex) + for txres in testres: + assert_equal(txres["package-error"], "package-mempool-limits") + + # Clear mempool and check that the package passes now + node.generate(1) + assert all([res["allowed"] for res in node.testmempoolaccept(rawtxs=package_hex)]) + + def test_anc_size_limits(self): + """Test Case with 2 independent transactions in the mempool and a parent + child in the + package, where the package parent is the child of both mempool transactions (30KvB each): + A B + ^ ^ + C + ^ + D + The lowest descendant, D, exceeds ancestor size limits, but only if the in-mempool + and in-package ancestors are all considered together. + """ + node = self.nodes[0] + assert_equal(0, node.getmempoolinfo()["size"]) + parents_tx = [] + values = [] + scripts = [] + target_weight = 1000 * 30 # 30KvB + high_fee = Decimal("0.003") # 10 sats/vB + self.log.info("Check that in-mempool and in-package ancestor size limits are calculated properly in packages") + # Mempool transactions A and B + for _ in range(2): + spk = None + top_coin = self.coins.pop() + txid = top_coin["txid"] + value = top_coin["amount"] + (tx, _, _, _) = make_chain(node, self.address, self.privkeys, txid, value, 0, spk, high_fee) + bulked_tx = bulk_transaction(tx, node, target_weight, self.privkeys) + node.sendrawtransaction(bulked_tx.serialize().hex()) + parents_tx.append(bulked_tx) + values.append(Decimal(bulked_tx.vout[0].nValue) / COIN) + scripts.append(bulked_tx.vout[0].scriptPubKey.hex()) + + # Package transaction C + small_pc_hex = create_child_with_parents(node, self.address, self.privkeys, parents_tx, values, scripts, high_fee) + pc_tx = bulk_transaction(tx_from_hex(small_pc_hex), node, target_weight, self.privkeys) + pc_value = Decimal(pc_tx.vout[0].nValue) / COIN + pc_spk = pc_tx.vout[0].scriptPubKey.hex() + pc_hex = pc_tx.serialize().hex() + + # Package transaction D + (small_pd, _, val, spk) = make_chain(node, self.address, self.privkeys, pc_tx.rehash(), pc_value, 0, pc_spk, high_fee) + prevtxs = [{ + "txid": pc_tx.rehash(), + "vout": 0, + "scriptPubKey": spk, + "amount": val, + }] + pd_tx = bulk_transaction(small_pd, node, target_weight, self.privkeys, prevtxs) + pd_hex = pd_tx.serialize().hex() + + assert_equal(2, node.getmempoolinfo()["size"]) + testres_too_heavy = node.testmempoolaccept(rawtxs=[pc_hex, pd_hex]) + for txres in testres_too_heavy: + assert_equal(txres["package-error"], "package-mempool-limits") + + # Clear mempool and check that the package passes now + node.generate(1) + assert all([res["allowed"] for res in node.testmempoolaccept(rawtxs=[pc_hex, pd_hex])]) + + def test_desc_size_limits(self): + """Create 3 mempool transactions and 2 package transactions (25KvB each): + Ma + ^ ^ + Mb Mc + ^ ^ + Pd Pe + The top ancestor in the package exceeds descendant size limits but only if the in-mempool + and in-package descendants are all considered together. + """ + node = self.nodes[0] + assert_equal(0, node.getmempoolinfo()["size"]) + target_weight = 21 * 1000 + high_fee = Decimal("0.0021") # 10 sats/vB + self.log.info("Check that in-mempool and in-package descendant sizes are calculated properly in packages") + # Top parent in mempool, Ma + first_coin = self.coins.pop() + parent_value = (first_coin["amount"] - high_fee) / 2 # Deduct fee and make 2 outputs + inputs = [{"txid": first_coin["txid"], "vout": 0}] + outputs = [{self.address : parent_value}, {ADDRESS_BCRT1_P2SH_OP_TRUE: parent_value}] + rawtx = node.createrawtransaction(inputs, outputs) + parent_tx = bulk_transaction(tx_from_hex(rawtx), node, target_weight, self.privkeys) + node.sendrawtransaction(parent_tx.serialize().hex()) + + package_hex = [] + for j in range(2): # Two legs (left and right) + # Mempool transaction (Mb and Mc) + mempool_tx = CTransaction() + spk = parent_tx.vout[j].scriptPubKey.hex() + value = Decimal(parent_tx.vout[j].nValue) / COIN + txid = parent_tx.rehash() + prevtxs = [{ + "txid": txid, + "vout": j, + "scriptPubKey": spk, + "amount": value, + }] + if j == 0: # normal key + (tx_small, _, _, _) = make_chain(node, self.address, self.privkeys, txid, value, j, spk, high_fee) + mempool_tx = bulk_transaction(tx_small, node, target_weight, self.privkeys, prevtxs) + else: # OP_TRUE + inputs = [{"txid": txid, "vout": 1}] + outputs = {self.address: value - high_fee} + small_tx = tx_from_hex(node.createrawtransaction(inputs, outputs)) + mempool_tx = bulk_transaction(small_tx, node, target_weight, None, prevtxs) + node.sendrawtransaction(mempool_tx.serialize().hex()) + + # Package transaction (Pd and Pe) + spk = mempool_tx.vout[0].scriptPubKey.hex() + value = Decimal(mempool_tx.vout[0].nValue) / COIN + txid = mempool_tx.rehash() + (tx_small, _, _, _) = make_chain(node, self.address, self.privkeys, txid, value, 0, spk, high_fee) + prevtxs = [{ + "txid": txid, + "vout": 0, + "scriptPubKey": spk, + "amount": value, + }] + package_tx = bulk_transaction(tx_small, node, target_weight, self.privkeys, prevtxs) + package_hex.append(package_tx.serialize().hex()) + + assert_equal(3, node.getmempoolinfo()["size"]) + assert_equal(2, len(package_hex)) + testres_too_heavy = node.testmempoolaccept(rawtxs=package_hex) + for txres in testres_too_heavy: + assert_equal(txres["package-error"], "package-mempool-limits") + + # Clear mempool and check that the package passes now + node.generate(1) + assert all([res["allowed"] for res in node.testmempoolaccept(rawtxs=package_hex)]) + +if __name__ == "__main__": + MempoolPackageLimitsTest().main() diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py index c2e14ce8cd7cc5..1b207061df22d6 100755 --- a/test/functional/rpc_packages.py +++ b/test/functional/rpc_packages.py @@ -19,6 +19,11 @@ from test_framework.util import ( assert_equal, ) +from test_framework.wallet import ( + create_child_with_parents, + create_raw_chain, + make_chain, +) class RPCPackagesTest(BitcoinTestFramework): def set_test_params(self): @@ -74,27 +79,6 @@ def run_test(self): self.test_multiple_parents() self.test_conflicting() - def chain_transaction(self, parent_txid, parent_value, n=0, parent_locking_script=None): - """Build a transaction that spends parent_txid.vout[n] and produces one output with - amount = parent_value with a fee deducted. - Return tuple (CTransaction object, raw hex, nValue, scriptPubKey of the output created). - """ - node = self.nodes[0] - inputs = [{"txid": parent_txid, "vout": n}] - my_value = parent_value - Decimal("0.0001") - outputs = {self.address : my_value} - rawtx = node.createrawtransaction(inputs, outputs) - prevtxs = [{ - "txid": parent_txid, - "vout": n, - "scriptPubKey": parent_locking_script, - "amount": parent_value, - }] if parent_locking_script else None - signedtx = node.signrawtransactionwithkey(hexstring=rawtx, privkeys=self.privkeys, prevtxs=prevtxs) - tx = tx_from_hex(signedtx["hex"]) - tx.rehash() - assert signedtx["complete"] - return (tx, signedtx["hex"], my_value, tx.vout[0].scriptPubKey.hex()) def test_independent(self): self.log.info("Test multiple independent transactions in a package") @@ -146,20 +130,7 @@ def test_independent(self): def test_chain(self): node = self.nodes[0] first_coin = self.coins.pop() - - # Chain of 25 transactions - parent_locking_script = None - txid = first_coin["txid"] - chain_hex = [] - chain_txns = [] - value = first_coin["amount"] - - for _ in range(25): - (tx, txhex, value, parent_locking_script) = self.chain_transaction(txid, value, 0, parent_locking_script) - txid = tx.rehash() - chain_hex.append(txhex) - chain_txns.append(tx) - + (chain_hex, chain_txns) = create_raw_chain(node, first_coin, self.address, self.privkeys) self.log.info("Check that testmempoolaccept requires packages to be sorted by dependency") assert_equal(node.testmempoolaccept(rawtxs=chain_hex[::-1]), [{"txid": tx.rehash(), "package-error": "package-not-sorted"} for tx in chain_txns[::-1]]) @@ -199,7 +170,7 @@ def test_multiple_children(self): child_value = value - Decimal("0.0001") # Child A - (_, tx_child_a_hex, _, _) = self.chain_transaction(parent_txid, child_value, 0, parent_locking_script_a) + (_, tx_child_a_hex, _, _) = make_chain(node, self.address, self.privkeys, parent_txid, child_value, 0, parent_locking_script_a) assert not node.testmempoolaccept([tx_child_a_hex])[0]["allowed"] # Child B @@ -223,19 +194,6 @@ def test_multiple_children(self): node.sendrawtransaction(rawtx) assert_equal(testres_single, testres_multiple_ab) - def create_child_with_parents(self, parents_tx, values, locking_scripts): - """Creates a transaction that spends the first output of each parent in parents_tx.""" - num_parents = len(parents_tx) - total_value = sum(values) - inputs = [{"txid": tx.rehash(), "vout": 0} for tx in parents_tx] - outputs = {self.address : total_value - num_parents * Decimal("0.0001")} - rawtx_child = self.nodes[0].createrawtransaction(inputs, outputs) - prevtxs = [] - for i in range(num_parents): - prevtxs.append({"txid": parents_tx[i].rehash(), "vout": 0, "scriptPubKey": locking_scripts[i], "amount": values[i]}) - signedtx_child = self.nodes[0].signrawtransactionwithkey(hexstring=rawtx_child, privkeys=self.privkeys, prevtxs=prevtxs) - assert signedtx_child["complete"] - return signedtx_child["hex"] def test_multiple_parents(self): node = self.nodes[0] @@ -250,12 +208,12 @@ def test_multiple_parents(self): for _ in range(num_parents): parent_coin = self.coins.pop() value = parent_coin["amount"] - (tx, txhex, value, parent_locking_script) = self.chain_transaction(parent_coin["txid"], value) + (tx, txhex, value, parent_locking_script) = make_chain(node, self.address, self.privkeys, parent_coin["txid"], value) package_hex.append(txhex) parents_tx.append(tx) values.append(value) parent_locking_scripts.append(parent_locking_script) - child_hex = self.create_child_with_parents(parents_tx, values, parent_locking_scripts) + child_hex = create_child_with_parents(node, self.address, self.privkeys, parents_tx, values, parent_locking_scripts) # Package accept should work with the parents in any order (as long as parents come before child) for _ in range(10): random.shuffle(package_hex) diff --git a/test/functional/test_framework/wallet.py b/test/functional/test_framework/wallet.py index ff2d73efdd1ade..cc486a70ae3658 100644 --- a/test/functional/test_framework/wallet.py +++ b/test/functional/test_framework/wallet.py @@ -4,8 +4,10 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """A limited-functionality wallet, which may replace a real wallet in tests""" +from copy import deepcopy from decimal import Decimal from test_framework.address import ADDRESS_BCRT1_P2SH_OP_TRUE +from random import choice from typing import Optional from test_framework.messages import ( COIN, @@ -13,6 +15,7 @@ CTransaction, CTxIn, CTxOut, + tx_from_hex, ) from test_framework.script import ( CScript, @@ -21,9 +24,11 @@ from test_framework.util import ( assert_equal, hex_str_to_bytes, + assert_greater_than_or_equal, satoshi_round, ) +DEFAULT_FEE = Decimal("0.0001") class MiniWallet: def __init__(self, test_node): @@ -88,3 +93,73 @@ def send_self_transfer(self, *, fee_rate=Decimal("0.003"), from_node, utxo_to_sp assert_equal(len(tx_hex) // 2, vsize) assert_equal(tx_info['fees']['base'], fee) return {'txid': tx_info['txid'], 'hex': tx_hex} + self.scan_tx(from_node.decoderawtransaction(tx_hex)) + +def make_chain(node, address, privkeys, parent_txid, parent_value, n=0, parent_locking_script=None, fee=DEFAULT_FEE): + """Build a transaction that spends parent_txid.vout[n] and produces one output with + amount = parent_value with a fee deducted. + Return tuple (CTransaction object, raw hex, nValue, scriptPubKey of the output created). + """ + inputs = [{"txid": parent_txid, "vout": n}] + my_value = parent_value - fee + outputs = {address : my_value} + rawtx = node.createrawtransaction(inputs, outputs) + prevtxs = [{ + "txid": parent_txid, + "vout": n, + "scriptPubKey": parent_locking_script, + "amount": parent_value, + }] if parent_locking_script else None + signedtx = node.signrawtransactionwithkey(hexstring=rawtx, privkeys=privkeys, prevtxs=prevtxs) + assert signedtx["complete"] + tx = tx_from_hex(signedtx["hex"]) + return (tx, signedtx["hex"], my_value, tx.vout[0].scriptPubKey.hex()) + +def create_child_with_parents(node, address, privkeys, parents_tx, values, locking_scripts, fee=DEFAULT_FEE): + """Creates a transaction that spends the first output of each parent in parents_tx.""" + num_parents = len(parents_tx) + total_value = sum(values) + inputs = [{"txid": tx.rehash(), "vout": 0} for tx in parents_tx] + outputs = {address : total_value - fee} + rawtx_child = node.createrawtransaction(inputs, outputs) + prevtxs = [] + for i in range(num_parents): + prevtxs.append({"txid": parents_tx[i].rehash(), "vout": 0, "scriptPubKey": locking_scripts[i], "amount": values[i]}) + signedtx_child = node.signrawtransactionwithkey(hexstring=rawtx_child, privkeys=privkeys, prevtxs=prevtxs) + assert signedtx_child["complete"] + return signedtx_child["hex"] + +def create_raw_chain(node, first_coin, address, privkeys, chain_length=25): + """Helper function: create a "chain" of chain_length transactions. The nth transaction in the + chain is a child of the n-1th transaction and parent of the n+1th transaction. + """ + parent_locking_script = None + txid = first_coin["txid"] + chain_hex = [] + chain_txns = [] + value = first_coin["amount"] + + for _ in range(chain_length): + (tx, txhex, value, parent_locking_script) = make_chain(node, address, privkeys, txid, value, 0, parent_locking_script) + txid = tx.rehash() + chain_hex.append(txhex) + chain_txns.append(tx) + + return (chain_hex, chain_txns) + +def bulk_transaction(tx, node, target_weight, privkeys, prevtxs=None): + """Pad a transaction with extra outputs until it reaches a target weight (or higher). + returns CTransaction object + """ + tx_heavy = deepcopy(tx) + assert_greater_than_or_equal(target_weight, tx_heavy.get_weight()) + while tx_heavy.get_weight() < target_weight: + random_spk = "6a4d0200" # OP_RETURN OP_PUSH2 512 bytes + for _ in range(512*2): + random_spk += choice("0123456789ABCDEF") + tx_heavy.vout.append(CTxOut(0, bytes.fromhex(random_spk))) + # Re-sign the transaction + if privkeys: + signed = node.signrawtransactionwithkey(tx_heavy.serialize().hex(), privkeys, prevtxs) + return tx_from_hex(signed["hex"]) + return tx_heavy diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 3af377ba30dde3..7bf25e0f591962 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -243,6 +243,7 @@ 'rpc_createmultisig.py --legacy-wallet', 'rpc_createmultisig.py --descriptors', 'rpc_packages.py', + 'mempool_package_limits.py', 'feature_versionbits_warning.py', 'rpc_preciousblock.py', 'wallet_importprunedfunds.py --legacy-wallet',