diff --git a/doc/release-notes-20833.md b/doc/release-notes-20833.md new file mode 100644 index 00000000000..9a02bbd275f --- /dev/null +++ b/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) + diff --git a/src/Makefile.am b/src/Makefile.am index 2c25f04d086..80c142009c3 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -193,6 +193,7 @@ BITCOIN_CORE_H = \ outputtype.h \ policy/feerate.h \ policy/fees.h \ + policy/packages.h \ policy/policy.h \ policy/rbf.h \ policy/settings.h \ diff --git a/src/policy/packages.h b/src/policy/packages.h new file mode 100644 index 00000000000..4b1463dcb34 --- /dev/null +++ b/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 +#include + +#include + +/** 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; + +class PackageValidationState : public ValidationState {}; + +#endif // BITCOIN_POLICY_PACKAGES_H diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index adb8ac05955..339d711ac98 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -885,8 +886,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" @@ -895,17 +899,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)", { @@ -932,62 +940,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 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; }, }; } diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index ad11f2c5f28..bab34ea3402 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -219,6 +219,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(); diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 9ba004cc38e..c47d0eae1ef 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -28,7 +28,8 @@ struct MinerTestingSetup : public TestingSetup { void TestPackageSelection(const CChainParams& chainparams, const CScript& scriptPubKey, const std::vector& 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); }; diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp index 8d14071297b..95ad85d0f83 100644 --- a/src/test/txvalidation_tests.cpp +++ b/src/test/txvalidation_tests.cpp @@ -3,8 +3,12 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include +#include +#include #include #include