aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--src/txmempool.cpp32
-rw-r--r--src/txmempool.h68
-rw-r--r--src/validation.cpp4
-rwxr-xr-xtest/functional/mempool_updatefromblock.py2
4 files changed, 73 insertions, 33 deletions
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index dc2769b81e..fb5652d0a0 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -116,10 +116,9 @@ size_t CTxMemPoolEntry::GetTxSize() const
return GetVirtualTransactionSize(nTxWeight, sigOpCost);
}
-// Update the given tx for any in-mempool descendants.
-// Assumes that CTxMemPool::m_children is correct for the given tx and all
-// descendants.
-void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap &cachedDescendants, const std::set<uint256> &setExclude)
+void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap& cachedDescendants,
+ const std::set<uint256>& setExclude, std::set<uint256>& descendants_to_remove,
+ uint64_t ancestor_size_limit, uint64_t ancestor_count_limit)
{
CTxMemPoolEntry::Children stageEntries, descendants;
stageEntries = updateIt->GetMemPoolChildrenConst();
@@ -156,17 +155,18 @@ void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap &cachedDescendan
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()));
+ // Don't directly remove the transaction here -- doing so would
+ // invalidate iterators in cachedDescendants. Mark it for removal
+ // by inserting into descendants_to_remove.
+ if (descendant.GetCountWithAncestors() > ancestor_count_limit || descendant.GetSizeWithAncestors() > ancestor_size_limit) {
+ descendants_to_remove.insert(descendant.GetTx().GetHash());
+ }
}
}
mapTx.modify(updateIt, update_descendant_state(modifySize, modifyFee, modifyCount));
}
-// vHashesToUpdate is the set of transaction hashes from a disconnected block
-// which has been re-added to the mempool.
-// for each entry, look for descendants that are outside vHashesToUpdate, and
-// add fee/size information for such descendants to the parent.
-// for each such descendant, also update the ancestor state to include the parent.
-void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256> &vHashesToUpdate)
+void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256> &vHashesToUpdate, uint64_t ancestor_size_limit, uint64_t ancestor_count_limit)
{
AssertLockHeld(cs);
// For each entry in vHashesToUpdate, store the set of in-mempool, but not
@@ -178,6 +178,8 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256> &vHashes
// accounted for in the state of their ancestors)
std::set<uint256> setAlreadyIncluded(vHashesToUpdate.begin(), vHashesToUpdate.end());
+ std::set<uint256> descendants_to_remove;
+
// Iterate in reverse, so that whenever we are looking at a transaction
// we are sure that all in-mempool descendants have already been processed.
// This maximizes the benefit of the descendant cache and guarantees that
@@ -207,7 +209,15 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256> &vHashes
}
}
} // release epoch guard for UpdateForDescendants
- UpdateForDescendants(it, mapMemPoolDescendantsToUpdate, setAlreadyIncluded);
+ UpdateForDescendants(it, mapMemPoolDescendantsToUpdate, setAlreadyIncluded, descendants_to_remove, ancestor_size_limit, ancestor_count_limit);
+ }
+
+ for (const auto& txid : descendants_to_remove) {
+ // This txid may have been removed already in a prior call to removeRecursive.
+ // Therefore we ensure it is not yet removed already.
+ if (const std::optional<txiter> txiter = GetIter(txid)) {
+ removeRecursive((*txiter)->GetTx(), MemPoolRemovalReason::SIZELIMIT);
+ }
}
}
diff --git a/src/txmempool.h b/src/txmempool.h
index e025dafd91..e7e5a3c402 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -636,16 +636,25 @@ public:
*/
void RemoveStaged(setEntries& stage, bool updateDescendants, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs);
- /** When adding transactions from a disconnected block back to the mempool,
- * new mempool entries may have children in the mempool (which is generally
- * not the case when otherwise adding transactions).
- * UpdateTransactionsFromBlock() will find child transactions and update the
- * descendant state for each transaction in vHashesToUpdate (excluding any
- * child transactions present in vHashesToUpdate, which are already accounted
- * for). Note: vHashesToUpdate should be the set of transactions from the
- * disconnected block that have been accepted back into the mempool.
+ /** UpdateTransactionsFromBlock is called when adding transactions from a
+ * disconnected block back to the mempool, new mempool entries may have
+ * children in the mempool (which is generally not the case when otherwise
+ * adding transactions).
+ * @post updated descendant state for descendants of each transaction in
+ * vHashesToUpdate (excluding any child transactions present in
+ * vHashesToUpdate, which are already accounted for). Updated state
+ * includes add fee/size information for such descendants to the
+ * parent and updated ancestor state to include the parent.
+ *
+ * @param[in] vHashesToUpdate The set of txids from the
+ * disconnected block that have been accepted back into the mempool.
+ * @param[in] ancestor_size_limit The maximum allowed size in virtual
+ * bytes of an entry and its ancestors
+ * @param[in] ancestor_count_limit The maximum allowed number of
+ * transactions including the entry and its ancestors.
*/
- void UpdateTransactionsFromBlock(const std::vector<uint256>& vHashesToUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main) LOCKS_EXCLUDED(m_epoch);
+ void UpdateTransactionsFromBlock(const std::vector<uint256>& vHashesToUpdate,
+ uint64_t ancestor_size_limit, uint64_t ancestor_count_limit) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main) LOCKS_EXCLUDED(m_epoch);
/** Try to calculate all in-mempool ancestors of entry.
* (these are all calculated including the tx itself)
@@ -794,19 +803,38 @@ private:
/** UpdateForDescendants is used by UpdateTransactionsFromBlock to update
* the descendants for a single transaction that has been added to the
* mempool but may have child transactions in the mempool, eg during a
- * chain reorg. setExclude is the set of descendant transactions in the
- * mempool that must not be accounted for (because any descendants in
- * setExclude were added to the mempool after the transaction being
- * updated and hence their state is already reflected in the parent
- * state).
+ * chain reorg.
+ *
+ * @pre CTxMemPool::m_children is correct for the given tx and all
+ * descendants.
+ * @pre cachedDescendants is an accurate cache where each entry has all
+ * descendants of the corresponding key, including those that should
+ * be removed for violation of ancestor limits.
+ * @post if updateIt has any non-excluded descendants, cachedDescendants has
+ * a new cache line for updateIt.
+ * @post descendants_to_remove has a new entry for any descendant which exceeded
+ * ancestor limits relative to updateIt.
*
- * cachedDescendants will be updated with the descendants of the transaction
- * being updated, so that future invocations don't need to walk the
- * same transaction again, if encountered in another transaction chain.
+ * @param[in] updateIt the entry to update for its descendants
+ * @param[in,out] cachedDescendants a cache where each line corresponds to all
+ * descendants. It will be updated with the descendants of the transaction
+ * being updated, so that future invocations don't need to walk the same
+ * transaction again, if encountered in another transaction chain.
+ * @param[in] setExclude the set of descendant transactions in the mempool
+ * that must not be accounted for (because any descendants in setExclude
+ * were added to the mempool after the transaction being updated and hence
+ * their state is already reflected in the parent state).
+ * @param[out] descendants_to_remove Populated with the txids of entries that
+ * exceed ancestor limits. It's the responsibility of the caller to
+ * removeRecursive them.
+ * @param[in] ancestor_size_limit the max number of ancestral bytes allowed
+ * for any descendant
+ * @param[in] ancestor_count_limit the max number of ancestor transactions
+ * allowed for any descendant
*/
- void UpdateForDescendants(txiter updateIt,
- cacheMap &cachedDescendants,
- const std::set<uint256> &setExclude) EXCLUSIVE_LOCKS_REQUIRED(cs);
+ void UpdateForDescendants(txiter updateIt, cacheMap& cachedDescendants,
+ const std::set<uint256>& setExclude, std::set<uint256>& descendants_to_remove,
+ uint64_t ancestor_size_limit, uint64_t ancestor_count_limit) EXCLUSIVE_LOCKS_REQUIRED(cs);
/** Update ancestors of hash to add/remove it as a descendant transaction. */
void UpdateAncestorsOf(bool add, txiter hash, setEntries &setAncestors) EXCLUSIVE_LOCKS_REQUIRED(cs);
/** Set ancestor state for an entry */
diff --git a/src/validation.cpp b/src/validation.cpp
index 72e0b6218e..1b1d01a4c2 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -347,7 +347,9 @@ void CChainState::MaybeUpdateMempoolForReorg(
// previously-confirmed transactions back to the mempool.
// UpdateTransactionsFromBlock finds descendants of any transactions in
// the disconnectpool that were added back and cleans up the mempool state.
- m_mempool->UpdateTransactionsFromBlock(vHashUpdate);
+ const uint64_t ancestor_count_limit = gArgs.GetIntArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT);
+ const uint64_t ancestor_size_limit = gArgs.GetIntArg("-limitancestorsize", DEFAULT_ANCESTOR_SIZE_LIMIT) * 1000;
+ m_mempool->UpdateTransactionsFromBlock(vHashUpdate, ancestor_size_limit, ancestor_count_limit);
// Predicate to use for filtering transactions in removeForReorg.
// Checks whether the transaction is still final and, if it spends a coinbase output, mature.
diff --git a/test/functional/mempool_updatefromblock.py b/test/functional/mempool_updatefromblock.py
index 16c15e3f74..51de582ce0 100755
--- a/test/functional/mempool_updatefromblock.py
+++ b/test/functional/mempool_updatefromblock.py
@@ -17,7 +17,7 @@ from test_framework.util import assert_equal
class MempoolUpdateFromBlockTest(BitcoinTestFramework):
def set_test_params(self):
self.num_nodes = 1
- self.extra_args = [['-limitdescendantsize=1000', '-limitancestorsize=1000']]
+ self.extra_args = [['-limitdescendantsize=1000', '-limitancestorsize=1000', '-limitancestorcount=100']]
def skip_test_if_missing_module(self):
self.skip_if_no_wallet()