diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h index 8be0a77e00d..e12047debda 100644 --- a/src/kernel/mempool_entry.h +++ b/src/kernel/mempool_entry.h @@ -128,10 +128,6 @@ public: size_t DynamicMemoryUsage() const { return nUsageSize; } const LockPoints& GetLockPoints() const { return lockPoints; } - // Adjusts the descendant state. - void UpdateDescendantState(int32_t modifySize, CAmount modifyFee, int64_t modifyCount); - // Adjusts the ancestor state - void UpdateAncestorState(int32_t modifySize, CAmount modifyFee, int64_t modifyCount, int64_t modifySigOps); // Updates the modified fees with descendants/ancestors. void UpdateModifiedFee(CAmount fee_diff) { diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 960296e2d8c..233e34210c7 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -54,70 +54,16 @@ bool TestLockPointValidity(CChain& active_chain, const LockPoints& lp) return true; } -void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap& cachedDescendants, - const std::set& setExclude) -{ - CTxMemPoolEntry::Children stageEntries, descendants; - stageEntries = updateIt->GetMemPoolChildrenConst(); - - while (!stageEntries.empty()) { - const CTxMemPoolEntry& descendant = *stageEntries.begin(); - descendants.insert(descendant); - stageEntries.erase(descendant); - const CTxMemPoolEntry::Children& children = descendant.GetMemPoolChildrenConst(); - for (const CTxMemPoolEntry& childEntry : children) { - cacheMap::iterator cacheIt = cachedDescendants.find(mapTx.iterator_to(childEntry)); - if (cacheIt != cachedDescendants.end()) { - // We've already calculated this one, just add the entries for this set - // but don't traverse again. - for (txiter cacheEntry : cacheIt->second) { - descendants.insert(*cacheEntry); - } - } else if (!descendants.count(childEntry)) { - // Schedule for later processing - stageEntries.insert(childEntry); - } - } - } - // descendants now contains all in-mempool descendants of updateIt. - // Update and add to cached descendant map - int32_t modifySize = 0; - CAmount modifyFee = 0; - int64_t modifyCount = 0; - for (const CTxMemPoolEntry& descendant : descendants) { - if (!setExclude.count(descendant.GetTx().GetHash())) { - modifySize += descendant.GetTxSize(); - modifyFee += descendant.GetModifiedFee(); - modifyCount++; - cachedDescendants[updateIt].insert(mapTx.iterator_to(descendant)); - // Update ancestor state for each descendant - mapTx.modify(mapTx.iterator_to(descendant), [=](CTxMemPoolEntry& e) { - e.UpdateAncestorState(updateIt->GetTxSize(), updateIt->GetModifiedFee(), 1, updateIt->GetSigOpCost()); - }); - } - } - mapTx.modify(updateIt, [=](CTxMemPoolEntry& e) { e.UpdateDescendantState(modifySize, modifyFee, modifyCount); }); -} - void CTxMemPool::UpdateTransactionsFromBlock(const std::vector& vHashesToUpdate) { AssertLockHeld(cs); - // For each entry in vHashesToUpdate, store the set of in-mempool, but not - // in-vHashesToUpdate transactions, so that we don't have to recalculate - // descendants when we come across a previously seen entry. - cacheMap mapMemPoolDescendantsToUpdate; // 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()); - std::set descendants_to_remove; - // Iterate in reverse, so that whenever we are looking at a transaction // we are sure that all in-mempool descendants have already been processed. - // This maximizes the benefit of the descendant cache and guarantees that - // CTxMemPoolEntry::m_children will be updated, an assumption made in - // UpdateForDescendants. for (const Txid& hash : vHashesToUpdate | std::views::reverse) { // calculate children from mapNextTx txiter it = mapTx.find(hash); @@ -143,22 +89,13 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector& vHashesToU // block and transactions that were in the mempool to txgraph. m_txgraph->AddDependency(/*parent=*/*it, /*child=*/*childIter); } - } // release epoch guard for UpdateForDescendants - UpdateForDescendants(it, mapMemPoolDescendantsToUpdate, setAlreadyIncluded); + } } auto txs_to_remove = m_txgraph->Trim(); // Enforce cluster size limits. for (auto txptr : txs_to_remove) { const CTxMemPoolEntry& entry = *(static_cast(txptr)); - descendants_to_remove.insert(entry.GetTx().GetHash()); - } - - for (const auto& txid : descendants_to_remove) { - // This txid may have been removed already in a prior call to removeRecursive. - // Therefore we ensure it is not yet removed already. - if (const std::optional txiter = GetIter(txid)) { - removeRecursive((*txiter)->GetTx(), MemPoolRemovalReason::SIZELIMIT); - } + removeUnchecked(mapTx.iterator_to(entry), MemPoolRemovalReason::SIZELIMIT); } } @@ -265,33 +202,13 @@ CTxMemPool::setEntries CTxMemPool::AssumeCalculateMemPoolAncestors( return std::move(result).value_or(CTxMemPool::setEntries{}); } -void CTxMemPool::UpdateAncestorsOf(bool add, txiter it, setEntries &setAncestors) +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); } - const int32_t updateCount = (add ? 1 : -1); - const int32_t updateSize{updateCount * it->GetTxSize()}; - const CAmount updateFee = updateCount * it->GetModifiedFee(); - for (txiter ancestorIt : setAncestors) { - mapTx.modify(ancestorIt, [=](CTxMemPoolEntry& e) { e.UpdateDescendantState(updateSize, updateFee, updateCount); }); - } -} - -void CTxMemPool::UpdateEntryForAncestors(txiter it, const setEntries &setAncestors) -{ - int64_t updateCount = setAncestors.size(); - int64_t updateSize = 0; - CAmount updateFee = 0; - int64_t updateSigOpsCost = 0; - for (txiter ancestorIt : setAncestors) { - updateSize += ancestorIt->GetTxSize(); - updateFee += ancestorIt->GetModifiedFee(); - updateSigOpsCost += ancestorIt->GetSigOpCost(); - } - mapTx.modify(it, [=](CTxMemPoolEntry& e){ e.UpdateAncestorState(updateSize, updateFee, updateCount, updateSigOpsCost); }); } void CTxMemPool::UpdateChildrenForRemoval(txiter it) @@ -304,52 +221,10 @@ void CTxMemPool::UpdateChildrenForRemoval(txiter it) void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove, bool updateDescendants) { - // For each entry, walk back all ancestors and decrement size associated with this - // transaction - if (updateDescendants) { - // updateDescendants should be true whenever we're not recursively - // removing a tx and all its descendants, eg when a transaction is - // confirmed in a block. - // Here we only update statistics and not data in CTxMemPool::Parents - // and CTxMemPoolEntry::Children (which we need to preserve until we're - // finished with all operations that need to traverse the mempool). - for (txiter removeIt : entriesToRemove) { - setEntries setDescendants; - CalculateDescendants(removeIt, setDescendants); - setDescendants.erase(removeIt); // don't update state for self - int32_t modifySize = -removeIt->GetTxSize(); - CAmount modifyFee = -removeIt->GetModifiedFee(); - int modifySigOps = -removeIt->GetSigOpCost(); - for (txiter dit : setDescendants) { - mapTx.modify(dit, [=](CTxMemPoolEntry& e){ e.UpdateAncestorState(modifySize, modifyFee, -1, modifySigOps); }); - } - } - } for (txiter removeIt : entriesToRemove) { - const CTxMemPoolEntry &entry = *removeIt; - // Since this is a tx that is already in the mempool, we can call CMPA - // with fSearchForParents = false. If the mempool is in a consistent - // state, then using true or false should both be correct, though false - // should be a bit faster. - // However, if we happen to be in the middle of processing a reorg, then - // the mempool can be in an inconsistent state. In this case, the set - // of ancestors reachable via GetMemPoolParents()/GetMemPoolChildren() - // will be the same as the set of ancestors whose packages include this - // transaction, because when we add a new transaction to the mempool in - // addNewTransaction(), we assume it has no children, and in the case of a - // reorg where that assumption is false, the in-mempool children aren't - // linked to the in-block tx's until UpdateTransactionsFromBlock() is - // called. - // So if we're being called during a reorg, ie before - // UpdateTransactionsFromBlock() has been called, then - // GetMemPoolParents()/GetMemPoolChildren() will differ from the set of - // mempool parents we'd calculate by searching, and it's important that - // we use the cached notion of ancestor transactions as the set of - // things to update for removal. - auto ancestors{AssumeCalculateMemPoolAncestors(__func__, entry, Limits::NoLimits(), /*fSearchForParents=*/false)}; // Note that UpdateAncestorsOf severs the child links that point to // removeIt in the entries for the parents of removeIt. - UpdateAncestorsOf(false, removeIt, ancestors); + 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 @@ -359,15 +234,6 @@ void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove, b } } -void CTxMemPoolEntry::UpdateDescendantState(int32_t modifySize, CAmount modifyFee, int64_t modifyCount) -{ -} - -void CTxMemPoolEntry::UpdateAncestorState(int32_t modifySize, CAmount modifyFee, int64_t modifyCount, int64_t modifySigOps) -{ -} - -//! Clamp option values and populate the error if options are not valid. static CTxMemPool::Options&& Flatten(CTxMemPool::Options&& opts, bilingual_str& error) { opts.check_ratio = std::clamp(opts.check_ratio, 0, 1'000'000); @@ -470,8 +336,7 @@ void CTxMemPool::addNewTransaction(CTxMemPool::txiter newit, CTxMemPool::setEntr for (const auto& pit : GetIterSet(setParentTransactions)) { UpdateParent(newit, pit, true); } - UpdateAncestorsOf(true, newit, setAncestors); - UpdateEntryForAncestors(newit, setAncestors); + UpdateAncestorsOf(true, newit); nTransactionsUpdated++; totalTxSize += entry.GetTxSize(); @@ -823,18 +688,6 @@ void CTxMemPool::PrioritiseTransaction(const Txid& hash, const CAmount& nFeeDelt // transaction fee to be current modified fee + feedelta. mapTx.modify(it, [&nFeeDelta](CTxMemPoolEntry& e) { e.UpdateModifiedFee(nFeeDelta); }); m_txgraph->SetTransactionFee(*it, it->GetModifiedFee()); - // Now update all ancestors' modified fees with descendants - auto ancestors{AssumeCalculateMemPoolAncestors(__func__, *it, Limits::NoLimits(), /*fSearchForParents=*/false)}; - for (txiter ancestorIt : ancestors) { - mapTx.modify(ancestorIt, [=](CTxMemPoolEntry& e){ e.UpdateDescendantState(0, nFeeDelta, 0);}); - } - // Now update all descendants' modified fees with ancestors - setEntries setDescendants; - CalculateDescendants(it, setDescendants); - setDescendants.erase(it); - for (txiter descendantIt : setDescendants) { - mapTx.modify(descendantIt, [=](CTxMemPoolEntry& e){ e.UpdateAncestorState(0, nFeeDelta, 0, 0); }); - } ++nTransactionsUpdated; } if (delta == 0) { diff --git a/src/txmempool.h b/src/txmempool.h index 9a4389c366f..b5da8c4cd37 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -350,7 +350,6 @@ public: */ void check(const CCoinsViewCache& active_coins_tip, int64_t spendheight) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main); - void removeRecursive(const CTransaction& tx, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs); /** After reorg, filter the entries that would no longer be valid in the next block, and update * the entries' cached LockPoints if needed. The mempool does not have any knowledge of @@ -639,35 +638,8 @@ private: */ void RemoveStaged(setEntries& stage, bool updateDescendants, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs); - /** UpdateForDescendants is used by UpdateTransactionsFromBlock to update - * the descendants for a single transaction that has been added to the - * mempool but may have child transactions in the mempool, eg during a - * chain reorg. - * - * @pre CTxMemPoolEntry::m_children is correct for the given tx and all - * descendants. - * @pre cachedDescendants is an accurate cache where each entry has all - * descendants of the corresponding key, including those that should - * be removed for violation of ancestor limits. - * @post if updateIt has any non-excluded descendants, cachedDescendants has - * a new cache line for updateIt. - * - * @param[in] updateIt the entry to update for its descendants - * @param[in,out] cachedDescendants a cache where each line corresponds to all - * descendants. It will be updated with the descendants of the transaction - * being updated, so that future invocations don't need to walk the same - * transaction again, if encountered in another transaction chain. - * @param[in] setExclude the set of descendant transactions in the mempool - * that must not be accounted for (because any descendants in setExclude - * were added to the mempool after the transaction being updated and hence - * their state is already reflected in the parent state). - */ - void UpdateForDescendants(txiter updateIt, cacheMap& cachedDescendants, - const std::set& setExclude) EXCLUSIVE_LOCKS_REQUIRED(cs); /** Update ancestors of hash to add/remove it as a descendant transaction. */ - void UpdateAncestorsOf(bool add, txiter hash, setEntries &setAncestors) EXCLUSIVE_LOCKS_REQUIRED(cs); - /** Set ancestor state for an entry */ - void UpdateEntryForAncestors(txiter it, const setEntries &setAncestors) EXCLUSIVE_LOCKS_REQUIRED(cs); + 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. */