Merge 7257e50dba into merged_master (Bitcoin PR bitcoin/bitcoin#20833)

doc/release-notes-20833.md

@@ -0,0 +1,12 @@
+Updated RPCs
+------------
+
+- The `testmempoolaccept` RPC now accepts multiple transactions (still experimental at the moment,
+  API may be unstable). This is intended for testing transaction packages with dependency
+  relationships; it is not recommended for batch-validating independent transactions. In addition to
+  mempool policy, package policies apply: the list cannot contain more than 25 transactions or have a
+  total size exceeding 101K virtual bytes, and cannot conflict with (spend the same inputs as) each other or
+  the mempool, even if it would be a valid BIP125 replace-by-fee. There are some known limitations to
+  the accuracy of the test accept: it's possible for `testmempoolaccept` to return "allowed"=True for a
+  group of transactions, but "too-long-mempool-chain" if they are actually submitted. (#20833)
+

src/Makefile.am

@@ -203,6 +203,7 @@ BITCOIN_CORE_H = \
   pegins.h \
   policy/feerate.h \
   policy/fees.h \
+  policy/packages.h \
   policy/policy.h \
   policy/rbf.h \
   policy/settings.h \

src/policy/packages.h

@@ -0,0 +1,34 @@
+// Copyright (c) 2021 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_POLICY_PACKAGES_H
+#define BITCOIN_POLICY_PACKAGES_H
+
+#include <consensus/validation.h>
+#include <primitives/transaction.h>
+
+#include <vector>
+
+/** Default maximum number of transactions in a package. */
+static constexpr uint32_t MAX_PACKAGE_COUNT{25};
+/** Default maximum total virtual size of transactions in a package in KvB. */
+static constexpr uint32_t MAX_PACKAGE_SIZE{101};
+
+/** A "reason" why a package was invalid. It may be that one or more of the included
+ * transactions is invalid or the package itself violates our rules.
+ * We don't distinguish between consensus and policy violations right now.
+ */
+enum class PackageValidationResult {
+    PCKG_RESULT_UNSET = 0,        //!< Initial value. The package has not yet been rejected.
+    PCKG_POLICY,                  //!< The package itself is invalid (e.g. too many transactions).
+    PCKG_TX,                      //!< At least one tx is invalid.
+};
+
+/** A package is an ordered list of transactions. The transactions cannot conflict with (spend the
+ * same inputs as) one another. */
+using Package = std::vector<CTransactionRef>;
+
+class PackageValidationState : public ValidationState<PackageValidationResult> {};
+
+#endif // BITCOIN_POLICY_PACKAGES_H

src/rpc/rawtransaction.cpp

@@ -19,6 +19,7 @@
 #include <node/psbt.h>
 #include <node/transaction.h>
 #include <pegins.h>
+#include <policy/packages.h>
 #include <policy/policy.h>
 #include <policy/rbf.h>
 #include <primitives/transaction.h>
@@ -940,8 +941,11 @@ static RPCHelpMan sendrawtransaction()
 static RPCHelpMan testmempoolaccept()
 {
     return RPCHelpMan{"testmempoolaccept",
-                "\nReturns result of mempool acceptance tests indicating if raw transaction (serialized, hex-encoded) would be accepted by mempool.\n"
-                "\nThis checks if the transaction violates the consensus or policy rules.\n"
+                "\nReturns result of mempool acceptance tests indicating if raw transaction(s) (serialized, hex-encoded) would be accepted by mempool.\n"
+                "\nIf multiple transactions are passed in, parents must come before children and package policies apply: the transactions cannot conflict with any mempool transactions or each other.\n"
+                "\nIf one transaction fails, other transactions may not be fully validated (the 'allowed' key will be blank).\n"
+                "\nThe maximum number of transactions allowed is 25 (MAX_PACKAGE_COUNT)\n"
+                "\nThis checks if transactions violate the consensus or policy rules.\n"
                 "\nSee sendrawtransaction call.\n",
                 {
                     {"rawtxs", RPCArg::Type::ARR, RPCArg::Optional::NO, "An array of hex strings of raw transactions.\n"
@@ -950,17 +954,21 @@ static RPCHelpMan testmempoolaccept()
                             {"rawtx", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, ""},
                         },
                         },
-                    {"maxfeerate", RPCArg::Type::AMOUNT, RPCArg::Default{FormatMoney(DEFAULT_MAX_RAW_TX_FEE_RATE.GetFeePerK())}, "Reject transactions whose fee rate is higher than the specified value, expressed in " + CURRENCY_UNIT + "/kvB\n"},
+                    {"maxfeerate", RPCArg::Type::AMOUNT, RPCArg::Default{FormatMoney(DEFAULT_MAX_RAW_TX_FEE_RATE.GetFeePerK())},
+                     "Reject transactions whose fee rate is higher than the specified value, expressed in " + CURRENCY_UNIT + "/kvB\n"},
                 },
                 RPCResult{
                     RPCResult::Type::ARR, "", "The result of the mempool acceptance test for each raw transaction in the input array.\n"
-                        "Length is exactly one for now.",
+                        "Returns results for each transaction in the same order they were passed in.\n"
+                        "It is possible for transactions to not be fully validated ('allowed' unset) if an earlier transaction failed.\n",
                     {
                         {RPCResult::Type::OBJ, "", "",
                         {
                             {RPCResult::Type::STR_HEX, "txid", "The transaction hash in hex"},
                             {RPCResult::Type::STR_HEX, "wtxid", "The transaction witness hash in hex"},
-                            {RPCResult::Type::BOOL, "allowed", "If the mempool allows this tx to be inserted"},
+                            {RPCResult::Type::STR, "package-error", "Package validation error, if any (only possible if rawtxs had more than 1 transaction)."},
+                            {RPCResult::Type::BOOL, "allowed", "Whether this tx would be accepted to the mempool and pass client-specified maxfeerate."
+                                                               "If not present, the tx was not fully validated due to a failure in another tx in the list."},
                             {RPCResult::Type::NUM, "vsize", "Virtual transaction size as defined in BIP 141. This is different from actual serialized size for witness transactions as witness data is discounted (only present when 'allowed' is true)"},
                             {RPCResult::Type::OBJ, "fees", "Transaction fees (only present if 'allowed' is true)",
                             {
@@ -987,62 +995,86 @@ static RPCHelpMan testmempoolaccept()
         UniValueType(), // VNUM or VSTR, checked inside AmountFromValue()
     });
 
-    if (request.params[0].get_array().size() != 1) {
-        throw JSONRPCError(RPC_INVALID_PARAMETER, "Array must contain exactly one raw transaction for now");
+    const UniValue raw_transactions = request.params[0].get_array();
+    if (raw_transactions.size() < 1 || raw_transactions.size() > MAX_PACKAGE_COUNT) {
+        throw JSONRPCError(RPC_INVALID_PARAMETER,
+                           "Array must contain between 1 and " + ToString(MAX_PACKAGE_COUNT) + " transactions.");
     }
 
-    CMutableTransaction mtx;
-    if (!DecodeHexTx(mtx, request.params[0].get_array()[0].get_str())) {
-        throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed. Make sure the tx has at least one input.");
-    }
-    CTransactionRef tx(MakeTransactionRef(std::move(mtx)));
-
     const CFeeRate max_raw_tx_fee_rate = request.params[1].isNull() ?
                                              DEFAULT_MAX_RAW_TX_FEE_RATE :
                                              CFeeRate(AmountFromValue(request.params[1]));
 
-    NodeContext& node = EnsureAnyNodeContext(request.context);
-
-    CTxMemPool& mempool = EnsureMemPool(node);
-    int64_t virtual_size = GetVirtualTransactionSize(*tx);
-    CAmount max_raw_tx_fee = max_raw_tx_fee_rate.GetFee(virtual_size);
-
-    UniValue result(UniValue::VARR);
-    UniValue result_0(UniValue::VOBJ);
-    result_0.pushKV("txid", tx->GetHash().GetHex());
-    result_0.pushKV("wtxid", tx->GetWitnessHash().GetHex());
-
-    ChainstateManager& chainman = EnsureChainman(node);
-    const MempoolAcceptResult accept_result = WITH_LOCK(cs_main, return AcceptToMemoryPool(chainman.ActiveChainstate(), mempool, std::move(tx),
-                                                  false /* bypass_limits */, /* test_accept */ true));
-
-    // Only return the fee and vsize if the transaction would pass ATMP.
-    // These can be used to calculate the feerate.
-    if (accept_result.m_result_type == MempoolAcceptResult::ResultType::VALID) {
-        const CAmount fee = accept_result.m_base_fees.value();
-        // Check that fee does not exceed maximum fee
-        if (max_raw_tx_fee && fee > max_raw_tx_fee) {
-            result_0.pushKV("allowed", false);
-            result_0.pushKV("reject-reason", "max-fee-exceeded");
-        } else {
-            result_0.pushKV("allowed", true);
-            result_0.pushKV("vsize", virtual_size);
-            UniValue fees(UniValue::VOBJ);
-            fees.pushKV("base", ValueFromAmount(fee));
-            result_0.pushKV("fees", fees);
+    std::vector<CTransactionRef> txns;
+    for (const auto& rawtx : raw_transactions.getValues()) {
+        CMutableTransaction mtx;
+        if (!DecodeHexTx(mtx, rawtx.get_str())) {
+            throw JSONRPCError(RPC_DESERIALIZATION_ERROR,
+                               "TX decode failed: " + rawtx.get_str() + " Make sure the tx has at least one input.");
         }
-        result.push_back(std::move(result_0));
-    } else {
-        result_0.pushKV("allowed", false);
-        const TxValidationState state = accept_result.m_state;
-        if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) {
-            result_0.pushKV("reject-reason", "missing-inputs");
-        } else {
-            result_0.pushKV("reject-reason", state.GetRejectReason());
-        }
-        result.push_back(std::move(result_0));
+        txns.emplace_back(MakeTransactionRef(std::move(mtx)));
     }
-    return result;
+
+    NodeContext& node = EnsureAnyNodeContext(request.context);
+    CTxMemPool& mempool = EnsureMemPool(node);
+    CChainState& chainstate = EnsureChainman(node).ActiveChainstate();
+    const PackageMempoolAcceptResult package_result = [&] {
+        LOCK(::cs_main);
+        if (txns.size() > 1) return ProcessNewPackage(chainstate, mempool, txns, /* test_accept */ true);
+        return PackageMempoolAcceptResult(txns[0]->GetWitnessHash(),
+               AcceptToMemoryPool(chainstate, mempool, txns[0], /* bypass_limits */ false, /* test_accept*/ true));
+    }();
+
+    UniValue rpc_result(UniValue::VARR);
+    // We will check transaction fees we iterate through txns in order. If any transaction fee
+    // exceeds maxfeerate, we will keave the rest of the validation results blank, because it
+    // doesn't make sense to return a validation result for a transaction if its ancestor(s) would
+    // not be submitted.
+    bool exit_early{false};
+    for (const auto& tx : txns) {
+        UniValue result_inner(UniValue::VOBJ);
+        result_inner.pushKV("txid", tx->GetHash().GetHex());
+        result_inner.pushKV("wtxid", tx->GetWitnessHash().GetHex());
+        if (package_result.m_state.GetResult() == PackageValidationResult::PCKG_POLICY) {
+            result_inner.pushKV("package-error", package_result.m_state.GetRejectReason());
+        }
+        auto it = package_result.m_tx_results.find(tx->GetWitnessHash());
+        if (exit_early || it == package_result.m_tx_results.end()) {
+            // Validation unfinished. Just return the txid and wtxid.
+            rpc_result.push_back(result_inner);
+            continue;
+        }
+        const auto& tx_result = it->second;
+        if (tx_result.m_result_type == MempoolAcceptResult::ResultType::VALID) {
+            const CAmount fee = tx_result.m_base_fees.value();
+            // Check that fee does not exceed maximum fee
+            const int64_t virtual_size = GetVirtualTransactionSize(*tx);
+            const CAmount max_raw_tx_fee = max_raw_tx_fee_rate.GetFee(virtual_size);
+            if (max_raw_tx_fee && fee > max_raw_tx_fee) {
+                result_inner.pushKV("allowed", false);
+                result_inner.pushKV("reject-reason", "max-fee-exceeded");
+                exit_early = true;
+            } else {
+                // Only return the fee and vsize if the transaction would pass ATMP.
+                // These can be used to calculate the feerate.
+                result_inner.pushKV("allowed", true);
+                result_inner.pushKV("vsize", virtual_size);
+                UniValue fees(UniValue::VOBJ);
+                fees.pushKV("base", ValueFromAmount(fee));
+                result_inner.pushKV("fees", fees);
+            }
+        } else {
+            result_inner.pushKV("allowed", false);
+            const TxValidationState state = tx_result.m_state;
+            if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) {
+                result_inner.pushKV("reject-reason", "missing-inputs");
+            } else {
+                result_inner.pushKV("reject-reason", state.GetRejectReason());
+            }
+        }
+        rpc_result.push_back(result_inner);
+    }
+    return rpc_result;
 },
     };
 }

src/test/fuzz/tx_pool.cpp

@@ -222,6 +222,16 @@ FUZZ_TARGET_INIT(tx_pool_standard, initialize_tx_pool)
         RegisterSharedValidationInterface(txr);
         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.
+        // 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(node.chainman->ActiveChainstate(), 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);
+
         const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, tx_pool, tx, bypass_limits));
         const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID;
         SyncWithValidationInterfaceQueue();

src/test/miner_tests.cpp

@@ -28,7 +28,8 @@ struct MinerTestingSetup : public TestingSetup {
     void TestPackageSelection(const CChainParams& chainparams, const CScript& scriptPubKey, const std::vector<CTransactionRef>& txFirst) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_node.mempool->cs);
     bool TestSequenceLocks(const CTransaction& tx, int flags) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_node.mempool->cs)
     {
-        return CheckSequenceLocks(::ChainstateActive(), *m_node.mempool, tx, flags);
+        CCoinsViewMemPool viewMempool(&m_node.chainman->ActiveChainstate().CoinsTip(), *m_node.mempool);
+        return CheckSequenceLocks(m_node.chainman->ActiveChain().Tip(), viewMempool, tx, flags);
     }
     BlockAssembler AssemblerForTest(const CChainParams& params);
 };

src/test/txvalidation_tests.cpp

@@ -3,8 +3,12 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <consensus/validation.h>
+#include <key_io.h>
+#include <policy/packages.h>
+#include <policy/policy.h>
 #include <primitives/transaction.h>
 #include <script/script.h>
+#include <script/standard.h>
 #include <test/util/setup_common.h>
 #include <validation.h>
 
@@ -47,4 +51,98 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_reject_coinbase, TestChain100Setup)
     BOOST_CHECK(result.m_state.GetResult() == TxValidationResult::TX_CONSENSUS);
 }
 
+// Create placeholder transactions that have no meaning.
+inline CTransactionRef create_placeholder_tx(size_t num_inputs, size_t num_outputs)
+{
+    CMutableTransaction mtx = CMutableTransaction();
+    mtx.vin.resize(num_inputs);
+    mtx.vout.resize(num_outputs);
+    auto random_script = CScript() << ToByteVector(InsecureRand256()) << ToByteVector(InsecureRand256());
+    for (size_t i{0}; i < num_inputs; ++i) {
+        mtx.vin[i].prevout.hash = InsecureRand256();
+        mtx.vin[i].prevout.n = 0;
+        mtx.vin[i].scriptSig = random_script;
+    }
+    for (size_t o{0}; o < num_outputs; ++o) {
+        mtx.vout[o].nValue = 1 * CENT;
+        mtx.vout[o].scriptPubKey = random_script;
+    }
+    return MakeTransactionRef(mtx);
+}
+
+BOOST_FIXTURE_TEST_CASE(package_tests, TestChain100Setup)
+{
+    LOCK(cs_main);
+    unsigned int initialPoolSize = m_node.mempool->size();
+
+    // Parent and Child Package
+    CKey parent_key;
+    parent_key.MakeNewKey(true);
+    CScript parent_locking_script = GetScriptForDestination(PKHash(parent_key.GetPubKey()));
+    auto mtx_parent = CreateValidMempoolTransaction(/* input_transaction */ m_coinbase_txns[0], /* 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);
+
+    CKey child_key;
+    child_key.MakeNewKey(true);
+    CScript child_locking_script = GetScriptForDestination(PKHash(child_key.GetPubKey()));
+    auto mtx_child = CreateValidMempoolTransaction(/* input_transaction */ tx_parent, /* 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);
+    const auto result_parent_child = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, {tx_parent, tx_child}, /* test_accept */ true);
+    BOOST_CHECK_MESSAGE(result_parent_child.m_state.IsValid(),
+                        "Package validation unexpectedly failed: " << result_parent_child.m_state.GetRejectReason());
+    auto it_parent = result_parent_child.m_tx_results.find(tx_parent->GetWitnessHash());
+    auto it_child = result_parent_child.m_tx_results.find(tx_child->GetWitnessHash());
+    BOOST_CHECK(it_parent != result_parent_child.m_tx_results.end());
+    BOOST_CHECK_MESSAGE(it_parent->second.m_state.IsValid(),
+                        "Package validation unexpectedly failed: " << it_parent->second.m_state.GetRejectReason());
+    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());
+
+    // Packages can't have more than 25 transactions.
+    Package package_too_many;
+    package_too_many.reserve(MAX_PACKAGE_COUNT + 1);
+    for (size_t i{0}; i < MAX_PACKAGE_COUNT + 1; ++i) {
+        package_too_many.emplace_back(create_placeholder_tx(1, 1));
+    }
+    auto result_too_many = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_too_many, /* test_accept */ true);
+    BOOST_CHECK(result_too_many.m_state.IsInvalid());
+    BOOST_CHECK_EQUAL(result_too_many.m_state.GetResult(), PackageValidationResult::PCKG_POLICY);
+    BOOST_CHECK_EQUAL(result_too_many.m_state.GetRejectReason(), "package-too-many-transactions");
+
+    // Packages can't have a total size of more than 101KvB.
+    CTransactionRef large_ptx = create_placeholder_tx(150, 150);
+    Package package_too_large;
+    auto size_large = GetVirtualTransactionSize(*large_ptx);
+    size_t total_size{0};
+    while (total_size <= MAX_PACKAGE_SIZE * 1000) {
+        package_too_large.push_back(large_ptx);
+        total_size += size_large;
+    }
+    BOOST_CHECK(package_too_large.size() <= MAX_PACKAGE_COUNT);
+    auto result_too_large = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_too_large, /* test_accept */ true);
+    BOOST_CHECK(result_too_large.m_state.IsInvalid());
+    BOOST_CHECK_EQUAL(result_too_large.m_state.GetResult(), PackageValidationResult::PCKG_POLICY);
+    BOOST_CHECK_EQUAL(result_too_large.m_state.GetRejectReason(), "package-too-large");
+
+    // A single, giant transaction submitted through ProcessNewPackage fails on single tx policy.
+    CTransactionRef giant_ptx = create_placeholder_tx(999, 999);
+    BOOST_CHECK(GetVirtualTransactionSize(*giant_ptx) > MAX_PACKAGE_SIZE * 1000);
+    auto result_single_large = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, {giant_ptx}, /* test_accept */ true);
+    BOOST_CHECK(result_single_large.m_state.IsInvalid());
+    BOOST_CHECK_EQUAL(result_single_large.m_state.GetResult(), PackageValidationResult::PCKG_TX);
+    BOOST_CHECK_EQUAL(result_single_large.m_state.GetRejectReason(), "transaction failed");
+    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");
+
+    // Check that mempool size hasn't changed.
+    BOOST_CHECK_EQUAL(m_node.mempool->size(), initialPoolSize);
+}
 BOOST_AUTO_TEST_SUITE_END()

src/test/util/setup_common.cpp

@@ -288,7 +288,8 @@ CMutableTransaction TestChain100Setup::CreateValidMempoolTransaction(CTransactio
                                                                      int input_height,
                                                                      CKey input_signing_key,
                                                                      CScript output_destination,
-                                                                     CAmount output_amount)
+                                                                     CAmount output_amount,
+                                                                     bool submit)
 {
     // Transaction we will submit to the mempool
     CMutableTransaction mempool_txn;
@@ -321,8 +322,8 @@ CMutableTransaction TestChain100Setup::CreateValidMempoolTransaction(CTransactio
     std::map<int, std::string> input_errors;
     assert(SignTransaction(mempool_txn, &keystore, input_coins, nHashType, input_errors));
 
-    // Add transaction to the mempool
-    {
+    // If submit=true, add transaction to the mempool.
+    if (submit) {
         LOCK(cs_main);
         const MempoolAcceptResult result = AcceptToMemoryPool(::ChainstateActive(), *m_node.mempool.get(), MakeTransactionRef(mempool_txn), /* bypass_limits */ false);
         assert(result.m_result_type == MempoolAcceptResult::ResultType::VALID);

src/test/util/setup_common.h

@@ -135,13 +135,15 @@ struct TestChain100Setup : public RegTestingSetup {
      * @param input_signing_key  The key to spend the input_transaction
      * @param output_destination Where to send the output
      * @param output_amount      How much to send
+     * @param submit             Whether or not to submit to mempool
      */
     CMutableTransaction CreateValidMempoolTransaction(CTransactionRef input_transaction,
                                                       int input_vout,
                                                       int input_height,
                                                       CKey input_signing_key,
                                                       CScript output_destination,
-                                                      CAmount output_amount = CAmount(1 * COIN));
+                                                      CAmount output_amount = CAmount(1 * COIN),
+                                                      bool submit = true);
 
     ~TestChain100Setup();
 

src/txmempool.cpp

@@ -518,7 +518,9 @@ void CTxMemPool::removeForReorg(CChainState& active_chainstate, int flags)
         LockPoints lp = it->GetLockPoints();
         assert(std::addressof(::ChainstateActive()) == std::addressof(active_chainstate));
         bool validLP =  TestLockPointValidity(active_chainstate.m_chain, &lp);
-        if (!CheckFinalTx(active_chainstate.m_chain.Tip(), tx, flags) || !CheckSequenceLocks(active_chainstate, *this, tx, flags, &lp, validLP)) {
+        CCoinsViewMemPool viewMempool(&active_chainstate.CoinsTip(), *this);
+        if (!CheckFinalTx(active_chainstate.m_chain.Tip(), tx, flags)
+            || !CheckSequenceLocks(active_chainstate.m_chain.Tip(), viewMempool, tx, flags, &lp, validLP)) {
             // Note if CheckSequenceLocks fails the LockPoints may still be invalid
             // So it's critical that we remove the tx and not depend on the LockPoints.
             txToRemove.insert(it);
@@ -997,6 +999,13 @@ bool CTxMemPool::HasNoInputsOf(const CTransaction &tx) const
 CCoinsViewMemPool::CCoinsViewMemPool(CCoinsView* baseIn, const CTxMemPool& mempoolIn) : CCoinsViewBacked(baseIn), mempool(mempoolIn) { }
 
 bool CCoinsViewMemPool::GetCoin(const COutPoint &outpoint, Coin &coin) const {
+    // Check to see if the inputs are made available by another tx in the package.
+    // These Coins would not be available in the underlying CoinsView.
+    if (auto it = m_temp_added.find(outpoint); it != m_temp_added.end()) {
+        coin = it->second;
+        return true;
+    }
+
     // If an entry in the mempool exists, always return that one, as it's guaranteed to never
     // conflict with the underlying cache, and it cannot have pruned entries (as it contains full)
     // transactions. First checking the underlying cache risks returning a pruned entry instead.
@@ -1012,6 +1021,13 @@ bool CCoinsViewMemPool::GetCoin(const COutPoint &outpoint, Coin &coin) const {
     return base->GetCoin(outpoint, coin);
 }
 
+void CCoinsViewMemPool::PackageAddTransaction(const CTransactionRef& tx)
+{
+    for (unsigned int n = 0; n < tx->vout.size(); ++n) {
+        m_temp_added.emplace(COutPoint(tx->GetHash(), n), Coin(tx->vout[n], MEMPOOL_HEIGHT, false));
+    }
+}
+
 // ELEMENTS:
 bool CCoinsViewMemPool::IsPeginSpent(const std::pair<uint256, COutPoint> &outpoint) const {
     return base->IsPeginSpent(outpoint);

src/txmempool.h

@@ -859,7 +859,8 @@ public:
  * CCoinsView that brings transactions from a mempool into view.
  * It does not check for spendings by memory pool transactions.
  * Instead, it provides access to all Coins which are either unspent in the
- * base CCoinsView, or are outputs from any mempool transaction!
+ * base CCoinsView, are outputs from any mempool transaction, or are
+ * tracked temporarily to allow transaction dependencies in package validation.
  * This allows transaction replacement to work as expected, as you want to
  * have all inputs "available" to check signatures, and any cycles in the
  * dependency graph are checked directly in AcceptToMemoryPool.
@@ -869,12 +870,19 @@ public:
  */
 class CCoinsViewMemPool : public CCoinsViewBacked
 {
+    /**
+    * Coins made available by transactions being validated. Tracking these allows for package
+    * validation, since we can access transaction outputs without submitting them to mempool.
+    */
+    std::unordered_map<COutPoint, Coin, SaltedOutpointHasher> m_temp_added;
 protected:
     const CTxMemPool& mempool;
 
 public:
     CCoinsViewMemPool(CCoinsView* baseIn, const CTxMemPool& mempoolIn);
     bool GetCoin(const COutPoint &outpoint, Coin &coin) const override;
+    /** Add the coins created by this transaction. */
+    void PackageAddTransaction(const CTransactionRef& tx);
     // ELEMENTS:
     bool IsPeginSpent(const std::pair<uint256, COutPoint> &outpoint) const override;
 };

src/validation.cpp

@@ -44,6 +44,7 @@
 #include <uint256.h>
 #include <undo.h>
 #include <util/check.h> // For NDEBUG compile time check
+#include <util/hasher.h>
 #include <util/moneystr.h>
 #include <util/rbf.h>
 #include <util/strencodings.h>
@@ -56,6 +57,7 @@
 #include <block_proof.h> // CheckChallenge, CheckProof
 #include <dynafed.h>
 
+#include <numeric>
 #include <optional>
 #include <string>
 
@@ -251,18 +253,13 @@ bool TestLockPointValidity(CChain& active_chain, const LockPoints* lp)
     return true;
 }
 
-bool CheckSequenceLocks(CChainState& active_chainstate,
-                        const CTxMemPool& pool,
+bool CheckSequenceLocks(CBlockIndex* tip,
+                        const CCoinsView& coins_view,
                         const CTransaction& tx,
                         int flags,
                         LockPoints* lp,
                         bool useExistingLockPoints)
 {
-    AssertLockHeld(cs_main);
-    AssertLockHeld(pool.cs);
-    assert(std::addressof(::ChainstateActive()) == std::addressof(active_chainstate));
-
-    CBlockIndex* tip = active_chainstate.m_chain.Tip();
     assert(tip != nullptr);
 
     CBlockIndex index;
@@ -282,8 +279,6 @@ bool CheckSequenceLocks(CChainState& active_chainstate,
         lockPair.second = lp->time;
     }
     else {
-        // CoinsTip() contains the UTXO set for active_chainstate.m_chain.Tip()
-        CCoinsViewMemPool viewMemPool(&active_chainstate.CoinsTip(), pool);
         std::vector<int> prevheights;
         prevheights.resize(tx.vin.size());
         for (size_t txinIndex = 0; txinIndex < tx.vin.size(); txinIndex++) {
@@ -295,7 +290,7 @@ bool CheckSequenceLocks(CChainState& active_chainstate,
             }
 
             Coin coin;
-            if (!viewMemPool.GetCoin(txin.prevout, coin)) {
+            if (!coins_view.GetCoin(txin.prevout, coin)) {
                 return error("%s: Missing input", __func__);
             }
             if (coin.nHeight == MEMPOOL_HEIGHT) {
@@ -493,11 +488,20 @@ public:
          */
         std::vector<COutPoint>& m_coins_to_uncache;
         const bool m_test_accept;
+        /** Disable BIP125 RBFing; disallow all conflicts with mempool transactions. */
+        const bool disallow_mempool_conflicts;
     };
 
     // Single transaction acceptance
     MempoolAcceptResult AcceptSingleTransaction(const CTransactionRef& ptx, ATMPArgs& args) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
+    /**
+    * Multiple transaction acceptance. Transactions may or may not be interdependent,
+    * but must not conflict with each other. Parents must come before children if any
+    * dependencies exist, otherwise a TX_MISSING_INPUTS error will be returned.
+    */
+    PackageMempoolAcceptResult AcceptMultipleTransactions(const std::vector<CTransactionRef>& txns, ATMPArgs& args) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
+
 private:
     // All the intermediate state that gets passed between the various levels
     // of checking a given transaction.
@@ -664,7 +668,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
                         break;
                     }
                 }
-                if (fReplacementOptOut) {
+                if (fReplacementOptOut || args.disallow_mempool_conflicts) {
                     return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "txn-mempool-conflict");
                 }
 
@@ -749,10 +753,10 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
     // Only accept BIP68 sequence locked transactions that can be mined in the next
     // block; we don't want our mempool filled up with transactions that can't
     // be mined yet.
-    // Must keep pool.cs for this unless we change CheckSequenceLocks to take a
-    // CoinsViewCache instead of create its own
+    // Pass in m_view which has all of the relevant inputs cached. Note that, since m_view's
+    // backend was removed, it no longer pulls coins from the mempool.
     assert(std::addressof(::ChainstateActive()) == std::addressof(m_active_chainstate));
-    if (!CheckSequenceLocks(m_active_chainstate, m_pool, tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp))
+    if (!CheckSequenceLocks(m_active_chainstate.m_chain.Tip(), m_view, tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp))
         return state.Invalid(TxValidationResult::TX_PREMATURE_SPEND, "non-BIP68-final");
 
     assert(std::addressof(g_chainman.m_blockman) == std::addressof(m_active_chainstate.m_blockman));
@@ -1123,7 +1127,7 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef
 
     Workspace ws(ptx);
 
-    if (!PreChecks(args, ws)) return MempoolAcceptResult(ws.m_state);
+    if (!PreChecks(args, ws)) return MempoolAcceptResult::Failure(ws.m_state);
 
     // Only compute the precomputed transaction data if we need to verify
     // scripts (ie, other policy checks pass). We perform the inexpensive
@@ -1131,20 +1135,121 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef
     // checks pass, to mitigate CPU exhaustion denial-of-service attacks.
     PrecomputedTransactionData txdata;
 
-    if (!PolicyScriptChecks(args, ws, txdata)) return MempoolAcceptResult(ws.m_state);
+    if (!PolicyScriptChecks(args, ws, txdata)) return MempoolAcceptResult::Failure(ws.m_state);
 
-    if (!ConsensusScriptChecks(args, ws, txdata)) return MempoolAcceptResult(ws.m_state);
+    if (!ConsensusScriptChecks(args, ws, txdata)) return MempoolAcceptResult::Failure(ws.m_state);
 
     // Tx was accepted, but not added
     if (args.m_test_accept) {
-        return MempoolAcceptResult(std::move(ws.m_replaced_transactions), ws.m_base_fees);
+        return MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_base_fees);
     }
 
-    if (!Finalize(args, ws)) return MempoolAcceptResult(ws.m_state);
+    if (!Finalize(args, ws)) return MempoolAcceptResult::Failure(ws.m_state);
 
     GetMainSignals().TransactionAddedToMempool(ptx, m_pool.GetAndIncrementSequence());
 
-    return MempoolAcceptResult(std::move(ws.m_replaced_transactions), ws.m_base_fees);
+    return MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_base_fees);
+}
+
+PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::vector<CTransactionRef>& txns, ATMPArgs& args)
+{
+    AssertLockHeld(cs_main);
+
+    PackageValidationState package_state;
+    const unsigned int package_count = txns.size();
+
+    // These context-free package limits can be checked before taking the mempool lock.
+    if (package_count > MAX_PACKAGE_COUNT) {
+        package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package-too-many-transactions");
+        return PackageMempoolAcceptResult(package_state, {});
+    }
+
+    const int64_t total_size = std::accumulate(txns.cbegin(), txns.cend(), 0,
+                               [](int64_t sum, const auto& tx) { return sum + GetVirtualTransactionSize(*tx); });
+    // If the package only contains 1 tx, it's better to report the policy violation on individual tx size.
+    if (package_count > 1 && total_size > MAX_PACKAGE_SIZE * 1000) {
+        package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package-too-large");
+        return PackageMempoolAcceptResult(package_state, {});
+    }
+
+    // Construct workspaces and check package policies.
+    std::vector<Workspace> workspaces{};
+    workspaces.reserve(package_count);
+    {
+        std::unordered_set<uint256, SaltedTxidHasher> later_txids;
+        std::transform(txns.cbegin(), txns.cend(), std::inserter(later_txids, later_txids.end()),
+                       [](const auto& tx) { return tx->GetHash(); });
+        // Require the package to be sorted in order of dependency, i.e. parents appear before children.
+        // An unsorted package will fail anyway on missing-inputs, but it's better to quit earlier and
+        // fail on something less ambiguous (missing-inputs could also be an orphan or trying to
+        // spend nonexistent coins).
+        for (const auto& tx : txns) {
+            for (const auto& input : tx->vin) {
+                if (later_txids.find(input.prevout.hash) != later_txids.end()) {
+                    // The parent is a subsequent transaction in the package.
+                    package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package-not-sorted");
+                    return PackageMempoolAcceptResult(package_state, {});
+                }
+            }
+            later_txids.erase(tx->GetHash());
+            workspaces.emplace_back(Workspace(tx));
+        }
+    }
+    std::map<const uint256, const MempoolAcceptResult> results;
+    {
+        // Don't allow any conflicting transactions, i.e. spending the same inputs, in a package.
+        std::unordered_set<COutPoint, SaltedOutpointHasher> inputs_seen;
+        for (const auto& tx : txns) {
+            for (const auto& input : tx->vin) {
+                if (inputs_seen.find(input.prevout) != inputs_seen.end()) {
+                    // This input is also present in another tx in the package.
+                    package_state.Invalid(PackageValidationResult::PCKG_POLICY, "conflict-in-package");
+                    return PackageMempoolAcceptResult(package_state, {});
+                }
+            }
+            // Batch-add all the inputs for a tx at a time. If we added them 1 at a time, we could
+            // catch duplicate inputs within a single tx.  This is a more severe, consensus error,
+            // and we want to report that from CheckTransaction instead.
+            std::transform(tx->vin.cbegin(), tx->vin.cend(), std::inserter(inputs_seen, inputs_seen.end()),
+                           [](const auto& input) { return input.prevout; });
+        }
+    }
+
+    LOCK(m_pool.cs);
+
+    // Do all PreChecks first and fail fast to avoid running expensive script checks when unnecessary.
+    for (Workspace& ws : workspaces) {
+        if (!PreChecks(args, ws)) {
+            package_state.Invalid(PackageValidationResult::PCKG_TX, "transaction failed");
+            // Exit early to avoid doing pointless work. Update the failed tx result; the rest are unfinished.
+            results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Failure(ws.m_state));
+            return PackageMempoolAcceptResult(package_state, std::move(results));
+        }
+        // Make the coins created by this transaction available for subsequent transactions in the
+        // package to spend. Since we already checked conflicts in the package and RBFs are
+        // impossible, we don't need to track the coins spent. Note that this logic will need to be
+        // updated if RBFs in packages are allowed in the future.
+        assert(args.disallow_mempool_conflicts);
+        m_viewmempool.PackageAddTransaction(ws.m_ptx);
+    }
+
+    for (Workspace& ws : workspaces) {
+        PrecomputedTransactionData txdata;
+        if (!PolicyScriptChecks(args, ws, txdata)) {
+            // 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));
+        }
+        if (args.m_test_accept) {
+            // When test_accept=true, transactions that pass PolicyScriptChecks are valid because there are
+            // no further mempool checks (passing PolicyScriptChecks implies passing ConsensusScriptChecks).
+            results.emplace(ws.m_ptx->GetWitnessHash(),
+                            MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_base_fees));
+        }
+    }
+
+    return PackageMempoolAcceptResult(package_state, std::move(results));
 }
 
 } // anon namespace
@@ -1157,7 +1262,8 @@ static MempoolAcceptResult AcceptToMemoryPoolWithTime(const CChainParams& chainp
                                                       EXCLUSIVE_LOCKS_REQUIRED(cs_main)
 {
     std::vector<COutPoint> coins_to_uncache;
-    MemPoolAccept::ATMPArgs args { chainparams, nAcceptTime, bypass_limits, coins_to_uncache, test_accept };
+    MemPoolAccept::ATMPArgs args { chainparams, nAcceptTime, bypass_limits, coins_to_uncache,
+                                   test_accept, /* disallow_mempool_conflicts */ false };
 
     assert(std::addressof(::ChainstateActive()) == std::addressof(active_chainstate));
     const MempoolAcceptResult result = MemPoolAccept(pool, active_chainstate).AcceptSingleTransaction(tx, args);
@@ -1183,6 +1289,29 @@ MempoolAcceptResult AcceptToMemoryPool(CChainState& active_chainstate, CTxMemPoo
     return AcceptToMemoryPoolWithTime(Params(), pool, active_chainstate, tx, GetTime(), bypass_limits, test_accept);
 }
 
+PackageMempoolAcceptResult ProcessNewPackage(CChainState& active_chainstate, CTxMemPool& pool,
+                                                   const Package& package, bool test_accept)
+{
+    AssertLockHeld(cs_main);
+    assert(test_accept); // Only allow package accept dry-runs (testmempoolaccept RPC).
+    assert(!package.empty());
+    assert(std::all_of(package.cbegin(), package.cend(), [](const auto& tx){return tx != nullptr;}));
+
+    std::vector<COutPoint> coins_to_uncache;
+    const CChainParams& chainparams = Params();
+    MemPoolAccept::ATMPArgs args { chainparams, GetTime(), /* bypass_limits */ false, coins_to_uncache,
+                                   test_accept, /* disallow_mempool_conflicts */ true };
+    assert(std::addressof(::ChainstateActive()) == std::addressof(active_chainstate));
+    const PackageMempoolAcceptResult result = MemPoolAccept(pool, active_chainstate).AcceptMultipleTransactions(package, args);
+
+    // Uncache coins pertaining to transactions that were not submitted to the mempool.
+    // Ensure the cache is still within its size limits.
+    for (const COutPoint& hashTx : coins_to_uncache) {
+        active_chainstate.CoinsTip().Uncache(hashTx);
+    }
+    return result;
+}
+
 CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMemPool* const mempool, const uint256& hash, const Consensus::Params& consensusParams, uint256& hashBlock)
 {
     LOCK(cs_main);

src/validation.h

@@ -19,6 +19,7 @@
 #include <fs.h>
 #include <node/utxo_snapshot.h>
 #include <policy/feerate.h>
+#include <policy/packages.h>
 #include <protocol.h> // For CMessageHeader::MessageStartChars
 #include <script/script_error.h>
 #include <sync.h>
@@ -177,9 +178,7 @@ void PruneBlockFilesManual(CChainState& active_chainstate, int nManualPruneHeigh
 * Validation result for a single transaction mempool acceptance.
 */
 struct MempoolAcceptResult {
-    /** Used to indicate the results of mempool validation,
-    * including the possibility of unfinished validation.
-    */
+    /** Used to indicate the results of mempool validation. */
     enum class ResultType {
         VALID, //!> Fully validated, valid.
         INVALID, //!> Invalid.
@@ -192,7 +191,16 @@ struct MempoolAcceptResult {
     const std::optional<std::list<CTransactionRef>> m_replaced_transactions;
     /** Raw base fees in satoshis. */
     const std::optional<CAmount> m_base_fees;
+    static MempoolAcceptResult Failure(TxValidationState state) {
+        return MempoolAcceptResult(state);
+    }
 
+    static MempoolAcceptResult Success(std::list<CTransactionRef>&& replaced_txns, CAmount fees) {
+        return MempoolAcceptResult(std::move(replaced_txns), fees);
+    }
+
+// Private constructors. Use static methods MempoolAcceptResult::Success, etc. to construct.
+private:
     /** Constructor for failure case */
     explicit MempoolAcceptResult(TxValidationState state)
         : m_result_type(ResultType::INVALID), m_state(state) {
@@ -205,6 +213,28 @@ struct MempoolAcceptResult {
         m_replaced_transactions(std::move(replaced_txns)), m_base_fees(fees) {}
 };
 
+/**
+* Validation result for package mempool acceptance.
+*/
+struct PackageMempoolAcceptResult
+{
+    const PackageValidationState m_state;
+    /**
+    * Map from wtxid to finished MempoolAcceptResults. The client is responsible
+    * for keeping track of the transaction objects themselves. If a result is not
+    * present, it means validation was unfinished for that transaction.
+    */
+    std::map<const uint256, const MempoolAcceptResult> m_tx_results;
+
+    explicit PackageMempoolAcceptResult(PackageValidationState state,
+                                        std::map<const uint256, const MempoolAcceptResult>&& results)
+        : m_state{state}, m_tx_results(std::move(results)) {}
+
+    /** Constructor to create a PackageMempoolAcceptResult from a single MempoolAcceptResult */
+    explicit PackageMempoolAcceptResult(const uint256& wtxid, const MempoolAcceptResult& result)
+        : m_tx_results{ {wtxid, result} } {}
+};
+
 /**
  * (Try to) add a transaction to the memory pool.
  * @param[in]  bypass_limits   When true, don't enforce mempool fee limits.
@@ -213,6 +243,18 @@ struct MempoolAcceptResult {
 MempoolAcceptResult AcceptToMemoryPool(CChainState& active_chainstate, CTxMemPool& pool, const CTransactionRef& tx,
                                        bool bypass_limits, bool test_accept=false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
+/**
+* Atomically test acceptance of a package. If the package only contains one tx, package rules still apply.
+* @param[in]    txns                Group of transactions which may be independent or contain
+*                                   parent-child dependencies. The transactions must not conflict, i.e.
+*                                   must not spend the same inputs, even if it would be a valid BIP125
+*                                   replace-by-fee. Parents must appear before children.
+* @returns a PackageMempoolAcceptResult which includes a MempoolAcceptResult for each transaction.
+* If a transaction fails, validation will exit early and some results may be missing.
+*/
+PackageMempoolAcceptResult ProcessNewPackage(CChainState& active_chainstate, CTxMemPool& pool,
+                                                   const Package& txns, bool test_accept)
+                                                   EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
 /** Apply the effects of this transaction on the UTXO set represented by view */
 void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight);
@@ -234,9 +276,13 @@ bool CheckFinalTx(const CBlockIndex* active_chain_tip, const CTransaction &tx, i
 bool TestLockPointValidity(CChain& active_chain, const LockPoints* lp) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
 /**
- * Check if transaction will be BIP 68 final in the next block to be created.
- *
- * Simulates calling SequenceLocks() with data from the tip of the current active chain.
+ * Check if transaction will be BIP68 final in the next block to be created on top of tip.
+ * @param[in]   tip             Chain tip to check tx sequence locks against. For example,
+ *                              the tip of the current active chain.
+ * @param[in]   coins_view      Any CCoinsView that provides access to the relevant coins
+ *                              for checking sequence locks. Any CCoinsView can be passed in;
+ *                              it is assumed to be consistent with the tip.
+ * Simulates calling SequenceLocks() with data from the tip passed in.
  * Optionally stores in LockPoints the resulting height and time calculated and the hash
  * of the block needed for calculation or skips the calculation and uses the LockPoints
  * passed in for evaluation.
@@ -244,12 +290,12 @@ bool TestLockPointValidity(CChain& active_chain, const LockPoints* lp) EXCLUSIVE
  *
  * See consensus/consensus.h for flag definitions.
  */
-bool CheckSequenceLocks(CChainState& active_chainstate,
-                        const CTxMemPool& pool,
+bool CheckSequenceLocks(CBlockIndex* tip,
+                        const CCoinsView& coins_view,
                         const CTransaction& tx,
                         int flags,
                         LockPoints* lp = nullptr,
-                        bool useExistingLockPoints = false) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, pool.cs);
+                        bool useExistingLockPoints = false);
 
 /**
  * Closure representing one script verification

test/functional/mempool_accept.py

@@ -70,7 +70,8 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
 
         self.log.info('Should not accept garbage to testmempoolaccept')
         assert_raises_rpc_error(-3, 'Expected type array, got string', lambda: node.testmempoolaccept(rawtxs='ff00baar'))
-        assert_raises_rpc_error(-8, 'Array must contain exactly one raw transaction for now', lambda: node.testmempoolaccept(rawtxs=['ff00baar', 'ff22']))
+        assert_raises_rpc_error(-8, 'Array must contain between 1 and 25 transactions.', lambda: node.testmempoolaccept(rawtxs=['ff22']*26))
+        assert_raises_rpc_error(-8, 'Array must contain between 1 and 25 transactions.', lambda: node.testmempoolaccept(rawtxs=[]))
         assert_raises_rpc_error(-22, 'TX decode failed', lambda: node.testmempoolaccept(rawtxs=['ff00baar']))
 
         self.log.info('A transaction already in the blockchain')

test/functional/rpc_packages.py

@@ -0,0 +1,364 @@
+#!/usr/bin/env python3
+# Copyright (c) 2021 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""RPCs that handle raw transaction packages."""
+
+from decimal import Decimal
+from io import BytesIO
+import random
+
+from test_framework.address import ADDRESS_BCRT1_P2WSH_OP_TRUE
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.messages import (
+    BIP125_SEQUENCE_NUMBER,
+    COIN,
+    CTransaction,
+    CTxInWitness,
+    CTxOutValue,
+)
+from test_framework.script import (
+    CScript,
+    OP_TRUE,
+)
+from test_framework.util import (
+    assert_equal,
+    hex_str_to_bytes,
+)
+
+class RPCPackagesTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.num_nodes = 1
+        self.setup_clean_chain = True
+
+    def assert_testres_equal(self, package_hex, testres_expected):
+        """Shuffle package_hex and assert that the testmempoolaccept result matches testres_expected. This should only
+        be used to test packages where the order does not matter. The ordering of transactions in package_hex and
+        testres_expected must match.
+        """
+        shuffled_indeces = list(range(len(package_hex)))
+        random.shuffle(shuffled_indeces)
+        shuffled_package = [package_hex[i] for i in shuffled_indeces]
+        shuffled_testres = [testres_expected[i] for i in shuffled_indeces]
+        assert_equal(shuffled_testres, self.nodes[0].testmempoolaccept(shuffled_package))
+
+    def run_test(self):
+        self.log.info("Generate blocks to create UTXOs")
+        node = self.nodes[0]
+        self.privkeys = [node.get_deterministic_priv_key().key]
+        self.address = node.get_deterministic_priv_key().address
+        self.coins = []
+        # The last 100 coinbase transactions are premature
+        for b in node.generatetoaddress(200, self.address)[:100]:
+            coinbase = node.getblock(blockhash=b, verbosity=2)["tx"][0]
+            self.coins.append({
+                "txid": coinbase["txid"],
+                "amount": coinbase["vout"][0]["value"],
+                "scriptPubKey": coinbase["vout"][0]["scriptPubKey"],
+            })
+
+        # Create some transactions that can be reused throughout the test. Never submit these to mempool.
+        self.independent_txns_hex = []
+        self.independent_txns_testres = []
+        for _ in range(3):
+            coin = self.coins.pop()
+            rawtx = node.createrawtransaction([{"txid": coin["txid"], "vout": 0}],
+                {self.address : coin["amount"] - Decimal("0.0001"), "fee" : Decimal("0.0001") })
+            signedtx = node.signrawtransactionwithkey(hexstring=rawtx, privkeys=self.privkeys)
+            assert signedtx["complete"]
+            testres = node.testmempoolaccept([signedtx["hex"]])
+            assert testres[0]["allowed"]
+            self.independent_txns_hex.append(signedtx["hex"])
+            # testmempoolaccept returns a list of length one, avoid creating a 2D list
+            self.independent_txns_testres.append(testres[0])
+        self.independent_txns_testres_blank = [{
+            "txid": res["txid"], "wtxid": res["wtxid"]} for res in self.independent_txns_testres]
+
+        self.test_independent()
+        self.test_chain()
+        self.test_multiple_children()
+        self.test_multiple_parents()
+        self.test_conflicting()
+        self.test_rbf()
+
+    def chain_transaction(self, parent_txid, parent_value, n=0, parent_locking_script=None):
+        """Build a transaction that spends parent_txid.vout[n] and produces one output with
+        amount = parent_value with a fee deducted.
+        Return tuple (CTransaction object, raw hex, nValue, scriptPubKey of the output created).
+        """
+        node = self.nodes[0]
+        inputs = [{"txid": parent_txid, "vout": n}]
+        my_value = parent_value - Decimal("0.0001")
+        outputs = {self.address : my_value, "fee" : Decimal("0.0001")}
+        rawtx = node.createrawtransaction(inputs, outputs)
+        prevtxs = [{
+            "txid": parent_txid,
+            "vout": n,
+            "scriptPubKey": parent_locking_script,
+            "amount": parent_value,
+        }] if parent_locking_script else None
+        signedtx = node.signrawtransactionwithkey(hexstring=rawtx, privkeys=self.privkeys, prevtxs=prevtxs)
+        tx = CTransaction()
+        assert signedtx["complete"]
+        tx.deserialize(BytesIO(hex_str_to_bytes(signedtx["hex"])))
+        return (tx, signedtx["hex"], my_value, tx.vout[0].scriptPubKey.hex())
+
+    def test_independent(self):
+        self.log.info("Test multiple independent transactions in a package")
+        node = self.nodes[0]
+        # For independent transactions, order doesn't matter.
+        self.assert_testres_equal(self.independent_txns_hex, self.independent_txns_testres)
+
+        self.log.info("Test an otherwise valid package with an extra garbage tx appended")
+        garbage_tx = node.createrawtransaction([{"txid": "00" * 32, "vout": 5}], {self.address: 1})
+        tx = CTransaction()
+        tx.deserialize(BytesIO(hex_str_to_bytes(garbage_tx)))
+        # Only the txid and wtxids are returned because validation is incomplete for the independent txns.
+        # Package validation is atomic: if the node cannot find a UTXO for any single tx in the package,
+        # it terminates immediately to avoid unnecessary, expensive signature verification.
+        package_bad = self.independent_txns_hex + [garbage_tx]
+        testres_bad = self.independent_txns_testres_blank + [{"txid": tx.rehash(), "wtxid": tx.getwtxid(), "allowed": False, "reject-reason": "missing-inputs"}]
+        self.assert_testres_equal(package_bad, testres_bad)
+
+        self.log.info("Check testmempoolaccept tells us when some transactions completed validation successfully")
+        coin = self.coins.pop()
+        tx_bad_sig_hex = node.createrawtransaction([{"txid": coin["txid"], "vout": 0}],
+                                           {self.address : coin["amount"] - Decimal("0.0001"), "fee" : Decimal("0.0001") })
+        tx_bad_sig = CTransaction()
+        tx_bad_sig.deserialize(BytesIO(hex_str_to_bytes(tx_bad_sig_hex)))
+        testres_bad_sig = node.testmempoolaccept(self.independent_txns_hex + [tx_bad_sig_hex])
+        # By the time the signature for the last transaction is checked, all the other transactions
+        # have been fully validated, which is why the node returns full validation results for all
+        # transactions here but empty results in other cases.
+        assert_equal(testres_bad_sig, self.independent_txns_testres + [{
+            "txid": tx_bad_sig.rehash(),
+            "wtxid": tx_bad_sig.getwtxid(), "allowed": False,
+            "reject-reason": "mandatory-script-verify-flag-failed (Operation not valid with the current stack size)"
+        }])
+
+        self.log.info("Check testmempoolaccept reports txns in packages that exceed max feerate")
+        coin = self.coins.pop()
+        tx_high_fee_raw = node.createrawtransaction([{"txid": coin["txid"], "vout": 0}],
+                                           {self.address : coin["amount"] - Decimal("0.999"), "fee" : Decimal("0.999")})
+        tx_high_fee_signed = node.signrawtransactionwithkey(hexstring=tx_high_fee_raw, privkeys=self.privkeys)
+        assert tx_high_fee_signed["complete"]
+        tx_high_fee = CTransaction()
+        tx_high_fee.deserialize(BytesIO(hex_str_to_bytes(tx_high_fee_signed["hex"])))
+        testres_high_fee = node.testmempoolaccept([tx_high_fee_signed["hex"]])
+        assert_equal(testres_high_fee, [
+            {"txid": tx_high_fee.rehash(), "wtxid": tx_high_fee.getwtxid(), "allowed": False, "reject-reason": "max-fee-exceeded"}
+        ])
+        package_high_fee = [tx_high_fee_signed["hex"]] + self.independent_txns_hex
+        testres_package_high_fee = node.testmempoolaccept(package_high_fee)
+        assert_equal(testres_package_high_fee, testres_high_fee + self.independent_txns_testres_blank)
+
+    def test_chain(self):
+        node = self.nodes[0]
+        first_coin = self.coins.pop()
+
+        # Chain of 25 transactions
+        parent_locking_script = None
+        txid = first_coin["txid"]
+        chain_hex = []
+        chain_txns = []
+        value = first_coin["amount"]
+
+        for _ in range(25):
+            (tx, txhex, value, parent_locking_script) = self.chain_transaction(txid, value, 0, parent_locking_script)
+            txid = tx.rehash()
+            chain_hex.append(txhex)
+            chain_txns.append(tx)
+
+        self.log.info("Check that testmempoolaccept requires packages to be sorted by dependency")
+        assert_equal(node.testmempoolaccept(rawtxs=chain_hex[::-1]),
+                [{"txid": tx.rehash(), "wtxid": tx.getwtxid(), "package-error": "package-not-sorted"} for tx in chain_txns[::-1]])
+
+        self.log.info("Testmempoolaccept a chain of 25 transactions")
+        testres_multiple = node.testmempoolaccept(rawtxs=chain_hex)
+
+        testres_single = []
+        # Test accept and then submit each one individually, which should be identical to package test accept
+        for rawtx in chain_hex:
+            testres = node.testmempoolaccept([rawtx])
+            testres_single.append(testres[0])
+            # Submit the transaction now so its child should have no problem validating
+            node.sendrawtransaction(rawtx)
+        assert_equal(testres_single, testres_multiple)
+
+        # Clean up by clearing the mempool
+        node.generate(1)
+
+    def test_multiple_children(self):
+        node = self.nodes[0]
+
+        self.log.info("Testmempoolaccept a package in which a transaction has two children within the package")
+        first_coin = self.coins.pop()
+        value = (first_coin["amount"] - Decimal("0.0002")) / 2 # Deduct reasonable fee and make 2 outputs
+        inputs = [{"txid": first_coin["txid"], "vout": 0}]
+        outputs = [{self.address : value}, {ADDRESS_BCRT1_P2WSH_OP_TRUE : value}, {"fee": Decimal("0.0002")}]
+        rawtx = node.createrawtransaction(inputs, outputs)
+
+        parent_signed = node.signrawtransactionwithkey(hexstring=rawtx, privkeys=self.privkeys)
+        parent_tx = CTransaction()
+        assert parent_signed["complete"]
+        parent_tx.deserialize(BytesIO(hex_str_to_bytes(parent_signed["hex"])))
+        parent_txid = parent_tx.rehash()
+        assert node.testmempoolaccept([parent_signed["hex"]])[0]["allowed"]
+
+        parent_locking_script_a = parent_tx.vout[0].scriptPubKey.hex()
+        child_value = value
+
+        # Child A
+        (_, tx_child_a_hex, _, _) = self.chain_transaction(parent_txid, child_value, 0, parent_locking_script_a)
+        assert not node.testmempoolaccept([tx_child_a_hex])[0]["allowed"]
+
+        # Child B
+        rawtx_b = node.createrawtransaction([{"txid": parent_txid, "vout": 1}], {self.address : child_value - Decimal("0.0001"), "fee": Decimal("0.0001")})
+        tx_child_b = CTransaction()
+        tx_child_b.deserialize(BytesIO(hex_str_to_bytes(rawtx_b)))
+        tx_child_b.wit.vtxinwit = [CTxInWitness()]
+        tx_child_b.wit.vtxinwit[0].scriptWitness.stack = [CScript([OP_TRUE])]
+        tx_child_b_hex = tx_child_b.serialize().hex()
+        assert not node.testmempoolaccept([tx_child_b_hex])[0]["allowed"]
+
+        self.log.info("Testmempoolaccept with entire package, should work with children in either order")
+        testres_multiple_ab = node.testmempoolaccept(rawtxs=[parent_signed["hex"], tx_child_a_hex, tx_child_b_hex])
+        testres_multiple_ba = node.testmempoolaccept(rawtxs=[parent_signed["hex"], tx_child_b_hex, tx_child_a_hex])
+        assert all([testres["allowed"] for testres in testres_multiple_ab + testres_multiple_ba])
+
+        testres_single = []
+        # Test accept and then submit each one individually, which should be identical to package testaccept
+        for rawtx in [parent_signed["hex"], tx_child_a_hex, tx_child_b_hex]:
+            testres = node.testmempoolaccept([rawtx])
+            testres_single.append(testres[0])
+            # Submit the transaction now so its child should have no problem validating
+            node.sendrawtransaction(rawtx)
+        assert_equal(testres_single, testres_multiple_ab)
+
+    def create_child_with_parents(self, parents_tx, values, locking_scripts):
+        """Creates a transaction that spends the first output of each parent in parents_tx."""
+        num_parents = len(parents_tx)
+        total_value = sum(values)
+        inputs = [{"txid": tx.rehash(), "vout": 0} for tx in parents_tx]
+        outputs = {self.address : total_value - num_parents * Decimal("0.0001"), "fee" : num_parents * Decimal("0.0001")}
+        rawtx_child = self.nodes[0].createrawtransaction(inputs, outputs)
+        prevtxs = []
+        for i in range(num_parents):
+            prevtxs.append({"txid": parents_tx[i].rehash(), "vout": 0, "scriptPubKey": locking_scripts[i], "amount": values[i]})
+        signedtx_child = self.nodes[0].signrawtransactionwithkey(hexstring=rawtx_child, privkeys=self.privkeys, prevtxs=prevtxs)
+        assert signedtx_child["complete"]
+        return signedtx_child["hex"]
+
+    def test_multiple_parents(self):
+        node = self.nodes[0]
+
+        self.log.info("Testmempoolaccept a package in which a transaction has multiple parents within the package")
+        for num_parents in [2, 10, 24]:
+            # Test a package with num_parents parents and 1 child transaction.
+            package_hex = []
+            parents_tx = []
+            values = []
+            parent_locking_scripts = []
+            for _ in range(num_parents):
+                parent_coin = self.coins.pop()
+                value = parent_coin["amount"]
+                (tx, txhex, value, parent_locking_script) = self.chain_transaction(parent_coin["txid"], value)
+                package_hex.append(txhex)
+                parents_tx.append(tx)
+                values.append(value)
+                parent_locking_scripts.append(parent_locking_script)
+            child_hex = self.create_child_with_parents(parents_tx, values, parent_locking_scripts)
+            # Package accept should work with the parents in any order (as long as parents come before child)
+            for _ in range(10):
+                random.shuffle(package_hex)
+                testres_multiple = node.testmempoolaccept(rawtxs=package_hex + [child_hex])
+                assert all([testres["allowed"] for testres in testres_multiple])
+
+            testres_single = []
+            # Test accept and then submit each one individually, which should be identical to package testaccept
+            for rawtx in package_hex + [child_hex]:
+                testres_single.append(node.testmempoolaccept([rawtx])[0])
+                # Submit the transaction now so its child should have no problem validating
+                node.sendrawtransaction(rawtx)
+            assert_equal(testres_single, testres_multiple)
+
+    def test_conflicting(self):
+        node = self.nodes[0]
+        prevtx = self.coins.pop()
+        inputs = [{"txid": prevtx["txid"], "vout": 0}]
+        output1 = {node.get_deterministic_priv_key().address: 50 - 0.00125, "fee" : 0.00125}
+        output2 = {ADDRESS_BCRT1_P2WSH_OP_TRUE: 50 - 0.00125, "fee" : 0.00125}
+
+        # tx1 and tx2 share the same inputs
+        rawtx1 = node.createrawtransaction(inputs, output1)
+        rawtx2 = node.createrawtransaction(inputs, output2)
+        signedtx1 = node.signrawtransactionwithkey(hexstring=rawtx1, privkeys=self.privkeys)
+        signedtx2 = node.signrawtransactionwithkey(hexstring=rawtx2, privkeys=self.privkeys)
+        tx1 = CTransaction()
+        tx1.deserialize(BytesIO(hex_str_to_bytes(signedtx1["hex"])))
+        tx2 = CTransaction()
+        tx2.deserialize(BytesIO(hex_str_to_bytes(signedtx2["hex"])))
+        assert signedtx1["complete"]
+        assert signedtx2["complete"]
+
+        # Ensure tx1 and tx2 are valid by themselves
+        assert node.testmempoolaccept([signedtx1["hex"]])[0]["allowed"]
+        assert node.testmempoolaccept([signedtx2["hex"]])[0]["allowed"]
+
+        self.log.info("Test duplicate transactions in the same package")
+        testres = node.testmempoolaccept([signedtx1["hex"], signedtx1["hex"]])
+        assert_equal(testres, [
+            {"txid": tx1.rehash(), "wtxid": tx1.getwtxid(), "package-error": "conflict-in-package"},
+            {"txid": tx1.rehash(), "wtxid": tx1.getwtxid(), "package-error": "conflict-in-package"}
+        ])
+
+        self.log.info("Test conflicting transactions in the same package")
+        testres = node.testmempoolaccept([signedtx1["hex"], signedtx2["hex"]])
+        assert_equal(testres, [
+            {"txid": tx1.rehash(), "wtxid": tx1.getwtxid(), "package-error": "conflict-in-package"},
+            {"txid": tx2.rehash(), "wtxid": tx2.getwtxid(), "package-error": "conflict-in-package"}
+        ])
+
+    def test_rbf(self):
+        node = self.nodes[0]
+        coin = self.coins.pop()
+        inputs = [{"txid": coin["txid"], "vout": 0, "sequence": BIP125_SEQUENCE_NUMBER}]
+        fee = Decimal('0.00125000')
+        output = {node.get_deterministic_priv_key().address: 50 - fee, "fee" : fee}
+        raw_replaceable_tx = node.createrawtransaction(inputs, output)
+        signed_replaceable_tx = node.signrawtransactionwithkey(hexstring=raw_replaceable_tx, privkeys=self.privkeys)
+        testres_replaceable = node.testmempoolaccept([signed_replaceable_tx["hex"]])
+        replaceable_tx = CTransaction()
+        replaceable_tx.deserialize(BytesIO(hex_str_to_bytes(signed_replaceable_tx["hex"])))
+        assert_equal(testres_replaceable, [
+            {"txid": replaceable_tx.rehash(), "wtxid": replaceable_tx.getwtxid(),
+            "allowed": True, "vsize": replaceable_tx.get_vsize(), "fees": { "base": fee }}
+        ])
+
+        # Replacement transaction is identical except has double the fee
+        replacement_tx = CTransaction()
+        replacement_tx.deserialize(BytesIO(hex_str_to_bytes(signed_replaceable_tx["hex"])))
+        replacement_tx.vout[0].nValue = CTxOutValue(int((50 - 2*fee) * COIN))  # Doubled fee
+        replacement_tx.vout[1].nValue = CTxOutValue(int(2*fee * COIN))  # Doubled fee
+        signed_replacement_tx = node.signrawtransactionwithkey(replacement_tx.serialize().hex(), self.privkeys)
+        replacement_tx.deserialize(BytesIO(hex_str_to_bytes(signed_replacement_tx["hex"])))
+
+        self.log.info("Test that transactions within a package cannot replace each other")
+        testres_rbf_conflicting = node.testmempoolaccept([signed_replaceable_tx["hex"], signed_replacement_tx["hex"]])
+        assert_equal(testres_rbf_conflicting, [
+            {"txid": replaceable_tx.rehash(), "wtxid": replaceable_tx.getwtxid(), "package-error": "conflict-in-package"},
+            {"txid": replacement_tx.rehash(), "wtxid": replacement_tx.getwtxid(), "package-error": "conflict-in-package"}
+        ])
+
+        self.log.info("Test that packages cannot conflict with mempool transactions, even if a valid BIP125 RBF")
+        node.sendrawtransaction(signed_replaceable_tx["hex"])
+        testres_rbf_single = node.testmempoolaccept([signed_replacement_tx["hex"]])
+        # This transaction is a valid BIP125 replace-by-fee
+        assert testres_rbf_single[0]["allowed"]
+        testres_rbf_package = self.independent_txns_testres_blank + [{
+            "txid": replacement_tx.rehash(), "wtxid": replacement_tx.getwtxid(), "allowed": False, "reject-reason": "txn-mempool-conflict"
+        }]
+        self.assert_testres_equal(self.independent_txns_hex + [signed_replacement_tx["hex"]], testres_rbf_package)
+
+if __name__ == "__main__":
+    RPCPackagesTest().main()

test/functional/test_runner.py

@@ -238,6 +238,7 @@ BASE_SCRIPTS = [
     'mempool_package_onemore.py',
     'rpc_createmultisig.py --legacy-wallet',
     'rpc_createmultisig.py --descriptors',
+    'rpc_packages.py',
     # ELEMENTS: no versionbits in use
     #'feature_versionbits_warning.py',
     'rpc_preciousblock.py',