From 53bbc791d47a51a7c2bbefb32835fd4001b68890 Mon Sep 17 00:00:00 2001 From: ismaelsadeeq Date: Wed, 13 Sep 2023 08:57:17 +0100 Subject: [PATCH] mempool, CValidationInterface: modify `MempoolTransactionsRemovedForConnectedBlock` parameter Add `parents`, `nSizeWithAncestors`, and `nModFeesWithAncestors` entries to `NewMempoolTransactionInfo`. This commit then changes `MempoolTransactionsRemovedForConnectedBlock` parameter `txs_removed_for_block` type to `NewMempoolTransactionInfo`. This added info will fix merge conflict with 25380. --- src/kernel/mempool_entry.h | 12 +++++++++ src/policy/fees.cpp | 8 +++--- src/policy/fees.h | 4 +-- src/test/fuzz/policy_estimator.cpp | 18 ++++++++++--- src/test/policyestimator_tests.cpp | 42 +++++++++++++++++++++--------- src/txmempool.cpp | 21 +++++++++++---- src/validation.cpp | 6 +++++ src/validationinterface.cpp | 2 +- src/validationinterface.h | 4 +-- 9 files changed, 87 insertions(+), 30 deletions(-) diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h index eb201cc333a75d..575ead55fb7221 100644 --- a/src/kernel/mempool_entry.h +++ b/src/kernel/mempool_entry.h @@ -170,6 +170,15 @@ class CTxMemPoolEntry const Parents& GetMemPoolParentsConst() const { return m_parents; } const Children& GetMemPoolChildrenConst() const { return m_children; } Parents& GetMemPoolParents() const { return m_parents; } + std::vector GetMemPoolParentsCopy() const + { + std::vector parents; + parents.reserve(m_parents.size()); + for (const auto& parent : m_parents) { + parents.push_back(parent.get().GetSharedTx()); + } + return parents; + } Children& GetMemPoolChildren() const { return m_children; } mutable size_t vTxHashesIdx; //!< Index in mempool's vTxHashes @@ -178,6 +187,7 @@ class CTxMemPoolEntry struct NewMempoolTransactionInfo { CTransactionRef m_tx; + std::vector m_parents; /* The fee the added transaction paid */ CAmount m_fee; /** @@ -191,6 +201,8 @@ struct NewMempoolTransactionInfo { */ int64_t m_virtual_transaction_size; unsigned int txHeight; + int64_t nSizeWithAncestors; + CAmount nModFeesWithAncestors; bool m_from_disconnected_block; bool m_submitted_in_package; bool m_chainstate_is_current; diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index fa4d56243c3a1a..ffd9154f17b923 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -637,7 +637,7 @@ bool CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTra } void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight, - const std::vector& txs_removed_for_block) + const std::vector& txs_removed_for_block) { LOCK(m_cs_fee_estimator); if (nBlockHeight <= nBestSeenHeight) { @@ -666,8 +666,8 @@ void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight, unsigned int countedTxs = 0; // Update averages with data points from current block - for (const auto& tx : txs_removed_for_block) { - if (processBlockTx(nBlockHeight, tx)) + for (const auto& tx_info : txs_removed_for_block) { + if (processBlockTx(nBlockHeight, tx_info.m_tx)) countedTxs++; } @@ -1049,7 +1049,7 @@ void CBlockPolicyEstimator::TransactionRemovedFromMempool(const CTransactionRef& removeTx(tx->GetHash(), false); } -void CBlockPolicyEstimator::MempoolTransactionsRemovedForConnectedBlock(const std::vector& txs_removed_for_block, unsigned int nBlockHeight) +void CBlockPolicyEstimator::MempoolTransactionsRemovedForConnectedBlock(const std::vector& txs_removed_for_block, unsigned int nBlockHeight) { processBlock(nBlockHeight, txs_removed_for_block); } diff --git a/src/policy/fees.h b/src/policy/fees.h index 52d8712c1fe554..a496712567cf45 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -205,7 +205,7 @@ class CBlockPolicyEstimator : public CValidationInterface /** Process all the transactions that have been included in a block */ void processBlock(unsigned int nBlockHeight, - const std::vector& txs_removed_for_block) + const std::vector& txs_removed_for_block) EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator); /** Process a transaction accepted to the mempool*/ @@ -269,7 +269,7 @@ class CBlockPolicyEstimator : public CValidationInterface EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator); void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) override EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator); - void MempoolTransactionsRemovedForConnectedBlock(const std::vector& txs_removed_for_block, unsigned int nBlockHeight) override + void MempoolTransactionsRemovedForConnectedBlock(const std::vector& txs_removed_for_block, unsigned int nBlockHeight) override EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator); private: diff --git a/src/test/fuzz/policy_estimator.cpp b/src/test/fuzz/policy_estimator.cpp index fb8f0a44d7e316..7d88c9d77d19d7 100644 --- a/src/test/fuzz/policy_estimator.cpp +++ b/src/test/fuzz/policy_estimator.cpp @@ -45,14 +45,16 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator) const CTxMemPoolEntry entry = ConsumeTxMemPoolEntry(fuzzed_data_provider, tx); const NewMempoolTransactionInfo tx_info = { .m_tx = entry.GetSharedTx(), + .m_parents = entry.GetMemPoolParentsCopy(), .m_fee = entry.GetFee(), .m_virtual_transaction_size = entry.GetTxSize(), .txHeight = entry.GetHeight(), + .nSizeWithAncestors = entry.GetSizeWithAncestors(), + .nModFeesWithAncestors = entry.GetModFeesWithAncestors(), .m_from_disconnected_block = fuzzed_data_provider.ConsumeBool(), .m_submitted_in_package = fuzzed_data_provider.ConsumeBool(), .m_chainstate_is_current = fuzzed_data_provider.ConsumeBool(), - .m_has_no_mempool_parents = fuzzed_data_provider.ConsumeBool() - }; + .m_has_no_mempool_parents = fuzzed_data_provider.ConsumeBool()}; block_policy_estimator.processTransaction(tx_info); if (fuzzed_data_provider.ConsumeBool()) { (void)block_policy_estimator.removeTx(tx.GetHash(), /*inBlock=*/fuzzed_data_provider.ConsumeBool()); @@ -68,10 +70,18 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator) const CTransaction tx{*mtx}; mempool_entries.push_back(ConsumeTxMemPoolEntry(fuzzed_data_provider, tx)); } - std::vector txs; + std::vector txs; txs.reserve(mempool_entries.size()); for (const CTxMemPoolEntry& mempool_entry : mempool_entries) { - txs.push_back(mempool_entry.GetSharedTx()); + NewMempoolTransactionInfo tx_info = { + .m_tx = mempool_entry.GetSharedTx(), + .m_parents = mempool_entry.GetMemPoolParentsCopy(), + .m_fee = mempool_entry.GetFee(), + .m_virtual_transaction_size = mempool_entry.GetTxSize(), + .txHeight = mempool_entry.GetHeight(), + .nSizeWithAncestors = mempool_entry.GetSizeWithAncestors(), + .nModFeesWithAncestors = mempool_entry.GetModFeesWithAncestors()}; + txs.push_back(tx_info); } block_policy_estimator.processBlock(fuzzed_data_provider.ConsumeIntegral(), txs); }, diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp index 377cdfd8525032..381349672fe94f 100644 --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -67,16 +67,16 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) // Since TransactionAddedToMempool callbacks are generated in ATMP, // not addUnchecked, we cheat and create one manually here int64_t virtual_size = GetVirtualTransactionSize(*MakeTransactionRef(tx)); - GetMainSignals().TransactionAddedToMempool( - {MakeTransactionRef(tx), - feeV[j], - virtual_size, - entry.nHeight, - false, - false, - true, - true}, - mpool.GetAndIncrementSequence()); + const NewMempoolTransactionInfo tx_info = { + .m_tx = MakeTransactionRef(tx), + .m_fee = feeV[j], + .m_virtual_transaction_size = virtual_size, + .txHeight = entry.nHeight, + .m_from_disconnected_block = false, + .m_submitted_in_package = false, + .m_chainstate_is_current = true, + .m_has_no_mempool_parents = true}; + GetMainSignals().TransactionAddedToMempool(tx_info, mpool.GetAndIncrementSequence()); } txHashes[j].push_back(hash); } @@ -165,7 +165,16 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) // Since TransactionAddedToMempool callbacks are generated in ATMP, // not addUnchecked, we cheat and create one manually here int64_t virtual_size = GetVirtualTransactionSize(*MakeTransactionRef(tx)); - GetMainSignals().TransactionAddedToMempool({MakeTransactionRef(tx), feeV[j], virtual_size, entry.nHeight, false, false, true, true}, mpool.GetAndIncrementSequence()); + const NewMempoolTransactionInfo tx_info = { + .m_tx = MakeTransactionRef(tx), + .m_fee = feeV[j], + .m_virtual_transaction_size = virtual_size, + .txHeight = entry.nHeight, + .m_from_disconnected_block = false, + .m_submitted_in_package = false, + .m_chainstate_is_current = true, + .m_has_no_mempool_parents = true}; + GetMainSignals().TransactionAddedToMempool(tx_info, mpool.GetAndIncrementSequence()); } txHashes[j].push_back(hash); } @@ -221,7 +230,16 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) // Since TransactionAddedToMempool callbacks are generated in ATMP, // not addUnchecked, we cheat and create one manually here int64_t virtual_size = GetVirtualTransactionSize(*MakeTransactionRef(tx)); - GetMainSignals().TransactionAddedToMempool({MakeTransactionRef(tx), feeV[j], virtual_size, entry.nHeight, false, false, true, true}, mpool.GetAndIncrementSequence()); + const NewMempoolTransactionInfo tx_info = { + .m_tx = MakeTransactionRef(tx), + .m_fee = feeV[j], + .m_virtual_transaction_size = virtual_size, + .txHeight = entry.nHeight, + .m_from_disconnected_block = false, + .m_submitted_in_package = false, + .m_chainstate_is_current = true, + .m_has_no_mempool_parents = true}; + GetMainSignals().TransactionAddedToMempool(tx_info, mpool.GetAndIncrementSequence()); } CTransactionRef ptx = mpool.get(hash); if (ptx) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index dd55ed983e09d2..e041a82a1156f9 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -630,21 +630,32 @@ void CTxMemPool::removeConflicts(const CTransaction &tx) void CTxMemPool::removeForBlock(const std::vector& vtx, unsigned int nBlockHeight) { AssertLockHeld(cs); - std::vector txs_removed_for_block; + std::vector txs_removed_for_block; txs_removed_for_block.reserve(vtx.size()); for (const auto& tx : vtx) { txiter it = mapTx.find(tx->GetHash()); if (it != mapTx.end()) { - setEntries stage; - stage.insert(it); - txs_removed_for_block.push_back(tx); - RemoveStaged(stage, true, MemPoolRemovalReason::BLOCK); + NewMempoolTransactionInfo tx_info = { + .m_tx = it->GetSharedTx(), + .m_parents = it->GetMemPoolParentsCopy(), + .m_fee = it->GetFee(), + .m_virtual_transaction_size = it->GetTxSize(), + .txHeight = it->GetHeight(), + .nSizeWithAncestors = it->GetSizeWithAncestors(), + .nModFeesWithAncestors = it->GetModFeesWithAncestors()}; + txs_removed_for_block.push_back(tx_info); } removeConflicts(*tx); ClearPrioritisation(tx->GetHash()); } GetMainSignals().MempoolTransactionsRemovedForConnectedBlock(txs_removed_for_block, nBlockHeight); + for (const auto& tx_info : txs_removed_for_block) { + txiter it = *CHECK_NONFATAL(GetIter(tx_info.m_tx->GetHash())); + setEntries stage; + stage.insert(it); + RemoveStaged(stage, true, MemPoolRemovalReason::BLOCK); + } lastRollingFeeUpdate = GetTime(); blockSinceLastRollingFeeBump = true; } diff --git a/src/validation.cpp b/src/validation.cpp index 208daabc52cd49..1fe431f65bf160 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1209,9 +1209,12 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector& const CTransaction& tx = *ws.m_ptx; const NewMempoolTransactionInfo tx_info = { .m_tx = ws.m_ptx, + .m_parents = ws.m_entry->GetMemPoolParentsCopy(), .m_fee = ws.m_base_fees, .m_virtual_transaction_size = ws.m_vsize, .txHeight = ws.m_entry->GetHeight(), + .nSizeWithAncestors = ws.m_entry->GetSizeWithAncestors(), + .nModFeesWithAncestors = ws.m_entry->GetModFeesWithAncestors(), .m_from_disconnected_block = args.m_bypass_limits, .m_submitted_in_package = args.m_package_submission, .m_chainstate_is_current = IsCurrentForFeeEstimation(m_active_chainstate), @@ -1251,9 +1254,12 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef const CTransaction& tx = *ws.m_ptx; const NewMempoolTransactionInfo tx_info = { .m_tx = ws.m_ptx, + .m_parents = ws.m_entry->GetMemPoolParentsCopy(), .m_fee = ws.m_base_fees, .m_virtual_transaction_size = ws.m_vsize, .txHeight = ws.m_entry->GetHeight(), + .nSizeWithAncestors = ws.m_entry->GetSizeWithAncestors(), + .nModFeesWithAncestors = ws.m_entry->GetModFeesWithAncestors(), .m_from_disconnected_block = args.m_bypass_limits, .m_submitted_in_package = args.m_package_submission, .m_chainstate_is_current = IsCurrentForFeeEstimation(m_active_chainstate), diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index 416fe3f40276eb..a2127456615720 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -216,7 +216,7 @@ void CMainSignals::TransactionAddedToMempool(const NewMempoolTransactionInfo& tx tx_info.m_tx->GetWitnessHash().ToString()); } -void CMainSignals::MempoolTransactionsRemovedForConnectedBlock(const std::vector& txs_removed_for_block, unsigned int nBlockHeight) +void CMainSignals::MempoolTransactionsRemovedForConnectedBlock(const std::vector& txs_removed_for_block, unsigned int nBlockHeight) { auto event = [txs_removed_for_block, nBlockHeight, this] { m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.MempoolTransactionsRemovedForConnectedBlock(txs_removed_for_block, nBlockHeight); }); diff --git a/src/validationinterface.h b/src/validationinterface.h index b49a72c696fd18..46a544623d12fe 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -138,7 +138,7 @@ class CValidationInterface { * * Called on a background thread. */ - virtual void MempoolTransactionsRemovedForConnectedBlock(const std::vector& txs_removed_for_block, unsigned int nBlockHeight) {} + virtual void MempoolTransactionsRemovedForConnectedBlock(const std::vector& txs_removed_for_block, unsigned int nBlockHeight) {} /** * Notifies listeners of a block being connected. * Provides the block that was connected. @@ -212,7 +212,7 @@ class CMainSignals { void UpdatedBlockTip(const CBlockIndex *, const CBlockIndex *, bool fInitialDownload); void TransactionAddedToMempool(const NewMempoolTransactionInfo&, uint64_t mempool_sequence); void TransactionRemovedFromMempool(const CTransactionRef&, MemPoolRemovalReason, uint64_t mempool_sequence); - void MempoolTransactionsRemovedForConnectedBlock(const std::vector&, unsigned int nBlockHeight); + void MempoolTransactionsRemovedForConnectedBlock(const std::vector&, unsigned int nBlockHeight); void BlockConnected(ChainstateRole, const std::shared_ptr &, const CBlockIndex *pindex); void BlockDisconnected(const std::shared_ptr &, const CBlockIndex* pindex); void ChainStateFlushed(ChainstateRole, const CBlockLocator &);