Merge bitcoin/bitcoin#28843: [refactor] Cleanup BlockAssembler mempool usage

192dac1 [refactor] Cleanup BlockAssembler mempool usage (TheCharlatan)

Pull request description:

  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.

  This was noticed in this PR review: bitcoin/bitcoin#25223 (comment).

ACKs for top commit:
  achow101:
    ACK 192dac1
  danielabrozzoni:
    re-ACK 192dac1
  stickies-v:
    ACK 192dac1

Tree-SHA512: a5ae7d6d771fbb5b54f23624b4d3429acf07bbe38179a462a078c825d60c89a725ad4e13fe7067eebea7dfec63c56c8f39b5077b0d949d594f500affcc1272d1

Author: achow101

src/node/miner.cpp

@@ -138,8 +138,7 @@ std::unique_ptr<CBlockTemplate> 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()};
@@ -288,9 +287,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

src/node/miner.h

@@ -187,8 +187,11 @@ class BlockAssembler
     // 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 */