refactor: Pass Logger instance to CTxMemPool

This allows libbitcoinkernel applications and test code to have the option to control where log output goes instead of having all output sent to the global logger.
ryanofsky · Jun 26, 2024 · 17cc01a · 17cc01a
1 parent fad5168
commit 17cc01a
Show file tree

Hide file tree

Showing 7 changed files with 102 additions and 7 deletions.
diff --git a/src/init.cpp b/src/init.cpp
@@ -1619,6 +1619,29 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     assert(!node.mempool);
     assert(!node.chainman);
 
+<<<<<<< HEAD
+||||||| parent of 4b6e43275bc1 (refactor: Pass Logger instance to CTxMemPool)
+    CTxMemPool::Options mempool_opts{
+        .check_ratio = chainparams.DefaultConsistencyChecks() ? 1 : 0,
+        .signals = &validation_signals,
+    };
+    auto result{ApplyArgsManOptions(args, chainparams, mempool_opts)};
+    if (!result) {
+        return InitError(util::ErrorString(result));
+    }
+
+=======
+    CTxMemPool::Options mempool_opts{
+        .check_ratio = chainparams.DefaultConsistencyChecks() ? 1 : 0,
+        .logger = &LogInstance(),
+        .signals = &validation_signals,
+    };
+    auto result{ApplyArgsManOptions(args, chainparams, mempool_opts)};
+    if (!result) {
+        return InitError(util::ErrorString(result));
+    }
+
+>>>>>>> 4b6e43275bc1 (refactor: Pass Logger instance to CTxMemPool)
     bool do_reindex{args.GetBoolArg("-reindex", false)};
     const bool do_reindex_chainstate{args.GetBoolArg("-reindex-chainstate", false)};
 

diff --git a/src/kernel/mempool_options.h b/src/kernel/mempool_options.h
@@ -6,6 +6,7 @@
 
 #include <kernel/mempool_limits.h>
 
+#include <logging.h>
 #include <policy/feerate.h>
 #include <policy/policy.h>
 
@@ -56,6 +57,7 @@ struct MemPoolOptions {
     bool persist_v1_dat{DEFAULT_PERSIST_V1_DAT};
     MemPoolLimits limits{};
 
+    BCLog::Logger* logger{nullptr};
     ValidationSignals* signals{nullptr};
 };
 } // namespace kernel

diff --git a/src/test/fuzz/mini_miner.cpp b/src/test/fuzz/mini_miner.cpp
@@ -36,7 +36,7 @@ FUZZ_TARGET(mini_miner, .init = initialize_miner)
 {
     FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
     bilingual_str error;
-    CTxMemPool pool{CTxMemPool::Options{}, error};
+    CTxMemPool pool{CTxMemPool::Options{.logger = &g_setup->m_logger}, error};
     Assert(error.empty());
     std::vector<COutPoint> outpoints;
     std::deque<COutPoint> available_coins = g_available_coins;
@@ -114,7 +114,7 @@ FUZZ_TARGET(mini_miner_selection, .init = initialize_miner)
 {
     FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
     bilingual_str error;
-    CTxMemPool pool{CTxMemPool::Options{}, error};
+    CTxMemPool pool{CTxMemPool::Options{.logger = &g_setup->m_logger}, error};
     Assert(error.empty());
     // Make a copy to preserve determinism.
     std::deque<COutPoint> available_coins = g_available_coins;

diff --git a/src/test/util/txmempool.cpp b/src/test/util/txmempool.cpp
@@ -23,6 +23,7 @@ CTxMemPool::Options MemPoolOptionsForTest(const NodeContext& node)
         // Default to always checking mempool regardless of
         // chainparams.DefaultConsistencyChecks for tests
         .check_ratio = 1,
+        .logger = &LogInstance(),
         .signals = node.validation_signals.get(),
     };
     const auto result{ApplyArgsManOptions(*node.args, ::Params(), mempool_opts)};

diff --git a/src/txmempool.cpp b/src/txmempool.cpp
@@ -280,7 +280,7 @@ CTxMemPool::setEntries CTxMemPool::AssumeCalculateMemPoolAncestors(
 {
     auto result{CalculateMemPoolAncestors(entry, limits, fSearchForParents)};
     if (!Assume(result)) {
-        LogPrintLevel(BCLog::MEMPOOL, BCLog::Level::Error, "%s: CalculateMemPoolAncestors failed unexpectedly, continuing with empty ancestor set (%s)\n",
+        LogError(m_log, "%s: CalculateMemPoolAncestors failed unexpectedly, continuing with empty ancestor set (%s)\n",
                       calling_fn_name, util::ErrorString(result).original);
     }
     return std::move(result).value_or(CTxMemPool::setEntries{});
@@ -412,7 +412,7 @@ static CTxMemPool::Options&& Flatten(CTxMemPool::Options&& opts, bilingual_str&
 }
 
 CTxMemPool::CTxMemPool(Options opts, bilingual_str& error)
-    : m_opts{Flatten(std::move(opts), error)}
+    : m_opts{Flatten(std::move(opts), error)}, m_log{BCLog::MEMPOOL, *Assert(m_opts.logger)}
 {
 }
 
@@ -697,7 +697,13 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei
 
     AssertLockHeld(::cs_main);
     LOCK(cs);
+<<<<<<< HEAD
     LogDebug(BCLog::MEMPOOL, "Checking mempool with %u transactions and %u inputs\n", (unsigned int)mapTx.size(), (unsigned int)mapNextTx.size());
+||||||| parent of 4b6e43275bc1 (refactor: Pass Logger instance to CTxMemPool)
+    LogPrint(BCLog::MEMPOOL, "Checking mempool with %u transactions and %u inputs\n", (unsigned int)mapTx.size(), (unsigned int)mapNextTx.size());
+=======
+    LogDebug(m_log, "Checking mempool with %u transactions and %u inputs\n", (unsigned int)mapTx.size(), (unsigned int)mapNextTx.size());
+>>>>>>> 4b6e43275bc1 (refactor: Pass Logger instance to CTxMemPool)
 
     uint64_t checkTotal = 0;
     CAmount check_total_fee{0};
@@ -935,9 +941,9 @@ void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeD
         }
         if (delta == 0) {
             mapDeltas.erase(hash);
-            LogPrintf("PrioritiseTransaction: %s (%sin mempool) delta cleared\n", hash.ToString(), it == mapTx.end() ? "not " : "");
+            LogInfo(m_log, "PrioritiseTransaction: %s (%sin mempool) delta cleared\n", hash.ToString(), it == mapTx.end() ? "not " : "");
         } else {
-            LogPrintf("PrioritiseTransaction: %s (%sin mempool) fee += %s, new delta=%s\n",
+            LogInfo(m_log, "PrioritiseTransaction: %s (%sin mempool) fee += %s, new delta=%s\n",
                       hash.ToString(),
                       it == mapTx.end() ? "not " : "",
                       FormatMoney(nFeeDelta),
@@ -1071,7 +1077,13 @@ void CTxMemPool::RemoveUnbroadcastTx(const uint256& txid, const bool unchecked)
 
     if (m_unbroadcast_txids.erase(txid))
     {
+<<<<<<< HEAD
         LogDebug(BCLog::MEMPOOL, "Removed %i from set of unbroadcast txns%s\n", txid.GetHex(), (unchecked ? " before confirmation that txn was sent out" : ""));
+||||||| parent of 4b6e43275bc1 (refactor: Pass Logger instance to CTxMemPool)
+        LogPrint(BCLog::MEMPOOL, "Removed %i from set of unbroadcast txns%s\n", txid.GetHex(), (unchecked ? " before confirmation that txn was sent out" : ""));
+=======
+        LogDebug(m_log, "Removed %i from set of unbroadcast txns%s\n", txid.GetHex(), (unchecked ? " before confirmation that txn was sent out" : ""));
+>>>>>>> 4b6e43275bc1 (refactor: Pass Logger instance to CTxMemPool)
     }
 }
 
@@ -1195,7 +1207,13 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpends
     }
 
     if (maxFeeRateRemoved > CFeeRate(0)) {
+<<<<<<< HEAD
         LogDebug(BCLog::MEMPOOL, "Removed %u txn, rolling minimum fee bumped to %s\n", nTxnRemoved, maxFeeRateRemoved.ToString());
+||||||| parent of 4b6e43275bc1 (refactor: Pass Logger instance to CTxMemPool)
+        LogPrint(BCLog::MEMPOOL, "Removed %u txn, rolling minimum fee bumped to %s\n", nTxnRemoved, maxFeeRateRemoved.ToString());
+=======
+        LogDebug(m_log, "Removed %u txn, rolling minimum fee bumped to %s\n", nTxnRemoved, maxFeeRateRemoved.ToString());
+>>>>>>> 4b6e43275bc1 (refactor: Pass Logger instance to CTxMemPool)
     }
 }
 

diff --git a/src/txmempool.h b/src/txmempool.h
@@ -14,6 +14,7 @@
 #include <kernel/mempool_limits.h>         // IWYU pragma: export
 #include <kernel/mempool_options.h>        // IWYU pragma: export
 #include <kernel/mempool_removal_reason.h> // IWYU pragma: export
+#include <logging.h>
 #include <policy/feerate.h>
 #include <policy/packages.h>
 #include <primitives/transaction.h>
@@ -437,6 +438,7 @@ class CTxMemPool
     using Options = kernel::MemPoolOptions;
 
     const Options m_opts;
+    const BCLog::Source m_log;
 
     /** Create a new CTxMemPool.
      * Sanity checks will be off by default for performance, because otherwise

diff --git a/src/validation.cpp b/src/validation.cpp
@@ -282,7 +282,13 @@ static void LimitMempoolSize(CTxMemPool& pool, CCoinsViewCache& coins_cache)
     AssertLockHeld(pool.cs);
     int expired = pool.Expire(GetTime<std::chrono::seconds>() - pool.m_opts.expiry);
     if (expired != 0) {
+<<<<<<< HEAD
         LogDebug(BCLog::MEMPOOL, "Expired %i transactions from the memory pool\n", expired);
+||||||| parent of 4b6e43275bc1 (refactor: Pass Logger instance to CTxMemPool)
+        LogPrint(BCLog::MEMPOOL, "Expired %i transactions from the memory pool\n", expired);
+=======
+        LogDebug(pool.m_log, "Expired %i transactions from the memory pool\n", expired);
+>>>>>>> 4b6e43275bc1 (refactor: Pass Logger instance to CTxMemPool)
     }
 
     std::vector<COutPoint> vNoSpendsRemaining;
@@ -1150,6 +1156,7 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector<CTransactionRef>& txn
 
     assert(txns.size() == workspaces.size());
 
+    const BCLog::Source& log_packages{BCLog::TXPACKAGES, m_pool.m_log.logger};
     auto result = m_pool.CheckPackageLimits(txns, total_vsize);
     if (!result) {
         // This is a package-wide error, separate from an individual transaction error.
@@ -1227,7 +1234,13 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector<CTransactionRef>& txn
                                      "package RBF failed: " + err_tup.value().second, "");
     }
 
+<<<<<<< HEAD
     LogDebug(BCLog::TXPACKAGES, "package RBF checks passed: parent %s (wtxid=%s), child %s (wtxid=%s), package hash (%s)\n",
+||||||| parent of 4b6e43275bc1 (refactor: Pass Logger instance to CTxMemPool)
+    LogPrint(BCLog::TXPACKAGES, "package RBF checks passed: parent %s (wtxid=%s), child %s (wtxid=%s)\n",
+=======
+    LogDebug(log_packages, "package RBF checks passed: parent %s (wtxid=%s), child %s (wtxid=%s)\n",
+>>>>>>> 4b6e43275bc1 (refactor: Pass Logger instance to CTxMemPool)
         txns.front()->GetHash().ToString(), txns.front()->GetWitnessHash().ToString(),
         txns.back()->GetHash().ToString(), txns.back()->GetWitnessHash().ToString(),
         GetPackageHash(txns).ToString());
@@ -1290,7 +1303,7 @@ bool MemPoolAccept::ConsensusScriptChecks(const ATMPArgs& args, Workspace& ws)
     unsigned int currentBlockScriptVerifyFlags{GetBlockScriptFlags(*m_active_chainstate.m_chain.Tip(), m_active_chainstate.m_chainman)};
     if (!CheckInputsFromMempoolAndCache(tx, state, m_view, m_pool, currentBlockScriptVerifyFlags,
                                         ws.m_precomputed_txdata, m_active_chainstate.CoinsTip(), GetValidationCache())) {
-        LogPrintf("BUG! PLEASE REPORT THIS! CheckInputScripts failed against latest-block but not STANDARD flags %s, %s\n", hash.ToString(), state.ToString());
+        LogInfo(m_pool.m_log, "BUG! PLEASE REPORT THIS! CheckInputScripts failed against latest-block but not STANDARD flags %s, %s\n", hash.ToString(), state.ToString());
         return Assume(false);
     }
 
@@ -1306,6 +1319,7 @@ void MemPoolAccept::FinalizeSubpackage(const ATMPArgs& args)
     // Remove conflicting transactions from the mempool
     for (CTxMemPool::txiter it : m_subpackage.m_changeset->GetRemovals())
     {
+<<<<<<< HEAD
         std::string log_string = strprintf("replacing mempool tx %s (wtxid=%s, fees=%s, vsize=%s). ",
                                       it->GetTx().GetHash().ToString(),
                                       it->GetTx().GetWitnessHash().ToString(),
@@ -1333,6 +1347,29 @@ void MemPoolAccept::FinalizeSubpackage(const ATMPArgs& args)
         }
         LogDebug(BCLog::MEMPOOL, "%s\n", log_string);
         TRACEPOINT(mempool, replaced,
+||||||| parent of 4b6e43275bc1 (refactor: Pass Logger instance to CTxMemPool)
+        LogPrint(BCLog::MEMPOOL, "replacing mempool tx %s (wtxid=%s, fees=%s, vsize=%s). New tx %s (wtxid=%s, fees=%s, vsize=%s)\n",
+                it->GetTx().GetHash().ToString(),
+                it->GetTx().GetWitnessHash().ToString(),
+                it->GetFee(),
+                it->GetTxSize(),
+                hash.ToString(),
+                tx.GetWitnessHash().ToString(),
+                entry->GetFee(),
+                entry->GetTxSize());
+        TRACE7(mempool, replaced,
+=======
+        LogDebug(m_pool.m_log, "replacing mempool tx %s (wtxid=%s, fees=%s, vsize=%s). New tx %s (wtxid=%s, fees=%s, vsize=%s)\n",
+                it->GetTx().GetHash().ToString(),
+                it->GetTx().GetWitnessHash().ToString(),
+                it->GetFee(),
+                it->GetTxSize(),
+                hash.ToString(),
+                tx.GetWitnessHash().ToString(),
+                entry->GetFee(),
+                entry->GetTxSize());
+        TRACE7(mempool, replaced,
+>>>>>>> 4b6e43275bc1 (refactor: Pass Logger instance to CTxMemPool)
                 it->GetTx().GetHash().data(),
                 it->GetTxSize(),
                 it->GetFee(),
@@ -1395,7 +1432,13 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector<Workspace>&
                    [](const auto& ws) { return ws.m_ptx->GetWitnessHash(); });
 
     if (!m_subpackage.m_replaced_transactions.empty()) {
+<<<<<<< HEAD
         LogDebug(BCLog::MEMPOOL, "replaced %u mempool transactions with %u new one(s) for %s additional fees, %d delta bytes\n",
+||||||| parent of 4b6e43275bc1 (refactor: Pass Logger instance to CTxMemPool)
+        LogPrint(BCLog::MEMPOOL, "replaced %u mempool transactions with %u new one(s) for %s additional fees, %d delta bytes\n",
+=======
+        LogDebug(m_pool.m_log, "replaced %u mempool transactions with %u new one(s) for %s additional fees, %d delta bytes\n",
+>>>>>>> 4b6e43275bc1 (refactor: Pass Logger instance to CTxMemPool)
                  m_subpackage.m_replaced_transactions.size(), workspaces.size(),
                  m_subpackage.m_total_modified_fees - m_subpackage.m_conflicting_fees,
                  m_subpackage.m_total_vsize - static_cast<int>(m_subpackage.m_conflicting_size));
@@ -1502,7 +1545,13 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef
     }
 
     if (!m_subpackage.m_replaced_transactions.empty()) {
+<<<<<<< HEAD
         LogDebug(BCLog::MEMPOOL, "replaced %u mempool transactions with 1 new transaction for %s additional fees, %d delta bytes\n",
+||||||| parent of 4b6e43275bc1 (refactor: Pass Logger instance to CTxMemPool)
+        LogPrint(BCLog::MEMPOOL, "replaced %u mempool transactions with 1 new transaction for %s additional fees, %d delta bytes\n",
+=======
+        LogDebug(m_pool.m_log, "replaced %u mempool transactions with 1 new transaction for %s additional fees, %d delta bytes\n",
+>>>>>>> 4b6e43275bc1 (refactor: Pass Logger instance to CTxMemPool)
                  m_subpackage.m_replaced_transactions.size(),
                  ws.m_modified_fees - m_subpackage.m_conflicting_fees,
                  ws.m_vsize - static_cast<int>(m_subpackage.m_conflicting_size));