Skip to content

Commit

Permalink
RPC: Add maxfeerate and maxburnamount args to submitpackage
Browse files Browse the repository at this point in the history
And thread the feerate value through ProcessNewPackage to
reject individual transactions that exceed the given
feerate. This allows subpackage processing, and is
compatible with future package RBF work.
  • Loading branch information
instagibbs committed Feb 20, 2024
1 parent 45b2a91 commit 1dcf47a
Show file tree
Hide file tree
Showing 9 changed files with 124 additions and 30 deletions.
6 changes: 6 additions & 0 deletions src/node/transaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ struct NodeContext;
*/
static const CFeeRate DEFAULT_MAX_RAW_TX_FEE_RATE{COIN / 10};

/** Maximum burn value for sendrawtransaction, submitpackage, and testmempoolaccept RPC calls.
* By default, a transaction with a burn value higher than this will be rejected
* by these RPCs and the GUI. This can be overridden with the maxburnamount argument.
*/
static const CAmount DEFAULT_MAX_BURN_AMOUNT{0};

/**
* Submit a transaction to the mempool and (optionally) relay it to all P2P peers.
*
Expand Down
2 changes: 2 additions & 0 deletions src/rpc/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "testmempoolaccept", 0, "rawtxs" },
{ "testmempoolaccept", 1, "maxfeerate" },
{ "submitpackage", 0, "package" },
{ "submitpackage", 1, "maxfeerate" },
{ "submitpackage", 2, "maxburnamount" },
{ "combinerawtransaction", 0, "txs" },
{ "fundrawtransaction", 1, "options" },
{ "fundrawtransaction", 1, "add_inputs"},
Expand Down
33 changes: 30 additions & 3 deletions src/rpc/mempool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

using kernel::DumpMempool;

using node::DEFAULT_MAX_BURN_AMOUNT;
using node::DEFAULT_MAX_RAW_TX_FEE_RATE;
using node::MempoolPath;
using node::NodeContext;
Expand All @@ -46,7 +47,7 @@ static RPCHelpMan sendrawtransaction()
{"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.\nFee rates larger than 1BTC/kvB are rejected.\nSet to 0 to accept any fee rate."},
{"maxburnamount", RPCArg::Type::AMOUNT, RPCArg::Default{FormatMoney(0)},
{"maxburnamount", RPCArg::Type::AMOUNT, RPCArg::Default{FormatMoney(DEFAULT_MAX_BURN_AMOUNT)},
"Reject transactions with provably unspendable outputs (e.g. 'datacarrier' outputs that use the OP_RETURN opcode) greater than the specified value, expressed in " + CURRENCY_UNIT + ".\n"
"If burning funds through unspendable outputs is desired, increase this value.\n"
"This check is based on heuristics and does not guarantee spendability of outputs.\n"},
Expand Down Expand Up @@ -180,7 +181,7 @@ static RPCHelpMan testmempoolaccept()
Chainstate& chainstate = chainman.ActiveChainstate();
const PackageMempoolAcceptResult package_result = [&] {
LOCK(::cs_main);
if (txns.size() > 1) return ProcessNewPackage(chainstate, mempool, txns, /*test_accept=*/true);
if (txns.size() > 1) return ProcessNewPackage(chainstate, mempool, txns, /*test_accept=*/true, /*max_sane_feerate=*/{});
return PackageMempoolAcceptResult(txns[0]->GetWitnessHash(),
chainman.ProcessTransaction(txns[0], /*test_accept=*/true));
}();
Expand Down Expand Up @@ -823,6 +824,14 @@ static RPCHelpMan submitpackage()
{"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.\nFee rates larger than 1BTC/kvB are rejected.\nSet to 0 to accept any fee rate."},
{"maxburnamount", RPCArg::Type::AMOUNT, RPCArg::Default{FormatMoney(DEFAULT_MAX_BURN_AMOUNT)},
"Reject transactions with provably unspendable outputs (e.g. 'datacarrier' outputs that use the OP_RETURN opcode) greater than the specified value, expressed in " + CURRENCY_UNIT + ".\n"
"If burning funds through unspendable outputs is desired, increase this value.\n"
"This check is based on heuristics and does not guarantee spendability of outputs.\n"
},
},
RPCResult{
RPCResult::Type::OBJ, "", "",
Expand Down Expand Up @@ -862,6 +871,17 @@ static RPCHelpMan submitpackage()
"Array must contain between 1 and " + ToString(MAX_PACKAGE_COUNT) + " transactions.");
}

// Fee check needs to be run with chainstate and package context
const CFeeRate max_raw_tx_fee_rate = ParseFeeRate(self.Arg<UniValue>(1));
std::optional<CFeeRate> max_sane_feerate{max_raw_tx_fee_rate};
// 0-value is special; it's mapped to no sanity check
if (max_raw_tx_fee_rate == CFeeRate(0)) {
max_sane_feerate = std::nullopt;
}

// Burn sanity check is run with no context
const CAmount max_burn_amount = request.params[2].isNull() ? 0 : AmountFromValue(request.params[2]);

std::vector<CTransactionRef> txns;
txns.reserve(raw_transactions.size());
for (const auto& rawtx : raw_transactions.getValues()) {
Expand All @@ -870,6 +890,13 @@ static RPCHelpMan submitpackage()
throw JSONRPCError(RPC_DESERIALIZATION_ERROR,
"TX decode failed: " + rawtx.get_str() + " Make sure the tx has at least one input.");
}

for (const auto& out : mtx.vout) {
if((out.scriptPubKey.IsUnspendable() || !out.scriptPubKey.HasValidOps()) && out.nValue > max_burn_amount) {
throw JSONRPCTransactionError(TransactionError::MAX_BURN_EXCEEDED);
}
}

txns.emplace_back(MakeTransactionRef(std::move(mtx)));
}
if (!IsChildWithParentsTree(txns)) {
Expand All @@ -879,7 +906,7 @@ static RPCHelpMan submitpackage()
NodeContext& node = EnsureAnyNodeContext(request.context);
CTxMemPool& mempool = EnsureMemPool(node);
Chainstate& 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(chainstate, mempool, txns, /*test_accept=*/ false, max_sane_feerate));

std::string package_msg = "success";

Expand Down
2 changes: 1 addition & 1 deletion src/test/fuzz/package_eval.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool)
auto single_submit = txs.size() == 1 && fuzzed_data_provider.ConsumeBool();

const auto result_package = WITH_LOCK(::cs_main,
return ProcessNewPackage(chainstate, tx_pool, txs, /*test_accept=*/single_submit));
return ProcessNewPackage(chainstate, tx_pool, txs, /*test_accept=*/single_submit, /*max_sane_feerate=*/{}));

// Always set bypass_limits to false because it is not supported in ProcessNewPackage and
// can be a source of divergence.
Expand Down
2 changes: 1 addition & 1 deletion src/test/fuzz/tx_pool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ FUZZ_TARGET(tx_pool_standard, .init = 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 auto result_package = WITH_LOCK(::cs_main,
return ProcessNewPackage(chainstate, tx_pool, {tx}, true));
return ProcessNewPackage(chainstate, tx_pool, {tx}, true, /*max_sane_feerate=*/{}));
// 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 Down
Loading

0 comments on commit 1dcf47a

Please sign in to comment.