Merge bitcoin/bitcoin#24152: policy / validation: CPFP fee bumping within packages

9bebf35e26 [validation] don't package validate if not policy or missing inputs (glozow)
51edcffa0e [unit test] package feerate and package cpfp (glozow)
1b93748c93 [validation] try individual validation before package validation (glozow)
17a8ffd802 [packages/policy] use package feerate in package validation (glozow)
09f32cffa6 [docs] package feerate (glozow)

Pull request description:

  Part of #22290, aka [Package Mempool Accept](https://gist.github.com/glozow/dc4e9d5c5b14ade7cdfac40f43adb18a).

  This enables CPFP fee bumping in child-with-unconfirmed-parents packages by introducing [package feerate](https://gist.github.com/glozow/dc4e9d5c5b14ade7cdfac40f43adb18a#fee-related-checks-use-package-feerate) (total modified fees divided by total virtual size) and using it in place of individual feerate. We also always [validate individual transactions first](https://gist.github.com/glozow/dc4e9d5c5b14ade7cdfac40f43adb18a#always-try-individual-submission-first) to avoid incentive-incompatible policies like "parents pay for children" or "siblings pay for siblings" behavior.

ACKs for top commit:
  instagibbs:
    reACK 9bebf35e26
  mzumsande:
    Code review ACK 9bebf35e26
  t-bast:
    ACK 9bebf35e26

Tree-SHA512: 5117cfcc3ce55c00384d9e8003a0589ceac1e6f738b1c299007d9cd9cdd2d7c530d31cfd23658b041a6604d39073bcc6e81f0639a300082a92097682a6ea8c8f

src/test/fuzz/tx_pool.cpp

@@ -234,14 +234,18 @@ FUZZ_TARGET_INIT(tx_pool_standard, initialize_tx_pool)
         const bool bypass_limits = fuzzed_data_provider.ConsumeBool();
         ::fRequireStandard = fuzzed_data_provider.ConsumeBool();
 
-        // Make sure ProcessNewPackage on one transaction works and always fully validates the transaction.
+        // Make sure ProcessNewPackage on one transaction works.
         // The result is not guaranteed to be the same as what is returned by ATMP.
         const auto result_package = WITH_LOCK(::cs_main,
                                     return ProcessNewPackage(chainstate, tx_pool, {tx}, true));
-        auto it = result_package.m_tx_results.find(tx->GetWitnessHash());
-        Assert(it != result_package.m_tx_results.end());
-        Assert(it->second.m_result_type == MempoolAcceptResult::ResultType::VALID ||
-               it->second.m_result_type == MempoolAcceptResult::ResultType::INVALID);
+        // If something went wrong due to a package-specific policy, it might not return a
+        // validation result for the transaction.
+        if (result_package.m_state.GetResult() != PackageValidationResult::PCKG_POLICY) {
+            auto it = result_package.m_tx_results.find(tx->GetWitnessHash());
+            Assert(it != result_package.m_tx_results.end());
+            Assert(it->second.m_result_type == MempoolAcceptResult::ResultType::VALID ||
+                   it->second.m_result_type == MempoolAcceptResult::ResultType::INVALID);
+        }
 
         const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, tx, GetTime(), bypass_limits, /*test_accept=*/false));
         const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID;

src/test/txpackage_tests.cpp

@@ -98,7 +98,9 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup)
     BOOST_CHECK(it_child != result_parent_child.m_tx_results.end());
     BOOST_CHECK_MESSAGE(it_child->second.m_state.IsValid(),
                         "Package validation unexpectedly failed: " << it_child->second.m_state.GetRejectReason());
-
+    BOOST_CHECK(result_parent_child.m_package_feerate.has_value());
+    BOOST_CHECK(result_parent_child.m_package_feerate.value() ==
+                CFeeRate(2 * COIN, GetVirtualTransactionSize(*tx_parent) + GetVirtualTransactionSize(*tx_child)));
 
     // A single, giant transaction submitted through ProcessNewPackage fails on single tx policy.
     CTransactionRef giant_ptx = create_placeholder_tx(999, 999);
@@ -110,6 +112,7 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup)
     auto it_giant_tx = result_single_large.m_tx_results.find(giant_ptx->GetWitnessHash());
     BOOST_CHECK(it_giant_tx != result_single_large.m_tx_results.end());
     BOOST_CHECK_EQUAL(it_giant_tx->second.m_state.GetRejectReason(), "tx-size");
+    BOOST_CHECK(result_single_large.m_package_feerate == std::nullopt);
 
     // Check that mempool size hasn't changed.
     BOOST_CHECK_EQUAL(m_node.mempool->size(), initialPoolSize);
@@ -230,6 +233,7 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup)
     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_EQUAL(m_node.mempool->size(), expected_pool_size);
+    BOOST_CHECK(result_unrelated_submit.m_package_feerate == std::nullopt);
 
     // Parent and Child (and Grandchild) Package
     Package package_parent_child;
@@ -271,6 +275,7 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup)
         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");
         BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
+        BOOST_CHECK(result_3gen_submit.m_package_feerate == std::nullopt);
     }
 
     // Child with missing parent.
@@ -286,6 +291,7 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup)
         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);
 
+        BOOST_CHECK(result_missing_parent.m_package_feerate == std::nullopt);
     }
 
     // Submit package with parent + child.
@@ -305,6 +311,10 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup)
         BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
         BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_parent->GetHash())));
         BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_child->GetHash())));
+
+        // Since both transactions have high feerates, they each passed validation individually.
+        // Package validation was unnecessary, so there is no package feerate.
+        BOOST_CHECK(submit_parent_child.m_package_feerate == std::nullopt);
     }
 
     // Already-in-mempool transactions should be detected and de-duplicated.
@@ -325,6 +335,8 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup)
         BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
         BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_parent->GetHash())));
         BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_child->GetHash())));
+
+        BOOST_CHECK(submit_deduped.m_package_feerate == std::nullopt);
     }
 }
 
@@ -399,8 +411,12 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup)
         BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(ptx_parent->GetHash())));
         BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(ptx_child1->GetHash())));
 
+        // Child2 would have been validated individually.
+        BOOST_CHECK(submit_witness1.m_package_feerate == std::nullopt);
+
         const auto submit_witness2 = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
                                                        {ptx_parent, ptx_child2}, /*test_accept=*/false);
+        BOOST_CHECK(submit_witness2.m_package_feerate == std::nullopt);
         BOOST_CHECK_MESSAGE(submit_witness2.m_state.IsValid(),
                             "Package validation unexpectedly failed: " << submit_witness2.m_state.GetRejectReason());
         auto it_parent2_deduped = submit_witness2.m_tx_results.find(ptx_parent->GetWitnessHash());
@@ -424,6 +440,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup)
         auto it_child_dup = submit_segwit_dedup.m_tx_results.find(ptx_child1->GetWitnessHash());
         BOOST_CHECK(it_parent_dup->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY);
         BOOST_CHECK(it_child_dup->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY);
+        BOOST_CHECK(submit_witness2.m_package_feerate == std::nullopt);
     }
 
     // Try submitting Package1{child2, grandchild} where child2 is same-txid-different-witness as
@@ -458,6 +475,9 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup)
         BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(ptx_child2->GetHash())));
         BOOST_CHECK(!m_node.mempool->exists(GenTxid::Wtxid(ptx_child2->GetWitnessHash())));
         BOOST_CHECK(m_node.mempool->exists(GenTxid::Wtxid(ptx_grandchild->GetWitnessHash())));
+
+        // Since child2 is ignored, grandchild would be validated individually.
+        BOOST_CHECK(submit_spend_ignored.m_package_feerate == std::nullopt);
     }
 
     // A package Package{parent1, parent2, parent3, child} where the parents are a mixture of
@@ -516,11 +536,11 @@ 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
+    // parent3 will be a new transaction. Put 0 fees on it 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(49 * COIN), /*submit=*/false);
+                                                     /*output_amount=*/CAmount(50 * COIN), /*submit=*/false);
     CTransactionRef ptx_parent3 = MakeTransactionRef(mtx_parent3);
     package_mixed.push_back(ptx_parent3);
 
@@ -536,7 +556,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup)
     mtx_mixed_child.vin[0].scriptWitness = acs_witness;
     mtx_mixed_child.vin[1].scriptWitness = acs_witness;
     mtx_mixed_child.vin[2].scriptWitness = acs_witness;
-    mtx_mixed_child.vout.push_back(CTxOut(145 * COIN, mixed_child_spk));
+    mtx_mixed_child.vout.push_back(CTxOut((48 + 49 + 50 - 1) * COIN, mixed_child_spk));
     CTransactionRef ptx_mixed_child = MakeTransactionRef(mtx_mixed_child);
     package_mixed.push_back(ptx_mixed_child);
 
@@ -568,6 +588,205 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup)
         BOOST_CHECK(!m_node.mempool->exists(GenTxid::Wtxid(ptx_parent2_v1->GetWitnessHash())));
         BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(ptx_parent3->GetHash())));
         BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(ptx_mixed_child->GetHash())));
+
+        // package feerate should include parent3 and child. It should not include parent1 or parent2_v1.
+        BOOST_CHECK(mixed_result.m_package_feerate.has_value());
+        const CFeeRate expected_feerate(1 * COIN, GetVirtualTransactionSize(*ptx_parent3) + GetVirtualTransactionSize(*ptx_mixed_child));
+        BOOST_CHECK_MESSAGE(mixed_result.m_package_feerate.value() == expected_feerate,
+                            strprintf("Expected package feerate %s, got %s", expected_feerate.ToString(),
+                                      mixed_result.m_package_feerate.value().ToString()));
+    }
+}
+
+BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup)
+{
+    mineBlocks(5);
+    LOCK(::cs_main);
+    size_t expected_pool_size = m_node.mempool->size();
+    CKey child_key;
+    child_key.MakeNewKey(true);
+    CScript parent_spk = GetScriptForDestination(WitnessV0KeyHash(child_key.GetPubKey()));
+    CKey grandchild_key;
+    grandchild_key.MakeNewKey(true);
+    CScript child_spk = GetScriptForDestination(WitnessV0KeyHash(grandchild_key.GetPubKey()));
+
+    // zero-fee parent and high-fee child package
+    const CAmount coinbase_value{50 * COIN};
+    const CAmount parent_value{coinbase_value - 0};
+    const CAmount child_value{parent_value - COIN};
+
+    Package package_cpfp;
+    auto mtx_parent = CreateValidMempoolTransaction(/*input_transaction=*/ m_coinbase_txns[0], /*vout=*/ 0,
+                                                    /*input_height=*/ 0, /*input_signing_key=*/ coinbaseKey,
+                                                    /*output_destination=*/ parent_spk,
+                                                    /*output_amount=*/ parent_value, /*submit=*/ false);
+    CTransactionRef tx_parent = MakeTransactionRef(mtx_parent);
+    package_cpfp.push_back(tx_parent);
+
+    auto mtx_child = CreateValidMempoolTransaction(/*input_transaction=*/ tx_parent, /*vout=*/ 0,
+                                                   /*input_height=*/ 101, /*input_signing_key=*/ child_key,
+                                                   /*output_destination=*/ child_spk,
+                                                   /*output_amount=*/ child_value, /*submit=*/ false);
+    CTransactionRef tx_child = MakeTransactionRef(mtx_child);
+    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);
+    {
+        BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
+        const auto submit_cpfp_deprio = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
+                                                   package_cpfp, /*test_accept=*/ false);
+        BOOST_CHECK_MESSAGE(submit_cpfp_deprio.m_state.IsInvalid(),
+                            "Package validation unexpectedly succeeded: " << submit_cpfp_deprio.m_state.GetRejectReason());
+        BOOST_CHECK(submit_cpfp_deprio.m_tx_results.empty());
+        BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
+        const CFeeRate expected_feerate(0, GetVirtualTransactionSize(*tx_parent) + GetVirtualTransactionSize(*tx_child));
+        BOOST_CHECK(submit_cpfp_deprio.m_package_feerate.has_value());
+        BOOST_CHECK(submit_cpfp_deprio.m_package_feerate.value() == CFeeRate{0});
+        BOOST_CHECK_MESSAGE(submit_cpfp_deprio.m_package_feerate.value() == expected_feerate,
+                            strprintf("Expected package feerate %s, got %s", expected_feerate.ToString(),
+                                      submit_cpfp_deprio.m_package_feerate.value().ToString()));
+    }
+
+    // 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.
+    {
+        BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
+        const auto submit_cpfp = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
+                                                   package_cpfp, /*test_accept=*/ false);
+        expected_pool_size += 2;
+        BOOST_CHECK_MESSAGE(submit_cpfp.m_state.IsValid(),
+                            "Package validation unexpectedly failed: " << submit_cpfp.m_state.GetRejectReason());
+        auto it_parent = submit_cpfp.m_tx_results.find(tx_parent->GetWitnessHash());
+        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_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);
+
+        BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
+        BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_parent->GetHash())));
+        BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_child->GetHash())));
+
+        const CFeeRate expected_feerate(coinbase_value - child_value,
+                                        GetVirtualTransactionSize(*tx_parent) + GetVirtualTransactionSize(*tx_child));
+        BOOST_CHECK(expected_feerate.GetFeePerK() > 1000);
+        BOOST_CHECK(submit_cpfp.m_package_feerate.has_value());
+        BOOST_CHECK_MESSAGE(submit_cpfp.m_package_feerate.value() == expected_feerate,
+                            strprintf("Expected package feerate %s, got %s", expected_feerate.ToString(),
+                                      submit_cpfp.m_package_feerate.value().ToString()));
+    }
+
+    // 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.
+    Package package_still_too_low;
+    auto mtx_parent_cheap = CreateValidMempoolTransaction(/*input_transaction=*/ m_coinbase_txns[1], /*vout=*/ 0,
+                                                          /*input_height=*/ 0, /*input_signing_key=*/ coinbaseKey,
+                                                          /*output_destination=*/ parent_spk,
+                                                          /*output_amount=*/ coinbase_value, /*submit=*/ false);
+    CTransactionRef tx_parent_cheap = MakeTransactionRef(mtx_parent_cheap);
+    package_still_too_low.push_back(tx_parent_cheap);
+
+    auto mtx_child_cheap = CreateValidMempoolTransaction(/*input_transaction=*/ tx_parent_cheap, /*vout=*/ 0,
+                                                         /*input_height=*/ 101, /* input_signing_key */ child_key,
+                                                         /*output_destination=*/ child_spk,
+                                                         /*output_amount=*/ coinbase_value - 200, /*submit=*/ false);
+    CTransactionRef tx_child_cheap = MakeTransactionRef(mtx_child_cheap);
+    package_still_too_low.push_back(tx_child_cheap);
+
+    // Cheap package should fail with package-fee-too-low.
+    {
+        BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
+        const auto submit_package_too_low = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
+                                                   package_still_too_low, /* test_accept */ false);
+        BOOST_CHECK_MESSAGE(submit_package_too_low.m_state.IsInvalid(), "Package validation unexpectedly succeeded");
+        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);
+        BOOST_CHECK(submit_package_too_low.m_package_feerate.has_value());
+        BOOST_CHECK_MESSAGE(submit_package_too_low.m_package_feerate.value() == expected_feerate,
+                            strprintf("Expected package feerate %s, got %s", expected_feerate.ToString(),
+                                      submit_package_too_low.m_package_feerate.value().ToString()));
+    }
+
+    // Package feerate includes the modified fees of the transactions.
+    // This means a child with its fee delta from prioritisetransaction can pay for a parent.
+    m_node.mempool->PrioritiseTransaction(tx_child_cheap->GetHash(), 1 * COIN);
+    // Now that the child's fees have "increased" by 1 BTC, the cheap package should succeed.
+    {
+        const auto submit_prioritised_package = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
+                                                                  package_still_too_low, /*test_accept=*/ false);
+        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,
+            GetVirtualTransactionSize(*tx_parent_cheap) + GetVirtualTransactionSize(*tx_child_cheap));
+        BOOST_CHECK(submit_prioritised_package.m_package_feerate.has_value());
+        BOOST_CHECK_MESSAGE(submit_prioritised_package.m_package_feerate.value() == expected_feerate,
+                            strprintf("Expected package feerate %s, got %s", expected_feerate.ToString(),
+                                      submit_prioritised_package.m_package_feerate.value().ToString()));
+    }
+
+    // Package feerate is calculated without topology in mind; it's just aggregating fees and sizes.
+    // However, this should not allow parents to pay for children. Each transaction should be
+    // validated individually first, eliminating sufficient-feerate parents before they are unfairly
+    // included in the package feerate. It's also important that the low-fee child doesn't prevent
+    // the parent from being accepted.
+    Package package_rich_parent;
+    const CAmount high_parent_fee{1 * COIN};
+    auto mtx_parent_rich = CreateValidMempoolTransaction(/*input_transaction=*/ m_coinbase_txns[2], /*vout=*/ 0,
+                                                         /*input_height=*/ 0, /*input_signing_key=*/ coinbaseKey,
+                                                         /*output_destination=*/ parent_spk,
+                                                         /*output_amount=*/ coinbase_value - high_parent_fee, /*submit=*/ false);
+    CTransactionRef tx_parent_rich = MakeTransactionRef(mtx_parent_rich);
+    package_rich_parent.push_back(tx_parent_rich);
+
+    auto mtx_child_poor = CreateValidMempoolTransaction(/*input_transaction=*/ tx_parent_rich, /*vout=*/ 0,
+                                                        /*input_height=*/ 101, /*input_signing_key=*/ child_key,
+                                                        /*output_destination=*/ child_spk,
+                                                        /*output_amount=*/ coinbase_value - high_parent_fee, /*submit=*/ false);
+    CTransactionRef tx_child_poor = MakeTransactionRef(mtx_child_poor);
+    package_rich_parent.push_back(tx_child_poor);
+
+    // Parent pays 1 BTC and child pays none. The parent should be accepted without the child.
+    {
+        BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
+        const auto submit_rich_parent = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
+                                                          package_rich_parent, /* test_accept */ false);
+        expected_pool_size += 1;
+        BOOST_CHECK_MESSAGE(submit_rich_parent.m_state.IsInvalid(), "Package validation unexpectedly succeeded");
+
+        // The child would have been validated on its own and failed, then submitted as a "package" of 1.
+        // The package feerate is just the child's feerate, which is 0sat/vb.
+        BOOST_CHECK(submit_rich_parent.m_package_feerate.has_value());
+        BOOST_CHECK_MESSAGE(submit_rich_parent.m_package_feerate.value() == CFeeRate(),
+                            "expected 0, got " << submit_rich_parent.m_package_feerate.value().ToString());
+        BOOST_CHECK_EQUAL(submit_rich_parent.m_state.GetResult(), PackageValidationResult::PCKG_POLICY);
+        BOOST_CHECK_EQUAL(submit_rich_parent.m_state.GetRejectReason(), "package-fee-too-low");
+
+        auto it_parent = submit_rich_parent.m_tx_results.find(tx_parent_rich->GetWitnessHash());
+        BOOST_CHECK(it_parent != submit_rich_parent.m_tx_results.end());
+        BOOST_CHECK(it_parent->second.m_result_type == MempoolAcceptResult::ResultType::VALID);
+        BOOST_CHECK(it_parent->second.m_state.GetRejectReason() == "");
+        BOOST_CHECK_MESSAGE(it_parent->second.m_base_fees.value() == high_parent_fee,
+                strprintf("rich parent: expected fee %s, got %s", high_parent_fee, it_parent->second.m_base_fees.value()));
+
+        BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
+        BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_parent_rich->GetHash())));
+        BOOST_CHECK(!m_node.mempool->exists(GenTxid::Txid(tx_child_poor->GetHash())));
     }
 }
 BOOST_AUTO_TEST_SUITE_END()

src/validation.cpp

@@ -460,6 +460,10 @@ public:
          * partially submitted.
          */
         const bool m_package_submission;
+        /** When true, use package feerates instead of individual transaction feerates for fee-based
+         * policies such as mempool min fee and min relay fee.
+         */
+        const bool m_package_feerates;
 
         /** Parameters for single transaction mempool validation. */
         static ATMPArgs SingleAccept(const CChainParams& chainparams, int64_t accept_time,
@@ -472,6 +476,7 @@ public:
                             /* m_test_accept */ test_accept,
                             /* m_allow_bip125_replacement */ true,
                             /* m_package_submission */ false,
+                            /* m_package_feerates */ false,
             };
         }
 
@@ -485,6 +490,7 @@ public:
                             /* m_test_accept */ true,
                             /* m_allow_bip125_replacement */ false,
                             /* m_package_submission */ false, // not submitting to mempool
+                            /* m_package_feerates */ false,
             };
         }
 
@@ -498,6 +504,20 @@ public:
                             /* m_test_accept */ false,
                             /* m_allow_bip125_replacement */ false,
                             /* m_package_submission */ true,
+                            /* m_package_feerates */ true,
+            };
+        }
+
+        /** Parameters for a single transaction within a package. */
+        static ATMPArgs SingleInPackageAccept(const ATMPArgs& package_args) {
+            return ATMPArgs{/* m_chainparams */ package_args.m_chainparams,
+                            /* m_accept_time */ package_args.m_accept_time,
+                            /* m_bypass_limits */ false,
+                            /* m_coins_to_uncache */ package_args.m_coins_to_uncache,
+                            /* m_test_accept */ package_args.m_test_accept,
+                            /* m_allow_bip125_replacement */ true,
+                            /* m_package_submission */ false,
+                            /* m_package_feerates */ false, // only 1 transaction
             };
         }
 
@@ -510,14 +530,16 @@ public:
                  std::vector<COutPoint>& coins_to_uncache,
                  bool test_accept,
                  bool allow_bip125_replacement,
-                 bool package_submission)
+                 bool package_submission,
+                 bool package_feerates)
             : m_chainparams{chainparams},
               m_accept_time{accept_time},
               m_bypass_limits{bypass_limits},
               m_coins_to_uncache{coins_to_uncache},
               m_test_accept{test_accept},
               m_allow_bip125_replacement{allow_bip125_replacement},
-              m_package_submission{package_submission}
+              m_package_submission{package_submission},
+              m_package_feerates{package_feerates}
         {
         }
     };
@@ -819,9 +841,10 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
         return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "bad-txns-too-many-sigops",
                 strprintf("%d", nSigOpsCost));
 
-    // No transactions are allowed below minRelayTxFee except from disconnected
-    // blocks
-    if (!bypass_limits && !CheckFeeRate(ws.m_vsize, ws.m_modified_fees, state)) return false;
+    // No individual transactions are allowed below minRelayTxFee and mempool min fee except from
+    // disconnected blocks and transactions in a package. Package transactions will be checked using
+    // package feerate later.
+    if (!bypass_limits && !args.m_package_feerates && !CheckFeeRate(ws.m_vsize, ws.m_modified_fees, state)) return false;
 
     ws.m_iters_conflicting = m_pool.GetIterSet(ws.m_conflicts);
     // Calculate in-mempool ancestors, up to a limit.
@@ -1205,12 +1228,27 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::
         m_viewmempool.PackageAddTransaction(ws.m_ptx);
     }
 
+    // Transactions must meet two minimum feerates: the mempool minimum fee and min relay fee.
+    // For transactions consisting of exactly one child and its parents, it suffices to use the
+    // package feerate (total modified fees / total virtual size) to check this requirement.
+    const auto m_total_vsize = std::accumulate(workspaces.cbegin(), workspaces.cend(), int64_t{0},
+        [](int64_t sum, auto& ws) { return sum + ws.m_vsize; });
+    const auto m_total_modified_fees = std::accumulate(workspaces.cbegin(), workspaces.cend(), CAmount{0},
+        [](CAmount sum, auto& ws) { return sum + ws.m_modified_fees; });
+    const CFeeRate package_feerate(m_total_modified_fees, m_total_vsize);
+    TxValidationState placeholder_state;
+    if (args.m_package_feerates &&
+        !CheckFeeRate(m_total_vsize, m_total_modified_fees, placeholder_state)) {
+        package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package-fee-too-low");
+        return PackageMempoolAcceptResult(package_state, package_feerate, {});
+    }
+
     // Apply package mempool ancestor/descendant limits. Skip if there is only one transaction,
     // because it's unnecessary. Also, CPFP carve out can increase the limit for individual
     // transactions, but this exemption is not extended to packages in CheckPackageLimits().
     std::string err_string;
     if (txns.size() > 1 && !PackageMempoolChecks(txns, package_state)) {
-        return PackageMempoolAcceptResult(package_state, std::move(results));
+        return PackageMempoolAcceptResult(package_state, package_feerate, std::move(results));
     }
 
     for (Workspace& ws : workspaces) {
@@ -1218,7 +1256,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::
             // Exit early to avoid doing pointless work. Update the failed tx result; the rest are unfinished.
             package_state.Invalid(PackageValidationResult::PCKG_TX, "transaction failed");
             results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Failure(ws.m_state));
-            return PackageMempoolAcceptResult(package_state, std::move(results));
+            return PackageMempoolAcceptResult(package_state, package_feerate, std::move(results));
         }
         if (args.m_test_accept) {
             // When test_accept=true, transactions that pass PolicyScriptChecks are valid because there are
@@ -1229,14 +1267,14 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::
         }
     }
 
-    if (args.m_test_accept) return PackageMempoolAcceptResult(package_state, std::move(results));
+    if (args.m_test_accept) return PackageMempoolAcceptResult(package_state, package_feerate, std::move(results));
 
     if (!SubmitPackage(args, workspaces, package_state, results)) {
         // PackageValidationState filled in by SubmitPackage().
-        return PackageMempoolAcceptResult(package_state, std::move(results));
+        return PackageMempoolAcceptResult(package_state, package_feerate, std::move(results));
     }
 
-    return PackageMempoolAcceptResult(package_state, std::move(results));
+    return PackageMempoolAcceptResult(package_state, package_feerate, std::move(results));
 }
 
 PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, ATMPArgs& args)
@@ -1303,6 +1341,8 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
     // transactions that are already in the mempool, and only call AcceptMultipleTransactions() with
     // the new transactions. This ensures we don't double-count transaction counts and sizes when
     // checking ancestor/descendant limits, or double-count transaction fees for fee-related policy.
+    ATMPArgs single_args = ATMPArgs::SingleInPackageAccept(args);
+    bool quit_early{false};
     std::vector<CTransactionRef> txns_new;
     for (const auto& tx : package) {
         const auto& wtxid = tx->GetWitnessHash();
@@ -1329,18 +1369,43 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
             results.emplace(wtxid, MempoolAcceptResult::MempoolTxDifferentWitness(iter.value()->GetTx().GetWitnessHash()));
         } else {
             // Transaction does not already exist in the mempool.
-            txns_new.push_back(tx);
+            // Try submitting the transaction on its own.
+            const auto single_res = AcceptSingleTransaction(tx, single_args);
+            if (single_res.m_result_type == MempoolAcceptResult::ResultType::VALID) {
+                // The transaction succeeded on its own and is now in the mempool. Don't include it
+                // in package validation, because its fees should only be "used" once.
+                assert(m_pool.exists(GenTxid::Wtxid(wtxid)));
+                results.emplace(wtxid, single_res);
+            } else if (single_res.m_state.GetResult() != TxValidationResult::TX_MEMPOOL_POLICY &&
+                       single_res.m_state.GetResult() != TxValidationResult::TX_MISSING_INPUTS) {
+                // Package validation policy only differs from individual policy in its evaluation
+                // of feerate. For example, if a transaction fails here due to violation of a
+                // consensus rule, the result will not change when it is submitted as part of a
+                // package. To minimize the amount of repeated work, unless the transaction fails
+                // due to feerate or missing inputs (its parent is a previous transaction in the
+                // package that failed due to feerate), don't run package validation. Note that this
+                // decision might not make sense if different types of packages are allowed in the
+                // future.  Continue individually validating the rest of the transactions, because
+                // some of them may still be valid.
+                quit_early = true;
+            } else {
+                txns_new.push_back(tx);
+            }
         }
     }
 
     // Nothing to do if the entire package has already been submitted.
-    if (txns_new.empty()) return PackageMempoolAcceptResult(package_state, std::move(results));
+    if (quit_early || txns_new.empty()) {
+        // No package feerate when no package validation was done.
+        return PackageMempoolAcceptResult(package_state, std::move(results));
+    }
     // Validate the (deduplicated) transactions as a package.
     auto submission_result = AcceptMultipleTransactions(txns_new, args);
     // Include already-in-mempool transaction results in the final result.
     for (const auto& [wtxid, mempoolaccept_res] : results) {
         submission_result.m_tx_results.emplace(wtxid, mempoolaccept_res);
     }
+    if (submission_result.m_state.IsValid()) assert(submission_result.m_package_feerate.has_value());
     return submission_result;
 }
 

src/validation.h

@@ -234,11 +234,19 @@ struct PackageMempoolAcceptResult
     * was a package-wide error (see result in m_state), m_tx_results will be empty.
     */
     std::map<const uint256, const MempoolAcceptResult> m_tx_results;
+    /** Package feerate, defined as the aggregated modified fees divided by the total virtual size
+     * of all transactions in the package.  May be unavailable if some inputs were not available or
+     * a transaction failure caused validation to terminate early. */
+    std::optional<CFeeRate> m_package_feerate;
 
     explicit PackageMempoolAcceptResult(PackageValidationState state,
                                         std::map<const uint256, const MempoolAcceptResult>&& results)
         : m_state{state}, m_tx_results(std::move(results)) {}
 
+    explicit PackageMempoolAcceptResult(PackageValidationState state, CFeeRate feerate,
+                                        std::map<const uint256, const MempoolAcceptResult>&& results)
+        : m_state{state}, m_tx_results(std::move(results)), m_package_feerate{feerate} {}
+
     /** Constructor to create a PackageMempoolAcceptResult from a single MempoolAcceptResult */
     explicit PackageMempoolAcceptResult(const uint256& wtxid, const MempoolAcceptResult& result)
         : m_tx_results{ {wtxid, result} } {}