From 79bcc5ca0679daf6e57fc4d7ce2244262a7cfd13 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 enable 25380 even after this, vice versa. assumed as being mined thanks to solely its own feerate when it's not". --- src/kernel/mempool_entry.h | 12 +++++++++ src/policy/fees.cpp | 8 +++--- src/policy/fees.h | 4 +-- src/test/fuzz/policy_estimator.cpp | 26 +++++++++++++++--- 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, 95 insertions(+), 30 deletions(-) diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h index eb201cc333a75..575ead55fb722 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 d2c2ef6e3e781..fc723e4692c70 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -638,7 +638,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) { @@ -667,8 +667,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++; } @@ -1050,7 +1050,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 52d8712c1fe55..a496712567cf4 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 835bbd67de588..72644c854373e 100644 --- a/src/test/fuzz/policy_estimator.cpp +++ b/src/test/fuzz/policy_estimator.cpp @@ -43,8 +43,18 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator) } const CTransaction tx{*mtx}; const CTxMemPoolEntry entry = ConsumeTxMemPoolEntry(fuzzed_data_provider, tx); - const NewMempoolTransactionInfo tx_info = {entry.GetSharedTx(), entry.GetFee(), entry.GetTxSize(), entry.GetHeight(), - fuzzed_data_provider.ConsumeBool(), fuzzed_data_provider.ConsumeBool(), fuzzed_data_provider.ConsumeBool(), fuzzed_data_provider.ConsumeBool()}; + NewMempoolTransactionInfo tx_info = { + entry.GetSharedTx(), + entry.GetMemPoolParentsCopy(), + entry.GetFee(), + entry.GetTxSize(), + entry.GetHeight(), + entry.GetSizeWithAncestors(), + entry.GetModFeesWithAncestors(), + fuzzed_data_provider.ConsumeBool(), + fuzzed_data_provider.ConsumeBool(), + fuzzed_data_provider.ConsumeBool(), + 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()); @@ -60,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; + tx_info.m_tx = mempool_entry.GetSharedTx(); + tx_info.m_parents = mempool_entry.GetMemPoolParentsCopy(); + tx_info.m_fee = mempool_entry.GetFee(); + tx_info.m_virtual_transaction_size = mempool_entry.GetTxSize(); + tx_info.txHeight = mempool_entry.GetHeight(); + tx_info.nSizeWithAncestors = mempool_entry.GetSizeWithAncestors(); + tx_info.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 377cdfd852503..2373b57917a8e 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()); + NewMempoolTransactionInfo tx_info; + tx_info.m_tx = MakeTransactionRef(tx); + tx_info.m_fee = feeV[j]; + tx_info.m_virtual_transaction_size = virtual_size; + tx_info.txHeight = entry.nHeight; + tx_info.m_from_disconnected_block = false; + tx_info.m_submitted_in_package = false; + tx_info.m_chainstate_is_current = true; + tx_info.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()); + NewMempoolTransactionInfo tx_info; + tx_info.m_tx = MakeTransactionRef(tx); + tx_info.m_fee = feeV[j]; + tx_info.m_virtual_transaction_size = virtual_size; + tx_info.txHeight = entry.nHeight; + tx_info.m_from_disconnected_block = false; + tx_info.m_submitted_in_package = false; + tx_info.m_chainstate_is_current = true; + tx_info.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()); + NewMempoolTransactionInfo tx_info; + tx_info.m_tx = MakeTransactionRef(tx); + tx_info.m_fee = feeV[j]; + tx_info.m_virtual_transaction_size = virtual_size; + tx_info.txHeight = entry.nHeight; + tx_info.m_from_disconnected_block = false; + tx_info.m_submitted_in_package = false; + tx_info.m_chainstate_is_current = true; + tx_info.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 dd55ed983e09d..ddc66027fd8ce 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; + tx_info.m_tx = it->GetSharedTx(); + tx_info.m_parents = it->GetMemPoolParentsCopy(); + tx_info.m_fee = it->GetFee(); + tx_info.m_virtual_transaction_size = it->GetTxSize(); + tx_info.txHeight = it->GetHeight(); + tx_info.nSizeWithAncestors = it->GetSizeWithAncestors(); + tx_info.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 22884b46e62c8..033fbf23b9155 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; NewMempoolTransactionInfo tx_info = { ws.m_ptx, + ws.m_entry->GetMemPoolParentsCopy(), ws.m_base_fees, ws.m_vsize, ws.m_entry->GetHeight(), + ws.m_entry->GetSizeWithAncestors(), + ws.m_entry->GetModFeesWithAncestors(), args.m_bypass_limits, args.m_package_submission, IsCurrentForFeeEstimation(m_active_chainstate), @@ -1251,9 +1254,12 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef const CTransaction& tx = *ws.m_ptx; NewMempoolTransactionInfo tx_info = { ws.m_ptx, + ws.m_entry->GetMemPoolParentsCopy(), ws.m_base_fees, ws.m_vsize, ws.m_entry->GetHeight(), + ws.m_entry->GetSizeWithAncestors(), + ws.m_entry->GetModFeesWithAncestors(), args.m_bypass_limits, args.m_package_submission, IsCurrentForFeeEstimation(m_active_chainstate), diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index 416fe3f40276e..a212745661572 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 b49a72c696fd1..46a544623d12f 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 &);