From ca87fdd928b0645c88dd6d3852e14dc50ff2118a Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 26 Jun 2024 13:04:51 -0400 Subject: [PATCH] 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. --- src/init.cpp | 3 ++- src/kernel/mempool_options.h | 2 ++ src/test/fuzz/mini_miner.cpp | 4 ++-- src/test/util/txmempool.cpp | 1 + src/txmempool.cpp | 14 +++++++------- src/txmempool.h | 2 ++ src/validation.cpp | 13 +++++++------ 7 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 609e92fe5b28b..2ac0f9e58e755 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1062,7 +1062,7 @@ bool AppInitParameterInteraction(const ArgsManager& args) if (!blockman_result) { return InitError(util::ErrorString(blockman_result)); } - CTxMemPool::Options mempool_opts{}; + CTxMemPool::Options mempool_opts{.logger = &LogInstance()}; auto mempool_result{ApplyArgsManOptions(args, chainparams, mempool_opts)}; if (!mempool_result) { return InitError(util::ErrorString(mempool_result)); @@ -1183,6 +1183,7 @@ static ChainstateLoadResult InitAndLoadChainstate( const CChainParams& chainparams = Params(); CTxMemPool::Options mempool_opts{ .check_ratio = chainparams.DefaultConsistencyChecks() ? 1 : 0, + .logger = &LogInstance(), .signals = node.validation_signals.get(), }; Assert(ApplyArgsManOptions(args, chainparams, mempool_opts)); // no error can happen, already checked in AppInitParameterInteraction diff --git a/src/kernel/mempool_options.h b/src/kernel/mempool_options.h index d57dbb393f3b5..285bfee936b5d 100644 --- a/src/kernel/mempool_options.h +++ b/src/kernel/mempool_options.h @@ -6,6 +6,7 @@ #include +#include #include #include @@ -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 index d06594d5f8d29..4a183836f135c 100644 --- 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 outpoints; std::deque 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 available_coins = g_available_coins; diff --git a/src/test/util/txmempool.cpp b/src/test/util/txmempool.cpp index f1ca33bec7859..5cf68bbdafbf4 100644 --- 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 index 950004044ee1d..2302b3acd3144 100644 --- 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,7 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei AssertLockHeld(::cs_main); LOCK(cs); - LogDebug(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()); uint64_t checkTotal = 0; CAmount check_total_fee{0}; @@ -935,9 +935,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 +1071,7 @@ void CTxMemPool::RemoveUnbroadcastTx(const uint256& txid, const bool unchecked) if (m_unbroadcast_txids.erase(txid)) { - LogDebug(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" : "")); } } @@ -1195,7 +1195,7 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector* pvNoSpends } if (maxFeeRateRemoved > CFeeRate(0)) { - LogDebug(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()); } } diff --git a/src/txmempool.h b/src/txmempool.h index e505c87f09c56..80fe5f646739c 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -14,6 +14,7 @@ #include // IWYU pragma: export #include // IWYU pragma: export #include // IWYU pragma: export +#include #include #include #include @@ -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 index cf24780d5fff7..95a068840a806 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -270,7 +270,7 @@ static void LimitMempoolSize(CTxMemPool& pool, CCoinsViewCache& coins_cache) AssertLockHeld(pool.cs); int expired = pool.Expire(GetTime() - pool.m_opts.expiry); if (expired != 0) { - LogDebug(BCLog::MEMPOOL, "Expired %i transactions from the memory pool\n", expired); + LogDebug(pool.m_log, "Expired %i transactions from the memory pool\n", expired); } std::vector vNoSpendsRemaining; @@ -1138,6 +1138,7 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector& 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. @@ -1215,7 +1216,7 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector& txn "package RBF failed: " + err_tup.value().second, ""); } - LogDebug(BCLog::TXPACKAGES, "package RBF checks passed: parent %s (wtxid=%s), child %s (wtxid=%s), package hash (%s)\n", + LogDebug(log_packages, "package RBF checks passed: parent %s (wtxid=%s), child %s (wtxid=%s), package hash (%s)\n", txns.front()->GetHash().ToString(), txns.front()->GetWitnessHash().ToString(), txns.back()->GetHash().ToString(), txns.back()->GetWitnessHash().ToString(), GetPackageHash(txns).ToString()); @@ -1278,7 +1279,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); } @@ -1319,7 +1320,7 @@ void MemPoolAccept::FinalizeSubpackage(const ATMPArgs& args) feerate.size); } - LogDebug(BCLog::MEMPOOL, "%s\n", log_string); + LogDebug(m_pool.m_log, "%s\n", log_string); TRACEPOINT(mempool, replaced, it->GetTx().GetHash().data(), it->GetTxSize(), @@ -1383,7 +1384,7 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector& [](const auto& ws) { return ws.m_ptx->GetWitnessHash(); }); if (!m_subpackage.m_replaced_transactions.empty()) { - LogDebug(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", 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(m_subpackage.m_conflicting_size)); @@ -1490,7 +1491,7 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef } if (!m_subpackage.m_replaced_transactions.empty()) { - LogDebug(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", m_subpackage.m_replaced_transactions.size(), ws.m_modified_fees - m_subpackage.m_conflicting_fees, ws.m_vsize - static_cast(m_subpackage.m_conflicting_size));