relax child-with-unconfirmed-parents rule

doc/policy/packages.md

@@ -8,12 +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-unconfirmed-parents** package is a topologically sorted package that consists of
-exactly one child and all of its unconfirmed parents (no other transactions may be present).
-The last transaction in the package is the child, and its package can be canonically defined based
-on the current state: each of its inputs must be available in the UTXO set as of the current chain
-tip or some preceding transaction in the package.
-
 ## Package Mempool Acceptance Rules
 
 The following rules are enforced for all packages:
@@ -75,7 +69,7 @@ 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-unconfirmed-parents packages. This also means packages must contain at
+* 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

src/policy/packages.h

@@ -26,7 +26,7 @@ static_assert(MAX_PACKAGE_WEIGHT >= MAX_STANDARD_TX_WEIGHT);
 
 // If a package is to be evaluated, it must be at least as large as the mempool's ancestor/descendant limits,
 // otherwise transactions that would be individually accepted may be rejected in a package erroneously.
-// Since a submitted package must be child-with-unconfirmed-parents (all of the transactions are an ancestor
+// Since a submitted package must be child-with-parents (all of the transactions are an ancestor
 // of the child), package limits are ultimately bounded by mempool package limits. Ensure that the
 // defaults reflect this constraint.
 static_assert(DEFAULT_DESCENDANT_LIMIT >= MAX_PACKAGE_COUNT);

src/test/txpackage_tests.cpp

@@ -441,20 +441,6 @@ BOOST_AUTO_TEST_CASE(package_submission_tests)
         BOOST_CHECK_EQUAL(result_quit_early.m_state.GetResult(), PackageValidationResult::PCKG_TX);
     }
 
-    // Child with missing parent.
-    mtx_child.vin.emplace_back(COutPoint(package_unrelated[0]->GetHash(), 0));
-    Package package_missing_parent;
-    package_missing_parent.push_back(tx_parent);
-    package_missing_parent.push_back(MakeTransactionRef(mtx_child));
-    {
-        const auto result_missing_parent = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
-                                                             package_missing_parent, /*test_accept=*/false, /*client_maxfeerate=*/{});
-        BOOST_CHECK(result_missing_parent.m_state.IsInvalid());
-        BOOST_CHECK_EQUAL(result_missing_parent.m_state.GetResult(), PackageValidationResult::PCKG_POLICY);
-        BOOST_CHECK_EQUAL(result_missing_parent.m_state.GetRejectReason(), "package-not-child-with-unconfirmed-parents");
-        BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
-    }
-
     // Submit package with parent + child.
     {
         const auto submit_parent_child = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,

src/validation.cpp

@@ -512,7 +512,7 @@ class MemPoolAccept
             };
         }
 
-        /** Parameters for child-with-unconfirmed-parents package validation. */
+        /** Parameters for child-with-parents package validation. */
         static ATMPArgs PackageChildWithParents(const CChainParams& chainparams, int64_t accept_time,
                                                 std::vector<COutPoint>& coins_to_uncache, const std::optional<CFeeRate>& client_maxfeerate) {
             return ATMPArgs{/* m_chainparams */ chainparams,
@@ -1678,7 +1678,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
 
     // There are two topologies we are able to handle through this function:
     // (1) A single transaction
-    // (2) A child-with-unconfirmed-parents package.
+    // (2) A child-with-parents package.
     // 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.
 
@@ -1687,49 +1687,11 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
         return PackageMempoolAcceptResult(package_state_quit_early, {});
     }
 
-    if (package.size() > 1) {
+    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.
-        if (!IsChildWithParents(package)) {
-            package_state_quit_early.Invalid(PackageValidationResult::PCKG_POLICY, "package-not-child-with-parents");
-            return PackageMempoolAcceptResult(package_state_quit_early, {});
-        }
-
-        // IsChildWithParents() guarantees the package is > 1 transactions.
-        assert(package.size() > 1);
-        // The package must be 1 child with all of its unconfirmed parents. The package is expected to
-        // be sorted, so the last transaction is the child.
-        const auto& child = package.back();
-        std::unordered_set<uint256, SaltedTxidHasher> unconfirmed_parent_txids;
-        std::transform(package.cbegin(), package.cend() - 1,
-                       std::inserter(unconfirmed_parent_txids, unconfirmed_parent_txids.end()),
-                       [](const auto& tx) { return tx->GetHash(); });
-
-        // All child inputs must refer to a preceding package transaction or a confirmed UTXO. The only
-        // way to verify this is to look up the child's inputs in our current coins view (not including
-        // mempool), and enforce that all parents not present in the package be available at chain tip.
-        // Since this check can bring new coins into the coins cache, keep track of these coins and
-        // uncache them if we don't end up submitting this package to the mempool.
-        const CCoinsViewCache& coins_tip_cache = m_active_chainstate.CoinsTip();
-        for (const auto& input : child->vin) {
-            if (!coins_tip_cache.HaveCoinInCache(input.prevout)) {
-                args.m_coins_to_uncache.push_back(input.prevout);
-            }
-        }
-        // Using the MemPoolAccept m_view cache allows us to look up these same coins faster later.
-        // This should be connecting directly to CoinsTip, not to m_viewmempool, because we specifically
-        // require inputs to be confirmed if they aren't in the package.
-        m_view.SetBackend(m_active_chainstate.CoinsTip());
-        const auto package_or_confirmed = [this, &unconfirmed_parent_txids](const auto& input) {
-             return unconfirmed_parent_txids.count(input.prevout.hash) > 0 || m_view.HaveCoin(input.prevout);
-        };
-        if (!std::all_of(child->vin.cbegin(), child->vin.cend(), package_or_confirmed)) {
-            package_state_quit_early.Invalid(PackageValidationResult::PCKG_POLICY, "package-not-child-with-unconfirmed-parents");
-            return PackageMempoolAcceptResult(package_state_quit_early, {});
-        }
-        // Protect against bugs where we pull more inputs from disk that miss being added to
-        // coins_to_uncache. The backend will be connected again when needed in PreChecks.
-        m_view.SetBackend(m_dummy);
+        package_state_quit_early.Invalid(PackageValidationResult::PCKG_POLICY, "package-not-child-with-parents");
+        return PackageMempoolAcceptResult(package_state_quit_early, {});
     }
 
     LOCK(m_pool.cs);

test/functional/p2p_1p1c_network.py

@@ -103,7 +103,7 @@ def create_packages(self):
         package_hex_2, parent_2, child_2 = self.create_basic_1p1c(self.wallet_nonsegwit)
 
         # 3: 2-parent-1-child package. Both parents are above mempool min feerate. No package submission happens.
-        # We require packages to be child-with-unconfirmed-parents and only allow 1-parent-1-child packages.
+        # We require packages to be child-with-parents and only allow 1-parent-1-child packages.
         package_hex_3, parent_31, _parent_32, child_3 = self.create_package_2p1c(self.wallet)
 
         # 4: parent + child package where the child spends 2 different outputs from the parent.

test/functional/p2p_opportunistic_1p1c.py

@@ -340,7 +340,7 @@ def test_multiple_parents(self):
 
     @cleanup
     def test_other_parent_in_mempool(self):
-        self.log.info("Check opportunistic 1p1c fails if child already has another parent in mempool")
+        self.log.info("Check opportunistic 1p1c works even if child already has another parent in mempool")
         node = self.nodes[0]
 
         # This parent needs CPFP
@@ -361,20 +361,15 @@ def test_other_parent_in_mempool(self):
         # 2. Send child.
         peer_sender.send_and_ping(msg_tx(child["tx"]))
 
-        # 3. Node requests parent_low. However, 1p1c fails because package-not-child-with-unconfirmed-parents
+        # 3. Node requests parent_low.
         parent_low_txid_int = int(parent_low["txid"], 16)
         peer_sender.wait_for_getdata([parent_low_txid_int])
         peer_sender.send_and_ping(msg_tx(parent_low["tx"]))
 
         node_mempool = node.getrawmempool()
         assert parent_high["txid"] in node_mempool
-        assert parent_low["txid"] not in node_mempool
-        assert child["txid"] not in node_mempool
-
-        # Same error if submitted through submitpackage without parent_high
-        package_hex_missing_parent = [parent_low["hex"], child["hex"]]
-        result_missing_parent = node.submitpackage(package_hex_missing_parent)
-        assert_equal(result_missing_parent["package_msg"], "package-not-child-with-unconfirmed-parents")
+        assert parent_low["txid"] in node_mempool
+        assert child["txid"] in node_mempool
 
     def run_test(self):
         node = self.nodes[0]

test/functional/rpc_packages.py

@@ -267,9 +267,11 @@ def test_conflicting(self):
         assert_equal(submitres, {'package_msg': 'conflict-in-package', 'tx-results': {}, 'replaced-transactions': []})
         assert tx_child["txid"] not in node.getrawmempool()
 
-        # ... and without the in-mempool ancestor tx1 included in the call
+        # without the in-mempool ancestor tx1 included in the call, tx2 can be submitted, but
+        # tx_child is missing an input.
         submitres = node.submitpackage([tx2["hex"], tx_child["hex"]])
-        assert_equal(submitres, {'package_msg': 'package-not-child-with-unconfirmed-parents', 'tx-results': {}, 'replaced-transactions': []})
+        assert_equal(submitres["tx-results"][tx_child["wtxid"]], {"txid": tx_child["txid"], "error": "bad-txns-inputs-missingorspent"})
+        assert tx2["txid"] in node.getrawmempool()
 
         # Regardless of error type, the child can never enter the mempool
         assert tx_child["txid"] not in node.getrawmempool()