Skip to content

Commit

Permalink
refactor: Pass Logger instance to CTxMemPool
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ryanofsky committed Dec 5, 2024
1 parent b8ec171 commit ca87fdd
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 16 deletions.
3 changes: 2 additions & 1 deletion src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/kernel/mempool_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include <kernel/mempool_limits.h>

#include <logging.h>
#include <policy/feerate.h>
#include <policy/policy.h>

Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/test/fuzz/mini_miner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/test/util/txmempool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)};
Expand Down
14 changes: 7 additions & 7 deletions src/txmempool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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{});
Expand Down Expand Up @@ -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)}
{
}

Expand Down Expand Up @@ -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};
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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" : ""));
}
}

Expand Down Expand Up @@ -1195,7 +1195,7 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector<COutPoint>* 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());
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/txmempool.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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
Expand Down
13 changes: 7 additions & 6 deletions src/validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ 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) {
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<COutPoint> vNoSpendsRemaining;
Expand Down Expand Up @@ -1138,6 +1138,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.
Expand Down Expand Up @@ -1215,7 +1216,7 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector<CTransactionRef>& 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());
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -1383,7 +1384,7 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector<Workspace>&
[](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<int>(m_subpackage.m_conflicting_size));
Expand Down Expand Up @@ -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<int>(m_subpackage.m_conflicting_size));
Expand Down

0 comments on commit ca87fdd

Please sign in to comment.