diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp index 244afb91619..0be843d5d9f 100644 --- a/src/test/txpackage_tests.cpp +++ b/src/test/txpackage_tests.cpp @@ -96,6 +96,8 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup) 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()); @@ -103,6 +105,8 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup) BOOST_CHECK(result_parent_child.m_package_feerate.value() == CFeeRate(2 * COIN, GetVirtualTransactionSize(*tx_parent) + GetVirtualTransactionSize(*tx_child))); 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); @@ -326,9 +330,13 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) BOOST_CHECK(it_parent != submit_parent_child.m_tx_results.end()); BOOST_CHECK(it_parent->second.m_state.IsValid()); 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()))); @@ -619,6 +627,9 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) mixed_result.m_package_feerate.value().ToString())); 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); } } @@ -703,6 +714,9 @@ 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()}); + 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); BOOST_CHECK(submit_cpfp.m_package_feerate.has_value()); BOOST_CHECK_MESSAGE(submit_cpfp.m_package_feerate.value() == expected_feerate, @@ -774,6 +788,9 @@ 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() == 200); 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); } // Package feerate is calculated without topology in mind; it's just aggregating fees and sizes. diff --git a/src/validation.cpp b/src/validation.cpp index 8ae4a3ecb80..5d0bd0bee1e 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1149,14 +1149,21 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector& // make sure we haven't exceeded max mempool size. LimitMempoolSize(m_pool, m_active_chainstate.CoinsTip()); + 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(); }); // Find the wtxids of the transactions that made it into the mempool. Allow partial submission, // but don't report success unless they all made it into the mempool. for (Workspace& ws : workspaces) { 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()}); if (m_pool.exists(GenTxid::Wtxid(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)); + MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, + ws.m_base_fees, effective_feerate, effective_feerate_wtxids)); GetMainSignals().TransactionAddedToMempool(ws.m_ptx, m_pool.GetAndIncrementSequence()); } else { all_submitted = false; @@ -1185,16 +1192,19 @@ 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, ws.m_base_fees, effective_feerate); + return MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, + ws.m_base_fees, effective_feerate, single_wtxid); } if (!Finalize(args, ws)) return MempoolAcceptResult::Failure(ws.m_state); GetMainSignals().TransactionAddedToMempool(ptx, m_pool.GetAndIncrementSequence()); - return MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, ws.m_base_fees, effective_feerate); + return MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, ws.m_base_fees, + effective_feerate, single_wtxid); } PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::vector& txns, ATMPArgs& args) @@ -1252,6 +1262,10 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: return PackageMempoolAcceptResult(package_state, package_feerate, 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)) { @@ -1263,11 +1277,12 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: if (args.m_test_accept) { const auto effective_feerate = args.m_package_feerates ? ws.m_package_feerate : CFeeRate{ws.m_modified_fees, static_cast(ws.m_vsize)}; - // When test_accept=true, transactions that pass PolicyScriptChecks are valid because there are - // no further mempool checks (passing PolicyScriptChecks implies passing ConsensusScriptChecks). + const auto effective_feerate_wtxids = args.m_package_feerates ? all_package_wtxids : + 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)); + ws.m_vsize, ws.m_base_fees, effective_feerate, + effective_feerate_wtxids)); } } diff --git a/src/validation.h b/src/validation.h index 8701326277d..6746ca1c5ea 100644 --- a/src/validation.h +++ b/src/validation.h @@ -137,8 +137,16 @@ struct MempoolAcceptResult { /** The feerate at which this transaction was considered. This includes any fee delta added * 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. Only present when m_result_type = ResultType::VALID. */ + * 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 + * transaction's wtxid and may include others if this transaction was validated as part of a + * 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; // 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. */ @@ -151,8 +159,10 @@ struct MempoolAcceptResult { static MempoolAcceptResult Success(std::list&& replaced_txns, int64_t vsize, CAmount fees, - CFeeRate effective_feerate) { - return MempoolAcceptResult(std::move(replaced_txns), vsize, fees, effective_feerate); + CFeeRate effective_feerate, + const std::vector& wtxids_fee_calculations) { + return MempoolAcceptResult(std::move(replaced_txns), vsize, fees, + effective_feerate, wtxids_fee_calculations); } static MempoolAcceptResult MempoolTx(int64_t vsize, CAmount fees) { @@ -175,12 +185,14 @@ private: explicit MempoolAcceptResult(std::list&& replaced_txns, int64_t vsize, CAmount fees, - CFeeRate effective_feerate) + CFeeRate effective_feerate, + const std::vector& wtxids_fee_calculations) : m_result_type(ResultType::VALID), m_replaced_transactions(std::move(replaced_txns)), m_vsize{vsize}, m_base_fees(fees), - m_effective_feerate(effective_feerate) {} + 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)