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

192dac1d33 [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: https://github.com/bitcoin/bitcoin/pull/25223#discussion_r898164322.

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

Tree-SHA512: a5ae7d6d771fbb5b54f23624b4d3429acf07bbe38179a462a078c825d60c89a725ad4e13fe7067eebea7dfec63c56c8f39b5077b0d949d594f500affcc1272d1
This commit is contained in:
Ava Chow 2024-11-14 16:30:48 -05:00
commit 1a8f51e745
No known key found for this signature in database
GPG Key ID: 17565732E08E5E41
2 changed files with 9 additions and 6 deletions

View File

@ -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

View File

@ -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 */