mempool: Use m_limit for UpdateTransactionsFromBlock

Since:

- UpdateTransactionsFromBlock is only called by
  MaybeUpdateMempoolForReorg, which calls it with the gArgs-determined
  ancestor limits
- UpdateForDescendants is only called by UpdateTransactionsFromBlock
  with the ancestor limits unchanged

We can remove the requirement to specify the ancestor limits for both
UpdateTransactionsFromBlock and UpdateForDescendants and just use the
values in the m_limits member.

Also move some removed comments to MemPoolLimits struct members.

The uint64_t cast in UpdateForDescendants is not new behavior,
see the diff in CChainState::MaybeUpdateMempoolForReorg for where they
were previously.

src/kernel/mempool_limits.h

@@ -16,9 +16,13 @@ namespace kernel {
  * Most of the time, this struct should be referenced as CTxMemPool::Limits.
  */
 struct MemPoolLimits {
+    //! The maximum allowed number of transactions in a package including the entry and its ancestors.
     int64_t ancestor_count{DEFAULT_ANCESTOR_LIMIT};
+    //! The maximum allowed size in virtual bytes of an entry and its ancestors within a package.
     int64_t ancestor_size_vbytes{DEFAULT_ANCESTOR_SIZE_LIMIT_KVB * 1'000};
+    //! The maximum allowed number of transactions in a package including the entry and its descendants.
     int64_t descendant_count{DEFAULT_DESCENDANT_LIMIT};
+    //! The maximum allowed size in virtual bytes of an entry and its descendants within a package.
     int64_t descendant_size_vbytes{DEFAULT_DESCENDANT_SIZE_LIMIT_KVB * 1'000};
 };
 } // namespace kernel

src/txmempool.cpp

@@ -107,8 +107,7 @@ size_t CTxMemPoolEntry::GetTxSize() const
 }
 
 void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap& cachedDescendants,
-                                      const std::set<uint256>& setExclude, std::set<uint256>& descendants_to_remove,
+                                      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();
@@ -148,7 +147,7 @@ void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap& cachedDescendan
             // 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) {
+            if (descendant.GetCountWithAncestors() > uint64_t(m_limits.ancestor_count) || descendant.GetSizeWithAncestors() > uint64_t(m_limits.ancestor_size_vbytes)) {
                 descendants_to_remove.insert(descendant.GetTx().GetHash());
             }
         }
@@ -156,7 +155,7 @@ void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap& cachedDescendan
     mapTx.modify(updateIt, update_descendant_state(modifySize, modifyFee, modifyCount));
 }
 
-void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256> &vHashesToUpdate, uint64_t ancestor_size_limit, uint64_t ancestor_count_limit)
+void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256>& vHashesToUpdate)
 {
     AssertLockHeld(cs);
     // For each entry in vHashesToUpdate, store the set of in-mempool, but not
@@ -199,7 +198,7 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256> &vHashes
                 }
             }
         } // release epoch guard for UpdateForDescendants
-        UpdateForDescendants(it, mapMemPoolDescendantsToUpdate, setAlreadyIncluded, descendants_to_remove, ancestor_size_limit, ancestor_count_limit);
+        UpdateForDescendants(it, mapMemPoolDescendantsToUpdate, setAlreadyIncluded, descendants_to_remove);
     }
 
     for (const auto& txid : descendants_to_remove) {

src/txmempool.h

@@ -659,13 +659,8 @@ public:
      *
      * @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,
+    void UpdateTransactionsFromBlock(const std::vector<uint256>& vHashesToUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main) LOCKS_EXCLUDED(m_epoch);
-            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)
@@ -840,14 +835,9 @@ private:
      * @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, std::set<uint256>& descendants_to_remove,
+                              const std::set<uint256>& setExclude, std::set<uint256>& descendants_to_remove) EXCLUSIVE_LOCKS_REQUIRED(cs);
-                              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 */

src/validation.cpp

@@ -320,9 +320,7 @@ 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.
-    const uint64_t ancestor_count_limit = gArgs.GetIntArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT);
+    m_mempool->UpdateTransactionsFromBlock(vHashUpdate);
-    const uint64_t ancestor_size_limit = gArgs.GetIntArg("-limitancestorsize", DEFAULT_ANCESTOR_SIZE_LIMIT_KVB) * 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.