From 192dac1d3370edd579db235d69c034726f37c8da Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Mon, 2 Sep 2024 14:07:31 +0200 Subject: [PATCH] [refactor] Cleanup BlockAssembler mempool usage The `addPackageTxs` method of the `BlockAssembler` currently has access to two mempool variables, as an argument and as a member. Clean this up and clarify that they both are the same mempool instance by removing the argument and instead only using the member variable in the method. Co-Authored-By: Anthony Towns Co-authored-by: stickies-v --- src/node/miner.cpp | 8 ++++---- src/node/miner.h | 7 +++++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 97f6ac346a5..afa410da100 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -142,8 +142,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()}; @@ -292,9 +291,10 @@ 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); + const auto& mempool{*Assert(m_mempool)}; + LOCK(mempool.cs); // mapModifiedTx will store sorted packages after they are modified // because some of their txs are already in the block diff --git a/src/node/miner.h b/src/node/miner.h index 1b829437669..25ce110b348 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -187,8 +187,11 @@ private: // Methods for how to add transactions to a block. /** 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); + * statistics from the package selection (for logging statistics). + * + * @pre BlockAssembler::m_mempool must not be nullptr + */ + void addPackageTxs(int& nPackagesSelected, int& nDescendantsUpdated) EXCLUSIVE_LOCKS_REQUIRED(!m_mempool->cs); // helper functions for addPackageTxs() /** Remove confirmed (inBlock) entries from given set */