diff --git a/src/node/transaction.h b/src/node/transaction.h index 168273594ce50d..6782536aceba25 100644 --- a/src/node/transaction.h +++ b/src/node/transaction.h @@ -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. * diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 49820f25a35a55..eb91a22593061c 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -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"}, diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 04bd7fa928ec0b..114f04793a7598 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -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; @@ -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.\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"}, @@ -183,7 +184,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)); }(); @@ -828,6 +829,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(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, "", "", @@ -867,6 +876,19 @@ 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])); + std::optional 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 txns; txns.reserve(raw_transactions.size()); for (const auto& rawtx : raw_transactions.getValues()) { @@ -875,6 +897,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)) { @@ -884,7 +913,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"; diff --git a/src/test/fuzz/package_eval.cpp b/src/test/fuzz/package_eval.cpp index 064930c5aa6235..98cf811451fd3f 100644 --- a/src/test/fuzz/package_eval.cpp +++ b/src/test/fuzz/package_eval.cpp @@ -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=*/{})); 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; diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index ffa0c1216ef004..33805e54723142 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -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=*/{})); // 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) { diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp index 637f92bd0fcdf7..deeb3437671a11 100644 --- a/src/test/txpackage_tests.cpp +++ b/src/test/txpackage_tests.cpp @@ -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=*/{}); if (auto err_parent_child{CheckPackageMempoolAcceptResult(package_parent_child, result_parent_child, /*expect_valid=*/true, nullptr)}) { BOOST_ERROR(err_parent_child.value()); } else { @@ -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=*/{}); if (auto err_single_large{CheckPackageMempoolAcceptResult(package_single_giant, result_single_large, /*expect_valid=*/false, nullptr)}) { BOOST_ERROR(err_single_large.value()); } else { @@ -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=*/{}); // 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); @@ -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=*/{}); 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"); @@ -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=*/{}); 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 { @@ -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=*/{}); 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"); @@ -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=*/{}); expected_pool_size += 2; BOOST_CHECK_MESSAGE(submit_parent_child.m_state.IsValid(), "Package validation unexpectedly failed: " << submit_parent_child.m_state.GetRejectReason()); @@ -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=*/{}); if (auto err_deduped{CheckPackageMempoolAcceptResult(package_parent_child, submit_deduped, /*expect_valid=*/true, m_node.mempool.get())}) { BOOST_ERROR(err_deduped.value()); } else { @@ -464,7 +464,7 @@ 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=*/{}); if (auto err_witness1{CheckPackageMempoolAcceptResult(package_parent_child1, submit_witness1, /*expect_valid=*/true, m_node.mempool.get())}) { BOOST_ERROR(err_witness1.value()); } @@ -472,7 +472,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) // 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=*/{}); if (auto err_witness2{CheckPackageMempoolAcceptResult(package_parent_child2, submit_witness2, /*expect_valid=*/true, m_node.mempool.get())}) { BOOST_ERROR(err_witness2.value()); } else { @@ -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=*/{}); 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 { @@ -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=*/{}); 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 { @@ -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=*/{}); if (auto err_mixed{CheckPackageMempoolAcceptResult(package_mixed, mixed_result, /*expect_valid=*/true, m_node.mempool.get())}) { BOOST_ERROR(err_mixed.value()); } else { @@ -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=*/{}); 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 { @@ -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=*/{}); if (auto err_cpfp{CheckPackageMempoolAcceptResult(package_cpfp, submit_cpfp, /*expect_valid=*/true, m_node.mempool.get())}) { BOOST_ERROR(err_cpfp.value()); } else { @@ -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=*/{}); 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 { @@ -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=*/{}); 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 { @@ -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=*/{}); 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 { diff --git a/src/validation.cpp b/src/validation.cpp index 8a5bb93ef8463c..4ef1df9f94181d 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -464,6 +464,11 @@ class MemPoolAccept * policies such as mempool min fee and min relay fee. */ const bool m_package_feerates; + /** Used for local submission of transactions to catch "absurd" fees + * due to fee miscalulation by wallets. std:nullopt implies unset, allowing any feerates. + * Any individual transaction failing this check causes immediate failure. + */ + const std::optional m_client_maxfeerate; /** Parameters for single transaction mempool validation. */ static ATMPArgs SingleAccept(const CChainParams& chainparams, int64_t accept_time, @@ -477,6 +482,7 @@ class MemPoolAccept /* m_allow_replacement */ true, /* m_package_submission */ false, /* m_package_feerates */ false, + /* m_client_maxfeerate */ {}, // checked by caller }; } @@ -491,12 +497,13 @@ class MemPoolAccept /* m_allow_replacement */ false, /* m_package_submission */ false, // not submitting to mempool /* m_package_feerates */ false, + /* m_client_maxfeerate */ {}, // checked by caller }; } /** Parameters for child-with-unconfirmed-parents package validation. */ static ATMPArgs PackageChildWithParents(const CChainParams& chainparams, int64_t accept_time, - std::vector& coins_to_uncache) { + std::vector& coins_to_uncache, std::optional& client_maxfeerate) { return ATMPArgs{/* m_chainparams */ chainparams, /* m_accept_time */ accept_time, /* m_bypass_limits */ false, @@ -505,6 +512,7 @@ class MemPoolAccept /* m_allow_replacement */ false, /* m_package_submission */ true, /* m_package_feerates */ true, + /* m_client_maxfeerate */ client_maxfeerate, }; } @@ -518,6 +526,7 @@ class MemPoolAccept /* m_allow_replacement */ true, /* m_package_submission */ true, // do not LimitMempoolSize in Finalize() /* m_package_feerates */ false, // only 1 transaction + /* m_client_maxfeerate */ package_args.m_client_maxfeerate, }; } @@ -531,7 +540,8 @@ class MemPoolAccept bool test_accept, bool allow_replacement, bool package_submission, - bool package_feerates) + bool package_feerates, + std::optional client_maxfeerate) : m_chainparams{chainparams}, m_accept_time{accept_time}, m_bypass_limits{bypass_limits}, @@ -539,7 +549,8 @@ class MemPoolAccept m_test_accept{test_accept}, m_allow_replacement{allow_replacement}, m_package_submission{package_submission}, - m_package_feerates{package_feerates} + m_package_feerates{package_feerates}, + m_client_maxfeerate{client_maxfeerate} { } }; @@ -1243,6 +1254,12 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef return MempoolAcceptResult::Failure(ws.m_state); } + // Individual modified feerate exceeded caller-defined max; abort + if (args.m_client_maxfeerate && CFeeRate(ws.m_modified_fees, ws.m_vsize) > args.m_client_maxfeerate.value()) { + ws.m_state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "max feerate exceeded", ""); + return MempoolAcceptResult::Failure(ws.m_state); + } + if (m_rbf && !ReplacementChecks(ws)) return MempoolAcceptResult::Failure(ws.m_state); // Perform the inexpensive checks first and avoid hashing and signature verification unless @@ -1295,6 +1312,16 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Failure(ws.m_state)); return PackageMempoolAcceptResult(package_state, std::move(results)); } + + // Individual modified feerate exceeded caller-defined max; abort + // N.B. this doesn't take into account CPFPs. Chunk-aware validation may be more robust. + if (args.m_client_maxfeerate && CFeeRate(ws.m_modified_fees, ws.m_vsize) > args.m_client_maxfeerate.value()) { + package_state.Invalid(PackageValidationResult::PCKG_TX, "max feerate exceeded"); + // Exit early to avoid doing pointless work. Update the failed tx result; the rest are unfinished. + results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Failure(ws.m_state)); + return PackageMempoolAcceptResult(package_state, std::move(results)); + } + // Make the coins created by this transaction available for subsequent transactions in the // package to spend. Since we already checked conflicts in the package and we don't allow // replacements, we don't need to track the coins spent. Note that this logic will need to be @@ -1630,7 +1657,7 @@ MempoolAcceptResult AcceptToMemoryPool(Chainstate& active_chainstate, const CTra } PackageMempoolAcceptResult ProcessNewPackage(Chainstate& active_chainstate, CTxMemPool& pool, - const Package& package, bool test_accept) + const Package& package, bool test_accept, std::optional client_maxfeerate) { AssertLockHeld(cs_main); assert(!package.empty()); @@ -1644,7 +1671,7 @@ PackageMempoolAcceptResult ProcessNewPackage(Chainstate& active_chainstate, CTxM auto args = MemPoolAccept::ATMPArgs::PackageTestAccept(chainparams, GetTime(), coins_to_uncache); return MemPoolAccept(pool, active_chainstate).AcceptMultipleTransactions(package, args); } else { - auto args = MemPoolAccept::ATMPArgs::PackageChildWithParents(chainparams, GetTime(), coins_to_uncache); + auto args = MemPoolAccept::ATMPArgs::PackageChildWithParents(chainparams, GetTime(), coins_to_uncache, client_maxfeerate); return MemPoolAccept(pool, active_chainstate).AcceptPackage(package, args); } }(); diff --git a/src/validation.h b/src/validation.h index 093cecfcd104bf..b35a5749868040 100644 --- a/src/validation.h +++ b/src/validation.h @@ -278,13 +278,15 @@ MempoolAcceptResult AcceptToMemoryPool(Chainstate& active_chainstate, const CTra /** * Validate (and maybe submit) a package to the mempool. See doc/policy/packages.md for full details * on package validation rules. -* @param[in] test_accept When true, run validation checks but don't submit to mempool. +* @param[in] test_accept When true, run validation checks but don't submit to mempool. +* @param[in] max_sane_feerate If exceeded by an individual transaction, rest of (sub)package evalution is aborted. +* Only for sanity checks against local submission of transactions. * @returns a PackageMempoolAcceptResult which includes a MempoolAcceptResult for each transaction. * If a transaction fails, validation will exit early and some results may be missing. It is also * possible for the package to be partially submitted. */ PackageMempoolAcceptResult ProcessNewPackage(Chainstate& active_chainstate, CTxMemPool& pool, - const Package& txns, bool test_accept) + const Package& txns, bool test_accept, std::optional max_sane_feerate) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /* Mempool validation helper functions */ diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py index 664f2df3f19a0d..68d23d290ee1f0 100755 --- a/test/functional/rpc_packages.py +++ b/test/functional/rpc_packages.py @@ -81,6 +81,7 @@ def run_test(self): self.test_conflicting() self.test_rbf() self.test_submitpackage() + self.test_maxfeerate_maxburn_submitpackage() def test_independent(self, coin): self.log.info("Test multiple independent transactions in a package") @@ -356,5 +357,34 @@ def test_submitpackage(self): assert_equal(res["tx-results"][sec_wtxid]["error"], "version") peer.wait_for_broadcast([first_wtxid]) + def test_maxfeerate_maxburn_submitpackage(self): + node = self.nodes[0] + # clear mempool + deterministic_address = node.get_deterministic_priv_key().address + self.generatetoaddress(node, 1, deterministic_address) + + self.log.info("Submitpackage maxfeerate arg testing") + chained_txns = self.wallet.create_self_transfer_chain(chain_length=2) + minrate = min([chained_txn["fee"] / chained_txn["tx"].get_vsize() for chained_txn in chained_txns]) + chain_hex = [t["hex"] for t in chained_txns] + pkg_result = node.submitpackage(chain_hex, maxfeerate=minrate-Decimal("0.00000001")) + assert_equal(pkg_result["tx-results"][chained_txns[0]["wtxid"]]["error"], "max feerate exceeded") + assert_equal(pkg_result["tx-results"][chained_txns[1]["wtxid"]]["error"], "bad-txns-inputs-missingorspent") + assert_equal(node.getrawmempool(), []) + + self.log.info("Submitpackage maxburnamount arg testing") + tx = tx_from_hex(chain_hex[1]) + tx.vout[-1].scriptPubKey = b'a' * 10001 # scriptPubKey bigger than 10k IsUnspendable + chain_hex = [chain_hex[0], tx.serialize().hex()] + # burn test is run before any package evaluation; nothing makes it in and we get broader exception + assert_raises_rpc_error(-25, "Unspendable output exceeds maximum configured by user", node.submitpackage, chain_hex, 0, chained_txns[1]["new_utxo"]["value"] - Decimal("0.00000001")) + assert_equal(node.getrawmempool(), []) + + # Relax the restrictions for both and send it; parent gets through as own subpackage + pkg_result = node.submitpackage(chain_hex, maxfeerate=50, maxburnamount=chained_txns[1]["new_utxo"]["value"]) + assert "error" not in pkg_result["tx-results"][chained_txns[0]["wtxid"]] + assert_equal(pkg_result["tx-results"][tx.getwtxid()]["error"], "scriptpubkey") + assert_equal(node.getrawmempool(), [chained_txns[0]["txid"]]) + if __name__ == "__main__": RPCPackagesTest().main()