Avoid using mapTx.modify() to update modified fees
What changed, and why it matters
This is a small internal cleanup in Bitcoin Core's memory pool (mempool) code. It changes how transaction fee adjustments are updated, removing an unnecessary wrapper and making a fee field directly mutable. There is no indication this fixes a security bug; it appears to be a refactoring or simplification after an earlier data structure change removed fee-based indexes.
No immediate action required. Treat as routine code maintenance. Standard review and regression testing for mempool prioritisation behavior is sufficient.
Security signals we found
No security-relevant signal: change is a refactoring/simplification
Mutable fee field could theoretically affect mempool eviction/mining priority logic if misused, but the diff does not introduce such misuse
No bounds/safety change: SaturatingAdd remains in place
Evidence from the diff
The commit removes uses of boost::multi_index_container’s modify() helper to update CTxMemPoolEntry::m_modified_fee, instead mutating the field directly through a const method. To allow this, m_modified_fee is marked mutable and UpdateModifiedFee() is made const. This is feasible because the mempool no longer maintains feerate-based indices that would be invalidated by in-place mutation. The change touches PrioritiseTransaction() and ChangeSet::StageAddition().
Changed components
src/kernel/mempool_entry.hsrc/txmempool.cppCTxMemPool::PrioritiseTransactionCTxMemPool::ChangeSet::StageAdditionInspect captured patch +4 / −4
diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h
index b274e1f7..3053a77c 100644
--- a/src/kernel/mempool_entry.h
+++ b/src/kernel/mempool_entry.h
@@ -80,7 +80,7 @@ private:
const unsigned int entryHeight; //!< Chain height when entering the mempool
const bool spendsCoinbase; //!< keep track of transactions that spend a coinbase
const int64_t sigOpCost; //!< Total sigop cost
- CAmount m_modified_fee; //!< Used for determining the priority of the transaction for mining in a block
+ mutable CAmount m_modified_fee; //!< Used for determining the priority of the transaction for mining in a block
mutable LockPoints lockPoints; //!< Track the height and time at which tx was final
public:
@@ -124,7 +124,7 @@ public:
const LockPoints& GetLockPoints() const { return lockPoints; }
// Updates the modified fees with descendants/ancestors.
- void UpdateModifiedFee(CAmount fee_diff)
+ void UpdateModifiedFee(CAmount fee_diff) const
{
m_modified_fee = SaturatingAdd(m_modified_fee, fee_diff);
}
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index ee4ced3b..de4a5572 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -598,7 +598,7 @@ void CTxMemPool::PrioritiseTransaction(const Txid& hash, const CAmount& nFeeDelt
if (it != mapTx.end()) {
// PrioritiseTransaction calls stack on previous ones. Set the new
// transaction fee to be current modified fee + feedelta.
- mapTx.modify(it, [&nFeeDelta](CTxMemPoolEntry& e) { e.UpdateModifiedFee(nFeeDelta); });
+ it->UpdateModifiedFee(nFeeDelta);
m_txgraph->SetTransactionFee(*it, it->GetModifiedFee());
++nTransactionsUpdated;
}
@@ -975,7 +975,7 @@ CTxMemPool::ChangeSet::TxHandle CTxMemPool::ChangeSet::StageAddition(const CTran
TxGraph::Ref ref(m_pool->m_txgraph->AddTransaction(FeePerWeight(fee, GetSigOpsAdjustedWeight(GetTransactionWeight(*tx), sigops_cost, ::nBytesPerSigOp))));
auto newit = m_to_add.emplace(std::move(ref), tx, fee, time, entry_height, entry_sequence, spends_coinbase, sigops_cost, lp).first;
if (delta) {
- m_to_add.modify(newit, [&delta](CTxMemPoolEntry& e) { e.UpdateModifiedFee(delta); });
+ newit->UpdateModifiedFee(delta);
m_pool->m_txgraph->SetTransactionFee(*newit, newit->GetModifiedFee());
}
Why this scored 18/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.