Skip to content

Commit 9ede34a

Browse files
committed
[rpc] allow multiple txns in testmempoolaccept
Only allow "packages" with no conflicts, sorted in order of dependency, and no more than 25 for now. Note that these groups of transactions don't necessarily need to adhere to some strict definition of a package or have any dependency relationships. Clients are free to pass in a batch of 25 unrelated transactions if they want to.
1 parent ae8e6df commit 9ede34a

File tree

2 files changed

+83
-50
lines changed

2 files changed

+83
-50
lines changed

src/rpc/rawtransaction.cpp

Lines changed: 81 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include <node/context.h>
1616
#include <node/psbt.h>
1717
#include <node/transaction.h>
18+
#include <policy/packages.h>
1819
#include <policy/policy.h>
1920
#include <policy/rbf.h>
2021
#include <primitives/transaction.h>
@@ -885,8 +886,11 @@ static RPCHelpMan sendrawtransaction()
885886
static RPCHelpMan testmempoolaccept()
886887
{
887888
return RPCHelpMan{"testmempoolaccept",
888-
"\nReturns result of mempool acceptance tests indicating if raw transaction (serialized, hex-encoded) would be accepted by mempool.\n"
889-
"\nThis checks if the transaction violates the consensus or policy rules.\n"
889+
"\nReturns result of mempool acceptance tests indicating if raw transaction(s) (serialized, hex-encoded) would be accepted by mempool.\n"
890+
"\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"
891+
"\nIf one transaction fails, other transactions may not be fully validated (the 'allowed' key will be blank).\n"
892+
"\nThe maximum number of transactions allowed is 25 (MAX_PACKAGE_COUNT)\n"
893+
"\nThis checks if transactions violate the consensus or policy rules.\n"
890894
"\nSee sendrawtransaction call.\n",
891895
{
892896
{"rawtxs", RPCArg::Type::ARR, RPCArg::Optional::NO, "An array of hex strings of raw transactions.\n"
@@ -895,17 +899,21 @@ static RPCHelpMan testmempoolaccept()
895899
{"rawtx", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, ""},
896900
},
897901
},
898-
{"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"},
902+
{"maxfeerate", RPCArg::Type::AMOUNT, RPCArg::Default{FormatMoney(DEFAULT_MAX_RAW_TX_FEE_RATE.GetFeePerK())},
903+
"Reject transactions whose fee rate is higher than the specified value, expressed in " + CURRENCY_UNIT + "/kvB\n"},
899904
},
900905
RPCResult{
901906
RPCResult::Type::ARR, "", "The result of the mempool acceptance test for each raw transaction in the input array.\n"
902-
"Length is exactly one for now.",
907+
"Returns results for each transaction in the same order they were passed in.\n"
908+
"It is possible for transactions to not be fully validated ('allowed' unset) if an earlier transaction failed.\n",
903909
{
904910
{RPCResult::Type::OBJ, "", "",
905911
{
906912
{RPCResult::Type::STR_HEX, "txid", "The transaction hash in hex"},
907913
{RPCResult::Type::STR_HEX, "wtxid", "The transaction witness hash in hex"},
908-
{RPCResult::Type::BOOL, "allowed", "If the mempool allows this tx to be inserted"},
914+
{RPCResult::Type::STR, "package-error", "Package validation error, if any (only possible if rawtxs had more than 1 transaction)."},
915+
{RPCResult::Type::BOOL, "allowed", "Whether this tx would be accepted to the mempool and pass client-specified maxfeerate."
916+
"If not present, the tx was not fully validated due to a failure in another tx in the list."},
909917
{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)"},
910918
{RPCResult::Type::OBJ, "fees", "Transaction fees (only present if 'allowed' is true)",
911919
{
@@ -932,62 +940,86 @@ static RPCHelpMan testmempoolaccept()
932940
UniValueType(), // VNUM or VSTR, checked inside AmountFromValue()
933941
});
934942

935-
if (request.params[0].get_array().size() != 1) {
936-
throw JSONRPCError(RPC_INVALID_PARAMETER, "Array must contain exactly one raw transaction for now");
937-
}
938-
939-
CMutableTransaction mtx;
940-
if (!DecodeHexTx(mtx, request.params[0].get_array()[0].get_str())) {
941-
throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed. Make sure the tx has at least one input.");
943+
const UniValue raw_transactions = request.params[0].get_array();
944+
if (raw_transactions.size() < 1 || raw_transactions.size() > MAX_PACKAGE_COUNT) {
945+
throw JSONRPCError(RPC_INVALID_PARAMETER,
946+
"Array must contain between 1 and " + ToString(MAX_PACKAGE_COUNT) + " transactions.");
942947
}
943-
CTransactionRef tx(MakeTransactionRef(std::move(mtx)));
944948

945949
const CFeeRate max_raw_tx_fee_rate = request.params[1].isNull() ?
946950
DEFAULT_MAX_RAW_TX_FEE_RATE :
947951
CFeeRate(AmountFromValue(request.params[1]));
948952

949-
NodeContext& node = EnsureAnyNodeContext(request.context);
953+
std::vector<CTransactionRef> txns;
954+
for (const auto& rawtx : raw_transactions.getValues()) {
955+
CMutableTransaction mtx;
956+
if (!DecodeHexTx(mtx, rawtx.get_str())) {
957+
throw JSONRPCError(RPC_DESERIALIZATION_ERROR,
958+
"TX decode failed: " + rawtx.get_str() + " Make sure the tx has at least one input.");
959+
}
960+
txns.emplace_back(MakeTransactionRef(std::move(mtx)));
961+
}
950962

963+
NodeContext& node = EnsureAnyNodeContext(request.context);
951964
CTxMemPool& mempool = EnsureMemPool(node);
952-
int64_t virtual_size = GetVirtualTransactionSize(*tx);
953-
CAmount max_raw_tx_fee = max_raw_tx_fee_rate.GetFee(virtual_size);
954-
955-
UniValue result(UniValue::VARR);
956-
UniValue result_0(UniValue::VOBJ);
957-
result_0.pushKV("txid", tx->GetHash().GetHex());
958-
result_0.pushKV("wtxid", tx->GetWitnessHash().GetHex());
959-
960-
ChainstateManager& chainman = EnsureChainman(node);
961-
const MempoolAcceptResult accept_result = WITH_LOCK(cs_main, return AcceptToMemoryPool(chainman.ActiveChainstate(), mempool, std::move(tx),
962-
false /* bypass_limits */, /* test_accept */ true));
963-
964-
// Only return the fee and vsize if the transaction would pass ATMP.
965-
// These can be used to calculate the feerate.
966-
if (accept_result.m_result_type == MempoolAcceptResult::ResultType::VALID) {
967-
const CAmount fee = accept_result.m_base_fees.value();
968-
// Check that fee does not exceed maximum fee
969-
if (max_raw_tx_fee && fee > max_raw_tx_fee) {
970-
result_0.pushKV("allowed", false);
971-
result_0.pushKV("reject-reason", "max-fee-exceeded");
972-
} else {
973-
result_0.pushKV("allowed", true);
974-
result_0.pushKV("vsize", virtual_size);
975-
UniValue fees(UniValue::VOBJ);
976-
fees.pushKV("base", ValueFromAmount(fee));
977-
result_0.pushKV("fees", fees);
965+
CChainState& chainstate = EnsureChainman(node).ActiveChainstate();
966+
const PackageMempoolAcceptResult package_result = [&] {
967+
LOCK(::cs_main);
968+
if (txns.size() > 1) return ProcessNewPackage(chainstate, mempool, txns, /* test_accept */ true);
969+
return PackageMempoolAcceptResult(txns[0]->GetWitnessHash(),
970+
AcceptToMemoryPool(chainstate, mempool, txns[0], /* bypass_limits */ false, /* test_accept*/ true));
971+
}();
972+
973+
UniValue rpc_result(UniValue::VARR);
974+
// We will check transaction fees we iterate through txns in order. If any transaction fee
975+
// exceeds maxfeerate, we will keave the rest of the validation results blank, because it
976+
// doesn't make sense to return a validation result for a transaction if its ancestor(s) would
977+
// not be submitted.
978+
bool exit_early{false};
979+
for (const auto& tx : txns) {
980+
UniValue result_inner(UniValue::VOBJ);
981+
result_inner.pushKV("txid", tx->GetHash().GetHex());
982+
result_inner.pushKV("wtxid", tx->GetWitnessHash().GetHex());
983+
if (package_result.m_state.GetResult() == PackageValidationResult::PCKG_POLICY) {
984+
result_inner.pushKV("package-error", package_result.m_state.GetRejectReason());
978985
}
979-
result.push_back(std::move(result_0));
980-
} else {
981-
result_0.pushKV("allowed", false);
982-
const TxValidationState state = accept_result.m_state;
983-
if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) {
984-
result_0.pushKV("reject-reason", "missing-inputs");
986+
auto it = package_result.m_tx_results.find(tx->GetWitnessHash());
987+
if (exit_early || it == package_result.m_tx_results.end()) {
988+
// Validation unfinished. Just return the txid and wtxid.
989+
rpc_result.push_back(result_inner);
990+
continue;
991+
}
992+
const auto& tx_result = it->second;
993+
if (tx_result.m_result_type == MempoolAcceptResult::ResultType::VALID) {
994+
const CAmount fee = tx_result.m_base_fees.value();
995+
// Check that fee does not exceed maximum fee
996+
const int64_t virtual_size = GetVirtualTransactionSize(*tx);
997+
const CAmount max_raw_tx_fee = max_raw_tx_fee_rate.GetFee(virtual_size);
998+
if (max_raw_tx_fee && fee > max_raw_tx_fee) {
999+
result_inner.pushKV("allowed", false);
1000+
result_inner.pushKV("reject-reason", "max-fee-exceeded");
1001+
exit_early = true;
1002+
} else {
1003+
// Only return the fee and vsize if the transaction would pass ATMP.
1004+
// These can be used to calculate the feerate.
1005+
result_inner.pushKV("allowed", true);
1006+
result_inner.pushKV("vsize", virtual_size);
1007+
UniValue fees(UniValue::VOBJ);
1008+
fees.pushKV("base", ValueFromAmount(fee));
1009+
result_inner.pushKV("fees", fees);
1010+
}
9851011
} else {
986-
result_0.pushKV("reject-reason", state.GetRejectReason());
1012+
result_inner.pushKV("allowed", false);
1013+
const TxValidationState state = tx_result.m_state;
1014+
if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) {
1015+
result_inner.pushKV("reject-reason", "missing-inputs");
1016+
} else {
1017+
result_inner.pushKV("reject-reason", state.GetRejectReason());
1018+
}
9871019
}
988-
result.push_back(std::move(result_0));
1020+
rpc_result.push_back(result_inner);
9891021
}
990-
return result;
1022+
return rpc_result;
9911023
},
9921024
};
9931025
}

test/functional/mempool_accept.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ def run_test(self):
6767

6868
self.log.info('Should not accept garbage to testmempoolaccept')
6969
assert_raises_rpc_error(-3, 'Expected type array, got string', lambda: node.testmempoolaccept(rawtxs='ff00baar'))
70-
assert_raises_rpc_error(-8, 'Array must contain exactly one raw transaction for now', lambda: node.testmempoolaccept(rawtxs=['ff00baar', 'ff22']))
70+
assert_raises_rpc_error(-8, 'Array must contain between 1 and 25 transactions.', lambda: node.testmempoolaccept(rawtxs=['ff22']*26))
71+
assert_raises_rpc_error(-8, 'Array must contain between 1 and 25 transactions.', lambda: node.testmempoolaccept(rawtxs=[]))
7172
assert_raises_rpc_error(-22, 'TX decode failed', lambda: node.testmempoolaccept(rawtxs=['ff00baar']))
7273

7374
self.log.info('A transaction already in the blockchain')

0 commit comments

Comments
 (0)