tx fees, policy: bugfix: move `removeTx` into reason != `BLOCK` condition

If the removal reason of a transaction is BLOCK, then the `removeTx`
boolean argument should be true.

Before this PR, `CBlockPolicyEstimator` have to complete updating the fee stats
before the mempool clears that's why having removeTx call outside reason!= `BLOCK`
in `addUnchecked` was not a bug.

But in a case where the `CBlockPolicyEstimator` update is asynchronous, the mempool might
clear before we update the `CBlockPolicyEstimator` fee stats.
Transactions that are removed for `BLOCK` reasons will also be incorrectly removed from
`CBlockPolicyEstimator` stats as failures.
This commit is contained in:
ismaelsadeeq 2023-11-03 11:01:52 +01:00
parent 640b450530
commit a0e3eb7549
1 changed files with 4 additions and 2 deletions

View File

@ -497,12 +497,16 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
// even if not directly reported below.
uint64_t mempool_sequence = GetAndIncrementSequence();
const auto& hash = it->GetTx().GetHash();
if (reason != MemPoolRemovalReason::BLOCK) {
// Notify clients that a transaction has been removed from the mempool
// for any reason except being included in a block. Clients interested
// in transactions included in blocks can subscribe to the BlockConnected
// notification.
GetMainSignals().TransactionRemovedFromMempool(it->GetSharedTx(), reason, mempool_sequence);
if (minerPolicyEstimator) {
minerPolicyEstimator->removeTx(hash, false);
}
}
TRACE5(mempool, removed,
it->GetTx().GetHash().data(),
@ -512,7 +516,6 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
std::chrono::duration_cast<std::chrono::duration<std::uint64_t>>(it->GetTime()).count()
);
const uint256 hash = it->GetTx().GetHash();
for (const CTxIn& txin : it->GetTx().vin)
mapNextTx.erase(txin.prevout);
@ -535,7 +538,6 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
cachedInnerUsage -= memusage::DynamicUsage(it->GetMemPoolParentsConst()) + memusage::DynamicUsage(it->GetMemPoolChildrenConst());
mapTx.erase(it);
nTransactionsUpdated++;
if (minerPolicyEstimator) {minerPolicyEstimator->removeTx(hash, false);}
}
// Calculates descendants of entry that are not already in setDescendants, and adds to