Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mempool, refactor: add MemPoolBypass #25577

Closed
wants to merge 8 commits into from
2 changes: 1 addition & 1 deletion src/bench/block_assemble.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ static void AssembleBlock(benchmark::Bench& bench)
LOCK(::cs_main);

for (const auto& txr : txs) {
const MempoolAcceptResult res = test_setup->m_node.chainman->ProcessTransaction(txr);
const MempoolAcceptResult res = test_setup->m_node.chainman->ProcessTransaction(/*tx=*/txr, /*mempool_bypass=*/std::nullopt);
assert(res.m_result_type == MempoolAcceptResult::ResultType::VALID);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/kernel/mempool_persist.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, CChainState& activ
}
if (nTime > TicksSinceEpoch<std::chrono::seconds>(now - pool.m_expiry)) {
LOCK(cs_main);
const auto& accepted = AcceptToMemoryPool(active_chainstate, tx, nTime, /*bypass_limits=*/false, /*test_accept=*/false);
const auto& accepted = AcceptToMemoryPool(/*active_chainstate=*/active_chainstate, tx, /*accept_time=*/nTime, /*mempool_bypass=*/std::nullopt);
if (accepted.m_result_type == MempoolAcceptResult::ResultType::VALID) {
++count;
} else {
Expand Down
4 changes: 2 additions & 2 deletions src/net_processing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2537,7 +2537,7 @@ void PeerManagerImpl::ProcessOrphanTx(std::set<uint256>& orphan_work_set)
const auto [porphanTx, from_peer] = m_orphanage.GetTx(orphanHash);
if (porphanTx == nullptr) continue;

const MempoolAcceptResult result = m_chainman.ProcessTransaction(porphanTx);
const MempoolAcceptResult result = m_chainman.ProcessTransaction(/*tx=*/porphanTx, /*mempool_bypass=*/std::nullopt);
const TxValidationState& state = result.m_state;

if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) {
Expand Down Expand Up @@ -3561,7 +3561,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
return;
}

const MempoolAcceptResult result = m_chainman.ProcessTransaction(ptx);
const MempoolAcceptResult result = m_chainman.ProcessTransaction(/*tx=*/ptx, /*mempool_bypass=*/std::nullopt);
const TxValidationState& state = result.m_state;

if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) {
Expand Down
6 changes: 4 additions & 2 deletions src/node/transaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,17 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
if (max_tx_fee > 0) {
// First, call ATMP with test_accept and check the fee. If ATMP
// fails here, return error immediately.
const MempoolAcceptResult result = node.chainman->ProcessTransaction(tx, /*test_accept=*/ true);

const MemPoolBypass mempool_bypass{.m_test_accept=true, .m_bypass_limits=false};
const MempoolAcceptResult result = node.chainman->ProcessTransaction(tx, mempool_bypass);
if (result.m_result_type != MempoolAcceptResult::ResultType::VALID) {
return HandleATMPError(result.m_state, err_string);
} else if (result.m_base_fees.value() > max_tx_fee) {
return TransactionError::MAX_FEE_EXCEEDED;
}
}
// Try to submit the transaction to the mempool.
const MempoolAcceptResult result = node.chainman->ProcessTransaction(tx, /*test_accept=*/ false);
const MempoolAcceptResult result = node.chainman->ProcessTransaction(tx, /*mempool_bypass=*/std::nullopt);
if (result.m_result_type != MempoolAcceptResult::ResultType::VALID) {
return HandleATMPError(result.m_state, err_string);
}
Expand Down
1 change: 1 addition & 0 deletions src/rpc/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "sendrawtransaction", 1, "maxfeerate" },
{ "testmempoolaccept", 0, "rawtxs" },
{ "testmempoolaccept", 1, "maxfeerate" },
{ "testmempoolaccept", 1, "options" },
{ "submitpackage", 0, "package" },
{ "combinerawtransaction", 0, "txs" },
{ "fundrawtransaction", 1, "options" },
Expand Down
73 changes: 54 additions & 19 deletions src/rpc/mempool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,14 @@ 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"},
{"options|maxfeerate", {RPCArg::Type::OBJ, RPCArg::Type::AMOUNT}, RPCArg::Optional::OMITTED_NAMED_ARG, "",
{
{"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"},
},
"\"options\""
},

},
RPCResult{
RPCResult::Type::ARR, "", "The result of the mempool acceptance test for each raw transaction in the input array.\n"
Expand Down Expand Up @@ -150,19 +156,47 @@ static RPCHelpMan testmempoolaccept()
"Array must contain between 1 and " + ToString(MAX_PACKAGE_COUNT) + " transactions.");
}

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

switch (request.params[1].getType())
{
case UniValue::VNULL:
break;

case UniValue::VSTR:
case UniValue::VNUM: {
max_raw_tx_fee_rate = CFeeRate(AmountFromValue(request.params[1]));
break;
}

case UniValue::VOBJ: {

const UniValue& options = request.params[1];
RPCTypeCheckObj(options,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should have backward compatibility for now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 618a792.

{
{"maxfeerate", UniValueType()}, // will be checked by AmountFromValue() below
},
true, true);

if (options.exists("maxfeerate")) {
max_raw_tx_fee_rate = CFeeRate(AmountFromValue(options["maxfeerate"]));
}
break;
}

default:
throw JSONRPCError(RPC_INVALID_PARAMETER, "maxfeerate must be of type object or number.");
}

std::vector<CTransactionRef> txns;
txns.reserve(raw_transactions.size());
std::vector<CTransactionRef> package;
package.reserve(raw_transactions.size());
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)));
package.emplace_back(MakeTransactionRef(std::move(mtx)));
}

NodeContext& node = EnsureAnyNodeContext(request.context);
Expand All @@ -171,9 +205,10 @@ static RPCHelpMan testmempoolaccept()
CChainState& chainstate = chainman.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(),
chainman.ProcessTransaction(txns[0], /*test_accept=*/true));
const MemPoolBypass mempool_bypass{.m_test_accept=true, .m_bypass_limits=false};
if (package.size() > 1) return ProcessNewPackage(/*active_chainstate=*/chainstate, /*pool=*/mempool, package, mempool_bypass);
return PackageMempoolAcceptResult(package[0]->GetWitnessHash(),
chainman.ProcessTransaction(/*tx=*/package[0], mempool_bypass));
}();

UniValue rpc_result(UniValue::VARR);
Expand All @@ -182,7 +217,7 @@ static RPCHelpMan testmempoolaccept()
// 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) {
for (const auto& tx : package) {
UniValue result_inner(UniValue::VOBJ);
result_inner.pushKV("txid", tx->GetHash().GetHex());
result_inner.pushKV("wtxid", tx->GetWitnessHash().GetHex());
Expand Down Expand Up @@ -797,21 +832,21 @@ static RPCHelpMan submitpackage()
"Array must contain between 1 and " + ToString(MAX_PACKAGE_COUNT) + " transactions.");
}

std::vector<CTransactionRef> txns;
txns.reserve(raw_transactions.size());
std::vector<CTransactionRef> package;
package.reserve(raw_transactions.size());
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)));
package.emplace_back(MakeTransactionRef(std::move(mtx)));
}

NodeContext& node = EnsureAnyNodeContext(request.context);
CTxMemPool& mempool = EnsureMemPool(node);
CChainState& chainstate = EnsureChainman(node).ActiveChainstate();
const auto package_result = WITH_LOCK(::cs_main, return ProcessNewPackage(chainstate, mempool, txns, /*test_accept=*/ false));
const auto package_result = WITH_LOCK(::cs_main, return ProcessNewPackage(/*active_chainstate=*/chainstate, /*pool=*/mempool, package, /*mempool_bypass=*/std::nullopt));

// First catch any errors.
switch(package_result.m_state.GetResult()) {
Expand All @@ -828,7 +863,7 @@ static RPCHelpMan submitpackage()
}
case PackageValidationResult::PCKG_TX:
{
for (const auto& tx : txns) {
for (const auto& tx : package) {
auto it = package_result.m_tx_results.find(tx->GetWitnessHash());
if (it != package_result.m_tx_results.end() && it->second.m_state.IsInvalid()) {
throw JSONRPCTransactionError(TransactionError::MEMPOOL_REJECTED,
Expand All @@ -839,7 +874,7 @@ static RPCHelpMan submitpackage()
NONFATAL_UNREACHABLE();
}
}
for (const auto& tx : txns) {
for (const auto& tx : package) {
size_t num_submitted{0};
std::string err_string;
const auto err = BroadcastTransaction(node, tx, err_string, 0, true, true);
Expand All @@ -852,7 +887,7 @@ static RPCHelpMan submitpackage()
UniValue rpc_result{UniValue::VOBJ};
UniValue tx_result_map{UniValue::VOBJ};
std::set<uint256> replaced_txids;
for (const auto& tx : txns) {
for (const auto& tx : package) {
auto it = package_result.m_tx_results.find(tx->GetWitnessHash());
CHECK_NONFATAL(it != package_result.m_tx_results.end());
UniValue result_inner{UniValue::VOBJ};
Expand Down
13 changes: 11 additions & 2 deletions src/rpc/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -665,14 +665,23 @@ UniValue RPCHelpMan::GetArgMap() const
UniValue arr{UniValue::VARR};
for (int i{0}; i < int(m_args.size()); ++i) {
const auto& arg = m_args.at(i);

RPCArg::Type argtype = arg.m_type;
size_t arg_num = 0;

std::vector<std::string> arg_names = SplitString(arg.m_names, '|');
for (const auto& arg_name : arg_names) {

if (!arg.m_type_per_name.empty()) {
argtype = arg.m_type_per_name.at(arg_num++);
}

UniValue map{UniValue::VARR};
map.push_back(m_name);
map.push_back(i);
map.push_back(arg_name);
map.push_back(arg.m_type == RPCArg::Type::STR ||
arg.m_type == RPCArg::Type::STR_HEX);
map.push_back(UniValue(argtype == RPCArg::Type::STR ||
argtype == RPCArg::Type::STR_HEX));
arr.push_back(map);
}
}
Expand Down
23 changes: 23 additions & 0 deletions src/rpc/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <univalue.h>
#include <util/check.h>

#include <algorithm>
#include <string>
#include <variant>
#include <vector>
Expand Down Expand Up @@ -171,6 +172,7 @@ struct RPCArg {
using Fallback = std::variant<Optional, /* hint for default value */ DefaultHint, /* default constant value */ Default>;
const std::string m_names; //!< The name of the arg (can be empty for inner args, can contain multiple aliases separated by | for named request arguments)
const Type m_type;
const std::vector<Type> m_type_per_name;
const bool m_hidden;
const std::vector<RPCArg> m_inner; //!< Only used for arrays or dicts
const Fallback m_fallback;
Expand All @@ -197,6 +199,27 @@ struct RPCArg {
CHECK_NONFATAL(type != Type::ARR && type != Type::OBJ && type != Type::OBJ_USER_KEYS);
}

RPCArg(
const std::string name,
const std::vector<Type> types,
const Fallback fallback,
const std::string description,
const std::vector<RPCArg> inner,
const std::string oneline_description = "",
const std::vector<std::string> type_str = {},
const bool hidden = false)
: m_names{std::move(name)},
m_type{types.at(0)},
m_type_per_name{std::move(types)},
m_hidden{hidden},
m_fallback{std::move(fallback)},
m_description{std::move(description)},
m_oneline_description{std::move(oneline_description)},
m_type_str{std::move(type_str)}
{
CHECK_NONFATAL(m_type_per_name.size() == size_t(std::count(m_names.begin(), m_names.end(), '|')) + 1);
}

RPCArg(
const std::string name,
const Type type,
Expand Down
9 changes: 6 additions & 3 deletions src/test/fuzz/tx_pool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,9 @@ FUZZ_TARGET_INIT(tx_pool_standard, initialize_tx_pool)

// Make sure ProcessNewPackage on one transaction works.
// The result is not guaranteed to be the same as what is returned by ATMP.
const MemPoolBypass mempool_bypass1{.m_test_accept=true, .m_bypass_limits=bypass_limits};
const auto result_package = WITH_LOCK(::cs_main,
return ProcessNewPackage(chainstate, tx_pool, {tx}, true));
return ProcessNewPackage(/*active_chainstate=*/chainstate, /*pool=*/tx_pool, /*package=*/{tx}, mempool_bypass1));
// If something went wrong due to a package-specific policy, it might not return a
// validation result for the transaction.
if (result_package.m_state.GetResult() != PackageValidationResult::PCKG_POLICY) {
Expand All @@ -255,7 +256,8 @@ FUZZ_TARGET_INIT(tx_pool_standard, initialize_tx_pool)
it->second.m_result_type == MempoolAcceptResult::ResultType::INVALID);
}

const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, tx, GetTime(), bypass_limits, /*test_accept=*/false));
const MemPoolBypass mempool_bypass2{.m_test_accept=false, .m_bypass_limits=bypass_limits};
const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(/*active_chainstate=*/chainstate, tx, /*accept_time=*/GetTime(), mempool_bypass2));
const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID;
SyncWithValidationInterfaceQueue();
UnregisterSharedValidationInterface(txr);
Expand Down Expand Up @@ -355,7 +357,8 @@ FUZZ_TARGET_INIT(tx_pool, initialize_tx_pool)
const auto tx = MakeTransactionRef(mut_tx);
const bool bypass_limits = fuzzed_data_provider.ConsumeBool();
::fRequireStandard = fuzzed_data_provider.ConsumeBool();
const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, tx, GetTime(), bypass_limits, /*test_accept=*/false));
const MemPoolBypass mempool_bypass{.m_test_accept=false, .m_bypass_limits=bypass_limits};
const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(/*active_chainstate=*/chainstate, tx, /*accept_time=*/GetTime(), mempool_bypass));
const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID;
if (accepted) {
txids.push_back(tx->GetHash());
Expand Down
Loading