Skip to content

Commit

Permalink
RPC: Add maxfeerate and maxburnamount args to submitpackage
Browse files Browse the repository at this point in the history
  • Loading branch information
@instagibbs
instagibbs committed Nov 27, 2023
1 parent 5f9fd11 commit a573605
Show file tree
Hide file tree
Showing 8 changed files with 105 additions and 29 deletions.
2 changes: 2 additions & 0 deletions src/rpc/client.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,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
28 changes: 26 additions & 2 deletions src/rpc/mempool.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,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=*/CFeeRate(0));
return PackageMempoolAcceptResult(txns[0]->GetWitnessHash(),
chainman.ProcessTransaction(txns[0], /*test_accept=*/true));
}();
Expand Down Expand Up @@ -828,6 +828,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.\nSet to 0 to accept any fee rate."},
{"maxburnamount", RPCArg::Type::AMOUNT, RPCArg::Default{FormatMoney(0)},
"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 @@ -865,6 +873,14 @@ 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 = request.params[1].isNull() ?
DEFAULT_MAX_RAW_TX_FEE_RATE :
CFeeRate(AmountFromValue(request.params[1]));

// 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 @@ -873,16 +889,24 @@ 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)) {
throw JSONRPCTransactionError(TransactionError::INVALID_PACKAGE, "package topology disallowed. not child-with-parents or parents depend on each other.");
}


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_raw_tx_fee_rate));

// First catch any errors.
switch(package_result.m_state.GetResult()) {
Expand Down
2 changes: 1 addition & 1 deletion src/test/fuzz/package_eval.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,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=*/CFeeRate(0)));

const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, txs.back(), GetTime(), bypass_limits, /*test_accept=*/!single_submit));
const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID;
Expand Down
2 changes: 1 addition & 1 deletion src/test/fuzz/tx_pool.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,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=*/CFeeRate(0)));
// 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
36 changes: 18 additions & 18 deletions src/test/txpackage_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup)
/*output_amount=*/CAmount(48 * COIN), /*submit=*/false);
CTransactionRef tx_child = MakeTransactionRef(mtx_child);
Package package_parent_child{tx_parent, tx_child};
const auto result_parent_child = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_parent_child, /*test_accept=*/true);
const auto result_parent_child = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_parent_child, /*test_accept=*/true, /*max_sane_feerate=*/CFeeRate(0));
if (auto err_parent_child{CheckPackageMempoolAcceptResult(package_parent_child, result_parent_child, /*expect_valid=*/true, nullptr)}) {
BOOST_ERROR(err_parent_child.value());
} else {
Expand All @@ -153,7 +153,7 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup)
CTransactionRef giant_ptx = create_placeholder_tx(999, 999);
BOOST_CHECK(GetVirtualTransactionSize(*giant_ptx) > DEFAULT_ANCESTOR_SIZE_LIMIT_KVB * 1000);
Package package_single_giant{giant_ptx};
auto result_single_large = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_single_giant, /*test_accept=*/true);
auto result_single_large = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_single_giant, /*test_accept=*/true, /*max_sane_feerate=*/CFeeRate(0));
if (auto err_single_large{CheckPackageMempoolAcceptResult(package_single_giant, result_single_large, /*expect_valid=*/false, nullptr)}) {
BOOST_ERROR(err_single_large.value());
} else {
Expand Down Expand Up @@ -280,7 +280,7 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup)
package_unrelated.emplace_back(MakeTransactionRef(mtx));
}
auto result_unrelated_submit = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
package_unrelated, /*test_accept=*/false);
package_unrelated, /*test_accept=*/false, /*max_sane_feerate=*/CFeeRate(0));
// We don't expect m_tx_results for each transaction when basic sanity checks haven't passed.
BOOST_CHECK(result_unrelated_submit.m_state.IsInvalid());
BOOST_CHECK_EQUAL(result_unrelated_submit.m_state.GetResult(), PackageValidationResult::PCKG_POLICY);
Expand Down Expand Up @@ -322,7 +322,7 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup)
// 3 Generations is not allowed.
{
auto result_3gen_submit = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
package_3gen, /*test_accept=*/false);
package_3gen, /*test_accept=*/false, /*max_sane_feerate=*/CFeeRate(0));
BOOST_CHECK(result_3gen_submit.m_state.IsInvalid());
BOOST_CHECK_EQUAL(result_3gen_submit.m_state.GetResult(), PackageValidationResult::PCKG_POLICY);
BOOST_CHECK_EQUAL(result_3gen_submit.m_state.GetRejectReason(), "package-not-child-with-parents");
Expand All @@ -339,7 +339,7 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup)
CTransactionRef tx_parent_invalid = MakeTransactionRef(mtx_parent_invalid);
Package package_invalid_parent{tx_parent_invalid, tx_child};
auto result_quit_early = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
package_invalid_parent, /*test_accept=*/ false);
package_invalid_parent, /*test_accept=*/ false, /*max_sane_feerate=*/CFeeRate(0));
if (auto err_parent_invalid{CheckPackageMempoolAcceptResult(package_invalid_parent, result_quit_early, /*expect_valid=*/false, m_node.mempool.get())}) {
BOOST_ERROR(err_parent_invalid.value());
} else {
Expand All @@ -360,7 +360,7 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup)
package_missing_parent.push_back(MakeTransactionRef(mtx_child));
{
const auto result_missing_parent = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
package_missing_parent, /*test_accept=*/false);
package_missing_parent, /*test_accept=*/false, /*max_sane_feerate=*/CFeeRate(0));
BOOST_CHECK(result_missing_parent.m_state.IsInvalid());
BOOST_CHECK_EQUAL(result_missing_parent.m_state.GetResult(), PackageValidationResult::PCKG_POLICY);
BOOST_CHECK_EQUAL(result_missing_parent.m_state.GetRejectReason(), "package-not-child-with-unconfirmed-parents");
Expand All @@ -370,7 +370,7 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup)
// Submit package with parent + child.
{
const auto submit_parent_child = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
package_parent_child, /*test_accept=*/false);
package_parent_child, /*test_accept=*/false, /*max_sane_feerate=*/CFeeRate(0));
expected_pool_size += 2;
BOOST_CHECK_MESSAGE(submit_parent_child.m_state.IsValid(),
"Package validation unexpectedly failed: " << submit_parent_child.m_state.GetRejectReason());
Expand All @@ -392,7 +392,7 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup)
// Already-in-mempool transactions should be detected and de-duplicated.
{
const auto submit_deduped = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
package_parent_child, /*test_accept=*/false);
package_parent_child, /*test_accept=*/false, /*max_sane_feerate=*/CFeeRate(0));
if (auto err_deduped{CheckPackageMempoolAcceptResult(package_parent_child, submit_deduped, /*expect_valid=*/true, m_node.mempool.get())}) {
BOOST_ERROR(err_deduped.value());
} else {
Expand Down Expand Up @@ -464,15 +464,15 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup)
{
Package package_parent_child1{ptx_parent, ptx_child1};
const auto submit_witness1 = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
package_parent_child1, /*test_accept=*/false);
package_parent_child1, /*test_accept=*/false, /*max_sane_feerate=*/CFeeRate(0));
if (auto err_witness1{CheckPackageMempoolAcceptResult(package_parent_child1, submit_witness1, /*expect_valid=*/true, m_node.mempool.get())}) {
BOOST_ERROR(err_witness1.value());
}

// Child2 would have been validated individually.
Package package_parent_child2{ptx_parent, ptx_child2};
const auto submit_witness2 = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
package_parent_child2, /*test_accept=*/false);
package_parent_child2, /*test_accept=*/false, /*max_sane_feerate=*/CFeeRate(0));
if (auto err_witness2{CheckPackageMempoolAcceptResult(package_parent_child2, submit_witness2, /*expect_valid=*/true, m_node.mempool.get())}) {
BOOST_ERROR(err_witness2.value());
} else {
Expand All @@ -486,7 +486,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup)
// Deduplication should work when wtxid != txid. Submit package with the already-in-mempool
// transactions again, which should not fail.
const auto submit_segwit_dedup = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
package_parent_child1, /*test_accept=*/false);
package_parent_child1, /*test_accept=*/false, /*max_sane_feerate=*/CFeeRate(0));
if (auto err_segwit_dedup{CheckPackageMempoolAcceptResult(package_parent_child1, submit_segwit_dedup, /*expect_valid=*/true, m_node.mempool.get())}) {
BOOST_ERROR(err_segwit_dedup.value());
} else {
Expand Down Expand Up @@ -517,7 +517,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup)
{
Package package_child2_grandchild{ptx_child2, ptx_grandchild};
const auto submit_spend_ignored = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
package_child2_grandchild, /*test_accept=*/false);
package_child2_grandchild, /*test_accept=*/false, /*max_sane_feerate=*/CFeeRate(0));
if (auto err_spend_ignored{CheckPackageMempoolAcceptResult(package_child2_grandchild, submit_spend_ignored, /*expect_valid=*/true, m_node.mempool.get())}) {
BOOST_ERROR(err_spend_ignored.value());
} else {
Expand Down Expand Up @@ -616,7 +616,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup)
// parent3 should be accepted
// child should be accepted
{
const auto mixed_result = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_mixed, false);
const auto mixed_result = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_mixed, false, /*max_sane_feerate=*/CFeeRate(0));
if (auto err_mixed{CheckPackageMempoolAcceptResult(package_mixed, mixed_result, /*expect_valid=*/true, m_node.mempool.get())}) {
BOOST_ERROR(err_mixed.value());
} else {
Expand Down Expand Up @@ -682,7 +682,7 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup)
{
BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
const auto submit_cpfp_deprio = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
package_cpfp, /*test_accept=*/ false);
package_cpfp, /*test_accept=*/ false, /*max_sane_feerate=*/CFeeRate(0));
if (auto err_cpfp_deprio{CheckPackageMempoolAcceptResult(package_cpfp, submit_cpfp_deprio, /*expect_valid=*/false, m_node.mempool.get())}) {
BOOST_ERROR(err_cpfp_deprio.value());
} else {
Expand All @@ -704,7 +704,7 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup)
{
BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
const auto submit_cpfp = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
package_cpfp, /*test_accept=*/ false);
package_cpfp, /*test_accept=*/ false, /*max_sane_feerate=*/CFeeRate(0));
if (auto err_cpfp{CheckPackageMempoolAcceptResult(package_cpfp, submit_cpfp, /*expect_valid=*/true, m_node.mempool.get())}) {
BOOST_ERROR(err_cpfp.value());
} else {
Expand Down Expand Up @@ -756,7 +756,7 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup)
// Cheap package should fail for being too low fee.
{
const auto submit_package_too_low = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
package_still_too_low, /*test_accept=*/false);
package_still_too_low, /*test_accept=*/false, /*max_sane_feerate=*/CFeeRate(0));
if (auto err_package_too_low{CheckPackageMempoolAcceptResult(package_still_too_low, submit_package_too_low, /*expect_valid=*/false, m_node.mempool.get())}) {
BOOST_ERROR(err_package_too_low.value());
} else {
Expand All @@ -782,7 +782,7 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup)
// Now that the child's fees have "increased" by 1 BTC, the cheap package should succeed.
{
const auto submit_prioritised_package = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
package_still_too_low, /*test_accept=*/false);
package_still_too_low, /*test_accept=*/false, /*max_sane_feerate=*/CFeeRate(0));
if (auto err_prioritised{CheckPackageMempoolAcceptResult(package_still_too_low, submit_prioritised_package, /*expect_valid=*/true, m_node.mempool.get())}) {
BOOST_ERROR(err_prioritised.value());
} else {
Expand Down Expand Up @@ -830,7 +830,7 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup)
{
BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
const auto submit_rich_parent = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
package_rich_parent, /*test_accept=*/false);
package_rich_parent, /*test_accept=*/false, /*max_sane_feerate=*/CFeeRate(0));
if (auto err_rich_parent{CheckPackageMempoolAcceptResult(package_rich_parent, submit_rich_parent, /*expect_valid=*/false, m_node.mempool.get())}) {
BOOST_ERROR(err_rich_parent.value());
} else {
Expand Down
Loading

0 comments on commit a573605

Please sign in to comment.