Merge bitcoin/bitcoin#28237: refactor: Enforce C-str fmt strings in WalletLogPrintf()

fa60fa3b0cba4a30726af8e0e9d1e84e14849eda bitcoin-tidy: Apply bitcoin-unterminated-logprintf to spkm as well (MarcoFalke)
faa11434fe38aa82892802adb6d879d112ae1675 refactor: Enable all clang-tidy plugin bitcoin tests (MarcoFalke)
fa6dc57760e0a04dbb2e365ca7ad9fd8171ebfdb refactor: Enforce C-str fmt strings in WalletLogPrintf() (MarcoFalke)
fa244f3321de7884f530bb38493a8d0a0cec86ab doc: Fix bitcoin-unterminated-logprintf tidy comments (MarcoFalke)

Pull request description:

  All fmt functions only accept a raw C-string as argument.

  There should never be a need to pass a format string that is not a compile-time string literal, so disallow it in `WalletLogPrintf()` to avoid accidentally introducing it.

  Apart from consistency, this also fixes the clang-tidy plugin bug https://github.com/bitcoin/bitcoin/pull/26296#discussion_r1286821141.

ACKs for top commit:
  theuni:
    ACK fa60fa3b0cba4a30726af8e0e9d1e84e14849eda

Tree-SHA512: fa6f4984c50f9b34e850bdfee7236706af586e512d866cc869cf0cdfaf9aa707029c210ca72d91f85e75fcbd8efe0d77084701de8c3d2004abfd7e46b6fa9072

3 files changed, 23 insertions, 7 deletions

diff --git a/contrib/devtools/bitcoin-tidy/example_logprintf.cpp b/contrib/devtools/bitcoin-tidy/example_logprintf.cpp
index a3d2768964..a12a666c08 100644
--- a/contrib/devtools/bitcoin-tidy/example_logprintf.cpp
+++ b/contrib/devtools/bitcoin-tidy/example_logprintf.cpp
@@ -2,9 +2,10 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
-// Warn about any use of LogPrintf that does not end with a newline.
 #include <string>
 
+// Test for bitcoin-unterminated-logprintf
+
 enum LogFlags {
     NONE
 };
@@ -21,8 +22,6 @@ static inline void LogPrintf_(const std::string& logging_function, const std::st
 #define LogPrintLevel_(category, level, ...) LogPrintf_(__func__, __FILE__, __LINE__, category, level, __VA_ARGS__)
 #define LogPrintf(...) LogPrintLevel_(LogFlags::NONE, Level::None, __VA_ARGS__)
 
-// Use a macro instead of a function for conditional logging to prevent
-// evaluating arguments when logging for the category is not enabled.
 #define LogPrint(category, ...) \
     do {                        \
         LogPrintf(__VA_ARGS__); \
@@ -38,9 +37,23 @@ class CWallet
 
 public:
     template <typename... Params>
-    void WalletLogPrintf(std::string fmt, Params... parameters) const
+    void WalletLogPrintf(const char* fmt, Params... parameters) const
+    {
+        LogPrintf(("%s " + std::string{fmt}).c_str(), GetDisplayName(), parameters...);
+    };
+};
+
+struct ScriptPubKeyMan
+{
+    std::string GetDisplayName() const
+    {
+        return "default wallet";
+    }
+
+    template <typename... Params>
+    void WalletLogPrintf(const char* fmt, Params... parameters) const
     {
-        LogPrintf(("%s " + fmt).c_str(), GetDisplayName(), parameters...);
+        LogPrintf(("%s " + std::string{fmt}).c_str(), GetDisplayName(), parameters...);
     };
 };
 
@@ -52,6 +65,8 @@ void good_func2()
 {
     CWallet wallet;
     wallet.WalletLogPrintf("hi\n");
+    ScriptPubKeyMan spkm;
+    spkm.WalletLogPrintf("hi\n");
 
     const CWallet& walletref = wallet;
     walletref.WalletLogPrintf("hi\n");
@@ -81,6 +96,8 @@ void bad_func5()
 {
     CWallet wallet;
     wallet.WalletLogPrintf("hi");
+    ScriptPubKeyMan spkm;
+    spkm.WalletLogPrintf("hi");
 
     const CWallet& walletref = wallet;
     walletref.WalletLogPrintf("hi");
diff --git a/contrib/devtools/bitcoin-tidy/logprintf.cpp b/contrib/devtools/bitcoin-tidy/logprintf.cpp
index 1690c8fde0..36beac28c8 100644
--- a/contrib/devtools/bitcoin-tidy/logprintf.cpp
+++ b/contrib/devtools/bitcoin-tidy/logprintf.cpp
@@ -36,14 +36,12 @@ void LogPrintfCheck::registerMatchers(clang::ast_matchers::MatchFinder* finder)
         this);
 
     /*
-      CWallet wallet;
       auto walletptr = &wallet;
       wallet.WalletLogPrintf("foo");
       wallet->WalletLogPrintf("foo");
     */
     finder->addMatcher(
         cxxMemberCallExpr(
-            thisPointerType(qualType(hasDeclaration(cxxRecordDecl(hasName("CWallet"))))),
             callee(cxxMethodDecl(hasName("WalletLogPrintf"))),
             hasArgument(0, stringLiteral(unterminated()).bind("logstring"))),
         this);
diff --git a/contrib/devtools/bitcoin-tidy/logprintf.h b/contrib/devtools/bitcoin-tidy/logprintf.h
index 466849ef01..db95dfe143 100644
--- a/contrib/devtools/bitcoin-tidy/logprintf.h
+++ b/contrib/devtools/bitcoin-tidy/logprintf.h
@@ -9,6 +9,7 @@
 
 namespace bitcoin {
 
+// Warn about any use of LogPrintf that does not end with a newline.
 class LogPrintfCheck final : public clang::tidy::ClangTidyCheck
 {
 public: