logging: move macros to util

Move logging macros to util/log.h so the entire codebase can use the same
macros. Since (un)conditional logging is a node concept, it is no longer
mentioned in the util macros. BCLog::Logger still maintains its logic to
ensure the log level never drops below Info.

LogAcceptCategory is now called for all logging levels as an early-exit
check, not just for Debug and Trace.

Author: stickies-v

doc/developer-notes.md

@@ -785,6 +785,11 @@ logging messages. They should be used as follows:
   options `-debug=category -loglevel=category:trace` or `-debug=1
   -loglevel=trace` are selected.
 
+Be conservative when using `LogInfo`, `LogWarning` or `LogError` as they will
+always write to debug.log, unless basic rate limiting quotas are exceeded. It
+should not be the case that an inbound peer can fill up a user's storage with
+debug.log entries.
+
 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.

src/logging.h

@@ -8,23 +8,19 @@
 
 #include <crypto/siphash.h>
 #include <threadsafety.h>
-#include <tinyformat.h>
 #include <util/check.h>
 #include <util/fs.h>
 #include <util/log.h> // IWYU pragma: export
-#include <util/string.h>
 #include <util/time.h>
 
 #include <atomic>
+#include <chrono>
 #include <cstdint>
-#include <cstring>
 #include <functional>
 #include <list>
 #include <memory>
-#include <mutex>
 #include <string>
 #include <unordered_map>
-#include <unordered_set>
 #include <vector>
 
 static const bool DEFAULT_LOGTIMEMICROS = false;
@@ -306,35 +302,4 @@ BCLog::Logger& LogInstance();
 /** Return true if str parses as a log category and set the flag */
 bool GetLogCategory(BCLog::LogFlags& flag, std::string_view str);
 
-// Allow __func__ to be used in any context without warnings:
-// NOLINTNEXTLINE(bugprone-lambda-function-name)
-#define LogPrintLevel_(category, level, should_ratelimit, ...) util::log::g_dispatcher().Log(level, category, SourceLocation{__func__}, should_ratelimit, __VA_ARGS__)
-
-// Log unconditionally. Uses basic rate limiting to mitigate disk filling attacks.
-// Be conservative when using functions that unconditionally log to debug.log!
-// It should not be the case that an inbound peer can fill up a user's storage
-// with debug.log entries.
-#define LogInfo(...) LogPrintLevel_(BCLog::LogFlags::ALL, BCLog::Level::Info, /*should_ratelimit=*/true, __VA_ARGS__)
-#define LogWarning(...) LogPrintLevel_(BCLog::LogFlags::ALL, BCLog::Level::Warning, /*should_ratelimit=*/true, __VA_ARGS__)
-#define LogError(...) LogPrintLevel_(BCLog::LogFlags::ALL, BCLog::Level::Error, /*should_ratelimit=*/true, __VA_ARGS__)
-
-// Use a macro instead of a function for conditional logging to prevent
-// evaluating arguments when logging for the category is not enabled.
-
-// Log by prefixing the output with the passed category name and severity level. This logs conditionally if
-// the category is allowed. No rate limiting is applied, because users specifying -debug are assumed to be
-// developers or power users who are aware that -debug may cause excessive disk usage due to logging.
-#define detail_LogIfCategoryAndLevelEnabled(category, level, ...)     \
-    do {                                                              \
-        if (LogAcceptCategory((category), (level))) {                 \
-            bool rate_limit{level >= BCLog::Level::Info};             \
-            Assume(!rate_limit);/*Only called with the levels below*/ \
-            LogPrintLevel_(category, level, rate_limit, __VA_ARGS__); \
-        }                                                             \
-    } while (0)
-
-// Log conditionally, prefixing the output with the passed category name.
-#define LogDebug(category, ...) detail_LogIfCategoryAndLevelEnabled(category, BCLog::Level::Debug, __VA_ARGS__)
-#define LogTrace(category, ...) detail_LogIfCategoryAndLevelEnabled(category, BCLog::Level::Trace, __VA_ARGS__)
-
 #endif // BITCOIN_LOGGING_H

src/util/log.h

@@ -8,6 +8,7 @@
 #include <logging/categories.h> // IWYU pragma: export
 #include <threadsafety.h>
 #include <tinyformat.h>
+#include <util/check.h>
 #include <util/string.h>
 #include <util/threadnames.h>
 
@@ -171,4 +172,25 @@ static inline bool LogAcceptCategory(uint64_t category, util::log::Level level)
     return util::log::g_dispatcher().WillLog(level, category);
 }
 
+// Allow __func__ to be used in any context without warnings:
+// NOLINTNEXTLINE(bugprone-lambda-function-name)
+#define LogPrintLevel_(category, level, should_ratelimit, ...) util::log::g_dispatcher().Log(level, category, SourceLocation{__func__}, should_ratelimit, __VA_ARGS__)
+
+// Arguments are always evaluated, even when logging is disabled.
+#define LogInfo(...) LogPrintLevel_(BCLog::LogFlags::ALL, util::log::Level::Info, /*should_ratelimit=*/true, __VA_ARGS__)
+#define LogWarning(...) LogPrintLevel_(BCLog::LogFlags::ALL, util::log::Level::Warning, /*should_ratelimit=*/true, __VA_ARGS__)
+#define LogError(...) LogPrintLevel_(BCLog::LogFlags::ALL, util::log::Level::Error, /*should_ratelimit=*/true, __VA_ARGS__)
+
+// Use a macro instead of a function to prevent evaluating arguments when the log will be dropped anyway.
+#define detail_LogIfCategoryAndLevelEnabled(category, level, ...)                     \
+    do {                                                                              \
+        Assume(level < util::log::Level::Info);                                       \
+        if (LogAcceptCategory((category), (level))) {                                 \
+            LogPrintLevel_(category, level, /*should_ratelimit=*/false, __VA_ARGS__); \
+        }                                                                             \
+    } while (0)
+
+#define LogDebug(category, ...) detail_LogIfCategoryAndLevelEnabled(category, util::log::Level::Debug, __VA_ARGS__)
+#define LogTrace(category, ...) detail_LogIfCategoryAndLevelEnabled(category, util::log::Level::Trace, __VA_ARGS__)
+
 #endif // BITCOIN_UTIL_LOG_H