partial bitcoin#20833: enable packages through testmempoolaccept

excludes:
- c9e1a26 (will be added in future fuzzing PR)

inapplicable:
- 249f43f (we don't have RBF)

doc/release-notes-5834.md

@@ -3,3 +3,11 @@ Updated RPCs
 
 - The `testmempoolaccept` RPC returns `vsize` and a `fee` object with the `base` fee
   if the transaction passes validation.
+- 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. 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.

src/Makefile.am

@@ -275,6 +275,7 @@ BITCOIN_CORE_H = \
   outputtype.h \
   policy/feerate.h \
   policy/fees.h \
+  policy/packages.h \
   policy/policy.h \
   policy/settings.h \
   pow.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

@@ -20,6 +20,7 @@
 #include <node/context.h>
 #include <node/psbt.h>
 #include <node/transaction.h>
+#include <policy/packages.h>
 #include <policy/policy.h>
 #include <policy/settings.h>
 #include <primitives/transaction.h>
@@ -1054,25 +1055,32 @@ static UniValue testmempoolaccept(const JSONRPCRequest& request)
 {
     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"
+                "\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.",
                         {
                             {"rawtx", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, ""},
                         },
                         },
-                    {"maxfeerate", RPCArg::Type::AMOUNT, /* default */ FormatMoney(DEFAULT_MAX_RAW_TX_FEE_RATE.GetFeePerK()), "Reject transactions whose fee rate is higher than the specified value, expressed in " + CURRENCY_UNIT + "/kB\n"},
+                    {"maxfeerate", RPCArg::Type::AMOUNT, /* default */ FormatMoney(DEFAULT_MAX_RAW_TX_FEE_RATE.GetFeePerK()),
+                     "Reject transactions whose fee rate is higher than the specified value, expressed in " + CURRENCY_UNIT + "/kB\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::BOOL, "allowed", "If the mempool allows this tx to be inserted"},
-                            {RPCResult::Type::NUM, "vsize", "Virtual transaction size as defined in BIP 141"},
+                            {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."},
                             {RPCResult::Type::OBJ, "fees", "Transaction fees (only present if 'allowed' is true)",
                             {
                                 {RPCResult::Type::STR_AMOUNT, "base", "transaction fee in " + CURRENCY_UNIT},
@@ -1098,62 +1106,85 @@ static UniValue testmempoolaccept(const JSONRPCRequest& request)
         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");
-    }
-
-    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.");
+    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.");
     }
-    CTransactionRef tx(MakeTransactionRef(std::move(mtx)));
-    const uint256& tx_hash = tx->GetHash();
 
     const CFeeRate max_raw_tx_fee_rate = request.params[1].isNull() ?
                                              DEFAULT_MAX_RAW_TX_FEE_RATE :
                                              CFeeRate(AmountFromValue(request.params[1]));
 
-    const NodeContext& node = EnsureAnyNodeContext(request.context);
+    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.");
+        }
+        txns.emplace_back(MakeTransactionRef(std::move(mtx)));
+    }
 
+    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_hash.GetHex());
+    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]->GetHash(),
+               AcceptToMemoryPool(chainstate, mempool, txns[0], /* bypass_limits */ false, /* test_accept*/ true));
+    }();
 
-    ChainstateManager& chainman = EnsureChainman(node);
-    const MempoolAcceptResult accept_result = WITH_LOCK(cs_main, return AcceptToMemoryPool(chainman.ActiveChainstate(), mempool, std::move(tx),
-                                                  false /* bypass_limits */, true /* test_accept */));
-
-    // 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);
+    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());
+        if (package_result.m_state.GetResult() == PackageValidationResult::PCKG_POLICY) {
+            result_inner.pushKV("package-error", package_result.m_state.GetRejectReason());
         }
-        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");
+        auto it = package_result.m_tx_results.find(tx->GetHash());
+        if (exit_early || it == package_result.m_tx_results.end()) {
+            // Validation unfinished. Just return the txid.
+            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_0.pushKV("reject-reason", state.GetRejectReason());
+            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());
+            }
         }
-        result.push_back(std::move(result_0));
+        rpc_result.push_back(result_inner);
     }
-    return result;
+    return rpc_result;
 }
 
 UniValue decodepsbt(const JSONRPCRequest& request)

src/test/miner_tests.cpp

@@ -35,7 +35,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>
 
@@ -46,4 +50,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->GetHash());
+    auto it_child = result_parent_child.m_tx_results.find(tx_child->GetHash());
+    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->GetHash());
+    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()