diff options
| -rw-r--r-- | src/test/txpackage_tests.cpp | 52 | ||||
| -rwxr-xr-x | test/functional/mempool_limit.py | 49 | ||||
| -rwxr-xr-x | test/functional/rpc_packages.py | 40 |
3 files changed, 77 insertions, 64 deletions
diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp index 024526497c..af29c955bc 100644 --- a/src/test/txpackage_tests.cpp +++ b/src/test/txpackage_tests.cpp @@ -16,6 +16,9 @@ #include <boost/test/unit_test.hpp> BOOST_AUTO_TEST_SUITE(txpackage_tests) +// A fee amount that is above 1sat/vB but below 5sat/vB for most transactions created within these +// unit tests. +static const CAmount low_fee_amt{200}; // Create placeholder transactions that have no meaning. inline CTransactionRef create_placeholder_tx(size_t num_inputs, size_t num_outputs) @@ -373,6 +376,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) { // Mine blocks to mature coinbases. mineBlocks(5); + MockMempoolMinFee(CFeeRate(5000)); LOCK(cs_main); // Transactions with a same-txid-different-witness transaction in the mempool should be ignored, @@ -560,13 +564,15 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) BOOST_CHECK(parent2_v2_result.m_result_type == MempoolAcceptResult::ResultType::VALID); package_mixed.push_back(ptx_parent2_v1); - // parent3 will be a new transaction. Put 0 fees on it to make it invalid on its own. + // parent3 will be a new transaction. Put a low feerate to make it invalid on its own. auto mtx_parent3 = CreateValidMempoolTransaction(/*input_transaction=*/m_coinbase_txns[3], /*input_vout=*/0, /*input_height=*/0, /*input_signing_key=*/coinbaseKey, /*output_destination=*/acs_spk, - /*output_amount=*/CAmount(50 * COIN), /*submit=*/false); + /*output_amount=*/CAmount(50 * COIN - low_fee_amt), /*submit=*/false); CTransactionRef ptx_parent3 = MakeTransactionRef(mtx_parent3); package_mixed.push_back(ptx_parent3); + BOOST_CHECK(m_node.mempool->GetMinFee().GetFee(GetVirtualTransactionSize(*ptx_parent3)) > low_fee_amt); + BOOST_CHECK(m_node.mempool->m_min_relay_feerate.GetFee(GetVirtualTransactionSize(*ptx_parent3)) <= low_fee_amt); // child spends parent1, parent2, and parent3 CKey mixed_grandchild_key; @@ -627,6 +633,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) { mineBlocks(5); + MockMempoolMinFee(CFeeRate(5000)); LOCK(::cs_main); size_t expected_pool_size = m_node.mempool->size(); CKey child_key; @@ -636,9 +643,9 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) grandchild_key.MakeNewKey(true); CScript child_spk = GetScriptForDestination(WitnessV0KeyHash(grandchild_key.GetPubKey())); - // zero-fee parent and high-fee child package + // low-fee parent and high-fee child package const CAmount coinbase_value{50 * COIN}; - const CAmount parent_value{coinbase_value - 0}; + const CAmount parent_value{coinbase_value - low_fee_amt}; const CAmount child_value{parent_value - COIN}; Package package_cpfp; @@ -657,9 +664,9 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) package_cpfp.push_back(tx_child); // Package feerate is calculated using modified fees, and prioritisetransaction accepts negative - // fee deltas. This should be taken into account. De-prioritise the parent transaction by -1BTC, - // bringing the package feerate to 0. - m_node.mempool->PrioritiseTransaction(tx_parent->GetHash(), -1 * COIN); + // fee deltas. This should be taken into account. De-prioritise the parent transaction + // to bring the package feerate to 0. + m_node.mempool->PrioritiseTransaction(tx_parent->GetHash(), child_value - coinbase_value); { BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); const auto submit_cpfp_deprio = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, @@ -675,8 +682,8 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) // Clear the prioritisation of the parent transaction. WITH_LOCK(m_node.mempool->cs, m_node.mempool->ClearPrioritisation(tx_parent->GetHash())); - // Package CPFP: Even though the parent pays 0 absolute fees, the child pays 1 BTC which is - // enough for the package feerate to meet the threshold. + // Package CPFP: Even though the parent's feerate is below the mempool minimum feerate, the + // child pays enough for the package feerate to meet the threshold. { BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); const auto submit_cpfp = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, @@ -689,7 +696,7 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) auto it_child = submit_cpfp.m_tx_results.find(tx_child->GetWitnessHash()); BOOST_CHECK(it_parent != submit_cpfp.m_tx_results.end()); BOOST_CHECK(it_parent->second.m_result_type == MempoolAcceptResult::ResultType::VALID); - BOOST_CHECK(it_parent->second.m_base_fees.value() == 0); + BOOST_CHECK(it_parent->second.m_base_fees.value() == coinbase_value - parent_value); BOOST_CHECK(it_child != submit_cpfp.m_tx_results.end()); BOOST_CHECK(it_child->second.m_result_type == MempoolAcceptResult::ResultType::VALID); BOOST_CHECK(it_child->second.m_base_fees.value() == COIN); @@ -709,22 +716,28 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) } // Just because we allow low-fee parents doesn't mean we allow low-feerate packages. - // This package just pays 200 satoshis total. This would be enough to pay for the child alone, - // but isn't enough for the entire package to meet the 1sat/vbyte minimum. + // The mempool minimum feerate is 5sat/vB, but this package just pays 800 satoshis total. + // The child fees would be able to pay for itself, but isn't enough for the entire package. Package package_still_too_low; + const CAmount parent_fee{200}; + const CAmount child_fee{600}; auto mtx_parent_cheap = CreateValidMempoolTransaction(/*input_transaction=*/m_coinbase_txns[1], /*input_vout=*/0, /*input_height=*/0, /*input_signing_key=*/coinbaseKey, /*output_destination=*/parent_spk, - /*output_amount=*/coinbase_value, /*submit=*/false); + /*output_amount=*/coinbase_value - parent_fee, /*submit=*/false); CTransactionRef tx_parent_cheap = MakeTransactionRef(mtx_parent_cheap); package_still_too_low.push_back(tx_parent_cheap); + BOOST_CHECK(m_node.mempool->GetMinFee().GetFee(GetVirtualTransactionSize(*tx_parent_cheap)) > parent_fee); + BOOST_CHECK(m_node.mempool->m_min_relay_feerate.GetFee(GetVirtualTransactionSize(*tx_parent_cheap)) <= parent_fee); auto mtx_child_cheap = CreateValidMempoolTransaction(/*input_transaction=*/tx_parent_cheap, /*input_vout=*/0, /*input_height=*/101, /*input_signing_key=*/child_key, /*output_destination=*/child_spk, - /*output_amount=*/coinbase_value - 200, /*submit=*/false); + /*output_amount=*/coinbase_value - parent_fee - child_fee, /*submit=*/false); CTransactionRef tx_child_cheap = MakeTransactionRef(mtx_child_cheap); package_still_too_low.push_back(tx_child_cheap); + BOOST_CHECK(m_node.mempool->GetMinFee().GetFee(GetVirtualTransactionSize(*tx_child_cheap)) <= child_fee); + BOOST_CHECK(m_node.mempool->GetMinFee().GetFee(GetVirtualTransactionSize(*tx_parent_cheap) + GetVirtualTransactionSize(*tx_child_cheap)) > parent_fee + child_fee); // Cheap package should fail with package-fee-too-low. { @@ -735,11 +748,6 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) BOOST_CHECK_EQUAL(submit_package_too_low.m_state.GetResult(), PackageValidationResult::PCKG_POLICY); BOOST_CHECK_EQUAL(submit_package_too_low.m_state.GetRejectReason(), "package-fee-too-low"); BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); - const CFeeRate child_feerate(200, GetVirtualTransactionSize(*tx_child_cheap)); - BOOST_CHECK(child_feerate.GetFeePerK() > 1000); - const CFeeRate expected_feerate(200, - GetVirtualTransactionSize(*tx_parent_cheap) + GetVirtualTransactionSize(*tx_child_cheap)); - BOOST_CHECK(expected_feerate.GetFeePerK() < 1000); } // Package feerate includes the modified fees of the transactions. @@ -752,18 +760,18 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) expected_pool_size += 2; BOOST_CHECK_MESSAGE(submit_prioritised_package.m_state.IsValid(), "Package validation unexpectedly failed" << submit_prioritised_package.m_state.GetRejectReason()); - const CFeeRate expected_feerate(1 * COIN + 200, + const CFeeRate expected_feerate(1 * COIN + parent_fee + child_fee, GetVirtualTransactionSize(*tx_parent_cheap) + GetVirtualTransactionSize(*tx_child_cheap)); BOOST_CHECK_EQUAL(submit_prioritised_package.m_tx_results.size(), package_still_too_low.size()); auto it_parent = submit_prioritised_package.m_tx_results.find(tx_parent_cheap->GetWitnessHash()); auto it_child = submit_prioritised_package.m_tx_results.find(tx_child_cheap->GetWitnessHash()); BOOST_CHECK(it_parent != submit_prioritised_package.m_tx_results.end()); BOOST_CHECK(it_parent->second.m_result_type == MempoolAcceptResult::ResultType::VALID); - BOOST_CHECK(it_parent->second.m_base_fees.value() == 0); + BOOST_CHECK(it_parent->second.m_base_fees.value() == parent_fee); BOOST_CHECK(it_parent->second.m_effective_feerate.value() == expected_feerate); BOOST_CHECK(it_child != submit_prioritised_package.m_tx_results.end()); BOOST_CHECK(it_child->second.m_result_type == MempoolAcceptResult::ResultType::VALID); - BOOST_CHECK(it_child->second.m_base_fees.value() == 200); + BOOST_CHECK(it_child->second.m_base_fees.value() == child_fee); BOOST_CHECK(it_child->second.m_effective_feerate.value() == expected_feerate); std::vector<uint256> expected_wtxids({tx_parent_cheap->GetWitnessHash(), tx_child_cheap->GetWitnessHash()}); BOOST_CHECK(it_parent->second.m_wtxids_fee_calculations.value() == expected_wtxids); diff --git a/test/functional/mempool_limit.py b/test/functional/mempool_limit.py index d38a37f952..f8e86cafab 100755 --- a/test/functional/mempool_limit.py +++ b/test/functional/mempool_limit.py @@ -7,15 +7,21 @@ from decimal import Decimal from test_framework.blocktools import COINBASE_MATURITY +from test_framework.p2p import P2PTxInvStore from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, + assert_fee_amount, assert_greater_than, assert_raises_rpc_error, create_lots_of_big_transactions, gen_return_txouts, ) -from test_framework.wallet import MiniWallet +from test_framework.wallet import ( + COIN, + DEFAULT_FEE, + MiniWallet, +) class MempoolLimitTest(BitcoinTestFramework): @@ -44,7 +50,8 @@ class MempoolLimitTest(BitcoinTestFramework): # 1 to create a tx initially that will be evicted from the mempool later # 3 batches of multiple transactions with a fee rate much higher than the previous UTXO # And 1 more to verify that this tx does not get added to the mempool with a fee rate less than the mempoolminfee - self.generate(miniwallet, 1 + (num_of_batches * tx_batch_size) + 1) + # And 2 more for the package cpfp test + self.generate(miniwallet, 1 + (num_of_batches * tx_batch_size) + 1 + 2) # Mine 99 blocks so that the UTXOs are allowed to be spent self.generate(node, COINBASE_MATURITY - 1) @@ -76,6 +83,44 @@ class MempoolLimitTest(BitcoinTestFramework): self.log.info('Create a mempool tx that will not pass mempoolminfee') assert_raises_rpc_error(-26, "mempool min fee not met", miniwallet.send_self_transfer, from_node=node, fee_rate=relayfee) + self.log.info("Check that submitpackage allows cpfp of a parent below mempool min feerate") + node = self.nodes[0] + peer = node.add_p2p_connection(P2PTxInvStore()) + + # Package with 2 parents and 1 child. One parent has a high feerate due to modified fees, + # another is below the mempool minimum feerate but bumped by the child. + tx_poor = miniwallet.create_self_transfer(fee_rate=relayfee) + tx_rich = miniwallet.create_self_transfer(fee=0, fee_rate=0) + node.prioritisetransaction(tx_rich["txid"], 0, int(DEFAULT_FEE * COIN)) + package_txns = [tx_rich, tx_poor] + coins = [tx["new_utxo"] for tx in package_txns] + tx_child = miniwallet.create_self_transfer_multi(utxos_to_spend=coins, fee_per_output=10000) #DEFAULT_FEE + package_txns.append(tx_child) + + submitpackage_result = node.submitpackage([tx["hex"] for tx in package_txns]) + + rich_parent_result = submitpackage_result["tx-results"][tx_rich["wtxid"]] + poor_parent_result = submitpackage_result["tx-results"][tx_poor["wtxid"]] + child_result = submitpackage_result["tx-results"][tx_child["tx"].getwtxid()] + assert_fee_amount(poor_parent_result["fees"]["base"], tx_poor["tx"].get_vsize(), relayfee) + assert_equal(rich_parent_result["fees"]["base"], 0) + assert_equal(child_result["fees"]["base"], DEFAULT_FEE) + # The "rich" parent does not require CPFP so its effective feerate is just its individual feerate. + assert_fee_amount(DEFAULT_FEE, tx_rich["tx"].get_vsize(), rich_parent_result["fees"]["effective-feerate"]) + assert_equal(rich_parent_result["fees"]["effective-includes"], [tx_rich["wtxid"]]) + # The "poor" parent and child's effective feerates are the same, composed of their total + # fees divided by their combined vsize. + package_fees = poor_parent_result["fees"]["base"] + child_result["fees"]["base"] + package_vsize = tx_poor["tx"].get_vsize() + tx_child["tx"].get_vsize() + assert_fee_amount(package_fees, package_vsize, poor_parent_result["fees"]["effective-feerate"]) + assert_fee_amount(package_fees, package_vsize, child_result["fees"]["effective-feerate"]) + assert_equal([tx_poor["wtxid"], tx_child["tx"].getwtxid()], poor_parent_result["fees"]["effective-includes"]) + assert_equal([tx_poor["wtxid"], tx_child["tx"].getwtxid()], child_result["fees"]["effective-includes"]) + + # The node will broadcast each transaction, still abiding by its peer's fee filter + peer.wait_for_broadcast([tx["tx"].getwtxid() for tx in package_txns]) + self.generate(node, 1) + self.log.info('Test passing a value below the minimum (5 MB) to -maxmempool throws an error') self.stop_node(0) self.nodes[0].assert_start_raises_init_error(["-maxmempool=4"], "Error: -maxmempool must be at least 5 MB") diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py index 6cb9760b3d..ae1a498e28 100755 --- a/test/functional/rpc_packages.py +++ b/test/functional/rpc_packages.py @@ -20,7 +20,6 @@ from test_framework.util import ( assert_raises_rpc_error, ) from test_framework.wallet import ( - COIN, DEFAULT_FEE, MiniWallet, ) @@ -325,42 +324,6 @@ class RPCPackagesTest(BitcoinTestFramework): peer.wait_for_broadcast([tx["tx"].getwtxid() for tx in package_txns]) self.generate(node, 1) - def test_submit_cpfp(self): - node = self.nodes[0] - peer = node.add_p2p_connection(P2PTxInvStore()) - - # Package with 2 parents and 1 child. One parent pays for itself using modified fees, and - # another has 0 fees but is bumped by child. - tx_poor = self.wallet.create_self_transfer(fee=0, fee_rate=0) - tx_rich = self.wallet.create_self_transfer(fee=0, fee_rate=0) - node.prioritisetransaction(tx_rich["txid"], 0, int(DEFAULT_FEE * COIN)) - package_txns = [tx_rich, tx_poor] - coins = [tx["new_utxo"] for tx in package_txns] - tx_child = self.wallet.create_self_transfer_multi(utxos_to_spend=coins, fee_per_output=10000) #DEFAULT_FEE - package_txns.append(tx_child) - - submitpackage_result = node.submitpackage([tx["hex"] for tx in package_txns]) - - rich_parent_result = submitpackage_result["tx-results"][tx_rich["wtxid"]] - poor_parent_result = submitpackage_result["tx-results"][tx_poor["wtxid"]] - child_result = submitpackage_result["tx-results"][tx_child["tx"].getwtxid()] - assert_equal(rich_parent_result["fees"]["base"], 0) - assert_equal(poor_parent_result["fees"]["base"], 0) - assert_equal(child_result["fees"]["base"], DEFAULT_FEE) - # The "rich" parent does not require CPFP so its effective feerate. - assert_fee_amount(DEFAULT_FEE, tx_rich["tx"].get_vsize(), rich_parent_result["fees"]["effective-feerate"]) - assert_equal(rich_parent_result["fees"]["effective-includes"], [tx_rich["wtxid"]]) - # The "poor" parent and child's effective feerates are the same, composed of the child's fee - # divided by their combined vsize. - assert_fee_amount(DEFAULT_FEE, tx_poor["tx"].get_vsize() + tx_child["tx"].get_vsize(), poor_parent_result["fees"]["effective-feerate"]) - assert_fee_amount(DEFAULT_FEE, tx_poor["tx"].get_vsize() + tx_child["tx"].get_vsize(), child_result["fees"]["effective-feerate"]) - assert_equal([tx_poor["wtxid"], tx_child["tx"].getwtxid()], poor_parent_result["fees"]["effective-includes"]) - assert_equal([tx_poor["wtxid"], tx_child["tx"].getwtxid()], child_result["fees"]["effective-includes"]) - - # The node will broadcast each transaction, still abiding by its peer's fee filter - peer.wait_for_broadcast([tx["tx"].getwtxid() for tx in package_txns]) - self.generate(node, 1) - def test_submitpackage(self): node = self.nodes[0] @@ -369,9 +332,6 @@ class RPCPackagesTest(BitcoinTestFramework): self.test_submit_child_with_parents(num_parents, False) self.test_submit_child_with_parents(num_parents, True) - self.log.info("Submitpackage valid packages with CPFP") - self.test_submit_cpfp() - self.log.info("Submitpackage only allows packages of 1 child with its parents") # Chain of 3 transactions has too many generations chain_hex = [t["hex"] for t in self.wallet.create_self_transfer_chain(chain_length=25)] |