Skip to content

Commit

Permalink
Rename CTxMemPool::remove -> removeRecursive remove is no longer call…
Browse files Browse the repository at this point in the history 
…ed non-recursively, so simplify the logic and eliminate an unnecessary parameter

Coming from btc@5de2baa138cda501038a4558bc169b2cfe5b7d6b
  • Loading branch information
@furszy
furszy committed Jan 18, 2021
1 parent c30fa16 commit ba32375
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 25 deletions.
20 changes: 10 additions & 10 deletions src/test/mempool_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,12 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
std::list<CTransactionRef> removed;

// Nothing in pool, remove should do nothing:
testPool.remove(txParent, removed, true);
testPool.removeRecursive(txParent, removed);
BOOST_CHECK_EQUAL(removed.size(), 0);

// Just the parent:
testPool.addUnchecked(txParent.GetHash(), entry.FromTx(txParent));
testPool.remove(txParent, removed, true);
testPool.removeRecursive(txParent, removed);
BOOST_CHECK_EQUAL(removed.size(), 1);
removed.clear();

Expand All @@ -73,16 +73,16 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
testPool.addUnchecked(txGrandChild[i].GetHash(), entry.FromTx(txGrandChild[i]));
}
// Remove Child[0], GrandChild[0] should be removed:
testPool.remove(txChild[0], removed, true);
testPool.removeRecursive(txChild[0], removed);
BOOST_CHECK_EQUAL(removed.size(), 2);
removed.clear();
// ... make sure grandchild and child are gone:
testPool.remove(txGrandChild[0], removed, true);
testPool.removeRecursive(txGrandChild[0], removed);
BOOST_CHECK_EQUAL(removed.size(), 0);
testPool.remove(txChild[0], removed, true);
testPool.removeRecursive(txChild[0], removed);
BOOST_CHECK_EQUAL(removed.size(), 0);
// Remove parent, all children/grandchildren should go:
testPool.remove(txParent, removed, true);
testPool.removeRecursive(txParent, removed);
BOOST_CHECK_EQUAL(removed.size(), 5);
BOOST_CHECK_EQUAL(testPool.size(), 0);
removed.clear();
Expand All @@ -95,7 +95,7 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
}
// Now remove the parent, as might happen if a block-re-org occurs but the parent cannot be
// put into the mempool (maybe because it is non-standard):
testPool.remove(txParent, removed, true);
testPool.removeRecursive(txParent, removed);
BOOST_CHECK_EQUAL(removed.size(), 6);
BOOST_CHECK_EQUAL(testPool.size(), 0);
removed.clear();
Expand Down Expand Up @@ -280,11 +280,11 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest)

// Now try removing tx10 and verify the sort order returns to normal
std::list<CTransactionRef> removed;
pool.remove(pool.mapTx.find(tx10.GetHash())->GetTx(), removed, true);
pool.removeRecursive(pool.mapTx.find(tx10.GetHash())->GetTx(), removed);
CheckSort<descendant_score>(pool, snapshotOrder);

pool.remove(pool.mapTx.find(tx9.GetHash())->GetTx(), removed, true);
pool.remove(pool.mapTx.find(tx8.GetHash())->GetTx(), removed, true);
pool.removeRecursive(pool.mapTx.find(tx9.GetHash())->GetTx(), removed);
pool.removeRecursive(pool.mapTx.find(tx8.GetHash())->GetTx(), removed);
/* Now check the sort on the mining score index.
* Final order should be:
*
Expand Down
22 changes: 9 additions & 13 deletions src/txmempool.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ void CTxMemPool::CalculateDescendants(txiter entryit, setEntries &setDescendants
}
}

void CTxMemPool::remove(const CTransaction& origTx, std::list<CTransactionRef>& removed, bool fRecursive)
void CTxMemPool::removeRecursive(const CTransaction& origTx, std::list<CTransactionRef>& removed)
{
// Remove transaction from memory pool
{
Expand All @@ -477,8 +477,8 @@ void CTxMemPool::remove(const CTransaction& origTx, std::list<CTransactionRef>&
txiter origit = mapTx.find(origTx.GetHash());
if (origit != mapTx.end()) {
txToRemove.insert(origit);
} else if (fRecursive) {
// If recursively removing but origTx isn't in the mempool
} else {
// When recursively removing but origTx isn't in the mempool
// be sure to remove any children that are in the pool. This can
// happen during chain re-orgs if origTx isn't re-accepted into
// the mempool for any reason.
Expand All @@ -492,12 +492,8 @@ void CTxMemPool::remove(const CTransaction& origTx, std::list<CTransactionRef>&
}
}
setEntries setAllRemoves;
if (fRecursive) {
for (const txiter& it : txToRemove) {
CalculateDescendants(it, setAllRemoves);
}
} else {
setAllRemoves.swap(txToRemove);
for (const txiter& it : txToRemove) {
CalculateDescendants(it, setAllRemoves);
}
for (const txiter& it : setAllRemoves) {
removed.emplace_back(it->GetSharedTx());
Expand Down Expand Up @@ -532,7 +528,7 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem
}
for (const CTransaction& tx : transactionsToRemove) {
std::list<CTransactionRef> removed;
remove(tx, removed, true);
removeRecursive(tx, removed);
}
}

Expand All @@ -557,7 +553,7 @@ void CTxMemPool::removeWithAnchor(const uint256& invalidRoot)
}
for (const CTransaction& tx : transactionsToRemove) {
std::list<CTransactionRef> removed;
remove(tx, removed, true);
removeRecursive(tx, removed);
}
}

Expand All @@ -571,7 +567,7 @@ void CTxMemPool::removeConflicts(const CTransaction& tx, std::list<CTransactionR
if (it != mapNextTx.end()) {
const CTransaction& txConflict = *it->second.ptx;
if (txConflict != tx) {
remove(txConflict, removed, true);
removeRecursive(txConflict, removed);
}
}
}
Expand All @@ -582,7 +578,7 @@ void CTxMemPool::removeConflicts(const CTransaction& tx, std::list<CTransactionR
if (it != mapSaplingNullifiers.end()) {
const CTransaction& txConflict = *it->second;
if (txConflict != tx) {
remove(txConflict, removed, true);
removeRecursive(txConflict, removed);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/txmempool.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ class CTxMemPool
// then invoke the second version.
bool addUnchecked(const uint256& hash, const CTxMemPoolEntry& entry, bool fCurrentEstimate = true);
bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, setEntries &setAncestors, bool fCurrentEstimate = true);
void remove(const CTransaction& tx, std::list<CTransactionRef>& removed, bool fRecursive = false);
void removeRecursive(const CTransaction& tx, std::list<CTransactionRef>& removed);
void removeForReorg(const CCoinsViewCache* pcoins, unsigned int nMemPoolHeight, int flags);
void removeWithAnchor(const uint256& invalidRoot);
void removeConflicts(const CTransaction& tx, std::list<CTransactionRef>& removed);
Expand Down
2 changes: 1 addition & 1 deletion src/validation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1965,7 +1965,7 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara
std::list<CTransactionRef> removed;
CValidationState stateDummy;
if (tx->IsCoinBase() || tx->IsCoinStake() || !AcceptToMemoryPool(mempool, stateDummy, tx, false, nullptr, true)) {
mempool.remove(*tx, removed, true);
mempool.removeRecursive(*tx, removed);
} else if (mempool.exists(tx->GetHash())) {
vHashUpdate.push_back(tx->GetHash());
}
Expand Down

0 comments on commit ba32375

Please sign in to comment.