Skip to content

mempool: Add option to bypass contextual timelocks in testmempoolaccept #25434

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.

Sign up for GitHub

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

Closed
wants to merge 12 commits into from
Closed

mempool: Add option to bypass contextual timelocks in testmempoolaccept #25434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
199b314
mempool: change `maxfeerate` to be a members of an `options` argument
w0xlt Jun 20, 2022
563dd14
mempool: make `test_accept` parameter explicit
w0xlt Jul 1, 2022
9e8703c
validation: Add `MemPoolBypass` for bypassing mempool checks
w0xlt Jul 9, 2022
14eff47
mempool: add `MemPoolBypass` parameter to `ProcessNewPackage`
w0xlt Jul 9, 2022
420a3c4
mempool: add `MemPoolBypass` parameter to `AcceptToMemoryPool`
w0xlt Jul 9, 2022
17db819
mempool: add `MemPoolBypass` parameter to `ProcessTransaction`
w0xlt Jul 9, 2022
26ee1a4
mempool: add `MemPoolBypass` parameter to `ATMPArgs`
w0xlt Jul 9, 2022
12ac626
validation: add `m_bypass_{relative,absolute}_timelock`
w0xlt Jul 10, 2022
b70a926
validation: add option to bypass contextual timelock checks
w0xlt Jul 10, 2022
afc2998
rpc: add optional bypass_timelocks for testmempoolaccept
w0xlt Jul 10, 2022
7ec6416
test: check if `bypass_{absolute,relative}_timelock` works
w0xlt Jul 10, 2022
a006b8f
test: check if bypass_timelocks do not affect scripts
w0xlt Jul 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/bench/block_assemble.cpp
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions src/net_processing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2403,7 +2403,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 @@ -3432,7 +3432,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
View file
Open in desktop
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{/*test_accept=*/true, /*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
2 changes: 1 addition & 1 deletion src/rpc/client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "signrawtransactionwithwallet", 1, "prevtxs" },
{ "sendrawtransaction", 1, "maxfeerate" },
{ "testmempoolaccept", 0, "rawtxs" },
{ "testmempoolaccept", 1, "maxfeerate" },
{ "testmempoolaccept", 1, "options" },
{ "submitpackage", 0, "package" },
{ "combinerawtransaction", 0, "txs" },
{ "fundrawtransaction", 1, "options" },
Expand Down
64 changes: 45 additions & 19 deletions src/rpc/mempool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,16 @@ 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", RPCArg::Type::OBJ, 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"},
{"bypass_absolute_timelock", RPCArg::Type::BOOL, RPCArg::Default{false}, "Don't enforce nLocktime.\n"},
{"bypass_relative_timelock", RPCArg::Type::BOOL, RPCArg::Default{false}, "Don't consensus-enforce BIP68 relative lock-time.\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 @@ -143,19 +151,37 @@ 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;

MemPoolBypass mempool_bypass{/*test_accept=*/true, /*bypass_limits=*/false};

if (!request.params[1].isNull()) {
const UniValue& options = request.params[1];
RPCTypeCheckObj(options,
{
{"maxfeerate", UniValueType()}, // will be checked by AmountFromValue() below
{"bypass_absolute_timelock", UniValueType(UniValue::VBOOL)},
{"bypass_relative_timelock", UniValueType(UniValue::VBOOL)},
},
true, true);

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

Choose a reason for hiding this comment

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

if you like ternary I think you can do ternary here.

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 7ec6416

}

mempool_bypass.m_bypass_absolute_timelock = options.exists("bypass_absolute_timelock") ? options["bypass_absolute_timelock"].get_bool() : false;
mempool_bypass.m_bypass_relative_timelock = options.exists("bypass_relative_timelock") ? options["bypass_relative_timelock"].get_bool() : false;
}

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 @@ -164,9 +190,9 @@ 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));
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 @@ -175,7 +201,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 @@ -786,21 +812,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 @@ -817,7 +843,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 @@ -828,7 +854,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 @@ -841,7 +867,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
9 changes: 6 additions & 3 deletions src/test/fuzz/tx_pool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,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{/*test_accept=*/true, /*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 @@ -263,7 +264,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{/*test_accept=*/false, /*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 @@ -363,7 +365,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{/*test_accept=*/false, /*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