From 5c786a026aee434363ad54f4346211d0e2c5a38d Mon Sep 17 00:00:00 2001 From: glozow Date: Mon, 6 Nov 2023 13:32:45 +0000 Subject: [PATCH 1/4] [refactor] use Wtxid for m_wtxids_fee_calculations --- src/test/txpackage_tests.cpp | 6 +++--- src/validation.cpp | 10 +++++----- src/validation.h | 6 +++--- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp index debefa7f93c..9824a7a46f5 100644 --- a/src/test/txpackage_tests.cpp +++ b/src/test/txpackage_tests.cpp @@ -668,7 +668,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) const CFeeRate expected_feerate(1 * COIN, GetVirtualTransactionSize(*ptx_parent3) + GetVirtualTransactionSize(*ptx_mixed_child)); BOOST_CHECK(it_parent3->second.m_effective_feerate.value() == expected_feerate); BOOST_CHECK(it_child->second.m_effective_feerate.value() == expected_feerate); - std::vector expected_wtxids({ptx_parent3->GetWitnessHash(), ptx_mixed_child->GetWitnessHash()}); + std::vector expected_wtxids({ptx_parent3->GetWitnessHash(), ptx_mixed_child->GetWitnessHash()}); BOOST_CHECK(it_parent3->second.m_wtxids_fee_calculations.value() == expected_wtxids); BOOST_CHECK(it_child->second.m_wtxids_fee_calculations.value() == expected_wtxids); } @@ -756,7 +756,7 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) GetVirtualTransactionSize(*tx_parent) + GetVirtualTransactionSize(*tx_child)); BOOST_CHECK(it_parent->second.m_effective_feerate.value() == expected_feerate); BOOST_CHECK(it_child->second.m_effective_feerate.value() == expected_feerate); - std::vector expected_wtxids({tx_parent->GetWitnessHash(), tx_child->GetWitnessHash()}); + std::vector expected_wtxids({tx_parent->GetWitnessHash(), tx_child->GetWitnessHash()}); BOOST_CHECK(it_parent->second.m_wtxids_fee_calculations.value() == expected_wtxids); BOOST_CHECK(it_child->second.m_wtxids_fee_calculations.value() == expected_wtxids); BOOST_CHECK(expected_feerate.GetFeePerK() > 1000); @@ -820,7 +820,7 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) BOOST_CHECK(it_child->second.m_result_type == MempoolAcceptResult::ResultType::VALID); BOOST_CHECK(it_child->second.m_base_fees.value() == child_fee); BOOST_CHECK(it_child->second.m_effective_feerate.value() == expected_feerate); - std::vector expected_wtxids({tx_parent_cheap->GetWitnessHash(), tx_child_cheap->GetWitnessHash()}); + std::vector expected_wtxids({tx_parent_cheap->GetWitnessHash(), tx_child_cheap->GetWitnessHash()}); BOOST_CHECK(it_parent->second.m_wtxids_fee_calculations.value() == expected_wtxids); BOOST_CHECK(it_child->second.m_wtxids_fee_calculations.value() == expected_wtxids); } diff --git a/src/validation.cpp b/src/validation.cpp index 8d7e3661254..b0dba9a6c7c 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1201,7 +1201,7 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector& } } - std::vector all_package_wtxids; + std::vector all_package_wtxids; all_package_wtxids.reserve(workspaces.size()); std::transform(workspaces.cbegin(), workspaces.cend(), std::back_inserter(all_package_wtxids), [](const auto& ws) { return ws.m_ptx->GetWitnessHash(); }); @@ -1211,7 +1211,7 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector& const auto effective_feerate = args.m_package_feerates ? ws.m_package_feerate : CFeeRate{ws.m_modified_fees, static_cast(ws.m_vsize)}; const auto effective_feerate_wtxids = args.m_package_feerates ? all_package_wtxids : - std::vector({ws.m_ptx->GetWitnessHash()}); + std::vector{ws.m_ptx->GetWitnessHash()}; results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, ws.m_base_fees, effective_feerate, effective_feerate_wtxids)); @@ -1226,6 +1226,7 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef LOCK(m_pool.cs); // mempool "read lock" (held through GetMainSignals().TransactionAddedToMempool()) Workspace ws(ptx); + const std::vector single_wtxid{ws.m_ptx->GetWitnessHash()}; if (!PreChecks(args, ws)) return MempoolAcceptResult::Failure(ws.m_state); @@ -1238,7 +1239,6 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef if (!ConsensusScriptChecks(args, ws)) return MempoolAcceptResult::Failure(ws.m_state); const CFeeRate effective_feerate{ws.m_modified_fees, static_cast(ws.m_vsize)}; - const std::vector single_wtxid{ws.m_ptx->GetWitnessHash()}; // Tx was accepted, but not added if (args.m_test_accept) { return MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, @@ -1314,7 +1314,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: return PackageMempoolAcceptResult(package_state, std::move(results)); } - std::vector all_package_wtxids; + std::vector all_package_wtxids; all_package_wtxids.reserve(workspaces.size()); std::transform(workspaces.cbegin(), workspaces.cend(), std::back_inserter(all_package_wtxids), [](const auto& ws) { return ws.m_ptx->GetWitnessHash(); }); @@ -1330,7 +1330,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: const auto effective_feerate = args.m_package_feerates ? ws.m_package_feerate : CFeeRate{ws.m_modified_fees, static_cast(ws.m_vsize)}; const auto effective_feerate_wtxids = args.m_package_feerates ? all_package_wtxids : - std::vector{ws.m_ptx->GetWitnessHash()}; + std::vector{ws.m_ptx->GetWitnessHash()}; results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, ws.m_base_fees, effective_feerate, diff --git a/src/validation.h b/src/validation.h index 7ce60da6340..75deba64934 100644 --- a/src/validation.h +++ b/src/validation.h @@ -149,7 +149,7 @@ struct MempoolAcceptResult { * package. This is not necessarily equivalent to the list of transactions passed to * ProcessNewPackage(). * Only present when m_result_type = ResultType::VALID. */ - const std::optional> m_wtxids_fee_calculations; + const std::optional> m_wtxids_fee_calculations; // The following field is only present when m_result_type = ResultType::DIFFERENT_WITNESS /** The wtxid of the transaction in the mempool which has the same txid but different witness. */ @@ -163,7 +163,7 @@ struct MempoolAcceptResult { int64_t vsize, CAmount fees, CFeeRate effective_feerate, - const std::vector& wtxids_fee_calculations) { + const std::vector& wtxids_fee_calculations) { return MempoolAcceptResult(std::move(replaced_txns), vsize, fees, effective_feerate, wtxids_fee_calculations); } @@ -189,7 +189,7 @@ private: int64_t vsize, CAmount fees, CFeeRate effective_feerate, - const std::vector& wtxids_fee_calculations) + const std::vector& wtxids_fee_calculations) : m_result_type(ResultType::VALID), m_replaced_transactions(std::move(replaced_txns)), m_vsize{vsize}, From 3979f1afcbef5fdd3fad56312573a6733a7d78a4 Mon Sep 17 00:00:00 2001 From: glozow Date: Mon, 12 Sep 2022 10:43:33 +0100 Subject: [PATCH 2/4] [validation] add TxValidationResult::TX_RECONSIDERABLE, TX_UNKNOWN With package validation rules, transactions that fail individually may sometimes be eligible for reconsideration if submitted as part of a (different) package. For now, that includes trasactions that failed for being too low feerate. Add a new TxValidationResult type to distinguish these failures from others. In the next commits, we will abort package validation if a tx fails for any other reason. In the future, we will also decide whether to cache failures in recent_rejects based on this result (we won't want to reject a package containing a transaction that was rejected previously for being low feerate). Package validation also sometimes elects to skip some transactions when it knows the package will not be submitted in order to quit sooner. Add a result to specify this situation; we also don't want to cache these as rejections. --- src/consensus/validation.h | 2 ++ src/net_processing.cpp | 2 ++ src/validation.cpp | 17 +++++++++++++---- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/consensus/validation.h b/src/consensus/validation.h index d5bf08cd61a..bd3a5913c36 100644 --- a/src/consensus/validation.h +++ b/src/consensus/validation.h @@ -54,6 +54,8 @@ enum class TxValidationResult { TX_CONFLICT, TX_MEMPOOL_POLICY, //!< violated mempool's fee/size/descendant/RBF/etc limits TX_NO_MEMPOOL, //!< this node does not have a mempool so can't validate the transaction + TX_RECONSIDERABLE, //!< fails some policy, but might be acceptable if submitted in a (different) package + TX_UNKNOWN, //!< transaction was not validated because package failed }; /** A "reason" why a block was invalid, suitable for determining whether the diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 2f41eb2b1d7..c9596a36b8f 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1820,6 +1820,8 @@ bool PeerManagerImpl::MaybePunishNodeForTx(NodeId nodeid, const TxValidationStat case TxValidationResult::TX_CONFLICT: case TxValidationResult::TX_MEMPOOL_POLICY: case TxValidationResult::TX_NO_MEMPOOL: + case TxValidationResult::TX_RECONSIDERABLE: + case TxValidationResult::TX_UNKNOWN: break; } return false; diff --git a/src/validation.cpp b/src/validation.cpp index b0dba9a6c7c..c478252f182 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -670,11 +670,11 @@ private: AssertLockHeld(m_pool.cs); CAmount mempoolRejectFee = m_pool.GetMinFee().GetFee(package_size); if (mempoolRejectFee > 0 && package_fee < mempoolRejectFee) { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "mempool min fee not met", strprintf("%d < %d", package_fee, mempoolRejectFee)); + return state.Invalid(TxValidationResult::TX_RECONSIDERABLE, "mempool min fee not met", strprintf("%d < %d", package_fee, mempoolRejectFee)); } if (package_fee < m_pool.m_min_relay_feerate.GetFee(package_size)) { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "min relay fee not met", + return state.Invalid(TxValidationResult::TX_RECONSIDERABLE, "min relay fee not met", strprintf("%d < %d", package_fee, m_pool.m_min_relay_feerate.GetFee(package_size))); } return true; @@ -867,6 +867,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // method of ensuring the tx remains bumped. For example, the fee-bumping child could disappear // due to a replacement. if (!bypass_limits && ws.m_modified_fees < m_pool.m_min_relay_feerate.GetFee(ws.m_vsize)) { + // Even though this is a fee-related failure, this result is TX_MEMPOOL_POLICY, not + // TX_RECONSIDERABLE, because it cannot be bypassed using package validation. return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "min relay fee not met", strprintf("%d < %d", ws.m_modified_fees, m_pool.m_min_relay_feerate.GetFee(ws.m_vsize))); } @@ -981,6 +983,9 @@ bool MemPoolAccept::ReplacementChecks(Workspace& ws) // descendant transaction of a direct conflict to pay a higher feerate than the transaction that // might replace them, under these rules. if (const auto err_string{PaysMoreThanConflicts(ws.m_iters_conflicting, newFeeRate, hash)}) { + // Even though this is a fee-related failure, this result is TX_MEMPOOL_POLICY, not + // TX_RECONSIDERABLE, because it cannot be bypassed using package validation. + // This must be changed if package RBF is enabled. return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "insufficient fee", *err_string); } @@ -1002,6 +1007,9 @@ bool MemPoolAccept::ReplacementChecks(Workspace& ws) } if (const auto err_string{PaysForRBF(ws.m_conflicting_fees, ws.m_modified_fees, ws.m_vsize, m_pool.m_incremental_relay_feerate, hash)}) { + // Even though this is a fee-related failure, this result is TX_MEMPOOL_POLICY, not + // TX_RECONSIDERABLE, because it cannot be bypassed using package validation. + // This must be changed if package RBF is enabled. return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "insufficient fee", *err_string); } return true; @@ -1139,7 +1147,8 @@ bool MemPoolAccept::Finalize(const ATMPArgs& args, Workspace& ws) if (!args.m_package_submission && !bypass_limits) { LimitMempoolSize(m_pool, m_active_chainstate.CoinsTip()); if (!m_pool.exists(GenTxid::Txid(hash))) - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "mempool full"); + // The tx no longer meets our (new) mempool minimum feerate but could be reconsidered in a package. + return state.Invalid(TxValidationResult::TX_RECONSIDERABLE, "mempool full"); } return true; } @@ -1510,7 +1519,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, // in package validation, because its fees should only be "used" once. assert(m_pool.exists(GenTxid::Wtxid(wtxid))); results_final.emplace(wtxid, single_res); - } else if (single_res.m_state.GetResult() != TxValidationResult::TX_MEMPOOL_POLICY && + } else if (single_res.m_state.GetResult() != TxValidationResult::TX_RECONSIDERABLE && single_res.m_state.GetResult() != TxValidationResult::TX_MISSING_INPUTS) { // Package validation policy only differs from individual policy in its evaluation // of feerate. For example, if a transaction fails here due to violation of a From 10dd9f2441f4618321bfa2865449ac2223c572a0 Mon Sep 17 00:00:00 2001 From: glozow Date: Tue, 8 Aug 2023 18:26:00 +0100 Subject: [PATCH 3/4] [test] use CheckPackageMempoolAcceptResult in previous tests Increases test coverage (check every result field) and makes it easier to test the changes in the next commit. --- src/test/txpackage_tests.cpp | 383 ++++++++++++++++------------------- 1 file changed, 174 insertions(+), 209 deletions(-) diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp index 9824a7a46f5..30e16ee365f 100644 --- a/src/test/txpackage_tests.cpp +++ b/src/test/txpackage_tests.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -132,36 +133,35 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup) /*output_destination=*/child_locking_script, /*output_amount=*/CAmount(48 * COIN), /*submit=*/false); CTransactionRef tx_child = MakeTransactionRef(mtx_child); - const auto result_parent_child = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, {tx_parent, tx_child}, /*test_accept=*/true); - BOOST_CHECK_MESSAGE(result_parent_child.m_state.IsValid(), - "Package validation unexpectedly failed: " << result_parent_child.m_state.GetRejectReason()); - BOOST_CHECK(result_parent_child.m_tx_results.size() == 2); - auto it_parent = result_parent_child.m_tx_results.find(tx_parent->GetWitnessHash()); - auto it_child = result_parent_child.m_tx_results.find(tx_child->GetWitnessHash()); - BOOST_CHECK(it_parent != result_parent_child.m_tx_results.end()); - BOOST_CHECK_MESSAGE(it_parent->second.m_state.IsValid(), - "Package validation unexpectedly failed: " << it_parent->second.m_state.GetRejectReason()); - BOOST_CHECK(it_parent->second.m_effective_feerate.value().GetFee(GetVirtualTransactionSize(*tx_parent)) == COIN); - BOOST_CHECK_EQUAL(it_parent->second.m_wtxids_fee_calculations.value().size(), 1); - BOOST_CHECK_EQUAL(it_parent->second.m_wtxids_fee_calculations.value().front(), tx_parent->GetWitnessHash()); - BOOST_CHECK(it_child != result_parent_child.m_tx_results.end()); - BOOST_CHECK_MESSAGE(it_child->second.m_state.IsValid(), - "Package validation unexpectedly failed: " << it_child->second.m_state.GetRejectReason()); - BOOST_CHECK(it_child->second.m_effective_feerate.value().GetFee(GetVirtualTransactionSize(*tx_child)) == COIN); - BOOST_CHECK_EQUAL(it_child->second.m_wtxids_fee_calculations.value().size(), 1); - BOOST_CHECK_EQUAL(it_child->second.m_wtxids_fee_calculations.value().front(), tx_child->GetWitnessHash()); + 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); + if (auto err_parent_child{CheckPackageMempoolAcceptResult(package_parent_child, result_parent_child, /*expect_valid=*/true, nullptr)}) { + BOOST_ERROR(err_parent_child.value()); + } else { + auto it_parent = result_parent_child.m_tx_results.find(tx_parent->GetWitnessHash()); + auto it_child = result_parent_child.m_tx_results.find(tx_child->GetWitnessHash()); + BOOST_CHECK(it_parent->second.m_effective_feerate.value().GetFee(GetVirtualTransactionSize(*tx_parent)) == COIN); + BOOST_CHECK_EQUAL(it_parent->second.m_wtxids_fee_calculations.value().size(), 1); + BOOST_CHECK_EQUAL(it_parent->second.m_wtxids_fee_calculations.value().front(), tx_parent->GetWitnessHash()); + + BOOST_CHECK(it_child->second.m_effective_feerate.value().GetFee(GetVirtualTransactionSize(*tx_child)) == COIN); + BOOST_CHECK_EQUAL(it_child->second.m_wtxids_fee_calculations.value().size(), 1); + BOOST_CHECK_EQUAL(it_child->second.m_wtxids_fee_calculations.value().front(), tx_child->GetWitnessHash()); + } // A single, giant transaction submitted through ProcessNewPackage fails on single tx policy. CTransactionRef giant_ptx = create_placeholder_tx(999, 999); BOOST_CHECK(GetVirtualTransactionSize(*giant_ptx) > DEFAULT_ANCESTOR_SIZE_LIMIT_KVB * 1000); - auto result_single_large = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, {giant_ptx}, /*test_accept=*/true); - BOOST_CHECK(result_single_large.m_state.IsInvalid()); - BOOST_CHECK_EQUAL(result_single_large.m_state.GetResult(), PackageValidationResult::PCKG_TX); - BOOST_CHECK_EQUAL(result_single_large.m_state.GetRejectReason(), "transaction failed"); - BOOST_CHECK(result_single_large.m_tx_results.size() == 1); - auto it_giant_tx = result_single_large.m_tx_results.find(giant_ptx->GetWitnessHash()); - BOOST_CHECK(it_giant_tx != result_single_large.m_tx_results.end()); - BOOST_CHECK_EQUAL(it_giant_tx->second.m_state.GetRejectReason(), "tx-size"); + Package package_single_giant{giant_ptx}; + auto result_single_large = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_single_giant, /*test_accept=*/true); + if (auto err_single_large{CheckPackageMempoolAcceptResult(package_single_giant, result_single_large, /*expect_valid=*/false, nullptr)}) { + BOOST_ERROR(err_single_large.value()); + } else { + BOOST_CHECK_EQUAL(result_single_large.m_state.GetResult(), PackageValidationResult::PCKG_TX); + BOOST_CHECK_EQUAL(result_single_large.m_state.GetRejectReason(), "transaction failed"); + auto it_giant_tx = result_single_large.m_tx_results.find(giant_ptx->GetWitnessHash()); + BOOST_CHECK_EQUAL(it_giant_tx->second.m_state.GetRejectReason(), "tx-size"); + } // Check that mempool size hasn't changed. BOOST_CHECK_EQUAL(m_node.mempool->size(), initialPoolSize); @@ -281,6 +281,7 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) } auto result_unrelated_submit = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_unrelated, /*test_accept=*/false); + // 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); BOOST_CHECK_EQUAL(result_unrelated_submit.m_state.GetRejectReason(), "package-not-child-with-parents"); @@ -336,20 +337,20 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) CMutableTransaction mtx_parent_invalid{mtx_parent}; mtx_parent_invalid.vin[0].scriptWitness = bad_witness; 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, - {tx_parent_invalid, tx_child}, /*test_accept=*/ false); - BOOST_CHECK(result_quit_early.m_state.IsInvalid()); + package_invalid_parent, /*test_accept=*/ false); + 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 { + auto it_parent = result_quit_early.m_tx_results.find(tx_parent_invalid->GetWitnessHash()); + auto it_child = result_quit_early.m_tx_results.find(tx_child->GetWitnessHash()); + BOOST_CHECK_EQUAL(it_parent->second.m_state.GetResult(), TxValidationResult::TX_WITNESS_MUTATED); + BOOST_CHECK_EQUAL(it_parent->second.m_state.GetRejectReason(), "bad-witness-nonstandard"); + BOOST_CHECK_EQUAL(it_child->second.m_state.GetResult(), TxValidationResult::TX_MISSING_INPUTS); + BOOST_CHECK_EQUAL(it_child->second.m_state.GetRejectReason(), "bad-txns-inputs-missingorspent"); + } BOOST_CHECK_EQUAL(result_quit_early.m_state.GetResult(), PackageValidationResult::PCKG_TX); - BOOST_CHECK(!result_quit_early.m_tx_results.empty()); - BOOST_CHECK_EQUAL(result_quit_early.m_tx_results.size(), 2); - auto it_parent = result_quit_early.m_tx_results.find(tx_parent_invalid->GetWitnessHash()); - auto it_child = result_quit_early.m_tx_results.find(tx_child->GetWitnessHash()); - BOOST_CHECK(it_parent != result_quit_early.m_tx_results.end()); - BOOST_CHECK(it_child != result_quit_early.m_tx_results.end()); - BOOST_CHECK_EQUAL(it_parent->second.m_state.GetResult(), TxValidationResult::TX_WITNESS_MUTATED); - BOOST_CHECK_EQUAL(it_parent->second.m_state.GetRejectReason(), "bad-witness-nonstandard"); - BOOST_CHECK_EQUAL(it_child->second.m_state.GetResult(), TxValidationResult::TX_MISSING_INPUTS); - BOOST_CHECK_EQUAL(it_child->second.m_state.GetRejectReason(), "bad-txns-inputs-missingorspent"); } // Child with missing parent. @@ -381,36 +382,27 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) BOOST_CHECK(it_parent->second.m_effective_feerate == CFeeRate(1 * COIN, GetVirtualTransactionSize(*tx_parent))); BOOST_CHECK_EQUAL(it_parent->second.m_wtxids_fee_calculations.value().size(), 1); BOOST_CHECK_EQUAL(it_parent->second.m_wtxids_fee_calculations.value().front(), tx_parent->GetWitnessHash()); - BOOST_CHECK(it_child != submit_parent_child.m_tx_results.end()); - BOOST_CHECK(it_child->second.m_state.IsValid()); BOOST_CHECK(it_child->second.m_effective_feerate == CFeeRate(1 * COIN, GetVirtualTransactionSize(*tx_child))); BOOST_CHECK_EQUAL(it_child->second.m_wtxids_fee_calculations.value().size(), 1); BOOST_CHECK_EQUAL(it_child->second.m_wtxids_fee_calculations.value().front(), tx_child->GetWitnessHash()); BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); - BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_parent->GetHash()))); - BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_child->GetHash()))); } // 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); - BOOST_CHECK_MESSAGE(submit_deduped.m_state.IsValid(), - "Package validation unexpectedly failed: " << submit_deduped.m_state.GetRejectReason()); - BOOST_CHECK_EQUAL(submit_deduped.m_tx_results.size(), package_parent_child.size()); - auto it_parent_deduped = submit_deduped.m_tx_results.find(tx_parent->GetWitnessHash()); - auto it_child_deduped = submit_deduped.m_tx_results.find(tx_child->GetWitnessHash()); - BOOST_CHECK(it_parent_deduped != submit_deduped.m_tx_results.end()); - BOOST_CHECK(it_parent_deduped->second.m_state.IsValid()); - BOOST_CHECK(it_parent_deduped->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); - BOOST_CHECK(it_child_deduped != submit_deduped.m_tx_results.end()); - BOOST_CHECK(it_child_deduped->second.m_state.IsValid()); - BOOST_CHECK(it_child_deduped->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); + if (auto err_deduped{CheckPackageMempoolAcceptResult(package_parent_child, submit_deduped, /*expect_valid=*/true, m_node.mempool.get())}) { + BOOST_ERROR(err_deduped.value()); + } else { + auto it_parent_deduped = submit_deduped.m_tx_results.find(tx_parent->GetWitnessHash()); + auto it_child_deduped = submit_deduped.m_tx_results.find(tx_child->GetWitnessHash()); + BOOST_CHECK(it_parent_deduped->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); + BOOST_CHECK(it_child_deduped->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); + } BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); - BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_parent->GetHash()))); - BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_child->GetHash()))); } } @@ -470,51 +462,39 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) // Try submitting Package1{parent, child1} and Package2{parent, child2} where the children are // same-txid-different-witness. { + Package package_parent_child1{ptx_parent, ptx_child1}; const auto submit_witness1 = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, - {ptx_parent, ptx_child1}, /*test_accept=*/false); - BOOST_CHECK_MESSAGE(submit_witness1.m_state.IsValid(), - "Package validation unexpectedly failed: " << submit_witness1.m_state.GetRejectReason()); - BOOST_CHECK_EQUAL(submit_witness1.m_tx_results.size(), 2); - auto it_parent1 = submit_witness1.m_tx_results.find(ptx_parent->GetWitnessHash()); - auto it_child1 = submit_witness1.m_tx_results.find(ptx_child1->GetWitnessHash()); - BOOST_CHECK(it_parent1 != submit_witness1.m_tx_results.end()); - BOOST_CHECK_MESSAGE(it_parent1->second.m_state.IsValid(), - "Transaction unexpectedly failed: " << it_parent1->second.m_state.GetRejectReason()); - BOOST_CHECK(it_child1 != submit_witness1.m_tx_results.end()); - BOOST_CHECK_MESSAGE(it_child1->second.m_state.IsValid(), - "Transaction unexpectedly failed: " << it_child1->second.m_state.GetRejectReason()); - - BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(ptx_parent->GetHash()))); - BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(ptx_child1->GetHash()))); + package_parent_child1, /*test_accept=*/false); + 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, - {ptx_parent, ptx_child2}, /*test_accept=*/false); - BOOST_CHECK_MESSAGE(submit_witness2.m_state.IsValid(), - "Package validation unexpectedly failed: " << submit_witness2.m_state.GetRejectReason()); - BOOST_CHECK_EQUAL(submit_witness2.m_tx_results.size(), 2); - auto it_parent2_deduped = submit_witness2.m_tx_results.find(ptx_parent->GetWitnessHash()); - auto it_child2 = submit_witness2.m_tx_results.find(ptx_child2->GetWitnessHash()); - BOOST_CHECK(it_parent2_deduped != submit_witness2.m_tx_results.end()); - BOOST_CHECK(it_parent2_deduped->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); - BOOST_CHECK(it_child2 != submit_witness2.m_tx_results.end()); - BOOST_CHECK(it_child2->second.m_result_type == MempoolAcceptResult::ResultType::DIFFERENT_WITNESS); - BOOST_CHECK_EQUAL(ptx_child1->GetWitnessHash(), it_child2->second.m_other_wtxid.value()); - - BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(ptx_child2->GetHash()))); - BOOST_CHECK(!m_node.mempool->exists(GenTxid::Wtxid(ptx_child2->GetWitnessHash()))); + package_parent_child2, /*test_accept=*/false); + if (auto err_witness2{CheckPackageMempoolAcceptResult(package_parent_child2, submit_witness2, /*expect_valid=*/true, m_node.mempool.get())}) { + BOOST_ERROR(err_witness2.value()); + } else { + auto it_parent2_deduped = submit_witness2.m_tx_results.find(ptx_parent->GetWitnessHash()); + auto it_child2 = submit_witness2.m_tx_results.find(ptx_child2->GetWitnessHash()); + BOOST_CHECK(it_parent2_deduped->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); + BOOST_CHECK(it_child2->second.m_result_type == MempoolAcceptResult::ResultType::DIFFERENT_WITNESS); + BOOST_CHECK_EQUAL(ptx_child1->GetWitnessHash(), it_child2->second.m_other_wtxid.value()); + } // 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, - {ptx_parent, ptx_child1}, /*test_accept=*/false); - BOOST_CHECK_MESSAGE(submit_segwit_dedup.m_state.IsValid(), - "Package validation unexpectedly failed: " << submit_segwit_dedup.m_state.GetRejectReason()); - BOOST_CHECK_EQUAL(submit_segwit_dedup.m_tx_results.size(), 2); - auto it_parent_dup = submit_segwit_dedup.m_tx_results.find(ptx_parent->GetWitnessHash()); - auto it_child_dup = submit_segwit_dedup.m_tx_results.find(ptx_child1->GetWitnessHash()); - BOOST_CHECK(it_parent_dup->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); - BOOST_CHECK(it_child_dup->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); + package_parent_child1, /*test_accept=*/false); + 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 { + auto it_parent_dup = submit_segwit_dedup.m_tx_results.find(ptx_parent->GetWitnessHash()); + auto it_child_dup = submit_segwit_dedup.m_tx_results.find(ptx_child1->GetWitnessHash()); + BOOST_CHECK(it_parent_dup->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); + BOOST_CHECK(it_child_dup->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); + } } // Try submitting Package1{child2, grandchild} where child2 is same-txid-different-witness as @@ -535,21 +515,17 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) // We already submitted child1 above. { + Package package_child2_grandchild{ptx_child2, ptx_grandchild}; const auto submit_spend_ignored = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, - {ptx_child2, ptx_grandchild}, /*test_accept=*/false); - BOOST_CHECK_MESSAGE(submit_spend_ignored.m_state.IsValid(), - "Package validation unexpectedly failed: " << submit_spend_ignored.m_state.GetRejectReason()); - BOOST_CHECK_EQUAL(submit_spend_ignored.m_tx_results.size(), 2); - auto it_child2_ignored = submit_spend_ignored.m_tx_results.find(ptx_child2->GetWitnessHash()); - auto it_grandchild = submit_spend_ignored.m_tx_results.find(ptx_grandchild->GetWitnessHash()); - BOOST_CHECK(it_child2_ignored != submit_spend_ignored.m_tx_results.end()); - BOOST_CHECK(it_child2_ignored->second.m_result_type == MempoolAcceptResult::ResultType::DIFFERENT_WITNESS); - BOOST_CHECK(it_grandchild != submit_spend_ignored.m_tx_results.end()); - BOOST_CHECK(it_grandchild->second.m_result_type == MempoolAcceptResult::ResultType::VALID); - - BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(ptx_child2->GetHash()))); - BOOST_CHECK(!m_node.mempool->exists(GenTxid::Wtxid(ptx_child2->GetWitnessHash()))); - BOOST_CHECK(m_node.mempool->exists(GenTxid::Wtxid(ptx_grandchild->GetWitnessHash()))); + package_child2_grandchild, /*test_accept=*/false); + 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 { + auto it_child2_ignored = submit_spend_ignored.m_tx_results.find(ptx_child2->GetWitnessHash()); + auto it_grandchild = submit_spend_ignored.m_tx_results.find(ptx_grandchild->GetWitnessHash()); + BOOST_CHECK(it_child2_ignored->second.m_result_type == MempoolAcceptResult::ResultType::DIFFERENT_WITNESS); + BOOST_CHECK(it_grandchild->second.m_result_type == MempoolAcceptResult::ResultType::VALID); + } } // A package Package{parent1, parent2, parent3, child} where the parents are a mixture of @@ -641,36 +617,28 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) // child should be accepted { const auto mixed_result = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_mixed, false); - BOOST_CHECK_MESSAGE(mixed_result.m_state.IsValid(), mixed_result.m_state.GetRejectReason()); - BOOST_CHECK_EQUAL(mixed_result.m_tx_results.size(), package_mixed.size()); - auto it_parent1 = mixed_result.m_tx_results.find(ptx_parent1->GetWitnessHash()); - auto it_parent2 = mixed_result.m_tx_results.find(ptx_parent2_v1->GetWitnessHash()); - auto it_parent3 = mixed_result.m_tx_results.find(ptx_parent3->GetWitnessHash()); - auto it_child = mixed_result.m_tx_results.find(ptx_mixed_child->GetWitnessHash()); - BOOST_CHECK(it_parent1 != mixed_result.m_tx_results.end()); - BOOST_CHECK(it_parent2 != mixed_result.m_tx_results.end()); - BOOST_CHECK(it_parent3 != mixed_result.m_tx_results.end()); - BOOST_CHECK(it_child != mixed_result.m_tx_results.end()); + if (auto err_mixed{CheckPackageMempoolAcceptResult(package_mixed, mixed_result, /*expect_valid=*/true, m_node.mempool.get())}) { + BOOST_ERROR(err_mixed.value()); + } else { + auto it_parent1 = mixed_result.m_tx_results.find(ptx_parent1->GetWitnessHash()); + auto it_parent2 = mixed_result.m_tx_results.find(ptx_parent2_v1->GetWitnessHash()); + auto it_parent3 = mixed_result.m_tx_results.find(ptx_parent3->GetWitnessHash()); + auto it_child = mixed_result.m_tx_results.find(ptx_mixed_child->GetWitnessHash()); - BOOST_CHECK(it_parent1->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); - BOOST_CHECK(it_parent2->second.m_result_type == MempoolAcceptResult::ResultType::DIFFERENT_WITNESS); - BOOST_CHECK(it_parent3->second.m_result_type == MempoolAcceptResult::ResultType::VALID); - BOOST_CHECK(it_child->second.m_result_type == MempoolAcceptResult::ResultType::VALID); - BOOST_CHECK_EQUAL(ptx_parent2_v2->GetWitnessHash(), it_parent2->second.m_other_wtxid.value()); + BOOST_CHECK(it_parent1->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); + BOOST_CHECK(it_parent2->second.m_result_type == MempoolAcceptResult::ResultType::DIFFERENT_WITNESS); + BOOST_CHECK(it_parent3->second.m_result_type == MempoolAcceptResult::ResultType::VALID); + BOOST_CHECK(it_child->second.m_result_type == MempoolAcceptResult::ResultType::VALID); + BOOST_CHECK_EQUAL(ptx_parent2_v2->GetWitnessHash(), it_parent2->second.m_other_wtxid.value()); - BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(ptx_parent1->GetHash()))); - BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(ptx_parent2_v1->GetHash()))); - BOOST_CHECK(!m_node.mempool->exists(GenTxid::Wtxid(ptx_parent2_v1->GetWitnessHash()))); - BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(ptx_parent3->GetHash()))); - BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(ptx_mixed_child->GetHash()))); - - // package feerate should include parent3 and child. It should not include parent1 or parent2_v1. - const CFeeRate expected_feerate(1 * COIN, GetVirtualTransactionSize(*ptx_parent3) + GetVirtualTransactionSize(*ptx_mixed_child)); - BOOST_CHECK(it_parent3->second.m_effective_feerate.value() == expected_feerate); - BOOST_CHECK(it_child->second.m_effective_feerate.value() == expected_feerate); - std::vector expected_wtxids({ptx_parent3->GetWitnessHash(), ptx_mixed_child->GetWitnessHash()}); - BOOST_CHECK(it_parent3->second.m_wtxids_fee_calculations.value() == expected_wtxids); - BOOST_CHECK(it_child->second.m_wtxids_fee_calculations.value() == expected_wtxids); + // package feerate should include parent3 and child. It should not include parent1 or parent2_v1. + const CFeeRate expected_feerate(1 * COIN, GetVirtualTransactionSize(*ptx_parent3) + GetVirtualTransactionSize(*ptx_mixed_child)); + BOOST_CHECK(it_parent3->second.m_effective_feerate.value() == expected_feerate); + BOOST_CHECK(it_child->second.m_effective_feerate.value() == expected_feerate); + std::vector expected_wtxids({ptx_parent3->GetWitnessHash(), ptx_mixed_child->GetWitnessHash()}); + BOOST_CHECK(it_parent3->second.m_wtxids_fee_calculations.value() == expected_wtxids); + BOOST_CHECK(it_child->second.m_wtxids_fee_calculations.value() == expected_wtxids); + } } } @@ -715,15 +683,17 @@ 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); - BOOST_CHECK_EQUAL(submit_cpfp_deprio.m_state.GetResult(), PackageValidationResult::PCKG_TX); - BOOST_CHECK(submit_cpfp_deprio.m_state.IsInvalid()); - BOOST_CHECK_EQUAL(submit_cpfp_deprio.m_tx_results.find(tx_parent->GetWitnessHash())->second.m_state.GetResult(), - TxValidationResult::TX_MEMPOOL_POLICY); - BOOST_CHECK_EQUAL(submit_cpfp_deprio.m_tx_results.find(tx_child->GetWitnessHash())->second.m_state.GetResult(), - TxValidationResult::TX_MISSING_INPUTS); - BOOST_CHECK(submit_cpfp_deprio.m_tx_results.find(tx_parent->GetWitnessHash())->second.m_state.GetRejectReason() == "min relay fee not met"); - BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); - const CFeeRate expected_feerate(0, GetVirtualTransactionSize(*tx_parent) + GetVirtualTransactionSize(*tx_child)); + 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 { + BOOST_CHECK_EQUAL(submit_cpfp_deprio.m_state.GetResult(), PackageValidationResult::PCKG_TX); + BOOST_CHECK_EQUAL(submit_cpfp_deprio.m_tx_results.find(tx_parent->GetWitnessHash())->second.m_state.GetResult(), + TxValidationResult::TX_MEMPOOL_POLICY); + BOOST_CHECK_EQUAL(submit_cpfp_deprio.m_tx_results.find(tx_child->GetWitnessHash())->second.m_state.GetResult(), + TxValidationResult::TX_MISSING_INPUTS); + BOOST_CHECK(submit_cpfp_deprio.m_tx_results.find(tx_parent->GetWitnessHash())->second.m_state.GetRejectReason() == "min relay fee not met"); + BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); + } } // Clear the prioritisation of the parent transaction. @@ -735,31 +705,27 @@ 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); + if (auto err_cpfp{CheckPackageMempoolAcceptResult(package_cpfp, submit_cpfp, /*expect_valid=*/true, m_node.mempool.get())}) { + BOOST_ERROR(err_cpfp.value()); + } else { + auto it_parent = submit_cpfp.m_tx_results.find(tx_parent->GetWitnessHash()); + auto it_child = submit_cpfp.m_tx_results.find(tx_child->GetWitnessHash()); + BOOST_CHECK(it_parent->second.m_result_type == MempoolAcceptResult::ResultType::VALID); + BOOST_CHECK(it_parent->second.m_base_fees.value() == coinbase_value - parent_value); + BOOST_CHECK(it_child->second.m_result_type == MempoolAcceptResult::ResultType::VALID); + BOOST_CHECK(it_child->second.m_base_fees.value() == COIN); + + const CFeeRate expected_feerate(coinbase_value - child_value, + GetVirtualTransactionSize(*tx_parent) + GetVirtualTransactionSize(*tx_child)); + BOOST_CHECK(it_parent->second.m_effective_feerate.value() == expected_feerate); + BOOST_CHECK(it_child->second.m_effective_feerate.value() == expected_feerate); + std::vector expected_wtxids({tx_parent->GetWitnessHash(), tx_child->GetWitnessHash()}); + BOOST_CHECK(it_parent->second.m_wtxids_fee_calculations.value() == expected_wtxids); + BOOST_CHECK(it_child->second.m_wtxids_fee_calculations.value() == expected_wtxids); + BOOST_CHECK(expected_feerate.GetFeePerK() > 1000); + } expected_pool_size += 2; - BOOST_CHECK_MESSAGE(submit_cpfp.m_state.IsValid(), - "Package validation unexpectedly failed: " << submit_cpfp.m_state.GetRejectReason()); - BOOST_CHECK_EQUAL(submit_cpfp.m_tx_results.size(), package_cpfp.size()); - auto it_parent = submit_cpfp.m_tx_results.find(tx_parent->GetWitnessHash()); - auto it_child = submit_cpfp.m_tx_results.find(tx_child->GetWitnessHash()); - BOOST_CHECK(it_parent != submit_cpfp.m_tx_results.end()); - BOOST_CHECK(it_parent->second.m_result_type == MempoolAcceptResult::ResultType::VALID); - BOOST_CHECK(it_parent->second.m_base_fees.value() == coinbase_value - parent_value); - BOOST_CHECK(it_child != submit_cpfp.m_tx_results.end()); - BOOST_CHECK(it_child->second.m_result_type == MempoolAcceptResult::ResultType::VALID); - BOOST_CHECK(it_child->second.m_base_fees.value() == COIN); - BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); - BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_parent->GetHash()))); - BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_child->GetHash()))); - - const CFeeRate expected_feerate(coinbase_value - child_value, - GetVirtualTransactionSize(*tx_parent) + GetVirtualTransactionSize(*tx_child)); - BOOST_CHECK(it_parent->second.m_effective_feerate.value() == expected_feerate); - BOOST_CHECK(it_child->second.m_effective_feerate.value() == expected_feerate); - std::vector expected_wtxids({tx_parent->GetWitnessHash(), tx_child->GetWitnessHash()}); - BOOST_CHECK(it_parent->second.m_wtxids_fee_calculations.value() == expected_wtxids); - BOOST_CHECK(it_child->second.m_wtxids_fee_calculations.value() == expected_wtxids); - BOOST_CHECK(expected_feerate.GetFeePerK() > 1000); } // Just because we allow low-fee parents doesn't mean we allow low-feerate packages. @@ -785,15 +751,17 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) package_still_too_low.push_back(tx_child_cheap); BOOST_CHECK(m_node.mempool->GetMinFee().GetFee(GetVirtualTransactionSize(*tx_child_cheap)) <= child_fee); BOOST_CHECK(m_node.mempool->GetMinFee().GetFee(GetVirtualTransactionSize(*tx_parent_cheap) + GetVirtualTransactionSize(*tx_child_cheap)) > parent_fee + child_fee); + BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); // Cheap package should fail with package-fee-too-low. { - BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); const auto submit_package_too_low = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_still_too_low, /*test_accept=*/false); - BOOST_CHECK_MESSAGE(submit_package_too_low.m_state.IsInvalid(), "Package validation unexpectedly succeeded"); BOOST_CHECK_EQUAL(submit_package_too_low.m_state.GetResult(), PackageValidationResult::PCKG_POLICY); BOOST_CHECK_EQUAL(submit_package_too_low.m_state.GetRejectReason(), "package-fee-too-low"); + 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()); + } BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); } @@ -804,25 +772,26 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) { const auto submit_prioritised_package = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_still_too_low, /*test_accept=*/false); + 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 { + const CFeeRate expected_feerate(1 * COIN + parent_fee + child_fee, + GetVirtualTransactionSize(*tx_parent_cheap) + GetVirtualTransactionSize(*tx_child_cheap)); + BOOST_CHECK_EQUAL(submit_prioritised_package.m_tx_results.size(), package_still_too_low.size()); + auto it_parent = submit_prioritised_package.m_tx_results.find(tx_parent_cheap->GetWitnessHash()); + auto it_child = submit_prioritised_package.m_tx_results.find(tx_child_cheap->GetWitnessHash()); + BOOST_CHECK(it_parent->second.m_result_type == MempoolAcceptResult::ResultType::VALID); + BOOST_CHECK(it_parent->second.m_base_fees.value() == parent_fee); + BOOST_CHECK(it_parent->second.m_effective_feerate.value() == expected_feerate); + BOOST_CHECK(it_child->second.m_result_type == MempoolAcceptResult::ResultType::VALID); + BOOST_CHECK(it_child->second.m_base_fees.value() == child_fee); + BOOST_CHECK(it_child->second.m_effective_feerate.value() == expected_feerate); + std::vector expected_wtxids({tx_parent_cheap->GetWitnessHash(), tx_child_cheap->GetWitnessHash()}); + BOOST_CHECK(it_parent->second.m_wtxids_fee_calculations.value() == expected_wtxids); + BOOST_CHECK(it_child->second.m_wtxids_fee_calculations.value() == expected_wtxids); + } expected_pool_size += 2; - BOOST_CHECK_MESSAGE(submit_prioritised_package.m_state.IsValid(), - "Package validation unexpectedly failed" << submit_prioritised_package.m_state.GetRejectReason()); - const CFeeRate expected_feerate(1 * COIN + parent_fee + child_fee, - GetVirtualTransactionSize(*tx_parent_cheap) + GetVirtualTransactionSize(*tx_child_cheap)); - BOOST_CHECK_EQUAL(submit_prioritised_package.m_tx_results.size(), package_still_too_low.size()); - auto it_parent = submit_prioritised_package.m_tx_results.find(tx_parent_cheap->GetWitnessHash()); - auto it_child = submit_prioritised_package.m_tx_results.find(tx_child_cheap->GetWitnessHash()); - BOOST_CHECK(it_parent != submit_prioritised_package.m_tx_results.end()); - BOOST_CHECK(it_parent->second.m_result_type == MempoolAcceptResult::ResultType::VALID); - BOOST_CHECK(it_parent->second.m_base_fees.value() == parent_fee); - BOOST_CHECK(it_parent->second.m_effective_feerate.value() == expected_feerate); - BOOST_CHECK(it_child != submit_prioritised_package.m_tx_results.end()); - BOOST_CHECK(it_child->second.m_result_type == MempoolAcceptResult::ResultType::VALID); - BOOST_CHECK(it_child->second.m_base_fees.value() == child_fee); - BOOST_CHECK(it_child->second.m_effective_feerate.value() == expected_feerate); - std::vector expected_wtxids({tx_parent_cheap->GetWitnessHash(), tx_child_cheap->GetWitnessHash()}); - BOOST_CHECK(it_parent->second.m_wtxids_fee_calculations.value() == expected_wtxids); - BOOST_CHECK(it_child->second.m_wtxids_fee_calculations.value() == expected_wtxids); + BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); } // Package feerate is calculated without topology in mind; it's just aggregating fees and sizes. @@ -851,31 +820,27 @@ 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); + 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 { + // The child would have been validated on its own and failed. + BOOST_CHECK_EQUAL(submit_rich_parent.m_state.GetResult(), PackageValidationResult::PCKG_TX); + BOOST_CHECK_EQUAL(submit_rich_parent.m_state.GetRejectReason(), "transaction failed"); + + auto it_parent = submit_rich_parent.m_tx_results.find(tx_parent_rich->GetWitnessHash()); + auto it_child = submit_rich_parent.m_tx_results.find(tx_child_poor->GetWitnessHash()); + BOOST_CHECK(it_parent->second.m_result_type == MempoolAcceptResult::ResultType::VALID); + BOOST_CHECK(it_child->second.m_result_type == MempoolAcceptResult::ResultType::INVALID); + BOOST_CHECK(it_parent->second.m_state.GetRejectReason() == ""); + BOOST_CHECK_MESSAGE(it_parent->second.m_base_fees.value() == high_parent_fee, + strprintf("rich parent: expected fee %s, got %s", high_parent_fee, it_parent->second.m_base_fees.value())); + BOOST_CHECK(it_parent->second.m_effective_feerate == CFeeRate(high_parent_fee, GetVirtualTransactionSize(*tx_parent_rich))); + BOOST_CHECK_EQUAL(it_child->second.m_result_type, MempoolAcceptResult::ResultType::INVALID); + BOOST_CHECK_EQUAL(it_child->second.m_state.GetResult(), TxValidationResult::TX_MEMPOOL_POLICY); + BOOST_CHECK(it_child->second.m_state.GetRejectReason() == "min relay fee not met"); + } expected_pool_size += 1; - BOOST_CHECK_MESSAGE(submit_rich_parent.m_state.IsInvalid(), "Package validation unexpectedly succeeded"); - - // The child would have been validated on its own and failed. - BOOST_CHECK_EQUAL(submit_rich_parent.m_state.GetResult(), PackageValidationResult::PCKG_TX); - BOOST_CHECK_EQUAL(submit_rich_parent.m_state.GetRejectReason(), "transaction failed"); - - auto it_parent = submit_rich_parent.m_tx_results.find(tx_parent_rich->GetWitnessHash()); - auto it_child = submit_rich_parent.m_tx_results.find(tx_child_poor->GetWitnessHash()); - BOOST_CHECK(it_parent != submit_rich_parent.m_tx_results.end()); - BOOST_CHECK(it_child != submit_rich_parent.m_tx_results.end()); - BOOST_CHECK(it_parent->second.m_result_type == MempoolAcceptResult::ResultType::VALID); - BOOST_CHECK(it_child->second.m_result_type == MempoolAcceptResult::ResultType::INVALID); - BOOST_CHECK(it_parent->second.m_state.GetRejectReason() == ""); - BOOST_CHECK_MESSAGE(it_parent->second.m_base_fees.value() == high_parent_fee, - strprintf("rich parent: expected fee %s, got %s", high_parent_fee, it_parent->second.m_base_fees.value())); - BOOST_CHECK(it_parent->second.m_effective_feerate == CFeeRate(high_parent_fee, GetVirtualTransactionSize(*tx_parent_rich))); - BOOST_CHECK(it_child != submit_rich_parent.m_tx_results.end()); - BOOST_CHECK_EQUAL(it_child->second.m_result_type, MempoolAcceptResult::ResultType::INVALID); - BOOST_CHECK_EQUAL(it_child->second.m_state.GetResult(), TxValidationResult::TX_MEMPOOL_POLICY); - BOOST_CHECK(it_child->second.m_state.GetRejectReason() == "min relay fee not met"); - BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); - BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_parent_rich->GetHash()))); - BOOST_CHECK(!m_node.mempool->exists(GenTxid::Txid(tx_child_poor->GetHash()))); } } BOOST_AUTO_TEST_SUITE_END() From 1147e00e59e47f27024ec96629993c66a3ce4ef0 Mon Sep 17 00:00:00 2001 From: glozow Date: Wed, 4 Oct 2023 10:06:22 +0100 Subject: [PATCH 4/4] [validation] change package-fee-too-low, return wtxid(s) and effective feerate With subpackage evaluation and de-duplication, it's not always the entire package that is used in CheckFeerate. To be more helpful to the caller, specify which transactions were included in the evaluation and what the feerate was. Instead of PCKG_POLICY (which is supposed to be for package-wide errors), use PCKG_TX. --- src/test/fuzz/tx_pool.cpp | 9 +++++--- src/test/txpackage_tests.cpp | 17 ++++++++++++--- src/test/util/txmempool.cpp | 7 +++++-- src/validation.cpp | 28 +++++++++++++++++-------- src/validation.h | 40 ++++++++++++++++++++++++++++++++---- 5 files changed, 81 insertions(+), 20 deletions(-) diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index 66e537a57ba..96095539ece 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -154,12 +154,15 @@ void CheckATMPInvariants(const MempoolAcceptResult& res, bool txid_in_mempool, b // It may be already in the mempool since in ATMP cases we don't set MEMPOOL_ENTRY or DIFFERENT_WITNESS Assert(!res.m_state.IsValid()); Assert(res.m_state.IsInvalid()); + + const bool is_reconsiderable{res.m_state.GetResult() == TxValidationResult::TX_RECONSIDERABLE}; Assert(!res.m_replaced_transactions); Assert(!res.m_vsize); Assert(!res.m_base_fees); - // Unable or unwilling to calculate fees - Assert(!res.m_effective_feerate); - Assert(!res.m_wtxids_fee_calculations); + // Fee information is provided if the failure is TX_RECONSIDERABLE. + // In other cases, validation may be unable or unwilling to calculate the fees. + Assert(res.m_effective_feerate.has_value() == is_reconsiderable); + Assert(res.m_wtxids_fee_calculations.has_value() == is_reconsiderable); Assert(!res.m_other_wtxid); break; } diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp index 30e16ee365f..84c9ecc3d1e 100644 --- a/src/test/txpackage_tests.cpp +++ b/src/test/txpackage_tests.cpp @@ -753,15 +753,26 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) BOOST_CHECK(m_node.mempool->GetMinFee().GetFee(GetVirtualTransactionSize(*tx_parent_cheap) + GetVirtualTransactionSize(*tx_child_cheap)) > parent_fee + child_fee); BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); - // Cheap package should fail with package-fee-too-low. + // 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); - BOOST_CHECK_EQUAL(submit_package_too_low.m_state.GetResult(), PackageValidationResult::PCKG_POLICY); - BOOST_CHECK_EQUAL(submit_package_too_low.m_state.GetRejectReason(), "package-fee-too-low"); 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 { + // Individual feerate of parent is too low. + BOOST_CHECK_EQUAL(submit_package_too_low.m_tx_results.at(tx_parent_cheap->GetWitnessHash()).m_state.GetResult(), + TxValidationResult::TX_RECONSIDERABLE); + BOOST_CHECK(submit_package_too_low.m_tx_results.at(tx_parent_cheap->GetWitnessHash()).m_effective_feerate.value() == + CFeeRate(parent_fee, GetVirtualTransactionSize(*tx_parent_cheap))); + // Package feerate of parent + child is too low. + BOOST_CHECK_EQUAL(submit_package_too_low.m_tx_results.at(tx_child_cheap->GetWitnessHash()).m_state.GetResult(), + TxValidationResult::TX_RECONSIDERABLE); + BOOST_CHECK(submit_package_too_low.m_tx_results.at(tx_child_cheap->GetWitnessHash()).m_effective_feerate.value() == + CFeeRate(parent_fee + child_fee, GetVirtualTransactionSize(*tx_parent_cheap) + GetVirtualTransactionSize(*tx_child_cheap))); } + BOOST_CHECK_EQUAL(submit_package_too_low.m_state.GetResult(), PackageValidationResult::PCKG_TX); + BOOST_CHECK_EQUAL(submit_package_too_low.m_state.GetRejectReason(), "transaction failed"); BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size); } diff --git a/src/test/util/txmempool.cpp b/src/test/util/txmempool.cpp index 147e589debf..6d3bb01be86 100644 --- a/src/test/util/txmempool.cpp +++ b/src/test/util/txmempool.cpp @@ -89,11 +89,14 @@ std::optional CheckPackageMempoolAcceptResult(const Package& txns, } // m_effective_feerate and m_wtxids_fee_calculations should exist iff the result was valid - if (atmp_result.m_effective_feerate.has_value() != valid) { + // or if the failure was TX_RECONSIDERABLE + const bool valid_or_reconsiderable{atmp_result.m_result_type == MempoolAcceptResult::ResultType::VALID || + atmp_result.m_state.GetResult() == TxValidationResult::TX_RECONSIDERABLE}; + if (atmp_result.m_effective_feerate.has_value() != valid_or_reconsiderable) { return strprintf("tx %s result should %shave m_effective_feerate", wtxid.ToString(), valid ? "" : "not "); } - if (atmp_result.m_wtxids_fee_calculations.has_value() != valid) { + if (atmp_result.m_wtxids_fee_calculations.has_value() != valid_or_reconsiderable) { return strprintf("tx %s result should %shave m_effective_feerate", wtxid.ToString(), valid ? "" : "not "); } diff --git a/src/validation.cpp b/src/validation.cpp index c478252f182..018566fa9c3 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1237,7 +1237,13 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef Workspace ws(ptx); const std::vector single_wtxid{ws.m_ptx->GetWitnessHash()}; - if (!PreChecks(args, ws)) return MempoolAcceptResult::Failure(ws.m_state); + if (!PreChecks(args, ws)) { + if (ws.m_state.GetResult() == TxValidationResult::TX_RECONSIDERABLE) { + // Failed for fee reasons. Provide the effective feerate and which tx was included. + return MempoolAcceptResult::FeeFailure(ws.m_state, CFeeRate(ws.m_modified_fees, ws.m_vsize), single_wtxid); + } + return MempoolAcceptResult::Failure(ws.m_state); + } if (m_rbf && !ReplacementChecks(ws)) return MempoolAcceptResult::Failure(ws.m_state); @@ -1254,7 +1260,12 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef ws.m_base_fees, effective_feerate, single_wtxid); } - if (!Finalize(args, ws)) return MempoolAcceptResult::Failure(ws.m_state); + if (!Finalize(args, ws)) { + // The only possible failure reason is fee-related (mempool full). + // Failed for fee reasons. Provide the effective feerate and which txns were included. + Assume(ws.m_state.GetResult() == TxValidationResult::TX_RECONSIDERABLE); + return MempoolAcceptResult::FeeFailure(ws.m_state, CFeeRate(ws.m_modified_fees, ws.m_vsize), {ws.m_ptx->GetWitnessHash()}); + } GetMainSignals().TransactionAddedToMempool(ptx, m_pool.GetAndIncrementSequence()); @@ -1308,11 +1319,16 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: const auto m_total_modified_fees = std::accumulate(workspaces.cbegin(), workspaces.cend(), CAmount{0}, [](CAmount sum, auto& ws) { return sum + ws.m_modified_fees; }); const CFeeRate package_feerate(m_total_modified_fees, m_total_vsize); + std::vector all_package_wtxids; + all_package_wtxids.reserve(workspaces.size()); + std::transform(workspaces.cbegin(), workspaces.cend(), std::back_inserter(all_package_wtxids), + [](const auto& ws) { return ws.m_ptx->GetWitnessHash(); }); TxValidationState placeholder_state; if (args.m_package_feerates && !CheckFeeRate(m_total_vsize, m_total_modified_fees, placeholder_state)) { - package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package-fee-too-low"); - return PackageMempoolAcceptResult(package_state, {}); + package_state.Invalid(PackageValidationResult::PCKG_TX, "transaction failed"); + return PackageMempoolAcceptResult(package_state, {{workspaces.back().m_ptx->GetWitnessHash(), + MempoolAcceptResult::FeeFailure(placeholder_state, CFeeRate(m_total_modified_fees, m_total_vsize), all_package_wtxids)}}); } // Apply package mempool ancestor/descendant limits. Skip if there is only one transaction, @@ -1323,10 +1339,6 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: return PackageMempoolAcceptResult(package_state, std::move(results)); } - std::vector all_package_wtxids; - all_package_wtxids.reserve(workspaces.size()); - std::transform(workspaces.cbegin(), workspaces.cend(), std::back_inserter(all_package_wtxids), - [](const auto& ws) { return ws.m_ptx->GetWitnessHash(); }); for (Workspace& ws : workspaces) { ws.m_package_feerate = package_feerate; if (!PolicyScriptChecks(args, ws)) { diff --git a/src/validation.h b/src/validation.h index 75deba64934..e669ec46d5d 100644 --- a/src/validation.h +++ b/src/validation.h @@ -114,7 +114,27 @@ double GuessVerificationProgress(const ChainTxData& data, const CBlockIndex* pin void PruneBlockFilesManual(Chainstate& active_chainstate, int nManualPruneHeight); /** -* Validation result for a single transaction mempool acceptance. +* Validation result for a transaction evaluated by MemPoolAccept (single or package). +* Here are the expected fields and properties of a result depending on its ResultType, applicable to +* results returned from package evaluation: +*+---------------------------+----------------+-------------------+------------------+----------------+-------------------+ +*| Field or property | VALID | INVALID | MEMPOOL_ENTRY | DIFFERENT_WITNESS | +*| | |--------------------------------------| | | +*| | | TX_RECONSIDERABLE | Other | | | +*+---------------------------+----------------+-------------------+------------------+----------------+-------------------+ +*| txid in mempool? | yes | no | no* | yes | yes | +*| wtxid in mempool? | yes | no | no* | yes | no | +*| m_state | yes, IsValid() | yes, IsInvalid() | yes, IsInvalid() | yes, IsValid() | yes, IsValid() | +*| m_replaced_transactions | yes | no | no | no | no | +*| m_vsize | yes | no | no | yes | no | +*| m_base_fees | yes | no | no | yes | no | +*| m_effective_feerate | yes | yes | no | no | no | +*| m_wtxids_fee_calculations | yes | yes | no | no | no | +*| m_other_wtxid | no | no | no | no | yes | +*+---------------------------+----------------+-------------------+------------------+----------------+-------------------+ +* (*) Individual transaction acceptance doesn't return MEMPOOL_ENTRY and DIFFERENT_WITNESS. It returns +* INVALID, with the errors txn-already-in-mempool and txn-same-nonwitness-data-in-mempool +* respectively. In those cases, the txid or wtxid may be in the mempool for a TX_CONFLICT. */ struct MempoolAcceptResult { /** Used to indicate the results of mempool validation. */ @@ -130,7 +150,6 @@ struct MempoolAcceptResult { /** Contains information about why the transaction failed. */ const TxValidationState m_state; - // The following fields are only present when m_result_type = ResultType::VALID or MEMPOOL_ENTRY /** Mempool transactions replaced by the tx. */ const std::optional> m_replaced_transactions; /** Virtual size as used by the mempool, calculated using serialized size and sigops. */ @@ -141,7 +160,6 @@ struct MempoolAcceptResult { * using prioritisetransaction (i.e. modified fees). If this transaction was submitted as a * package, this is the package feerate, which may also include its descendants and/or * ancestors (see m_wtxids_fee_calculations below). - * Only present when m_result_type = ResultType::VALID. */ const std::optional m_effective_feerate; /** Contains the wtxids of the transactions used for fee-related checks. Includes this @@ -151,7 +169,6 @@ struct MempoolAcceptResult { * Only present when m_result_type = ResultType::VALID. */ const std::optional> m_wtxids_fee_calculations; - // The following field is only present when m_result_type = ResultType::DIFFERENT_WITNESS /** The wtxid of the transaction in the mempool which has the same txid but different witness. */ const std::optional m_other_wtxid; @@ -159,6 +176,12 @@ struct MempoolAcceptResult { return MempoolAcceptResult(state); } + static MempoolAcceptResult FeeFailure(TxValidationState state, + CFeeRate effective_feerate, + const std::vector& wtxids_fee_calculations) { + return MempoolAcceptResult(state, effective_feerate, wtxids_fee_calculations); + } + static MempoolAcceptResult Success(std::list&& replaced_txns, int64_t vsize, CAmount fees, @@ -197,6 +220,15 @@ private: m_effective_feerate(effective_feerate), m_wtxids_fee_calculations(wtxids_fee_calculations) {} + /** Constructor for fee-related failure case */ + explicit MempoolAcceptResult(TxValidationState state, + CFeeRate effective_feerate, + const std::vector& wtxids_fee_calculations) + : m_result_type(ResultType::INVALID), + m_state(state), + m_effective_feerate(effective_feerate), + m_wtxids_fee_calculations(wtxids_fee_calculations) {} + /** Constructor for already-in-mempool case. It wouldn't replace any transactions. */ explicit MempoolAcceptResult(int64_t vsize, CAmount fees) : m_result_type(ResultType::MEMPOOL_ENTRY), m_vsize{vsize}, m_base_fees(fees) {}