Merge bitcoin#25290: [kernel 3a/n] Decouple CTxMemPool from ArgsManager

BACKPORT NOTE:
includes changes from Merge bitcoin#25223: [kernel 2e/n] miner: Make `mempool` optional, stop constructing temporary empty mempools
includes changes from Merge bitcoin#26286: test: Remove unused txmempool include from tests

d1684be fees: Pass in a filepath instead of referencing gArgs (Carl Dong)
9a3d825 init: Remove redundant -*mempool*, -limit* queries (Carl Dong)
6c5c60c mempool: Use m_limit for UpdateTransactionsFromBlock (Carl Dong)
9e93b10 node/ifaces: Use existing MemPoolLimits (Carl Dong)
38af2bc mempoolaccept: Use limits from mempool in constructor (Carl Dong)
9333427 mempool: Introduce (still-unused) MemPoolLimits (Carl Dong)
716bb5f scripted-diff: Rename anc/desc size limit vars to indicate SI unit (Carl Dong)
1ecc773 scripted-diff: Rename DEFAULT_MEMPOOL_EXPIRY to indicate time unit (Carl Dong)
aa9141c mempool: Pass in -mempoolexpiry instead of referencing gArgs (Carl Dong)
51c7a41 init: Only determine maxmempool once (Carl Dong)
386c947 mempool: Make GetMinFee() with custom size protected (Carl Dong)
82f00de mempool: Pass in -maxmempool instead of referencing gArgs (Carl Dong)
f1941e8 pool: Add and use MemPoolOptions, ApplyArgsManOptions (Carl Dong)
0199bd3 fuzz/rbf: Add missing TestingSetup (Carl Dong)
ccbaf54 scripted-diff: Rename DEFAULT_MAX_MEMPOOL_SIZE to indicate SI unit (Carl Dong)
fc02f77 ArgsMan: Add Get*Arg functions returning optional (Carl Dong)

Pull request description:

  This is part of the `libbitcoinkernel` project: bitcoin#24303, https://github.com/bitcoin/bitcoin/projects/18

  -----

  As mentioned in the Stage 1 Step 2 description of [the `libbitcoinkernel` project](bitcoin#24303), `ArgsManager` will not be part of `libbitcoinkernel`. Therefore, it is important that we remove any dependence on `ArgsManager` by code that will be part of `libbitcoinkernel`. This is the first in a series of PRs aiming to achieve this.

  This PR removes `CTxMemPool+MempoolAccept`'s dependency on `ArgsManager` by introducing a `CTxMemPool::Options` struct, which is used to specify `CTxMemPool`'s various options at construction time.

  These options are:
  - `-maxmempool` -> `CTxMemPool::Options::max_size`
  - `-mempoolexpiry` -> `CTxMemPool::Options::expiry`
  - `-limitancestorcount` -> `CTxMemPool::Options::limits::ancestor_count`
  - `-limitancestorsize` -> `CTxMemPool::Options::limits::ancestor_size`
  - `-limitdescendantcount` -> `CTxMemPool::Options::limits::descendant_count`
  - `-limitdescendantsize` -> `CTxMemPool::Options::limits::descendant_size`

  More context can be gleaned from the commit messages. The important commits are:

  - 56eb479ded8bfb2ef635bb6f3b484f9d5952c70d "pool: Add and use MemPoolOptions, ApplyArgsManOptions"
  - a1e08b70f3068f4e8def1c630d8f50cd54da7832 "mempool: Pass in -maxmempool instead of referencing gArgs"
  - 6f4bf3ede5812b374828f08fc728ceded2f10024 "mempool: Pass in -mempoolexpiry instead of referencing gArgs"
  - 5958a7fe4806599fc620ee8c1a881ca10fa2dd16 "mempool: Introduce (still-unused) MemPoolLimits"

  Reviewers: Help needed in the following commits (see commit messages):
  - a1e08b70f3068f4e8def1c630d8f50cd54da7832 "mempool: Pass in -maxmempool instead of referencing gArgs"
  - 0695081a797e9a5d7787b78b0f8289dafcc6bff7 "node/ifaces: Use existing MemPoolLimits"

  Note to Reviewers: There are perhaps an infinite number of ways to architect `CTxMemPool::Options`, the current one tries to keep it simple, usable, and flexible. I hope we don't spend too much time arguing over the design here since that's not the point. In the case that you're 100% certain that a different design is strictly better than this one in every regard, please show us a fully-implemented branch.

  -----

  TODO:
  - [x] Use the more ergonomic `CTxMemPool::Options` where appropriate
  - [x] Doxygen comments for `ApplyArgsManOptions`, `MemPoolOptions`

  -----

  Questions for Reviewers:
  1. Should we use `std::chrono::seconds` for `CTxMemPool::Options::expiry` and `CTxMemPool::m_expiry` instead of an `int64_t`? Something else? (`std::chrono::hours`?)
  2. Should I merge `CTxMemPool::Limits` inside `CTxMemPool::Options`?

ACKs for top commit:
  MarcoFalke:
    ACK d1684be 🍜
  ryanofsky:
    Code review ACK d1684be. Just minor cleanups since last review, mostly switching to brace initialization

Tree-SHA512: 2c138e52d69f61c263f1c3648f01c801338a8f576762c815f478ef5148b8b2f51e91ded5c1be915e678c0b14f6cfba894b82afec58d999d39a7bb7c914736e0b

Co-authored-by: MacroFake <falke.marco@gmail.com>

Author: knst

src/Makefile.am

@@ -284,6 +284,8 @@ BITCOIN_CORE_H = \
   kernel/checks.h \
   kernel/coinstats.h \
   kernel/context.h \
+  kernel/mempool_limits.h \
+  kernel/mempool_options.h \
   key.h \
   key_io.h \
   limitedmap.h \
@@ -315,6 +317,7 @@ BITCOIN_CORE_H = \
   masternode/payments.h \
   masternode/sync.h \
   masternode/utils.h \
+  mempool_args.h \
   memusage.h \
   merkleblock.h \
   messagesigner.h \
@@ -347,6 +350,7 @@ BITCOIN_CORE_H = \
   outputtype.h \
   policy/feerate.h \
   policy/fees.h \
+  policy/fees_args.h \
   policy/packages.h \
   policy/policy.h \
   policy/settings.h \
@@ -583,6 +587,7 @@ libbitcoin_node_a_SOURCES = \
   masternode/payments.cpp \
   masternode/sync.cpp \
   masternode/utils.cpp \
+  mempool_args.cpp \
   net.cpp \
   net_processing.cpp \
   netfulfilledman.cpp \
@@ -604,6 +609,7 @@ libbitcoin_node_a_SOURCES = \
   node/txreconciliation.cpp \
   noui.cpp \
   policy/fees.cpp \
+  policy/fees_args.cpp \
   policy/packages.cpp \
   policy/policy.cpp \
   policy/settings.cpp \

src/init.cpp

@@ -40,7 +40,7 @@
 #include <interfaces/wallet.h>
 #include <kernel/coinstats.h>
 #include <mapport.h>
-#include <node/miner.h>
+#include <mempool_args.h>
 #include <net.h>
 #include <net_permissions.h>
 #include <net_processing.h>
@@ -51,10 +51,12 @@
 #include <node/chainstate.h>
 #include <node/context.h>
 #include <node/interface_ui.h>
+#include <node/miner.h>
 #include <node/sync_manager.h>
 #include <node/txreconciliation.h>
 #include <policy/feerate.h>
 #include <policy/fees.h>
+#include <policy/fees_args.h>
 #include <policy/policy.h>
 #include <policy/settings.h>
 #include <rpc/blockchain.h>
@@ -583,10 +585,10 @@ void SetupServerArgs(ArgsManager& argsman)
     argsman.AddArg("-dbcache=<n>", strprintf("Maximum database cache size <n> MiB (%d to %d, default: %d). In addition, unused mempool memory is shared for this cache (see -maxmempool).", nMinDbCache, nMaxDbCache, nDefaultDbCache), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-includeconf=<file>", "Specify additional configuration file, relative to the -datadir path (only useable from configuration file, not command line)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-loadblock=<file>", "Imports blocks from external file on startup", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
-    argsman.AddArg("-maxmempool=<n>", strprintf("Keep the transaction memory pool below <n> megabytes (default: %u)", DEFAULT_MAX_MEMPOOL_SIZE), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    argsman.AddArg("-maxmempool=<n>", strprintf("Keep the transaction memory pool below <n> megabytes (default: %u)", DEFAULT_MAX_MEMPOOL_SIZE_MB), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-maxorphantxsize=<n>", strprintf("Maximum total size of all orphan transactions in megabytes (default: %u)", DEFAULT_MAX_ORPHAN_TRANSACTIONS_SIZE), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-maxrecsigsage=<n>", strprintf("Number of seconds to keep LLMQ recovery sigs (default: %u)", llmq::DEFAULT_MAX_RECOVERED_SIGS_AGE), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
-    argsman.AddArg("-mempoolexpiry=<n>", strprintf("Do not keep transactions in the mempool longer than <n> hours (default: %u)", DEFAULT_MEMPOOL_EXPIRY), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    argsman.AddArg("-mempoolexpiry=<n>", strprintf("Do not keep transactions in the mempool longer than <n> hours (default: %u)", DEFAULT_MEMPOOL_EXPIRY_HOURS), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-minimumchainwork=<hex>", strprintf("Minimum work assumed to exist on a valid chain in hex (default: %s, testnet: %s, devnet: %s)", defaultChainParams->GetConsensus().nMinimumChainWork.GetHex(), testnetChainParams->GetConsensus().nMinimumChainWork.GetHex(), devnetChainParams->GetConsensus().nMinimumChainWork.GetHex()), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::OPTIONS);
     argsman.AddArg("-par=<n>", strprintf("Set the number of script verification threads (%u to %d, 0 = auto, <0 = leave that many cores free, default: %d)",
         -GetNumCores(), MAX_SCRIPTCHECK_THREADS, DEFAULT_SCRIPTCHECK_THREADS), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
@@ -774,9 +776,9 @@ void SetupServerArgs(ArgsManager& argsman)
     argsman.AddArg("-deprecatedrpc=<method>", "Allows deprecated RPC method(s) to be used", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
     argsman.AddArg("-forceevodbrepair", "Force evodb masternode list diff verification and repair on startup, even if already repaired (default: 0)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
     argsman.AddArg("-limitancestorcount=<n>", strprintf("Do not accept transactions if number of in-mempool ancestors is <n> or more (default: %u)", DEFAULT_ANCESTOR_LIMIT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
-    argsman.AddArg("-limitancestorsize=<n>", strprintf("Do not accept transactions whose size with all in-mempool ancestors exceeds <n> kilobytes (default: %u)", DEFAULT_ANCESTOR_SIZE_LIMIT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
+    argsman.AddArg("-limitancestorsize=<n>", strprintf("Do not accept transactions whose size with all in-mempool ancestors exceeds <n> kilobytes (default: %u)", DEFAULT_ANCESTOR_SIZE_LIMIT_KVB), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
     argsman.AddArg("-limitdescendantcount=<n>", strprintf("Do not accept transactions if any ancestor would have <n> or more in-mempool descendants (default: %u)", DEFAULT_DESCENDANT_LIMIT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
-    argsman.AddArg("-limitdescendantsize=<n>", strprintf("Do not accept transactions if any ancestor would have more than <n> kilobytes of in-mempool descendants (default: %u).", DEFAULT_DESCENDANT_SIZE_LIMIT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
+    argsman.AddArg("-limitdescendantsize=<n>", strprintf("Do not accept transactions if any ancestor would have more than <n> kilobytes of in-mempool descendants (default: %u).", DEFAULT_DESCENDANT_SIZE_LIMIT_KVB), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
     argsman.AddArg("-stopafterblockimport", strprintf("Stop running after importing blocks from disk (default: %u)", DEFAULT_STOPAFTERBLOCKIMPORT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
     argsman.AddArg("-stopatheight", strprintf("Stop running after reaching the given height in the main chain (default: %u)", DEFAULT_STOPATHEIGHT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
     argsman.AddArg("-watchquorums=<n>", strprintf("Watch and validate quorum communication (default: %u)", llmq::DEFAULT_WATCH_QUORUMS), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
@@ -886,7 +888,6 @@ static void StartupNotify(const ArgsManager& args)
 static void PeriodicStats(NodeContext& node)
 {
     assert(::g_stats_client->active());
-    const ArgsManager& args = *Assert(node.args);
     ChainstateManager& chainman = *Assert(node.chainman);
     const CTxMemPool& mempool = *Assert(node.mempool);
     const llmq::CInstantSendManager& isman = *Assert(node.llmq_ctx->isman);
@@ -940,7 +941,7 @@ static void PeriodicStats(NodeContext& node)
         ::g_stats_client->gauge("transactions.mempool.totalTransactions", mempool.size(), 1.0f);
         ::g_stats_client->gauge("transactions.mempool.totalTxBytes", (int64_t) mempool.GetTotalTxSize(), 1.0f);
         ::g_stats_client->gauge("transactions.mempool.memoryUsageBytes", (int64_t) mempool.DynamicMemoryUsage(), 1.0f);
-        ::g_stats_client->gauge("transactions.mempool.minFeePerKb", mempool.GetMinFee(args.GetIntArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFeePerK(), 1.0f);
+        ::g_stats_client->gauge("transactions.mempool.minFeePerKb", mempool.GetMinFee().GetFeePerK(), 1.0f);
     }
     ::g_stats_client->gauge("transactions.mempool.lockedTransactions", isman.GetInstantSendLockCount(), 1.0f);
 }
@@ -1302,11 +1303,6 @@ bool AppInitParameterInteraction(const ArgsManager& args)
         LogPrintf("Warning: nMinimumChainWork set below default value of %s\n", chainparams.GetConsensus().nMinimumChainWork.GetHex());
     }
 
-    // mempool limits
-    int64_t nMempoolSizeMax = args.GetIntArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000;
-    int64_t nMempoolSizeMin = args.GetIntArg("-limitdescendantsize", DEFAULT_DESCENDANT_SIZE_LIMIT) * 1000 * 40;
-    if (nMempoolSizeMax < 0 || nMempoolSizeMax < nMempoolSizeMin)
-        return InitError(strprintf(_("-maxmempool must be at least %d MB"), std::ceil(nMempoolSizeMin / 1000000.0)));
     // incremental relay fee sets the amount the mempool min fee increases above the feerate of txs evicted due to mempool limiting.
     if (args.IsArgSet("-incrementalrelayfee")) {
         if (std::optional<CAmount> inc_relay_fee = ParseMoney(args.GetArg("-incrementalrelayfee", ""))) {
@@ -1710,7 +1706,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     assert(!node.fee_estimator);
     // Don't initialize fee estimation with old data if we don't relay transactions,
     // as they would never get updated.
-    if (!ignores_incoming_txs) node.fee_estimator = std::make_unique<CBlockPolicyEstimator>();
+    if (!ignores_incoming_txs) node.fee_estimator = std::make_unique<CBlockPolicyEstimator>(FeeestPath(args));
 
     assert(!node.mn_metaman);
     node.mn_metaman = std::make_unique<CMasternodeMetaMan>();
@@ -1970,7 +1966,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     // cache size calculations
     CacheSizes cache_sizes = CalculateCacheSizes(args, g_enabled_filter_types.size());
 
-    int64_t nMempoolSizeMax = args.GetIntArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000;
     LogPrintf("Cache configuration:\n");
     LogPrintf("* Using %.1f MiB for block index database\n", cache_sizes.block_tree_db * (1.0 / 1024 / 1024));
     if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX)) {
@@ -1990,15 +1985,26 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
                   cache_sizes.filter_index * (1.0 / 1024 / 1024), BlockFilterTypeName(filter_type));
     }
     LogPrintf("* Using %.1f MiB for chain state database\n", cache_sizes.coins_db * (1.0 / 1024 / 1024));
-    LogPrintf("* Using %.1f MiB for in-memory UTXO set (plus up to %.1f MiB of unused mempool space)\n", cache_sizes.coins * (1.0 / 1024 / 1024), nMempoolSizeMax * (1.0 / 1024 / 1024));
 
     assert(!node.mempool);
     assert(!node.chainman);
     assert(!node.mn_sync);
-    const int mempool_check_ratio = std::clamp<int>(args.GetIntArg("-checkmempool", chainparams.DefaultConsistencyChecks() ? 1 : 0), 0, 1000000);
+
+    CTxMemPool::Options mempool_opts{
+        .estimator = node.fee_estimator.get(),
+        .check_ratio = chainparams.DefaultConsistencyChecks() ? 1 : 0,
+    };
+    ApplyArgsManOptions(args, mempool_opts);
+    mempool_opts.check_ratio = std::clamp<int>(mempool_opts.check_ratio, 0, 1'000'000);
+
+    int64_t descendant_limit_bytes = mempool_opts.limits.descendant_size_vbytes * 40;
+    if (mempool_opts.max_size_bytes < 0 || mempool_opts.max_size_bytes < descendant_limit_bytes) {
+        return InitError(strprintf(_("-maxmempool must be at least %d MB"), std::ceil(descendant_limit_bytes / 1'000'000.0)));
+    }
+    LogPrintf("* Using %.1f MiB for in-memory UTXO set (plus up to %.1f MiB of unused mempool space)\n", cache_sizes.coins * (1.0 / 1024 / 1024), mempool_opts.max_size_bytes * (1.0 / 1024 / 1024));
 
     for (bool fLoaded = false; !fLoaded && !ShutdownRequested();) {
-        node.mempool = std::make_unique<CTxMemPool>(node.fee_estimator.get(), mempool_check_ratio);
+        node.mempool = std::make_unique<CTxMemPool>(mempool_opts);
 
         const ChainstateManager::Options chainman_opts{
             chainparams,

src/kernel/mempool_limits.h

@@ -0,0 +1,30 @@
+// Copyright (c) 2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+#ifndef BITCOIN_KERNEL_MEMPOOL_LIMITS_H
+#define BITCOIN_KERNEL_MEMPOOL_LIMITS_H
+
+#include <policy/policy.h>
+
+#include <cstdint>
+
+namespace kernel {
+/**
+ * Options struct containing limit options for a CTxMemPool. Default constructor
+ * populates the struct with sane default values which can be modified.
+ *
+ * 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
+
+#endif // BITCOIN_KERNEL_MEMPOOL_LIMITS_H

src/kernel/mempool_options.h

@@ -0,0 +1,38 @@
+// Copyright (c) 2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+#ifndef BITCOIN_KERNEL_MEMPOOL_OPTIONS_H
+#define BITCOIN_KERNEL_MEMPOOL_OPTIONS_H
+
+#include <kernel/mempool_limits.h>
+
+#include <chrono>
+#include <cstdint>
+
+class CBlockPolicyEstimator;
+
+/** Default for -maxmempool, maximum megabytes of mempool memory usage */
+static constexpr unsigned int DEFAULT_MAX_MEMPOOL_SIZE_MB{300};
+/** Default for -mempoolexpiry, expiration time for mempool transactions in hours */
+static constexpr unsigned int DEFAULT_MEMPOOL_EXPIRY_HOURS{336};
+
+namespace kernel {
+/**
+ * Options struct containing options for constructing a CTxMemPool. Default
+ * constructor populates the struct with sane default values which can be
+ * modified.
+ *
+ * Most of the time, this struct should be referenced as CTxMemPool::Options.
+ */
+struct MemPoolOptions {
+    /* Used to estimate appropriate transaction fees. */
+    CBlockPolicyEstimator* estimator{nullptr};
+    /* The ratio used to determine how often sanity checks will run.  */
+    int check_ratio{0};
+    int64_t max_size_bytes{DEFAULT_MAX_MEMPOOL_SIZE_MB * 1'000'000};
+    std::chrono::seconds expiry{std::chrono::hours{DEFAULT_MEMPOOL_EXPIRY_HOURS}};
+    MemPoolLimits limits{};
+};
+} // namespace kernel
+
+#endif // BITCOIN_KERNEL_MEMPOOL_OPTIONS_H

src/mempool_args.cpp

@@ -0,0 +1,37 @@
+// Copyright (c) 2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <mempool_args.h>
+
+#include <kernel/mempool_limits.h>
+#include <kernel/mempool_options.h>
+
+#include <util/system.h>
+
+using kernel::MemPoolLimits;
+using kernel::MemPoolOptions;
+
+namespace {
+void ApplyArgsManOptions(const ArgsManager& argsman, MemPoolLimits& mempool_limits)
+{
+    mempool_limits.ancestor_count = argsman.GetIntArg("-limitancestorcount", mempool_limits.ancestor_count);
+
+    if (auto vkb = argsman.GetIntArg("-limitancestorsize")) mempool_limits.ancestor_size_vbytes = *vkb * 1'000;
+
+    mempool_limits.descendant_count = argsman.GetIntArg("-limitdescendantcount", mempool_limits.descendant_count);
+
+    if (auto vkb = argsman.GetIntArg("-limitdescendantsize")) mempool_limits.descendant_size_vbytes = *vkb * 1'000;
+}
+}
+
+void ApplyArgsManOptions(const ArgsManager& argsman, MemPoolOptions& mempool_opts)
+{
+    mempool_opts.check_ratio = argsman.GetIntArg("-checkmempool", mempool_opts.check_ratio);
+
+    if (auto mb = argsman.GetIntArg("-maxmempool")) mempool_opts.max_size_bytes = *mb * 1'000'000;
+
+    if (auto hours = argsman.GetIntArg("-mempoolexpiry")) mempool_opts.expiry = std::chrono::hours{*hours};
+
+    ApplyArgsManOptions(argsman, mempool_opts.limits);
+}

src/mempool_args.h

@@ -0,0 +1,22 @@
+// Copyright (c) 2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_MEMPOOL_ARGS_H
+#define BITCOIN_MEMPOOL_ARGS_H
+
+class ArgsManager;
+namespace kernel {
+struct MemPoolOptions;
+};
+
+/**
+ * Overlay the options set in \p argsman on top of corresponding members in \p mempool_opts.
+ *
+ * @param[in]  argsman The ArgsManager in which to check set options.
+ * @param[in,out] mempool_opts The MemPoolOptions to modify according to \p argsman.
+ */
+void ApplyArgsManOptions(const ArgsManager& argsman, kernel::MemPoolOptions& mempool_opts);
+
+
+#endif // BITCOIN_MEMPOOL_ARGS_H

src/node/interfaces.cpp

@@ -858,7 +858,7 @@ class NodeImpl : public Node
     int64_t getTotalBytesSent() override { return m_context->connman ? m_context->connman->GetTotalBytesSent() : 0; }
     size_t getMempoolSize() override { return m_context->mempool ? m_context->mempool->size() : 0; }
     size_t getMempoolDynamicUsage() override { return m_context->mempool ? m_context->mempool->DynamicMemoryUsage() : 0; }
-    size_t getMempoolMaxUsage() override { return gArgs.GetIntArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000; }
+    size_t getMempoolMaxUsage() override { return gArgs.GetIntArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE_MB) * 1000000; }
     bool getHeaderTip(int& height, int64_t& block_time) override
     {
         LOCK(::cs_main);
@@ -1376,24 +1376,25 @@ class ChainImpl : public Chain
     }
     void getPackageLimits(unsigned int& limit_ancestor_count, unsigned int& limit_descendant_count) override
     {
-        limit_ancestor_count = gArgs.GetIntArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT);
-        limit_descendant_count = gArgs.GetIntArg("-limitdescendantcount", DEFAULT_DESCENDANT_LIMIT);
+        const CTxMemPool::Limits default_limits{};
+
+        const CTxMemPool::Limits& limits{m_node.mempool ? m_node.mempool->m_limits : default_limits};
+
+        limit_ancestor_count = limits.ancestor_count;
+        limit_descendant_count = limits.descendant_count;
     }
     bool checkChainLimits(const CTransactionRef& tx) override
     {
         if (!m_node.mempool) return true;
         LockPoints lp;
         CTxMemPoolEntry entry(tx, 0, 0, 0, false, 0, lp);
         CTxMemPool::setEntries ancestors;
-        auto limit_ancestor_count = gArgs.GetIntArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT);
-        auto limit_ancestor_size = gArgs.GetIntArg("-limitancestorsize", DEFAULT_ANCESTOR_SIZE_LIMIT) * 1000;
-        auto limit_descendant_count = gArgs.GetIntArg("-limitdescendantcount", DEFAULT_DESCENDANT_LIMIT);
-        auto limit_descendant_size = gArgs.GetIntArg("-limitdescendantsize", DEFAULT_DESCENDANT_SIZE_LIMIT) * 1000;
+        const CTxMemPool::Limits& limits{m_node.mempool->m_limits};
         std::string unused_error_string;
         LOCK(m_node.mempool->cs);
         return m_node.mempool->CalculateMemPoolAncestors(
-            entry, ancestors, limit_ancestor_count, limit_ancestor_size,
-            limit_descendant_count, limit_descendant_size, unused_error_string);
+            entry, ancestors, limits.ancestor_count, limits.ancestor_size_vbytes,
+            limits.descendant_count, limits.descendant_size_vbytes, unused_error_string);
     }
     CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc) override
     {
@@ -1408,7 +1409,7 @@ class ChainImpl : public Chain
     CFeeRate mempoolMinFee() override
     {
         if (!m_node.mempool) return {};
-        return m_node.mempool->GetMinFee(gArgs.GetIntArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000);
+        return m_node.mempool->GetMinFee();
     }
     CFeeRate relayMinFee() override { return ::minRelayTxFee; }
     CFeeRate relayIncrementalFee() override { return ::incrementalRelayFee; }