diff --git a/src/validation.cpp b/src/validation.cpp index 596f688bb14..50660ca75b7 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1014,40 +1014,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) if (auto ancestors{m_subpackage.m_changeset->CalculateMemPoolAncestors(ws.m_tx_handle, maybe_rbf_limits)}) { ws.m_ancestors = std::move(*ancestors); } else { - // If CalculateMemPoolAncestors fails second time, we want the original error string. - const auto error_message{util::ErrorString(ancestors).original}; - - // Carve-out is not allowed in this context; fail - if (!args.m_allow_carveouts) { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", error_message); - } - - // Contracting/payment channels CPFP carve-out: - // If the new transaction is relatively small (up to 40k weight) - // and has at most one ancestor (ie ancestor limit of 2, including - // the new transaction), allow it if its parent has exactly the - // descendant limit descendants. The transaction also cannot be TRUC, - // as its topology restrictions do not allow a second child. - // - // This allows protocols which rely on distrusting counterparties - // being able to broadcast descendants of an unconfirmed transaction - // to be secure by simply only having two immediately-spendable - // outputs - one for each counterparty. For more info on the uses for - // this, see https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html - CTxMemPool::Limits cpfp_carve_out_limits{ - .ancestor_count = 2, - .ancestor_size_vbytes = maybe_rbf_limits.ancestor_size_vbytes, - .descendant_count = maybe_rbf_limits.descendant_count + 1, - .descendant_size_vbytes = maybe_rbf_limits.descendant_size_vbytes + EXTRA_DESCENDANT_TX_SIZE_LIMIT, - }; - if (ws.m_vsize > EXTRA_DESCENDANT_TX_SIZE_LIMIT || ws.m_ptx->version == TRUC_VERSION) { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", error_message); - } - if (auto ancestors_retry{m_subpackage.m_changeset->CalculateMemPoolAncestors(ws.m_tx_handle, cpfp_carve_out_limits)}) { - ws.m_ancestors = std::move(*ancestors_retry); - } else { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", error_message); - } + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", util::ErrorString(ancestors).original); } // Even though just checking direct mempool parents for inheritance would be sufficient, we diff --git a/test/functional/mempool_package_onemore.py b/test/functional/mempool_package_onemore.py deleted file mode 100755 index 74c5e345202..00000000000 --- a/test/functional/mempool_package_onemore.py +++ /dev/null @@ -1,80 +0,0 @@ -#!/usr/bin/env python3 -# Copyright (c) 2014-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. -"""Test descendant package tracking carve-out allowing one final transaction in - an otherwise-full package as long as it has only one parent and is <= 10k in - size. -""" - -from test_framework.messages import ( - DEFAULT_ANCESTOR_LIMIT, -) -from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import ( - assert_equal, - assert_raises_rpc_error, -) -from test_framework.wallet import MiniWallet - - -class MempoolPackagesTest(BitcoinTestFramework): - def set_test_params(self): - self.num_nodes = 1 - - def chain_tx(self, utxos_to_spend, *, num_outputs=1): - return self.wallet.send_self_transfer_multi( - from_node=self.nodes[0], - utxos_to_spend=utxos_to_spend, - num_outputs=num_outputs)['new_utxos'] - - def run_test(self): - self.wallet = MiniWallet(self.nodes[0]) - - # DEFAULT_ANCESTOR_LIMIT transactions off a confirmed tx should be fine - chain = [] - utxo = self.wallet.get_utxo() - for _ in range(4): - utxo, utxo2 = self.chain_tx([utxo], num_outputs=2) - chain.append(utxo2) - for _ in range(DEFAULT_ANCESTOR_LIMIT - 4): - utxo, = self.chain_tx([utxo]) - chain.append(utxo) - second_chain, = self.chain_tx([self.wallet.get_utxo(confirmed_only=True)]) - - # Check mempool has DEFAULT_ANCESTOR_LIMIT + 1 transactions in it - assert_equal(len(self.nodes[0].getrawmempool()), DEFAULT_ANCESTOR_LIMIT + 1) - - # Adding one more transaction on to the chain should fail. - assert_raises_rpc_error(-26, "too-long-mempool-chain, too many unconfirmed ancestors [limit: 25]", self.chain_tx, [utxo]) - # ... or if it chains on from some point in the middle of the chain. - assert_raises_rpc_error(-26, "too-long-mempool-chain, too many descendants", self.chain_tx, [chain[2]]) - assert_raises_rpc_error(-26, "too-long-mempool-chain, too many descendants", self.chain_tx, [chain[1]]) - # ...even if it chains on to two parent transactions with one in the chain. - assert_raises_rpc_error(-26, "too-long-mempool-chain, too many descendants", self.chain_tx, [chain[0], second_chain]) - # ...especially if its > 40k weight - assert_raises_rpc_error(-26, "too-long-mempool-chain, too many descendants", self.chain_tx, [chain[0]], num_outputs=350) - # ...even if it's submitted with other transactions - replaceable_tx = self.wallet.create_self_transfer_multi(utxos_to_spend=[chain[0]]) - txns = [replaceable_tx["tx"], self.wallet.create_self_transfer_multi(utxos_to_spend=replaceable_tx["new_utxos"])["tx"]] - txns_hex = [tx.serialize().hex() for tx in txns] - assert_equal(self.nodes[0].testmempoolaccept(txns_hex)[0]["reject-reason"], "too-long-mempool-chain") - pkg_result = self.nodes[0].submitpackage(txns_hex) - assert "too-long-mempool-chain" in pkg_result["tx-results"][txns[0].wtxid_hex]["error"] - assert_equal(pkg_result["tx-results"][txns[1].wtxid_hex]["error"], "bad-txns-inputs-missingorspent") - # But not if it chains directly off the first transaction - self.nodes[0].sendrawtransaction(replaceable_tx["hex"]) - # and the second chain should work just fine - self.chain_tx([second_chain]) - - # Ensure an individual transaction with single direct conflict can RBF the chain which used our carve-out rule - replacement_tx = replaceable_tx["tx"] - replacement_tx.vout[0].nValue -= 1000000 - self.nodes[0].sendrawtransaction(replacement_tx.serialize().hex()) - - # Finally, check that we added two transactions - assert_equal(len(self.nodes[0].getrawmempool()), DEFAULT_ANCESTOR_LIMIT + 3) - - -if __name__ == '__main__': - MempoolPackagesTest(__file__).main() diff --git a/test/functional/mempool_packages.py b/test/functional/mempool_packages.py index a844a2a1d8f..b5f994e8030 100755 --- a/test/functional/mempool_packages.py +++ b/test/functional/mempool_packages.py @@ -20,7 +20,7 @@ from test_framework.wallet import MiniWallet # custom limits for node1 CUSTOM_ANCESTOR_LIMIT = 5 -CUSTOM_DESCENDANT_LIMIT = 10 +CUSTOM_DESCENDANT_LIMIT = 11 assert CUSTOM_DESCENDANT_LIMIT >= CUSTOM_ANCESTOR_LIMIT @@ -240,12 +240,15 @@ class MempoolPackagesTest(BitcoinTestFramework): # - parent tx for descendant test # - txs chained off parent tx (-> custom descendant limit) self.wait_until(lambda: len(self.nodes[1].getrawmempool()) == - CUSTOM_ANCESTOR_LIMIT + 1 + CUSTOM_DESCENDANT_LIMIT, timeout=10) + CUSTOM_ANCESTOR_LIMIT + CUSTOM_DESCENDANT_LIMIT, timeout=10) mempool0 = self.nodes[0].getrawmempool(False) mempool1 = self.nodes[1].getrawmempool(False) assert set(mempool1).issubset(set(mempool0)) assert parent_transaction in mempool1 - for tx in chain[:CUSTOM_DESCENDANT_LIMIT]: + # Note: this test is brittle, because it relies on the relay sort order + # of node0 to be based on ancestor count (so that the first 10 + # descendants of parent_transaction relay before the later ones). + for tx in chain[:CUSTOM_DESCENDANT_LIMIT-1]: assert tx in mempool1 for tx in chain[CUSTOM_DESCENDANT_LIMIT:]: assert tx not in mempool1 diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 5e820e43ab6..97b23921653 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -254,7 +254,6 @@ BASE_SCRIPTS = [ 'feature_utxo_set_hash.py', 'feature_rbf.py', 'mempool_packages.py', - 'mempool_package_onemore.py', 'mempool_package_limits.py', 'mempool_package_rbf.py', 'tool_utxo_to_sqlite.py',