diff --git a/doc/policy/packages.md b/doc/policy/packages.md index 4795f715e80..d48afed4ee5 100644 --- a/doc/policy/packages.md +++ b/doc/policy/packages.md @@ -99,14 +99,6 @@ submitted as a package. transaction (i.e. in which a replacement transaction with a higher fee cannot be signed) being rejected from the mempool when transaction volume is high and the mempool minimum feerate rises. -Note: Package feerate cannot be used to meet the minimum relay feerate (`-minrelaytxfee`) -requirement. For example, if the mempool minimum feerate is 5sat/vB and the minimum relay feerate is -set to 5sat/vB, a 1sat/vB parent transaction with a high-feerate child will not be accepted, even if -submitted as a package. Note that this rule does not apply to -[TRUC transactions](https://github.com/bitcoin/bips/blob/master/bip-0431.mediawiki) as an individual -TRUC transaction is permitted to be below the mempool min relay feerate, assuming it is considered within -a package that meets the mempool's feerate requirements. - *Rationale*: Avoid situations in which the mempool contains non-bumped transactions below min relay feerate (which we consider to have pay 0 fees and thus receiving free relay). While package submission would ensure these transactions are bumped at the time of entry, it is not guaranteed diff --git a/doc/release-notes-33892.md b/doc/release-notes-33892.md new file mode 100644 index 00000000000..e9d41b80122 --- /dev/null +++ b/doc/release-notes-33892.md @@ -0,0 +1,8 @@ +P2P and network changes +----------------------- + +- Transactions participating in one-parent-one-child package relay can now have the parent + with a feerate lower than the `-minrelaytxfee` feerate, even 0 fee. This expands the change + from 28.0 to also cover packages of non-TRUC transactions. Note that in general the + package child can have additional unconfirmed parents, but they must already be + in-mempool for the new package to be relayed. (#33892) diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp index 3de62a0b41e..bb4b83ed368 100644 --- a/src/test/txpackage_tests.cpp +++ b/src/test/txpackage_tests.cpp @@ -908,10 +908,10 @@ BOOST_AUTO_TEST_CASE(package_cpfp_tests) } else { BOOST_CHECK_EQUAL(submit_cpfp_deprio.m_state.GetResult(), PackageValidationResult::PCKG_TX); BOOST_CHECK_EQUAL(submit_cpfp_deprio.m_tx_results.find(tx_parent->GetWitnessHash())->second.m_state.GetResult(), - TxValidationResult::TX_MEMPOOL_POLICY); + TxValidationResult::TX_RECONSIDERABLE); BOOST_CHECK_EQUAL(submit_cpfp_deprio.m_tx_results.find(tx_child->GetWitnessHash())->second.m_state.GetResult(), - TxValidationResult::TX_MISSING_INPUTS); - BOOST_CHECK(submit_cpfp_deprio.m_tx_results.find(tx_parent->GetWitnessHash())->second.m_state.GetRejectReason() == "min relay fee not met"); + TxValidationResult::TX_RECONSIDERABLE); + BOOST_CHECK_EQUAL(submit_cpfp_deprio.m_tx_results.find(tx_parent->GetWitnessHash())->second.m_state.GetRejectReason(), "mempool min fee not met"); BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); } } @@ -1067,8 +1067,8 @@ BOOST_AUTO_TEST_CASE(package_cpfp_tests) strprintf("rich parent: expected fee %s, got %s", high_parent_fee, it_parent->second.m_base_fees.value())); BOOST_CHECK(it_parent->second.m_effective_feerate == CFeeRate(high_parent_fee, GetVirtualTransactionSize(*tx_parent_rich))); BOOST_CHECK_EQUAL(it_child->second.m_result_type, MempoolAcceptResult::ResultType::INVALID); - BOOST_CHECK_EQUAL(it_child->second.m_state.GetResult(), TxValidationResult::TX_MEMPOOL_POLICY); - BOOST_CHECK(it_child->second.m_state.GetRejectReason() == "min relay fee not met"); + BOOST_CHECK_EQUAL(it_child->second.m_state.GetResult(), TxValidationResult::TX_RECONSIDERABLE); + BOOST_CHECK_EQUAL(it_child->second.m_state.GetRejectReason(), "mempool min fee not met"); } expected_pool_size += 1; BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); diff --git a/src/validation.cpp b/src/validation.cpp index 04f60e7bd56..fe4237db2c8 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -943,18 +943,6 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "bad-txns-too-many-sigops", strprintf("%d", nSigOpsCost)); - // No individual transactions are allowed below the min relay feerate except from disconnected blocks. - // This requirement, unlike CheckFeeRate, cannot be bypassed using m_package_feerates because, - // 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. - // The only exception is TRUC transactions. - if (!bypass_limits && ws.m_ptx->version != TRUC_VERSION && ws.m_modified_fees < m_pool.m_opts.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", - strprintf("%d < %d", ws.m_modified_fees, m_pool.m_opts.min_relay_feerate.GetFee(ws.m_vsize))); - } // No individual transactions are allowed below the mempool min feerate except from disconnected // blocks and transactions in a package. Package transactions will be checked using package // feerate later. diff --git a/test/functional/mempool_ephemeral_dust.py b/test/functional/mempool_ephemeral_dust.py index 0616ae8fa4d..7d45fcb828c 100755 --- a/test/functional/mempool_ephemeral_dust.py +++ b/test/functional/mempool_ephemeral_dust.py @@ -216,18 +216,16 @@ class EphemeralDustTest(BitcoinTestFramework): self.connect_nodes(0, 1) assert_mempool_contents(self, self.nodes[0], expected=[]) - # N.B. If individual minrelay requirement is dropped, this test can be dropped def test_non_truc(self): - self.log.info("Test that v2 dust-having transaction is rejected even if spent, because of min relay requirement") + self.log.info("Test that v2 dust-having transaction is also accepted if spent") assert_equal(self.nodes[0].getrawmempool(), []) dusty_tx, sweep_tx = self.create_ephemeral_dust_package(tx_version=2) res = self.nodes[0].submitpackage([dusty_tx["hex"], sweep_tx["hex"]]) - assert_equal(res["package_msg"], "transaction failed") - assert_equal(res["tx-results"][dusty_tx["wtxid"]]["error"], "min relay fee not met, 0 < 15") - - assert_equal(self.nodes[0].getrawmempool(), []) + assert_equal(res["package_msg"], "success") + assert_mempool_contents(self, self.nodes[0], expected=[dusty_tx["tx"], sweep_tx["tx"]]) + self.generate(self.nodes[0], 1) def test_unspent_ephemeral(self): self.log.info("Test that spending from a tx with ephemeral outputs is only allowed if dust is spent as well") diff --git a/test/functional/mempool_truc.py b/test/functional/mempool_truc.py index 9cd30287e5b..2586308e4d3 100755 --- a/test/functional/mempool_truc.py +++ b/test/functional/mempool_truc.py @@ -612,7 +612,7 @@ class MempoolTRUC(BitcoinTestFramework): @cleanup(extra_args=None) def test_minrelay_in_package_combos(self): node = self.nodes[0] - self.log.info("Test that only TRUC transactions can be under minrelaytxfee for various settings...") + self.log.info("Test that all transaction versions can be under minrelaytxfee for various settings...") for minrelay_setting in (0, 5, 10, 100, 500, 1000, 5000, 333333, 2500000): self.log.info(f"-> Test -minrelaytxfee={minrelay_setting}sat/kvB...") @@ -649,14 +649,8 @@ class MempoolTRUC(BitcoinTestFramework): assert_equal(result_truc["package_msg"], "success") result_non_truc = node.submitpackage([tx_v2_0fee_parent["hex"], tx_v2_child["hex"]], maxfeerate=0) - if minrelayfeerate > 0: - assert_equal(result_non_truc["package_msg"], "transaction failed") - min_fee_parent = int(get_fee(tx_v2_0fee_parent["tx"].get_vsize(), minrelayfeerate) * COIN) - assert_equal(result_non_truc["tx-results"][tx_v2_0fee_parent["wtxid"]]["error"], f"min relay fee not met, 0 < {min_fee_parent}") - self.check_mempool([tx_v3_0fee_parent["txid"], tx_v3_child["txid"]]) - else: - assert_equal(result_non_truc["package_msg"], "success") - self.check_mempool([tx_v2_0fee_parent["txid"], tx_v2_child["txid"], tx_v3_0fee_parent["txid"], tx_v3_child["txid"]]) + assert_equal(result_non_truc["package_msg"], "success") + self.check_mempool([tx_v2_0fee_parent["txid"], tx_v2_child["txid"], tx_v3_0fee_parent["txid"], tx_v3_child["txid"]]) def run_test(self): diff --git a/test/functional/p2p_orphan_handling.py b/test/functional/p2p_orphan_handling.py index 48009343ea2..4cd59f8ff14 100755 --- a/test/functional/p2p_orphan_handling.py +++ b/test/functional/p2p_orphan_handling.py @@ -5,6 +5,7 @@ import time +from test_framework.blocktools import MAX_STANDARD_TX_WEIGHT from test_framework.mempool_util import ( create_large_orphan, tx_in_orphanage, @@ -188,15 +189,15 @@ class OrphanHandlingTest(BitcoinTestFramework): peer2 = node.add_p2p_connection(PeerTxRelayer()) self.log.info("Test orphan handling when a nonsegwit parent is known to be invalid") - parent_low_fee_nonsegwit = self.wallet_nonsegwit.create_self_transfer(fee_rate=0) - assert_equal(parent_low_fee_nonsegwit["txid"], parent_low_fee_nonsegwit["tx"].wtxid_hex) + parent_overly_large_nonsegwit = self.wallet_nonsegwit.create_self_transfer(target_vsize=int(MAX_STANDARD_TX_WEIGHT / 4) + 1) + assert_equal(parent_overly_large_nonsegwit["txid"], parent_overly_large_nonsegwit["tx"].wtxid_hex) parent_other = self.wallet_nonsegwit.create_self_transfer() child_nonsegwit = self.wallet_nonsegwit.create_self_transfer_multi( - utxos_to_spend=[parent_other["new_utxo"], parent_low_fee_nonsegwit["new_utxo"]]) + utxos_to_spend=[parent_other["new_utxo"], parent_overly_large_nonsegwit["new_utxo"]]) - # Relay the parent. It should be rejected because it pays 0 fees. - self.relay_transaction(peer1, parent_low_fee_nonsegwit["tx"]) - assert parent_low_fee_nonsegwit["txid"] not in node.getrawmempool() + # Relay the parent. It should be rejected (and not reconsiderable) because it violated size limitations. + self.relay_transaction(peer1, parent_overly_large_nonsegwit["tx"]) + assert parent_overly_large_nonsegwit["txid"] not in node.getrawmempool() # Relay the child. It should not be accepted because it has missing inputs. # Its parent should not be requested because its hash (txid == wtxid) has been added to the rejection filter. @@ -208,7 +209,7 @@ class OrphanHandlingTest(BitcoinTestFramework): self.nodes[0].bumpmocktime(GETDATA_TX_INTERVAL) peer1.assert_never_requested(int(parent_other["txid"], 16)) peer2.assert_never_requested(int(parent_other["txid"], 16)) - peer2.assert_never_requested(int(parent_low_fee_nonsegwit["txid"], 16)) + peer2.assert_never_requested(int(parent_overly_large_nonsegwit["txid"], 16)) self.log.info("Test orphan handling when a segwit parent was invalid but may be retried with another witness") parent_low_fee = self.wallet.create_self_transfer(fee_rate=0) @@ -391,23 +392,23 @@ class OrphanHandlingTest(BitcoinTestFramework): peer3 = node.add_p2p_connection(PeerTxRelayer(wtxidrelay=False)) self.log.info("Test that an orphan with rejected parents, along with any descendants, cannot be retried with an alternate witness") - parent_low_fee_nonsegwit = self.wallet_nonsegwit.create_self_transfer(fee_rate=0) - assert_equal(parent_low_fee_nonsegwit["txid"], parent_low_fee_nonsegwit["tx"].wtxid_hex) - child = self.wallet.create_self_transfer(utxo_to_spend=parent_low_fee_nonsegwit["new_utxo"]) + parent_overly_large_nonsegwit = self.wallet_nonsegwit.create_self_transfer(target_vsize=int(MAX_STANDARD_TX_WEIGHT / 4) + 1) + assert_equal(parent_overly_large_nonsegwit["txid"], parent_overly_large_nonsegwit["tx"].wtxid_hex) + child = self.wallet.create_self_transfer(utxo_to_spend=parent_overly_large_nonsegwit["new_utxo"]) grandchild = self.wallet.create_self_transfer(utxo_to_spend=child["new_utxo"]) assert_not_equal(child["txid"], child["tx"].wtxid_hex) assert_not_equal(grandchild["txid"], grandchild["tx"].wtxid_hex) # Relay the parent. It should be rejected because it pays 0 fees. - self.relay_transaction(peer1, parent_low_fee_nonsegwit["tx"]) - assert parent_low_fee_nonsegwit["txid"] not in node.getrawmempool() + self.relay_transaction(peer1, parent_overly_large_nonsegwit["tx"]) + assert parent_overly_large_nonsegwit["txid"] not in node.getrawmempool() # Relay the child. It should be rejected for having missing parents, and this rejection is # cached by txid and wtxid. self.relay_transaction(peer1, child["tx"]) assert_equal(0, len(node.getrawmempool())) assert not tx_in_orphanage(node, child["tx"]) - peer1.assert_never_requested(parent_low_fee_nonsegwit["txid"]) + peer1.assert_never_requested(parent_overly_large_nonsegwit["txid"]) # Grandchild should also not be kept in orphanage because its parent has been rejected. self.relay_transaction(peer2, grandchild["tx"])