From 944d8bc87690389de9bd51a6ea8a3223d10e4002 Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Sat, 11 Nov 2023 10:21:46 +1000 Subject: [PATCH] BlockAssembler::addPackageTxs - use m_mempool directly --- src/node/miner.cpp | 29 ++++++++++++++--------------- src/node/miner.h | 2 +- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/node/miner.cpp b/src/node/miner.cpp index caa299181933a..579009871f588 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -138,10 +138,7 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc int nPackagesSelected = 0; int nDescendantsUpdated = 0; - if (m_mempool) { - LOCK(m_mempool->cs); - addPackageTxs(*m_mempool, nPackagesSelected, nDescendantsUpdated); - } + addPackageTxs(nPackagesSelected, nDescendantsUpdated); const auto time_1{SteadyClock::now()}; @@ -290,9 +287,11 @@ void BlockAssembler::SortForBlock(const CTxMemPool::setEntries& package, std::ve // Each time through the loop, we compare the best transaction in // mapModifiedTxs with the next transaction in the mempool to decide what // transaction package to work on next. -void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSelected, int& nDescendantsUpdated) +void BlockAssembler::addPackageTxs(int& nPackagesSelected, int& nDescendantsUpdated) { - AssertLockHeld(mempool.cs); + if (m_mempool == nullptr) return; + + LOCK(m_mempool->cs); // mapModifiedTx will store sorted packages after they are modified // because some of their txs are already in the block @@ -300,7 +299,7 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele // Keep track of entries that failed inclusion, to avoid duplicate work CTxMemPool::setEntries failedTx; - CTxMemPool::indexed_transaction_set::index::type::iterator mi = mempool.mapTx.get().begin(); + CTxMemPool::indexed_transaction_set::index::type::iterator mi = m_mempool->mapTx.get().begin(); CTxMemPool::txiter iter; // Limit the number of attempts to add transactions to the block when it is @@ -309,7 +308,7 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele const int64_t MAX_CONSECUTIVE_FAILURES = 1000; int64_t nConsecutiveFailed = 0; - while (mi != mempool.mapTx.get().end() || !mapModifiedTx.empty()) { + while (mi != m_mempool->mapTx.get().end() || !mapModifiedTx.empty()) { // First try to find a new transaction in mapTx to evaluate. // // Skip entries in mapTx that are already in a block or are present @@ -323,9 +322,9 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele // cached size/sigops/fee values that are not actually correct. /** Return true if given transaction from mapTx has already been evaluated, * or if the transaction's cached data in mapTx is incorrect. */ - if (mi != mempool.mapTx.get().end()) { - auto it = mempool.mapTx.project<0>(mi); - assert(it != mempool.mapTx.end()); + if (mi != m_mempool->mapTx.get().end()) { + auto it = m_mempool->mapTx.project<0>(mi); + assert(it != m_mempool->mapTx.end()); if (mapModifiedTx.count(it) || inBlock.count(it) || failedTx.count(it)) { ++mi; continue; @@ -337,13 +336,13 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele bool fUsingModified = false; modtxscoreiter modit = mapModifiedTx.get().begin(); - if (mi == mempool.mapTx.get().end()) { + if (mi == m_mempool->mapTx.get().end()) { // We're out of entries in mapTx; use the entry from mapModifiedTx iter = modit->iter; fUsingModified = true; } else { // Try to compare the mapTx entry to the mapModifiedTx entry - iter = mempool.mapTx.project<0>(mi); + iter = m_mempool->mapTx.project<0>(mi); if (modit != mapModifiedTx.get().end() && CompareTxMemPoolEntryByAncestorFee()(*modit, CTxMemPoolModifiedEntry(iter))) { // The best entry in mapModifiedTx has higher score @@ -395,7 +394,7 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele continue; } - auto ancestors{mempool.AssumeCalculateMemPoolAncestors(__func__, *iter, CTxMemPool::Limits::NoLimits(), /*fSearchForParents=*/false)}; + auto ancestors{m_mempool->AssumeCalculateMemPoolAncestors(__func__, *iter, CTxMemPool::Limits::NoLimits(), /*fSearchForParents=*/false)}; onlyUnconfirmed(ancestors); ancestors.insert(iter); @@ -425,7 +424,7 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele ++nPackagesSelected; // Update transactions that depend on each of these - nDescendantsUpdated += UpdatePackagesForAdded(mempool, ancestors, mapModifiedTx); + nDescendantsUpdated += UpdatePackagesForAdded(*m_mempool, ancestors, mapModifiedTx); } } } // namespace node diff --git a/src/node/miner.h b/src/node/miner.h index 4173521585459..efd7fb439ac25 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -183,7 +183,7 @@ class BlockAssembler /** Add transactions based on feerate including unconfirmed ancestors * Increments nPackagesSelected / nDescendantsUpdated with corresponding * statistics from the package selection (for logging statistics). */ - void addPackageTxs(const CTxMemPool& mempool, int& nPackagesSelected, int& nDescendantsUpdated) EXCLUSIVE_LOCKS_REQUIRED(mempool.cs); + void addPackageTxs(int& nPackagesSelected, int& nDescendantsUpdated) EXCLUSIVE_LOCKS_REQUIRED(!m_mempool->cs); // helper functions for addPackageTxs() /** Remove confirmed (inBlock) entries from given set */