Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mempool clean up: replace update_* structs with lambdas #26048

Merged
merged 1 commit into from
Sep 12, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
48 changes: 9 additions & 39 deletions src/txmempool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,38 +24,6 @@
#include <cmath>
#include <optional>

// Helpers for modifying CTxMemPool::mapTx, which is a boost multi_index.
struct update_descendant_state
{
update_descendant_state(int64_t _modifySize, CAmount _modifyFee, int64_t _modifyCount) :
modifySize(_modifySize), modifyFee(_modifyFee), modifyCount(_modifyCount)
{}

void operator() (CTxMemPoolEntry &e)
{ e.UpdateDescendantState(modifySize, modifyFee, modifyCount); }

private:
int64_t modifySize;
CAmount modifyFee;
int64_t modifyCount;
};

struct update_ancestor_state
{
update_ancestor_state(int64_t _modifySize, CAmount _modifyFee, int64_t _modifyCount, int64_t _modifySigOpsCost) :
modifySize(_modifySize), modifyFee(_modifyFee), modifyCount(_modifyCount), modifySigOpsCost(_modifySigOpsCost)
{}

void operator() (CTxMemPoolEntry &e)
{ e.UpdateAncestorState(modifySize, modifyFee, modifyCount, modifySigOpsCost); }

private:
int64_t modifySize;
CAmount modifyFee;
int64_t modifyCount;
int64_t modifySigOpsCost;
};

bool TestLockPointValidity(CChain& active_chain, const LockPoints& lp)
{
AssertLockHeld(cs_main);
Expand Down Expand Up @@ -146,7 +114,9 @@ void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap& cachedDescendan
modifyCount++;
cachedDescendants[updateIt].insert(mapTx.iterator_to(descendant));
// Update ancestor state for each descendant
mapTx.modify(mapTx.iterator_to(descendant), update_ancestor_state(updateIt->GetTxSize(), updateIt->GetModifiedFee(), 1, updateIt->GetSigOpCost()));
mapTx.modify(mapTx.iterator_to(descendant), [=](CTxMemPoolEntry& e) {
e.UpdateAncestorState(updateIt->GetTxSize(), updateIt->GetModifiedFee(), 1, updateIt->GetSigOpCost());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
e.UpdateAncestorState(updateIt->GetTxSize(), updateIt->GetModifiedFee(), 1, updateIt->GetSigOpCost());
e.UpdateAncestorState(updateIt->GetTxSize(), updateIt->GetModifiedFee(), /*modifyCount=*/1, updateIt->GetSigOpCost());

});
// Don't directly remove the transaction here -- doing so would
// invalidate iterators in cachedDescendants. Mark it for removal
// by inserting into descendants_to_remove.
Expand All @@ -155,7 +125,7 @@ void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap& cachedDescendan
}
}
}
mapTx.modify(updateIt, update_descendant_state(modifySize, modifyFee, modifyCount));
mapTx.modify(updateIt, [=](CTxMemPoolEntry& e) { e.UpdateDescendantState(modifySize, modifyFee, modifyCount); });
}

void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256>& vHashesToUpdate)
Expand Down Expand Up @@ -347,7 +317,7 @@ void CTxMemPool::UpdateAncestorsOf(bool add, txiter it, setEntries &setAncestors
const int64_t updateSize = updateCount * it->GetTxSize();
const CAmount updateFee = updateCount * it->GetModifiedFee();
for (txiter ancestorIt : setAncestors) {
mapTx.modify(ancestorIt, update_descendant_state(updateSize, updateFee, updateCount));
mapTx.modify(ancestorIt, [=](CTxMemPoolEntry& e) { e.UpdateDescendantState(updateSize, updateFee, updateCount); });
}
}

Expand All @@ -362,7 +332,7 @@ void CTxMemPool::UpdateEntryForAncestors(txiter it, const setEntries &setAncesto
updateFee += ancestorIt->GetModifiedFee();
updateSigOpsCost += ancestorIt->GetSigOpCost();
}
mapTx.modify(it, update_ancestor_state(updateSize, updateFee, updateCount, updateSigOpsCost));
mapTx.modify(it, [=](CTxMemPoolEntry& e){ e.UpdateAncestorState(updateSize, updateFee, updateCount, updateSigOpsCost); });
}

void CTxMemPool::UpdateChildrenForRemoval(txiter it)
Expand Down Expand Up @@ -393,7 +363,7 @@ void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove, b
CAmount modifyFee = -removeIt->GetModifiedFee();
int modifySigOps = -removeIt->GetSigOpCost();
for (txiter dit : setDescendants) {
mapTx.modify(dit, update_ancestor_state(modifySize, modifyFee, -1, modifySigOps));
mapTx.modify(dit, [=](CTxMemPoolEntry& e){ e.UpdateAncestorState(modifySize, modifyFee, -1, modifySigOps); });
Copy link
Contributor

@jonatack jonatack Sep 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
mapTx.modify(dit, [=](CTxMemPoolEntry& e){ e.UpdateAncestorState(modifySize, modifyFee, -1, modifySigOps); });
mapTx.modify(dit, [=](CTxMemPoolEntry& e) { e.UpdateAncestorState(modifySize, modifyFee, /*modifyCount=*/-1, modifySigOps); });

}
}
}
Expand Down Expand Up @@ -942,14 +912,14 @@ void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeD
std::string dummy;
CalculateMemPoolAncestors(*it, setAncestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy, false);
for (txiter ancestorIt : setAncestors) {
mapTx.modify(ancestorIt, update_descendant_state(0, nFeeDelta, 0));
mapTx.modify(ancestorIt, [=](CTxMemPoolEntry& e){ e.UpdateDescendantState(0, nFeeDelta, 0);});
}
// Now update all descendants' modified fees with ancestors
setEntries setDescendants;
CalculateDescendants(it, setDescendants);
setDescendants.erase(it);
for (txiter descendantIt : setDescendants) {
mapTx.modify(descendantIt, update_ancestor_state(0, nFeeDelta, 0, 0));
mapTx.modify(descendantIt, [=](CTxMemPoolEntry& e){ e.UpdateAncestorState(0, nFeeDelta, 0, 0); });
}
++nTransactionsUpdated;
}
Expand Down