diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h index e12047debda..b274e1f7eee 100644 --- a/src/kernel/mempool_entry.h +++ b/src/kernel/mempool_entry.h @@ -67,16 +67,11 @@ class CTxMemPoolEntry : public TxGraph::Ref { public: typedef std::reference_wrapper CTxMemPoolEntryRef; - // two aliases, should the types ever diverge - typedef std::set Parents; - typedef std::set Children; private: CTxMemPoolEntry(const CTxMemPoolEntry&) = delete; const CTransactionRef tx; - mutable Parents m_parents; - mutable Children m_children; const CAmount nFee; //!< Cached to avoid expensive parent-transaction lookups const int32_t nTxWeight; //!< ... and avoid recomputing tx weight (also used for GetTxSize()) const size_t nUsageSize; //!< ... and total memory usage @@ -142,11 +137,6 @@ public: bool GetSpendsCoinbase() const { return spendsCoinbase; } - const Parents& GetMemPoolParentsConst() const { return m_parents; } - const Children& GetMemPoolChildrenConst() const { return m_children; } - Parents& GetMemPoolParents() const { return m_parents; } - Children& GetMemPoolChildren() const { return m_children; } - mutable size_t idx_randomized; //!< Index in mempool's txns_randomized mutable Epoch::Marker m_epoch_marker; //!< epoch when last touched, useful for graph algorithms }; diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 53d92b8d9ea..64acf95223b 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -90,10 +90,6 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector& vHashesToU { AssertLockHeld(cs); - // Use a set for lookups into vHashesToUpdate (these entries are already - // accounted for in the state of their ancestors) - std::set setAlreadyIncluded(vHashesToUpdate.begin(), vHashesToUpdate.end()); - // Iterate in reverse, so that whenever we are looking at a transaction // we are sure that all in-mempool descendants have already been processed. for (const Txid& hash : vHashesToUpdate | std::views::reverse) { @@ -103,20 +99,10 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector& vHashesToU continue; } auto iter = mapNextTx.lower_bound(COutPoint(hash, 0)); - // First calculate the children, and update CTxMemPoolEntry::m_children to - // include them, and update their CTxMemPoolEntry::m_parents to include this tx. - // we cache the in-mempool children to avoid duplicate updates { - WITH_FRESH_EPOCH(m_epoch); for (; iter != mapNextTx.end() && iter->first->hash == hash; ++iter) { txiter childIter = iter->second; assert(childIter != mapTx.end()); - // We can skip updating entries we've encountered before or that - // are in the block (which are already accounted for). - if (!visited(childIter) && !setAlreadyIncluded.count(iter->second->GetTx().GetHash())) { - UpdateChild(it, childIter, true); - UpdateParent(childIter, it, true); - } // Add dependencies that are discovered between transactions in the // block and transactions that were in the mempool to txgraph. m_txgraph->AddDependency(/*parent=*/*it, /*child=*/*childIter); @@ -158,8 +144,6 @@ CTxMemPool::setEntries CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEnt const CTransaction &tx = entry.GetTx(); // Get parents of this transaction that are in the mempool - // GetMemPoolParents() is only valid for entries in the mempool, so we - // iterate mapTx to find parents. for (unsigned int i = 0; i < tx.vin.size(); i++) { std::optional piter = GetIter(tx.vin[i].prevout.hash); if (piter) { @@ -177,38 +161,6 @@ CTxMemPool::setEntries CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEnt return ret; } -void CTxMemPool::UpdateAncestorsOf(bool add, txiter it) -{ - const CTxMemPoolEntry::Parents& parents = it->GetMemPoolParentsConst(); - // add or remove this tx as a child of each parent - for (const CTxMemPoolEntry& parent : parents) { - UpdateChild(mapTx.iterator_to(parent), it, add); - } -} - -void CTxMemPool::UpdateChildrenForRemoval(txiter it) -{ - const CTxMemPoolEntry::Children& children = it->GetMemPoolChildrenConst(); - for (const CTxMemPoolEntry& updateIt : children) { - UpdateParent(mapTx.iterator_to(updateIt), it, false); - } -} - -void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove, bool updateDescendants) -{ - for (txiter removeIt : entriesToRemove) { - // Note that UpdateAncestorsOf severs the child links that point to - // removeIt in the entries for the parents of removeIt. - UpdateAncestorsOf(false, removeIt); - } - // After updating all the ancestor sizes, we can now sever the link between each - // transaction being removed and any mempool children (ie, update CTxMemPoolEntry::m_parents - // for each direct child of a transaction being removed). - for (txiter removeIt : entriesToRemove) { - UpdateChildrenForRemoval(removeIt); - } -} - static CTxMemPool::Options&& Flatten(CTxMemPool::Options&& opts, bilingual_str& error) { opts.check_ratio = std::clamp(opts.check_ratio, 0, 1'000'000); @@ -271,10 +223,8 @@ void CTxMemPool::addNewTransaction(CTxMemPool::txiter newit) cachedInnerUsage += entry.DynamicMemoryUsage(); const CTransaction& tx = newit->GetTx(); - std::set setParentTransactions; for (unsigned int i = 0; i < tx.vin.size(); i++) { mapNextTx.insert(std::make_pair(&tx.vin[i].prevout, newit)); - setParentTransactions.insert(tx.vin[i].prevout.hash); } // Don't bother worrying about child transactions of this one. // Normal case of a new transaction arriving is that there can't be any @@ -283,12 +233,6 @@ void CTxMemPool::addNewTransaction(CTxMemPool::txiter newit) // In that case, our disconnect block logic will call UpdateTransactionsFromBlock // to clean up the mess we're leaving here. - // Update ancestors with information about this tx - for (const auto& pit : GetIterSet(setParentTransactions)) { - UpdateParent(newit, pit, true); - } - UpdateAncestorsOf(true, newit); - nTransactionsUpdated++; totalTxSize += entry.GetTxSize(); m_total_fee += entry.GetFee(); @@ -344,7 +288,6 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason) totalTxSize -= it->GetTxSize(); m_total_fee -= it->GetFee(); cachedInnerUsage -= it->DynamicMemoryUsage(); - cachedInnerUsage -= memusage::DynamicUsage(it->GetMemPoolParentsConst()) + memusage::DynamicUsage(it->GetMemPoolChildrenConst()); mapTx.erase(it); nTransactionsUpdated++; } @@ -483,8 +426,8 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei check_total_fee += it->GetFee(); innerUsage += it->DynamicMemoryUsage(); const CTransaction& tx = it->GetTx(); - innerUsage += memusage::DynamicUsage(it->GetMemPoolParentsConst()) + memusage::DynamicUsage(it->GetMemPoolChildrenConst()); - CTxMemPoolEntry::Parents setParentCheck; + std::set setParentCheck; + std::set setParentsStored; for (const CTxIn &txin : tx.vin) { // Check that every mempool transaction's inputs refer to available coins, or other mempool tx's. indexed_transaction_set::const_iterator it2 = mapTx.find(txin.prevout.hash); @@ -507,19 +450,26 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei auto comp = [](const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) -> bool { return a.GetTx().GetHash() == b.GetTx().GetHash(); }; - assert(setParentCheck.size() == it->GetMemPoolParentsConst().size()); - assert(std::equal(setParentCheck.begin(), setParentCheck.end(), it->GetMemPoolParentsConst().begin(), comp)); + for (auto &txentry : GetParents(*it)) { + setParentsStored.insert(dynamic_cast(txentry.get())); + } + assert(setParentCheck.size() == setParentsStored.size()); + assert(std::equal(setParentCheck.begin(), setParentCheck.end(), setParentsStored.begin(), comp)); // Check children against mapNextTx - CTxMemPoolEntry::Children setChildrenCheck; + std::set setChildrenCheck; + std::set setChildrenStored; auto iter = mapNextTx.lower_bound(COutPoint(it->GetTx().GetHash(), 0)); for (; iter != mapNextTx.end() && iter->first->hash == it->GetTx().GetHash(); ++iter) { txiter childit = iter->second; assert(childit != mapTx.end()); // mapNextTx points to in-mempool transactions setChildrenCheck.insert(*childit); } - assert(setChildrenCheck.size() == it->GetMemPoolChildrenConst().size()); - assert(std::equal(setChildrenCheck.begin(), setChildrenCheck.end(), it->GetMemPoolChildrenConst().begin(), comp)); + for (auto &txentry : GetChildren(*it)) { + setChildrenStored.insert(dynamic_cast(txentry.get())); + } + assert(setChildrenCheck.size() == setChildrenStored.size()); + assert(std::equal(setChildrenCheck.begin(), setChildrenCheck.end(), setChildrenStored.begin(), comp)); TxValidationState dummy_state; // Not used. CheckTxInputs() should always pass CAmount txfee = 0; @@ -777,7 +727,6 @@ void CTxMemPool::RemoveUnbroadcastTx(const Txid& txid, const bool unchecked) { void CTxMemPool::RemoveStaged(setEntries &stage, bool updateDescendants, MemPoolRemovalReason reason) { AssertLockHeld(cs); - UpdateForRemoveFromMempool(stage, updateDescendants); for (txiter it : stage) { removeUnchecked(it, reason); } @@ -812,28 +761,6 @@ int CTxMemPool::Expire(std::chrono::seconds time) return stage.size(); } -void CTxMemPool::UpdateChild(txiter entry, txiter child, bool add) -{ - AssertLockHeld(cs); - CTxMemPoolEntry::Children s; - if (add && entry->GetMemPoolChildren().insert(*child).second) { - cachedInnerUsage += memusage::IncrementalDynamicUsage(s); - } else if (!add && entry->GetMemPoolChildren().erase(*child)) { - cachedInnerUsage -= memusage::IncrementalDynamicUsage(s); - } -} - -void CTxMemPool::UpdateParent(txiter entry, txiter parent, bool add) -{ - AssertLockHeld(cs); - CTxMemPoolEntry::Parents s; - if (add && entry->GetMemPoolParents().insert(*parent).second) { - cachedInnerUsage += memusage::IncrementalDynamicUsage(s); - } else if (!add && entry->GetMemPoolParents().erase(*parent)) { - cachedInnerUsage -= memusage::IncrementalDynamicUsage(s); - } -} - CFeeRate CTxMemPool::GetMinFee(size_t sizelimit) const { LOCK(cs); if (!blockSinceLastRollingFeeBump || rollingMinimumFeeRate == 0) @@ -900,7 +827,6 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector* pvNoSpends for (auto ref : worst_chunk) { stage.insert(mapTx.iterator_to(static_cast(*ref))); } - UpdateForRemoveFromMempool(stage, false); for (auto e : stage) { removeUnchecked(e, MemPoolRemovalReason::SIZELIMIT); } diff --git a/src/txmempool.h b/src/txmempool.h index 0c0ec241888..1891c8d48a7 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -296,9 +296,6 @@ private: typedef std::map cacheMap; - void UpdateParent(txiter entry, txiter parent, bool add) EXCLUSIVE_LOCKS_REQUIRED(cs); - void UpdateChild(txiter entry, txiter child, bool add) EXCLUSIVE_LOCKS_REQUIRED(cs); - std::vector GetSortedScoreWithTopology() const EXCLUSIVE_LOCKS_REQUIRED(cs); /** @@ -584,15 +581,6 @@ private: */ void RemoveStaged(setEntries& stage, bool updateDescendants, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs); - /** Update ancestors of hash to add/remove it as a descendant transaction. */ - void UpdateAncestorsOf(bool add, txiter hash) EXCLUSIVE_LOCKS_REQUIRED(cs); - /** For each transaction being removed, update ancestors and any direct children. - * If updateDescendants is true, then also update in-mempool descendants' - * ancestor state. */ - void UpdateForRemoveFromMempool(const setEntries &entriesToRemove, bool updateDescendants) EXCLUSIVE_LOCKS_REQUIRED(cs); - /** Sever link between specified transaction and direct children. */ - void UpdateChildrenForRemoval(txiter entry) EXCLUSIVE_LOCKS_REQUIRED(cs); - /** Before calling removeUnchecked for a given transaction, * UpdateForRemoveFromMempool must be called on the entire (dependent) set * of transactions being removed at the same time. We use each