[rpc] permit any package, not just child-with-unconfirmed-parents

glozow · glozow · commit 9b1df49d0cdc · 2025-10-30T11:26:38.000-04:00
We can safely allow any package since MiniGraph can linearize and group
things into ancestor subsets.
diff --git a/doc/policy/packages.md b/doc/policy/packages.md
@@ -8,10 +8,6 @@ Graph (a directed edge exists between a transaction that spends the output of an
 For every transaction `t` in a **topologically sorted** package, if any of its parents are present
 in the package, they appear somewhere in the list before `t`.
 
-A **child-with-parents** package is a topologically sorted package that consists of exactly one child and at least one
-of its unconfirmed parents.  Not all unconfirmed parents need to be present but no other transactions may be present; the
-parent of a parent should not be in this package (unless this "grandparent" is also a direct parent of the child).
-
 ## Package Mempool Acceptance Rules
 
 The following rules are enforced for all packages:
@@ -57,18 +53,8 @@ The following rules are enforced for all packages:
 The following rules are only enforced for packages to be submitted to the mempool (not
 enforced for test accepts):
 
-* Packages must be child-with-parents packages. This also means packages must contain at
-  least 1 transaction. (#31096)
-
-   - *Rationale*: This allows for fee-bumping by CPFP. Allowing multiple parents makes it possible
-     to fee-bump a batch of transactions. Restricting packages to a defined topology is easier to
-     reason about and simplifies the validation logic greatly.
-
-   - Warning: Batched fee-bumping may be unsafe for some use cases. Users and application developers
-     should take caution if utilizing multi-parent packages.
-
-* Transactions in the package that have the same txid as another transaction already in the mempool
-  will be removed from the package prior to submission ("deduplication").
+* Transactions in the package that are already in the mempool or have the same txid as another
+  transaction already in the mempool are skipped ("deduplication").
 
    - *Rationale*: Node operators are free to set their mempool policies however they please, nodes
      may receive transactions in different orders, and malicious counterparties may try to take
diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp
@@ -1144,10 +1144,6 @@ static RPCHelpMan submitpackage()
                 txns.emplace_back(MakeTransactionRef(std::move(mtx)));
             }
             CHECK_NONFATAL(!txns.empty());
-            if (txns.size() > 1 && !IsChildWithParentsTree(txns)) {
-                throw JSONRPCTransactionError(TransactionError::INVALID_PACKAGE, "package topology disallowed. not child-with-parents or parents depend on each other.");
-            }
-
             NodeContext& node = EnsureAnyNodeContext(request.context);
             CTxMemPool& mempool = EnsureMemPool(node);
             Chainstate& chainstate = EnsureChainman(node).ActiveChainstate();
diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp
@@ -353,74 +353,122 @@ BOOST_AUTO_TEST_CASE(noncontextual_package_tests)
     }
 }
 
-BOOST_AUTO_TEST_CASE(package_submission_tests)
+BOOST_AUTO_TEST_CASE(package_topology)
 {
     // Mine blocks to mature coinbases.
-    mineBlocks(60);
+    mineBlocks(20);
     CFeeRate minfeerate(5000);
     MockMempoolMinFee(minfeerate);
     LOCK(cs_main);
     unsigned int expected_pool_size = m_node.mempool->size();
     CKey parent_key = GenerateRandomKey();
     CScript parent_locking_script = GetScriptForDestination(PKHash(parent_key.GetPubKey()));
     const CAmount coinbase_value{50 * COIN};
+    const CAmount generous_fee{1000};
 
-    // Unrelated transactions are not allowed in package submission.
     Package package_unrelated;
     for (size_t i{0}; i < 10; ++i) {
-        auto mtx = CreateValidMempoolTransaction(/*input_transaction=*/m_coinbase_txns[i + 25], /*input_vout=*/0,
+        auto mtx = CreateValidMempoolTransaction(/*input_transaction=*/m_coinbase_txns[i], /*input_vout=*/0,
                                                  /*input_height=*/0, /*input_signing_key=*/coinbaseKey,
                                                  /*output_destination=*/parent_locking_script,
-                                                 /*output_amount=*/CAmount(49 * COIN), /*submit=*/false);
+                                                 /*output_amount=*/coinbase_value - generous_fee, /*submit=*/false);
         package_unrelated.emplace_back(MakeTransactionRef(mtx));
     }
     auto result_unrelated_submit = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
                                                      package_unrelated, /*test_accept=*/false, /*client_maxfeerate=*/{});
-    // We don't expect m_tx_results for each transaction when basic sanity checks haven't passed.
-    BOOST_CHECK(result_unrelated_submit.m_state.IsInvalid());
-    BOOST_CHECK_EQUAL(result_unrelated_submit.m_state.GetResult(), PackageValidationResult::PCKG_POLICY);
-    BOOST_CHECK_EQUAL(result_unrelated_submit.m_state.GetRejectReason(), "package-not-child-with-parents");
+    BOOST_CHECK(result_unrelated_submit.m_state.IsValid());
+    expected_pool_size += 10;
     BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
-
+    // We should see a result for each transaction. They should have been validated individually.
+    for (const auto& tx : package_unrelated) {
+        auto it = result_unrelated_submit.m_tx_results.find(tx->GetWitnessHash());
+        BOOST_CHECK(it != result_unrelated_submit.m_tx_results.end());
+        BOOST_CHECK(it->second.m_state.IsValid());
+        BOOST_CHECK_EQUAL(it->second.m_subpackage_wtxids.value().size(), 1);
+    }
     // Parent and Child (and Grandchild) Package
-    Package package_parent_child;
     Package package_3gen;
-    auto mtx_parent = CreateValidMempoolTransaction(/*input_transaction=*/m_coinbase_txns[0], /*input_vout=*/0,
+    auto mtx_parent = CreateValidMempoolTransaction(/*input_transaction=*/m_coinbase_txns[10], /*input_vout=*/0,
                                                     /*input_height=*/0, /*input_signing_key=*/coinbaseKey,
                                                     /*output_destination=*/parent_locking_script,
-                                                    /*output_amount=*/CAmount(49 * COIN), /*submit=*/false);
+                                                    /*output_amount=*/coinbase_value - generous_fee, /*submit=*/false);
     CTransactionRef tx_parent = MakeTransactionRef(mtx_parent);
-    package_parent_child.push_back(tx_parent);
     package_3gen.push_back(tx_parent);
 
     CKey child_key = GenerateRandomKey();
     CScript child_locking_script = GetScriptForDestination(PKHash(child_key.GetPubKey()));
     auto mtx_child = CreateValidMempoolTransaction(/*input_transaction=*/tx_parent, /*input_vout=*/0,
                                                    /*input_height=*/101, /*input_signing_key=*/parent_key,
                                                    /*output_destination=*/child_locking_script,
-                                                   /*output_amount=*/CAmount(48 * COIN), /*submit=*/false);
+                                                   /*output_amount=*/coinbase_value - 2 * generous_fee, /*submit=*/false);
     CTransactionRef tx_child = MakeTransactionRef(mtx_child);
-    package_parent_child.push_back(tx_child);
     package_3gen.push_back(tx_child);
 
     CKey grandchild_key = GenerateRandomKey();
     CScript grandchild_locking_script = GetScriptForDestination(PKHash(grandchild_key.GetPubKey()));
     auto mtx_grandchild = CreateValidMempoolTransaction(/*input_transaction=*/tx_child, /*input_vout=*/0,
                                                        /*input_height=*/101, /*input_signing_key=*/child_key,
                                                        /*output_destination=*/grandchild_locking_script,
-                                                       /*output_amount=*/CAmount(47 * COIN), /*submit=*/false);
+                                                       /*output_amount=*/coinbase_value - 3 * generous_fee, /*submit=*/false);
     CTransactionRef tx_grandchild = MakeTransactionRef(mtx_grandchild);
     package_3gen.push_back(tx_grandchild);
 
-    // 3 Generations is not allowed.
+    // Submit package parent + child + grandchild.
     {
         auto result_3gen_submit = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
                                                     package_3gen, /*test_accept=*/false, /*client_maxfeerate=*/{});
-        BOOST_CHECK(result_3gen_submit.m_state.IsInvalid());
-        BOOST_CHECK_EQUAL(result_3gen_submit.m_state.GetResult(), PackageValidationResult::PCKG_POLICY);
-        BOOST_CHECK_EQUAL(result_3gen_submit.m_state.GetRejectReason(), "package-not-child-with-parents");
+        expected_pool_size += 3;
+        BOOST_CHECK_MESSAGE(result_3gen_submit.m_state.IsValid(),
+                            "Package validation unexpectedly failed: " << result_3gen_submit.m_state.GetRejectReason());
+        BOOST_CHECK_EQUAL(result_3gen_submit.m_tx_results.size(), package_3gen.size());
+        auto it_parent = result_3gen_submit.m_tx_results.find(tx_parent->GetWitnessHash());
+        auto it_child = result_3gen_submit.m_tx_results.find(tx_child->GetWitnessHash());
+        auto it_grandchild = result_3gen_submit.m_tx_results.find(tx_grandchild->GetWitnessHash());
+
+        BOOST_CHECK(it_parent->second.m_effective_feerate == CFeeRate(generous_fee, GetVirtualTransactionSize(*tx_parent)));
+        BOOST_CHECK_EQUAL(it_parent->second.m_subpackage_wtxids.value().size(), 1);
+        BOOST_CHECK_EQUAL(it_parent->second.m_subpackage_wtxids.value().front(), tx_parent->GetWitnessHash());
+        BOOST_CHECK(it_child->second.m_effective_feerate == CFeeRate(generous_fee, GetVirtualTransactionSize(*tx_child)));
+        BOOST_CHECK_EQUAL(it_child->second.m_subpackage_wtxids.value().size(), 1);
+        BOOST_CHECK_EQUAL(it_child->second.m_subpackage_wtxids.value().front(), tx_child->GetWitnessHash());
+
+        BOOST_CHECK(it_grandchild->second.m_effective_feerate == CFeeRate(generous_fee, GetVirtualTransactionSize(*tx_grandchild)));
+        BOOST_CHECK_EQUAL(it_grandchild->second.m_subpackage_wtxids.value().size(), 1);
+        BOOST_CHECK_EQUAL(it_grandchild->second.m_subpackage_wtxids.value().front(), tx_grandchild->GetWitnessHash());
+
         BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
     }
+}
+
+BOOST_AUTO_TEST_CASE(package_submission_tests)
+{
+    // Mine blocks to mature coinbases.
+    mineBlocks(60);
+    CFeeRate minfeerate(5000);
+    MockMempoolMinFee(minfeerate);
+    LOCK(cs_main);
+    unsigned int expected_pool_size = m_node.mempool->size();
+    CKey parent_key = GenerateRandomKey();
+    CScript parent_locking_script = GetScriptForDestination(PKHash(parent_key.GetPubKey()));
+    const CAmount coinbase_value{50 * COIN};
+
+    // Parent and Child
+    Package package_parent_child;
+    auto mtx_parent = CreateValidMempoolTransaction(/*input_transaction=*/m_coinbase_txns[0], /*input_vout=*/0,
+                                                    /*input_height=*/0, /*input_signing_key=*/coinbaseKey,
+                                                    /*output_destination=*/parent_locking_script,
+                                                    /*output_amount=*/CAmount(49 * COIN), /*submit=*/false);
+    CTransactionRef tx_parent = MakeTransactionRef(mtx_parent);
+    package_parent_child.push_back(tx_parent);
+
+    CKey child_key = GenerateRandomKey();
+    CScript child_locking_script = GetScriptForDestination(PKHash(child_key.GetPubKey()));
+    auto mtx_child = CreateValidMempoolTransaction(/*input_transaction=*/tx_parent, /*input_vout=*/0,
+                                                   /*input_height=*/101, /*input_signing_key=*/parent_key,
+                                                   /*output_destination=*/child_locking_script,
+                                                   /*output_amount=*/CAmount(48 * COIN), /*submit=*/false);
+    CTransactionRef tx_child = MakeTransactionRef(mtx_child);
+    package_parent_child.push_back(tx_child);
 
     // Parent and child package where transactions are invalid for reasons other than fee and
     // missing inputs, so the package validation isn't expected to happen.
@@ -448,22 +496,21 @@ BOOST_AUTO_TEST_CASE(package_submission_tests)
         BOOST_CHECK_EQUAL(result_quit_early.m_state.GetResult(), PackageValidationResult::PCKG_TX);
     }
 
-    // Submit package with parent + child.
+    // Submit package parent + child
     {
-        const auto submit_parent_child = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
-                                                           package_parent_child, /*test_accept=*/false, /*client_maxfeerate=*/{});
+        auto result_parent_child_submit = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
+                                                    package_parent_child, /*test_accept=*/false, /*client_maxfeerate=*/{});
         expected_pool_size += 2;
-        BOOST_CHECK_MESSAGE(submit_parent_child.m_state.IsValid(),
-                            "Package validation unexpectedly failed: " << submit_parent_child.m_state.GetRejectReason());
-        BOOST_CHECK_EQUAL(submit_parent_child.m_tx_results.size(), package_parent_child.size());
-        auto it_parent = submit_parent_child.m_tx_results.find(tx_parent->GetWitnessHash());
-        auto it_child = submit_parent_child.m_tx_results.find(tx_child->GetWitnessHash());
-        BOOST_CHECK(it_parent != submit_parent_child.m_tx_results.end());
-        BOOST_CHECK(it_parent->second.m_state.IsValid());
-        BOOST_CHECK(it_parent->second.m_effective_feerate == CFeeRate(1 * COIN, GetVirtualTransactionSize(*tx_parent)));
+        BOOST_CHECK_MESSAGE(result_parent_child_submit.m_state.IsValid(),
+                            "Package validation unexpectedly failed: " << result_parent_child_submit.m_state.GetRejectReason());
+        BOOST_CHECK_EQUAL(result_parent_child_submit.m_tx_results.size(), package_parent_child.size());
+        auto it_parent = result_parent_child_submit.m_tx_results.find(tx_parent->GetWitnessHash());
+        auto it_child = result_parent_child_submit.m_tx_results.find(tx_child->GetWitnessHash());
+
+        BOOST_CHECK(it_parent->second.m_effective_feerate == CFeeRate(COIN, GetVirtualTransactionSize(*tx_parent)));
         BOOST_CHECK_EQUAL(it_parent->second.m_subpackage_wtxids.value().size(), 1);
         BOOST_CHECK_EQUAL(it_parent->second.m_subpackage_wtxids.value().front(), tx_parent->GetWitnessHash());
-        BOOST_CHECK(it_child->second.m_effective_feerate == CFeeRate(1 * COIN, GetVirtualTransactionSize(*tx_child)));
+        BOOST_CHECK(it_child->second.m_effective_feerate == CFeeRate(COIN, GetVirtualTransactionSize(*tx_child)));
         BOOST_CHECK_EQUAL(it_child->second.m_subpackage_wtxids.value().size(), 1);
         BOOST_CHECK_EQUAL(it_child->second.m_subpackage_wtxids.value().front(), tx_child->GetWitnessHash());
 
@@ -1530,15 +1577,15 @@ BOOST_FIXTURE_TEST_CASE(linearization_tests, TestChain100Setup)
         //     3sat/vB     3sat/vB    20sat/vB
         //           ^     ^     ^    ^
         //          parent1     parent2
-        //          7sat/vB     7sat/vB
+        //          8sat/vB     8sat/vB
         //         ^      ^      ^     ^
         //                  child
         //                 1sat/vB
         //
         //  child is also spending all the grandparents so that this is a child-with-parents package.
         const CFeeRate feerate_grandparents_low(3000);
         const CFeeRate feerate_grandparent_high(20000);
-        const CFeeRate feerate_parents(8000);
+        const CFeeRate feerate_parents(8200);
         const CFeeRate feerate_child(1000);
         const CFeeRate mempool_min_feerate{m_node.mempool->GetMinFee()};
 
diff --git a/src/validation.cpp b/src/validation.cpp
@@ -512,8 +512,8 @@ class MemPoolAccept
             };
         }
 
-        /** Parameters for child-with-parents package validation. */
-        static ATMPArgs PackageChildWithParents(const CChainParams& chainparams, int64_t accept_time,
+        /** Parameters for package validation of a tx with ancestors. */
+        static ATMPArgs AnyPackageAccept(const CChainParams& chainparams, int64_t accept_time,
                                                 std::vector<COutPoint>& coins_to_uncache, const std::optional<CFeeRate>& client_maxfeerate) {
             return ATMPArgs{/* m_chainparams */ chainparams,
                             /* m_accept_time */ accept_time,
@@ -1616,26 +1616,14 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
     // Used if returning a PackageMempoolAcceptResult directly from this function.
     PackageValidationState package_state_quit_early;
 
-    // There are two topologies we are able to handle through this function:
-    // (1) A single transaction
-    // (2) A child-with-parents package.
     // 1. Check that the package is well-formed. If it isn't, we won't try to validate any of the
     // transactions and thus won't return any MempoolAcceptResults, just a package-wide error.
-
     // Context-free package checks.
     if (!IsWellFormedPackage(package, package_state_quit_early, /*require_sorted=*/true)) {
         return PackageMempoolAcceptResult(package_state_quit_early, {});
     }
 
-    if (package.size() > 1 && !IsChildWithParents(package)) {
-        // All transactions in the package must be a parent of the last transaction. This is just an
-        // opportunity for us to fail fast on a context-free check without taking the mempool lock.
-        package_state_quit_early.Invalid(PackageValidationResult::PCKG_POLICY, "package-not-child-with-parents");
-        return PackageMempoolAcceptResult(package_state_quit_early, {});
-    }
-
     PackageValidationState package_state_final;
-
     LOCK(m_pool.cs);
     MiniGraph package_sorter(package, std::max(m_pool.GetMinFee(), m_pool.m_opts.min_relay_feerate));
 
@@ -1950,7 +1938,7 @@ PackageMempoolAcceptResult ProcessNewPackage(Chainstate& active_chainstate, CTxM
             auto args = MemPoolAccept::ATMPArgs::PackageTestAccept(chainparams, GetTime(), coins_to_uncache);
             return MemPoolAccept(pool, active_chainstate).AcceptMultipleTransactions(package, args);
         } else {
-            auto args = MemPoolAccept::ATMPArgs::PackageChildWithParents(chainparams, GetTime(), coins_to_uncache, client_maxfeerate);
+            auto args = MemPoolAccept::ATMPArgs::AnyPackageAccept(chainparams, GetTime(), coins_to_uncache, client_maxfeerate);
             return MemPoolAccept(pool, active_chainstate).AcceptPackage(package, args);
         }
     }();
diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py
