diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp index e77fcecf378..244afb91619 100644 --- a/src/test/txpackage_tests.cpp +++ b/src/test/txpackage_tests.cpp @@ -95,12 +95,14 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup) 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(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(result_parent_child.m_package_feerate.has_value()); 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); // A single, giant transaction submitted through ProcessNewPackage fails on single tx policy. CTransactionRef giant_ptx = create_placeholder_tx(999, 999); @@ -323,8 +325,10 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup) auto it_child = submit_parent_child.m_tx_results.find(tx_child->GetWitnessHash()); 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(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(m_node.mempool->size(), expected_pool_size); BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_parent->GetHash()))); @@ -613,6 +617,8 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup) BOOST_CHECK_MESSAGE(mixed_result.m_package_feerate.value() == expected_feerate, strprintf("Expected package feerate %s, got %s", expected_feerate.ToString(), 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); } } @@ -695,6 +701,8 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) 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); 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, @@ -756,6 +764,16 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) BOOST_CHECK_MESSAGE(submit_prioritised_package.m_package_feerate.value() == expected_feerate, strprintf("Expected package feerate %s, got %s", expected_feerate.ToString(), submit_prioritised_package.m_package_feerate.value().ToString())); + 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() == 0); + 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() == 200); + BOOST_CHECK(it_child->second.m_effective_feerate.value() == expected_feerate); } // Package feerate is calculated without topology in mind; it's just aggregating fees and sizes. @@ -801,6 +819,7 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) 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(m_node.mempool->size(), expected_pool_size); BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_parent_rich->GetHash()))); diff --git a/src/validation.cpp b/src/validation.cpp index 9845e8ddc95..8ae4a3ecb80 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -583,6 +583,11 @@ private: /** Total virtual size of all transactions being replaced. */ size_t m_conflicting_size{0}; + /** If we're doing package validation (i.e. m_package_feerates=true), the "effective" + * package feerate of this transaction is the total fees divided by the total size of + * transactions (which may include its ancestors and/or descendants). */ + CFeeRate m_package_feerate{0}; + const CTransactionRef& m_ptx; /** Txid. */ const uint256& m_hash; @@ -1147,9 +1152,11 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector& // 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)}; 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)); + MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, ws.m_base_fees, effective_feerate)); GetMainSignals().TransactionAddedToMempool(ws.m_ptx, m_pool.GetAndIncrementSequence()); } else { all_submitted = false; @@ -1177,16 +1184,17 @@ 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)}; // 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); + return MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, ws.m_base_fees, effective_feerate); } 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); + return MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, ws.m_base_fees, effective_feerate); } PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::vector& txns, ATMPArgs& args) @@ -1245,6 +1253,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: } for (Workspace& ws : workspaces) { + ws.m_package_feerate = package_feerate; if (!PolicyScriptChecks(args, ws)) { // Exit early to avoid doing pointless work. Update the failed tx result; the rest are unfinished. package_state.Invalid(PackageValidationResult::PCKG_TX, "transaction failed"); @@ -1252,11 +1261,13 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: return PackageMempoolAcceptResult(package_state, package_feerate, std::move(results)); } 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). results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), - ws.m_vsize, ws.m_base_fees)); + ws.m_vsize, ws.m_base_fees, effective_feerate)); } } diff --git a/src/validation.h b/src/validation.h index 63ff9247bed..8701326277d 100644 --- a/src/validation.h +++ b/src/validation.h @@ -134,6 +134,11 @@ struct MempoolAcceptResult { const std::optional m_vsize; /** Raw base fees in satoshis. */ const std::optional m_base_fees; + /** 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. */ + const std::optional m_effective_feerate; // 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. */ @@ -143,8 +148,11 @@ struct MempoolAcceptResult { return MempoolAcceptResult(state); } - static MempoolAcceptResult Success(std::list&& replaced_txns, int64_t vsize, CAmount fees) { - return MempoolAcceptResult(std::move(replaced_txns), vsize, fees); + 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); } static MempoolAcceptResult MempoolTx(int64_t vsize, CAmount fees) { @@ -164,9 +172,15 @@ private: } /** Constructor for success case */ - explicit MempoolAcceptResult(std::list&& replaced_txns, int64_t vsize, CAmount fees) + explicit MempoolAcceptResult(std::list&& replaced_txns, + int64_t vsize, + CAmount fees, + CFeeRate effective_feerate) : m_result_type(ResultType::VALID), - m_replaced_transactions(std::move(replaced_txns)), m_vsize{vsize}, m_base_fees(fees) {} + m_replaced_transactions(std::move(replaced_txns)), + m_vsize{vsize}, + m_base_fees(fees), + m_effective_feerate(effective_feerate) {} /** Constructor for already-in-mempool case. It wouldn't replace any transactions. */ explicit MempoolAcceptResult(int64_t vsize, CAmount fees)