Browse Source

Remove work limit in UpdateForDescendants()

The work limit served to prevent the descendant walking algorithm from doing
too much work by marking the parent transaction as dirty.  However to implement
ancestor tracking, it's not possible to similarly mark those descendant
transactions as dirty without having to calculate them to begin with.

This commit removes the work limit altogether.  With appropriate
chain limits (-limitdescendantcount) the concern about doing too much
work inside this function should be mitigated.
0.13
Suhas Daftuar 9 years ago
parent
commit
76a76321d2
  1. 14
      src/main.cpp
  2. 47
      src/txmempool.cpp
  3. 19
      src/txmempool.h

14
src/main.cpp

@ -1194,20 +1194,6 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C
// Save these to avoid repeated lookups // Save these to avoid repeated lookups
setIterConflicting.insert(mi); setIterConflicting.insert(mi);
// If this entry is "dirty", then we don't have descendant
// state for this transaction, which means we probably have
// lots of in-mempool descendants.
// Don't allow replacements of dirty transactions, to ensure
// that we don't spend too much time walking descendants.
// This should be rare.
if (mi->IsDirty()) {
return state.DoS(0, false,
REJECT_NONSTANDARD, "too many potential replacements", false,
strprintf("too many potential replacements: rejecting replacement %s; cannot replace tx %s with untracked descendants",
hash.ToString(),
mi->GetTx().GetHash().ToString()));
}
// Don't allow the replacement to reduce the feerate of the // Don't allow the replacement to reduce the feerate of the
// mempool. // mempool.
// //

47
src/txmempool.cpp

@ -64,21 +64,13 @@ void CTxMemPoolEntry::UpdateFeeDelta(int64_t newFeeDelta)
// Update the given tx for any in-mempool descendants. // Update the given tx for any in-mempool descendants.
// Assumes that setMemPoolChildren is correct for the given tx and all // Assumes that setMemPoolChildren is correct for the given tx and all
// descendants. // descendants.
bool CTxMemPool::UpdateForDescendants(txiter updateIt, int maxDescendantsToVisit, cacheMap &cachedDescendants, const std::set<uint256> &setExclude) void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap &cachedDescendants, const std::set<uint256> &setExclude)
{ {
// Track the number of entries (outside setExclude) that we'd need to visit
// (will bail out if it exceeds maxDescendantsToVisit)
int nChildrenToVisit = 0;
setEntries stageEntries, setAllDescendants; setEntries stageEntries, setAllDescendants;
stageEntries = GetMemPoolChildren(updateIt); stageEntries = GetMemPoolChildren(updateIt);
while (!stageEntries.empty()) { while (!stageEntries.empty()) {
const txiter cit = *stageEntries.begin(); const txiter cit = *stageEntries.begin();
if (cit->IsDirty()) {
// Don't consider any more children if any descendant is dirty
return false;
}
setAllDescendants.insert(cit); setAllDescendants.insert(cit);
stageEntries.erase(cit); stageEntries.erase(cit);
const setEntries &setChildren = GetMemPoolChildren(cit); const setEntries &setChildren = GetMemPoolChildren(cit);
@ -88,22 +80,11 @@ bool CTxMemPool::UpdateForDescendants(txiter updateIt, int maxDescendantsToVisit
// We've already calculated this one, just add the entries for this set // We've already calculated this one, just add the entries for this set
// but don't traverse again. // but don't traverse again.
BOOST_FOREACH(const txiter cacheEntry, cacheIt->second) { BOOST_FOREACH(const txiter cacheEntry, cacheIt->second) {
// update visit count only for new child transactions setAllDescendants.insert(cacheEntry);
// (outside of setExclude and stageEntries)
if (setAllDescendants.insert(cacheEntry).second &&
!setExclude.count(cacheEntry->GetTx().GetHash()) &&
!stageEntries.count(cacheEntry)) {
nChildrenToVisit++;
}
} }
} else if (!setAllDescendants.count(childEntry)) { } else if (!setAllDescendants.count(childEntry)) {
// Schedule for later processing and update our visit count // Schedule for later processing
if (stageEntries.insert(childEntry).second && !setExclude.count(childEntry->GetTx().GetHash())) { stageEntries.insert(childEntry);
nChildrenToVisit++;
}
}
if (nChildrenToVisit > maxDescendantsToVisit) {
return false;
} }
} }
} }
@ -121,7 +102,6 @@ bool CTxMemPool::UpdateForDescendants(txiter updateIt, int maxDescendantsToVisit
} }
} }
mapTx.modify(updateIt, update_descendant_state(modifySize, modifyFee, modifyCount)); mapTx.modify(updateIt, update_descendant_state(modifySize, modifyFee, modifyCount));
return true;
} }
// vHashesToUpdate is the set of transaction hashes from a disconnected block // vHashesToUpdate is the set of transaction hashes from a disconnected block
@ -167,10 +147,7 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256> &vHashes
UpdateParent(childIter, it, true); UpdateParent(childIter, it, true);
} }
} }
if (!UpdateForDescendants(it, 100, mapMemPoolDescendantsToUpdate, setAlreadyIncluded)) { UpdateForDescendants(it, mapMemPoolDescendantsToUpdate, setAlreadyIncluded);
// Mark as dirty if we can't do the calculation.
mapTx.modify(it, set_dirty());
}
} }
} }
@ -301,23 +278,14 @@ void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove)
} }
} }
void CTxMemPoolEntry::SetDirty()
{
nCountWithDescendants = 0;
nSizeWithDescendants = nTxSize;
nModFeesWithDescendants = GetModifiedFee();
}
void CTxMemPoolEntry::UpdateState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount) void CTxMemPoolEntry::UpdateState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount)
{ {
if (!IsDirty()) {
nSizeWithDescendants += modifySize; nSizeWithDescendants += modifySize;
assert(int64_t(nSizeWithDescendants) > 0); assert(int64_t(nSizeWithDescendants) > 0);
nModFeesWithDescendants += modifyFee; nModFeesWithDescendants += modifyFee;
nCountWithDescendants += modifyCount; nCountWithDescendants += modifyCount;
assert(int64_t(nCountWithDescendants) > 0); assert(int64_t(nCountWithDescendants) > 0);
} }
}
CTxMemPool::CTxMemPool(const CFeeRate& _minReasonableRelayFee) : CTxMemPool::CTxMemPool(const CFeeRate& _minReasonableRelayFee) :
nTransactionsUpdated(0) nTransactionsUpdated(0)
@ -658,12 +626,7 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const
assert(setChildrenCheck == GetMemPoolChildren(it)); assert(setChildrenCheck == GetMemPoolChildren(it));
// Also check to make sure size is greater than sum with immediate children. // Also check to make sure size is greater than sum with immediate children.
// just a sanity check, not definitive that this calc is correct... // just a sanity check, not definitive that this calc is correct...
if (!it->IsDirty()) {
assert(it->GetSizeWithDescendants() >= childSizes + it->GetTxSize()); assert(it->GetSizeWithDescendants() >= childSizes + it->GetTxSize());
} else {
assert(it->GetSizeWithDescendants() == it->GetTxSize());
assert(it->GetModFeesWithDescendants() == it->GetModifiedFee());
}
if (fDependsWait) if (fDependsWait)
waitingOnDependants.push_back(&(*it)); waitingOnDependants.push_back(&(*it));

19
src/txmempool.h

@ -108,13 +108,6 @@ public:
// modified fees with descendants. // modified fees with descendants.
void UpdateFeeDelta(int64_t feeDelta); void UpdateFeeDelta(int64_t feeDelta);
/** We can set the entry to be dirty if doing the full calculation of in-
* mempool descendants will be too expensive, which can potentially happen
* when re-adding transactions from a block back to the mempool.
*/
void SetDirty();
bool IsDirty() const { return nCountWithDescendants == 0; }
uint64_t GetCountWithDescendants() const { return nCountWithDescendants; } uint64_t GetCountWithDescendants() const { return nCountWithDescendants; }
uint64_t GetSizeWithDescendants() const { return nSizeWithDescendants; } uint64_t GetSizeWithDescendants() const { return nSizeWithDescendants; }
CAmount GetModFeesWithDescendants() const { return nModFeesWithDescendants; } CAmount GetModFeesWithDescendants() const { return nModFeesWithDescendants; }
@ -138,12 +131,6 @@ struct update_descendant_state
int64_t modifyCount; int64_t modifyCount;
}; };
struct set_dirty
{
void operator() (CTxMemPoolEntry &e)
{ e.SetDirty(); }
};
struct update_fee_delta struct update_fee_delta
{ {
update_fee_delta(int64_t _feeDelta) : feeDelta(_feeDelta) { } update_fee_delta(int64_t _feeDelta) : feeDelta(_feeDelta) { }
@ -555,15 +542,11 @@ private:
* updated and hence their state is already reflected in the parent * updated and hence their state is already reflected in the parent
* state). * state).
* *
* If updating an entry requires looking at more than maxDescendantsToVisit
* transactions, outside of the ones in setExclude, then give up.
*
* cachedDescendants will be updated with the descendants of the transaction * cachedDescendants will be updated with the descendants of the transaction
* being updated, so that future invocations don't need to walk the * being updated, so that future invocations don't need to walk the
* same transaction again, if encountered in another transaction chain. * same transaction again, if encountered in another transaction chain.
*/ */
bool UpdateForDescendants(txiter updateIt, void UpdateForDescendants(txiter updateIt,
int maxDescendantsToVisit,
cacheMap &cachedDescendants, cacheMap &cachedDescendants,
const std::set<uint256> &setExclude); const std::set<uint256> &setExclude);
/** Update ancestors of hash to add/remove it as a descendant transaction. */ /** Update ancestors of hash to add/remove it as a descendant transaction. */

Loading…
Cancel
Save