diff --git a/doc/developer-notes.md b/doc/developer-notes.md index d838ddff4b0..67e47e9772d 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -754,13 +754,12 @@ logging messages. They should be used as follows: most of the time, and it should be used for log messages that are useful for debugging and can reasonably be enabled on a production system (that has sufficient free storage space). They will be logged - if the program is started with `-debug=category` or `-debug=1`. + if the program is started with `-debug=category` or `-debug=1`, or + the category is enabled through the `logging` RPC. - `LogInfo(fmt, params...)` should only be used rarely, e.g. for startup messages or for infrequent and important events such as a new block tip - being found or a new outbound connection being made. These log messages - are unconditional, so care must be taken that they can't be used by an - attacker to fill up storage. + being found or a new outbound connection being made. - `LogError(fmt, params...)` should be used in place of `LogInfo` for severe problems that require the node (or a subsystem) to shut down @@ -782,6 +781,13 @@ Note that the format strings and parameters of `LogDebug` and `LogTrace` are only evaluated if the logging category is enabled, so you must be careful to avoid side-effects in those expressions. +While `LogInfo`, `LogWarning` and `LogError` messages should be rare, +in case there are circumstances where they are not, those messages +are automatically rate-limited to prevent potential disk-filling +attacks. For the cases where this protection is undesirable, +rate-limiting can be avoided with the `util::log::NO_RATE_LIMIT` tag, eg +`LogInfo(util::log::NO_RATE_LIMIT, "UpdateTip: new best=%s ...",...)`. + ## General C++ For general C++ guidelines, you may refer to the [C++ Core diff --git a/src/addrdb.cpp b/src/addrdb.cpp index 5db632c7e1e..14baf1a643d 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -14,7 +14,6 @@ #include #include #include -#include #include #include #include @@ -24,6 +23,7 @@ #include #include #include +#include #include #include diff --git a/src/addrman.cpp b/src/addrman.cpp index d3dae59ae79..92178f30089 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -9,7 +9,6 @@ #include #include -#include #include #include #include @@ -19,6 +18,7 @@ #include #include #include +#include #include #include diff --git a/src/addrman_impl.h b/src/addrman_impl.h index c1716f891d6..88ee92e1b04 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -5,13 +5,13 @@ #ifndef BITCOIN_ADDRMAN_IMPL_H #define BITCOIN_ADDRMAN_IMPL_H -#include #include #include #include #include #include #include +#include #include #include diff --git a/src/banman.cpp b/src/banman.cpp index 3c1f64e2263..54789f8d7d4 100644 --- a/src/banman.cpp +++ b/src/banman.cpp @@ -6,10 +6,10 @@ #include #include -#include #include #include #include +#include #include #include diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index ac731587683..debc7845618 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index 18799cd8083..c2846539bdb 100644 --- a/src/blockencodings.cpp +++ b/src/blockencodings.cpp @@ -9,10 +9,10 @@ #include #include #include -#include #include #include #include +#include #include #include @@ -219,7 +219,7 @@ ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector< return READ_STATUS_FAILED; // Possible Short ID collision } - if (LogAcceptCategory(BCLog::CMPCTBLOCK, BCLog::Level::Debug)) { + if (util::log::ShouldDebugLog(BCLog::CMPCTBLOCK)) { const uint256 hash{block.GetHash()}; uint32_t tx_missing_size{0}; for (const auto& tx : vtx_missing) tx_missing_size += tx->ComputeTotalSize(); diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 32c3bff7348..290b54261f0 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -9,9 +9,9 @@ #include #include #include -#include #include #include +#include #include #include diff --git a/src/common/args.cpp b/src/common/args.cpp index e153d886f9c..c24ec4912a3 100644 --- a/src/common/args.cpp +++ b/src/common/args.cpp @@ -7,7 +7,6 @@ #include #include -#include #include #include #include @@ -15,6 +14,7 @@ #include #include #include +#include #include #include diff --git a/src/common/config.cpp b/src/common/config.cpp index a05c20d6f8f..cc7ffd59537 100644 --- a/src/common/config.cpp +++ b/src/common/config.cpp @@ -5,12 +5,12 @@ #include #include -#include #include #include #include #include #include +#include #include #include diff --git a/src/common/init.cpp b/src/common/init.cpp index 7a1d664c659..5c9742bec47 100644 --- a/src/common/init.cpp +++ b/src/common/init.cpp @@ -5,9 +5,9 @@ #include #include #include -#include #include #include +#include #include #include diff --git a/src/common/netif.cpp b/src/common/netif.cpp index 8c0c4fa0832..af69b8d13a1 100644 --- a/src/common/netif.cpp +++ b/src/common/netif.cpp @@ -6,9 +6,9 @@ #include -#include #include #include +#include #include #include diff --git a/src/common/pcp.cpp b/src/common/pcp.cpp index a640b823f9c..22c42477322 100644 --- a/src/common/pcp.cpp +++ b/src/common/pcp.cpp @@ -7,12 +7,12 @@ #include #include #include -#include #include #include #include #include #include +#include #include #include #include diff --git a/src/common/system.cpp b/src/common/system.cpp index 08c0c692711..ca7b857d983 100644 --- a/src/common/system.cpp +++ b/src/common/system.cpp @@ -7,7 +7,7 @@ #include -#include +#include #include #include diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp index 7f6c10beae3..1d2b6f0d9a8 100644 --- a/src/dbwrapper.cpp +++ b/src/dbwrapper.cpp @@ -14,7 +14,6 @@ #include #include #include -#include #include #include #include @@ -59,7 +58,7 @@ public: // This code is adapted from posix_logger.h, which is why it is using vsprintf. // Please do not do this in normal code void Logv(const char * format, va_list ap) override { - if (!LogAcceptCategory(BCLog::LEVELDB, util::log::Level::Debug)) { + if (!util::log::ShouldDebugLog(BCLog::LEVELDB)) { return; } char buffer[500]; @@ -288,7 +287,7 @@ CDBWrapper::~CDBWrapper() void CDBWrapper::WriteBatch(CDBBatch& batch, bool fSync) { - const bool log_memory = LogAcceptCategory(BCLog::LEVELDB, util::log::Level::Debug); + const bool log_memory = util::log::ShouldDebugLog(BCLog::LEVELDB); double mem_before = 0; if (log_memory) { mem_before = DynamicMemoryUsage() / double(1_MiB); diff --git a/src/dummywallet.cpp b/src/dummywallet.cpp index 24952fea197..27e23800521 100644 --- a/src/dummywallet.cpp +++ b/src/dummywallet.cpp @@ -3,7 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include -#include +#include #include class ArgsManager; diff --git a/src/headerssync.cpp b/src/headerssync.cpp index 039e7f68e8a..633ffef53ad 100644 --- a/src/headerssync.cpp +++ b/src/headerssync.cpp @@ -4,9 +4,9 @@ #include -#include #include #include +#include #include #include diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 9a076996656..9a6b8eb0b2e 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -7,12 +7,12 @@ #include #include #include -#include #include #include #include #include #include +#include #include #include #include diff --git a/src/i2p.cpp b/src/i2p.cpp index 03513576c45..cf37272632f 100644 --- a/src/i2p.cpp +++ b/src/i2p.cpp @@ -8,7 +8,6 @@ #include #include #include -#include #include #include #include @@ -16,6 +15,7 @@ #include #include #include +#include #include #include #include diff --git a/src/index/db_key.h b/src/index/db_key.h index b7334c81e41..7c31f8afb8a 100644 --- a/src/index/db_key.h +++ b/src/index/db_key.h @@ -7,9 +7,9 @@ #include #include -#include #include #include +#include #include #include diff --git a/src/interfaces/node.h b/src/interfaces/node.h index f6d8209f072..ad6298e621f 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -7,12 +7,12 @@ #include #include -#include #include #include #include #include #include +#include #include #include diff --git a/src/ipc/capnp/protocol.cpp b/src/ipc/capnp/protocol.cpp index 2645e46f70f..e39c5039da9 100644 --- a/src/ipc/capnp/protocol.cpp +++ b/src/ipc/capnp/protocol.cpp @@ -10,10 +10,10 @@ #include #include #include -#include #include #include #include +#include #include #include @@ -33,8 +33,8 @@ namespace { mp::Log GetRequestedIPCLogLevel() { - if (LogAcceptCategory(BCLog::IPC, BCLog::Level::Trace)) return mp::Log::Trace; - if (LogAcceptCategory(BCLog::IPC, BCLog::Level::Debug)) return mp::Log::Debug; + if (util::log::ShouldTraceLog(BCLog::IPC)) return mp::Log::Trace; + if (util::log::ShouldDebugLog(BCLog::IPC)) return mp::Log::Debug; // Info, Warning, and Error are logged unconditionally return mp::Log::Info; diff --git a/src/ipc/interfaces.cpp b/src/ipc/interfaces.cpp index 1171f706487..32febd35526 100644 --- a/src/ipc/interfaces.cpp +++ b/src/ipc/interfaces.cpp @@ -9,9 +9,9 @@ #include #include #include -#include #include #include +#include #include #include diff --git a/src/ipc/process.cpp b/src/ipc/process.cpp index e89414be8e9..b60718e9931 100644 --- a/src/ipc/process.cpp +++ b/src/ipc/process.cpp @@ -4,10 +4,10 @@ #include #include -#include #include #include #include +#include #include #include diff --git a/src/ipc/test/ipc_test.cpp b/src/ipc/test/ipc_test.cpp index 31607299ae7..46366cef1f0 100644 --- a/src/ipc/test/ipc_test.cpp +++ b/src/ipc/test/ipc_test.cpp @@ -3,16 +3,16 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include #include -#include -#include -#include #include #include #include +#include #include +#include #include #include diff --git a/src/logging.cpp b/src/logging.cpp index 3373063c05c..4b6fd96b1ec 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -224,7 +224,7 @@ static const std::unordered_map LOG_CATEGORIES_BY_ }(LOG_CATEGORIES_BY_STR) }; -std::optional GetLogCategory(std::string_view str) +std::optional BCLog::Logger::GetLogCategory(std::string_view str) { if (str.empty() || str == "1" || str == "all") { return BCLog::ALL; @@ -514,6 +514,8 @@ void BCLog::Logger::LogPrint_(util::log::Entry entry) void BCLog::Logger::ShrinkDebugFile() { + STDLOCK(m_cs); + // Amount of debug.log to save at end when shrinking (must fit in memory) constexpr size_t RECENT_DEBUG_HISTORY_SIZE = 10 * 1000000; @@ -535,7 +537,14 @@ void BCLog::Logger::ShrinkDebugFile() // Restart the file with some of the end std::vector vch(RECENT_DEBUG_HISTORY_SIZE, 0); if (fseek(file, -((long)vch.size()), SEEK_END)) { - LogWarning("Failed to shrink debug log file: fseek(...) failed"); + // LogWarning, except with m_cs held + LogPrint_({ + .category = BCLog::ALL, + .level = Level::Warning, + .should_ratelimit = true, + .source_loc = SourceLocation{__func__}, + .message = "Failed to shrink debug log file: fseek(...) failed", + }); fclose(file); return; } @@ -563,8 +572,7 @@ void BCLog::LogRateLimiter::Reset() } for (const auto& [source_loc, stats] : source_locations) { if (stats.m_dropped_bytes == 0) continue; - LogPrintLevel_( - LogFlags::ALL, Level::Warning, /*should_ratelimit=*/false, + LogWarning(util::log::NO_RATE_LIMIT, "Restarting logging from %s:%d (%s): %d bytes were dropped during the last %ss.", source_loc.file_name(), source_loc.line(), source_loc.function_name_short(), stats.m_dropped_bytes, Ticks(m_reset_window)); @@ -604,9 +612,14 @@ bool BCLog::Logger::SetCategoryLogLevel(std::string_view category_str, std::stri return true; } -bool util::log::ShouldLog(Category category, Level level) +bool util::log::ShouldDebugLog(Category category) { - return LogInstance().WillLogCategoryLevel(static_cast(category), level); + return LogInstance().WillLogCategoryLevel(static_cast(category), util::log::Level::Debug); +} + +bool util::log::ShouldTraceLog(Category category) +{ + return LogInstance().WillLogCategoryLevel(static_cast(category), util::log::Level::Trace); } void util::log::Log(util::log::Entry entry) diff --git a/src/logging.h b/src/logging.h index 67f5d6ef007..4bdcd0f241d 100644 --- a/src/logging.h +++ b/src/logging.h @@ -227,7 +227,7 @@ namespace BCLog { */ void DisableLogging() EXCLUSIVE_LOCKS_REQUIRED(!m_cs); - void ShrinkDebugFile(); + void ShrinkDebugFile() EXCLUSIVE_LOCKS_REQUIRED(!m_cs); std::unordered_map CategoryLevels() const EXCLUSIVE_LOCKS_REQUIRED(!m_cs) { @@ -275,19 +275,12 @@ namespace BCLog { static std::string LogLevelToStr(BCLog::Level level); bool DefaultShrinkDebugFile() const; - }; + //! Return log flag if str parses as a log category. + static std::optional GetLogCategory(std::string_view str); + }; } // namespace BCLog BCLog::Logger& LogInstance(); -/** Return true if log accepts specified category, at the specified level. */ -static inline bool LogAcceptCategory(BCLog::LogFlags category, BCLog::Level level) -{ - return LogInstance().WillLogCategoryLevel(category, level); -} - -/// Return log flag if str parses as a log category. -std::optional GetLogCategory(std::string_view str); - #endif // BITCOIN_LOGGING_H diff --git a/src/mapport.cpp b/src/mapport.cpp index 43918ebf101..358956fafd4 100644 --- a/src/mapport.cpp +++ b/src/mapport.cpp @@ -8,11 +8,11 @@ #include #include #include -#include #include #include #include #include +#include #include #include diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 631e968da7f..50ea62ff05c 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2607,7 +2607,7 @@ void PeerManagerImpl::SendBlockTransactions(CNode& pfrom, Peer& peer, const CBlo resp.txn[i] = block.vtx[req.indexes[i]]; } - if (LogAcceptCategory(BCLog::CMPCTBLOCK, BCLog::Level::Debug)) { + if (util::log::ShouldDebugLog(BCLog::CMPCTBLOCK)) { uint32_t tx_requested_size{0}; for (const auto& tx : resp.txn) tx_requested_size += tx->ComputeTotalSize(); LogDebug(BCLog::CMPCTBLOCK, "Peer %d sent us a GETBLOCKTXN for block %s, sending a BLOCKTXN with %u txns. (%u bytes)\n", pfrom.GetId(), block.GetHash().ToString(), resp.txn.size(), tx_requested_size); diff --git a/src/net_types.cpp b/src/net_types.cpp index 15a122a7bbd..339d698a80c 100644 --- a/src/net_types.cpp +++ b/src/net_types.cpp @@ -4,10 +4,10 @@ #include -#include #include #include #include +#include static const char* BANMAN_JSON_VERSION_KEY{"version"}; diff --git a/src/netbase.cpp b/src/netbase.cpp index 5434ec9f14c..cea6fa29489 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -8,9 +8,9 @@ #include #include -#include #include #include +#include #include #include #include diff --git a/src/netgroup.cpp b/src/netgroup.cpp index 9b0d4d8d3c2..69fbef3ef22 100644 --- a/src/netgroup.cpp +++ b/src/netgroup.cpp @@ -5,9 +5,9 @@ #include #include -#include #include #include +#include #include diff --git a/src/node/caches.cpp b/src/node/caches.cpp index 3250c51a8c2..94d0ced88f6 100644 --- a/src/node/caches.cpp +++ b/src/node/caches.cpp @@ -9,10 +9,10 @@ #include #include #include -#include #include #include #include +#include #include #include diff --git a/src/node/chainstatemanager_args.cpp b/src/node/chainstatemanager_args.cpp index bf91a750c14..46a12bf21ed 100644 --- a/src/node/chainstatemanager_args.cpp +++ b/src/node/chainstatemanager_args.cpp @@ -7,12 +7,12 @@ #include #include #include -#include #include #include #include #include #include +#include #include #include #include diff --git a/src/node/kernel_notifications.cpp b/src/node/kernel_notifications.cpp index ab0e5ccb69e..66b77da5d1c 100644 --- a/src/node/kernel_notifications.cpp +++ b/src/node/kernel_notifications.cpp @@ -11,11 +11,11 @@ #include #include #include -#include #include #include #include #include +#include #include #include #include diff --git a/src/node/mempool_args.cpp b/src/node/mempool_args.cpp index d5ff50be96b..f0f7308bcef 100644 --- a/src/node/mempool_args.cpp +++ b/src/node/mempool_args.cpp @@ -11,11 +11,11 @@ #include #include #include -#include #include #include #include #include +#include #include #include diff --git a/src/node/mempool_persist.cpp b/src/node/mempool_persist.cpp index dcb05267edd..00f5306e218 100644 --- a/src/node/mempool_persist.cpp +++ b/src/node/mempool_persist.cpp @@ -6,7 +6,6 @@ #include #include -#include #include #include #include @@ -16,6 +15,7 @@ #include #include #include +#include #include #include #include diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 7fc19deeb79..c9a491ef23c 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -15,13 +15,13 @@ #include #include #include -#include #include #include #include #include #include #include +#include #include #include #include diff --git a/src/node/minisketchwrapper.cpp b/src/node/minisketchwrapper.cpp index 37089d56a7c..079990b4428 100644 --- a/src/node/minisketchwrapper.cpp +++ b/src/node/minisketchwrapper.cpp @@ -4,7 +4,7 @@ #include -#include +#include #include #include diff --git a/src/node/timeoffsets.cpp b/src/node/timeoffsets.cpp index 002c00d2455..e1a6bf71987 100644 --- a/src/node/timeoffsets.cpp +++ b/src/node/timeoffsets.cpp @@ -2,11 +2,11 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include #include #include #include #include +#include #include #include diff --git a/src/node/txdownloadman_impl.cpp b/src/node/txdownloadman_impl.cpp index 91bc7f3bcbd..3198fb17e64 100644 --- a/src/node/txdownloadman_impl.cpp +++ b/src/node/txdownloadman_impl.cpp @@ -7,8 +7,8 @@ #include #include -#include #include +#include #include #include diff --git a/src/node/txorphanage.cpp b/src/node/txorphanage.cpp index 33d3c23e57a..009379c9cd4 100644 --- a/src/node/txorphanage.cpp +++ b/src/node/txorphanage.cpp @@ -5,12 +5,12 @@ #include #include -#include #include #include #include -#include #include +#include +#include #include #include diff --git a/src/node/txreconciliation.cpp b/src/node/txreconciliation.cpp index 2333181cead..9a1f640f5a4 100644 --- a/src/node/txreconciliation.cpp +++ b/src/node/txreconciliation.cpp @@ -5,8 +5,8 @@ #include #include -#include #include +#include #include #include diff --git a/src/noui.cpp b/src/noui.cpp index 6f33b22728d..12ebc401c7f 100644 --- a/src/noui.cpp +++ b/src/noui.cpp @@ -6,8 +6,8 @@ #include #include -#include #include +#include #include #include diff --git a/src/policy/fees/block_policy_estimator.cpp b/src/policy/fees/block_policy_estimator.cpp index 84941b6edfd..748c49bcdc6 100644 --- a/src/policy/fees/block_policy_estimator.cpp +++ b/src/policy/fees/block_policy_estimator.cpp @@ -8,7 +8,6 @@ #include #include #include -#include #include #include #include @@ -18,6 +17,7 @@ #include #include #include +#include #include #include #include diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 0b89c605b9c..bf42f78da04 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -15,7 +15,6 @@ #include #include #include -#include #include #include #include @@ -33,6 +32,7 @@ #include #include #include +#include #include #include #include diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp index d9c97d85297..79c1911549c 100644 --- a/src/qt/transactiondesc.cpp +++ b/src/qt/transactiondesc.cpp @@ -14,8 +14,8 @@ #include #include #include -#include #include +#include #include #include diff --git a/src/script/signingprovider.cpp b/src/script/signingprovider.cpp index 19596544495..b680d18c2c6 100644 --- a/src/script/signingprovider.cpp +++ b/src/script/signingprovider.cpp @@ -7,7 +7,7 @@ #include