backport: bitcoin#26296, #27675, #28148, #28237, #28245, #28258, #28321

.github/workflows/build.yml

@@ -512,9 +512,9 @@ jobs:
         uses: actions/cache/restore@v5
         with:
           path: /cache/ctcache
-          key: ctcache-${{ hashFiles('contrib/containers/ci/ci.Dockerfile', 'depends/packages/*') }}-linux64_multiprocess-${{ github.sha }}
+          key: ctcache-${{ hashFiles('contrib/containers/ci/ci.Dockerfile', 'contrib/devtools/bitcoin-tidy/*', 'depends/packages/*') }}-linux64_multiprocess-${{ github.sha }}
           restore-keys: |
-            ctcache-${{ hashFiles('contrib/containers/ci/ci.Dockerfile', 'depends/packages/*') }}-linux64_multiprocess-
+            ctcache-${{ hashFiles('contrib/containers/ci/ci.Dockerfile', 'contrib/devtools/bitcoin-tidy/*', 'depends/packages/*') }}-linux64_multiprocess-
 
       - name: Run linters
         run: |
@@ -530,7 +530,7 @@ jobs:
         uses: actions/cache/save@v5
         with:
           path: /cache/ctcache
-          key: ctcache-${{ hashFiles('contrib/containers/ci/ci.Dockerfile', 'depends/packages/*') }}-linux64_multiprocess-${{ github.sha }}
+          key: ctcache-${{ hashFiles('contrib/containers/ci/ci.Dockerfile', 'contrib/devtools/bitcoin-tidy/*', 'depends/packages/*') }}-linux64_multiprocess-${{ github.sha }}
 
   test-linux64:
     name: linux64-test

ci/dash/lint-tidy.sh

@@ -43,6 +43,8 @@ fi
 
 # Setup ctcache for clang-tidy result caching
 export CTCACHE_SAVE_OUTPUT=1
+# The cache key is the preprocessed source: keep comments so that a NOLINT edit invalidates it
+export CTCACHE_KEEP_COMMENTS=1
 export CTCACHE_CLANG_TIDY="${CLANG_TIDY_BIN}"
 mkdir -p "${CTCACHE_DIR}"
 
@@ -64,6 +66,11 @@ fi
 # Zero stats before run to get accurate statistics for this run only
 python3 "${CLANG_TIDY_CACHE_PY}" --zero-stats 2>&1 || true
 
+TIDY_BUILD_DIR="${BASE_ROOT_DIR}/build-ci/tidy-build"
+cmake -B "${TIDY_BUILD_DIR}" -DLLVM_DIR="$(llvm-config --cmakedir)" -DCMAKE_BUILD_TYPE=Release -S "${BASE_ROOT_DIR}/contrib/devtools/bitcoin-tidy"
+cmake --build "${TIDY_BUILD_DIR}" "${MAKEJOBS}"
+cmake --build "${TIDY_BUILD_DIR}" --target bitcoin-tidy-tests "${MAKEJOBS}"
+
 cd "${BASE_ROOT_DIR}/build-ci/dashcore-${BUILD_TARGET}/src"
 
 # Filter out files by regex here, because regex may not be
@@ -84,6 +91,7 @@ if ! ( run-clang-tidy \
   -checks=clang-diagnostic-old-style-cast,google-readability-casting \
   -warnings-as-errors=-clang-diagnostic-old-style-cast,-google-readability-casting \
   -clang-tidy-binary="${CLANG_TIDY_CACHE}" \
+  -load="${TIDY_BUILD_DIR}/libbitcoin-tidy.so" \
   -p "${CAST_LINT_DB}" \
   -quiet "${MAKEJOBS}" | \
   python3 "${BASE_ROOT_DIR}/ci/dash/lint-cstyle-casts.py" filter --source-root "${BASE_ROOT_DIR}" | \

contrib/devtools/bitcoin-tidy/CMakeLists.txt

@@ -0,0 +1,56 @@
+cmake_minimum_required(VERSION 3.9)
+
+project(bitcoin-tidy VERSION 1.0.0 DESCRIPTION "clang-tidy checks for Bitcoin Core")
+
+include(GNUInstallDirs)
+
+set(CMAKE_CXX_STANDARD 17)
+set(CMAKE_CXX_STANDARD_REQUIRED True)
+set(CMAKE_CXX_EXTENSIONS False)
+
+# TODO: Figure out how to avoid the terminfo check
+find_package(LLVM REQUIRED CONFIG)
+find_program(CLANG_TIDY_EXE NAMES "clang-tidy-${LLVM_VERSION_MAJOR}" "clang-tidy" HINTS ${LLVM_TOOLS_BINARY_DIR})
+message(STATUS "Found LLVM ${LLVM_PACKAGE_VERSION}")
+message(STATUS "Found clang-tidy: ${CLANG_TIDY_EXE}")
+
+add_library(bitcoin-tidy MODULE bitcoin-tidy.cpp logprintf.cpp)
+target_include_directories(bitcoin-tidy SYSTEM PRIVATE ${LLVM_INCLUDE_DIRS})
+
+# Disable RTTI and exceptions as necessary
+if (MSVC)
+    target_compile_options(bitcoin-tidy PRIVATE /GR-)
+else()
+    target_compile_options(bitcoin-tidy PRIVATE -fno-rtti)
+    target_compile_options(bitcoin-tidy PRIVATE -fno-exceptions)
+endif()
+
+if(CMAKE_HOST_APPLE)
+    # ld64 expects no undefined symbols by default
+    target_link_options(bitcoin-tidy PRIVATE -Wl,-flat_namespace)
+    target_link_options(bitcoin-tidy PRIVATE -Wl,-undefined -Wl,suppress)
+endif()
+
+# Add warnings
+if (MSVC)
+    target_compile_options(bitcoin-tidy PRIVATE /W4)
+else()
+    target_compile_options(bitcoin-tidy PRIVATE -Wall)
+    target_compile_options(bitcoin-tidy PRIVATE -Wextra)
+endif()
+
+if(CMAKE_VERSION VERSION_LESS 3.27)
+    set(CLANG_TIDY_COMMAND "${CLANG_TIDY_EXE}" "--load=${CMAKE_BINARY_DIR}/${CMAKE_SHARED_MODULE_PREFIX}bitcoin-tidy${CMAKE_SHARED_MODULE_SUFFIX}" "-checks=-*,bitcoin-*")
+else()
+    # CLANG_TIDY_COMMAND supports generator expressions as of 3.27
+    set(CLANG_TIDY_COMMAND "${CLANG_TIDY_EXE}" "--load=$<TARGET_FILE:bitcoin-tidy>" "-checks=-*,bitcoin-*")
+endif()
+
+# Create a dummy library that runs clang-tidy tests as a side-effect of building
+add_library(bitcoin-tidy-tests OBJECT EXCLUDE_FROM_ALL example_logprintf.cpp)
+add_dependencies(bitcoin-tidy-tests bitcoin-tidy)
+
+set_target_properties(bitcoin-tidy-tests PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_COMMAND}")
+
+
+install(TARGETS bitcoin-tidy LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR})

contrib/devtools/bitcoin-tidy/README

@@ -0,0 +1,11 @@
+# Bitcoin Tidy
+
+Example Usage:
+
+```bash
+cmake -S . -B build -DLLVM_DIR=$(llvm-config --cmakedir) -DCMAKE_BUILD_TYPE=Release
+
+cmake --build build -j$(nproc)
+
+cmake --build build --target bitcoin-tidy-tests -j$(nproc)
+```

contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp

@@ -0,0 +1,22 @@
+// Copyright (c) 2023 Bitcoin Developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include "logprintf.h"
+
+#include <clang-tidy/ClangTidyModule.h>
+#include <clang-tidy/ClangTidyModuleRegistry.h>
+
+class BitcoinModule final : public clang::tidy::ClangTidyModule
+{
+public:
+    void addCheckFactories(clang::tidy::ClangTidyCheckFactories& CheckFactories) override
+    {
+        CheckFactories.registerCheck<bitcoin::LogPrintfCheck>("bitcoin-unterminated-logprintf");
+    }
+};
+
+static clang::tidy::ClangTidyModuleRegistry::Add<BitcoinModule>
+    X("bitcoin-module", "Adds bitcoin checks.");
+
+volatile int BitcoinModuleAnchorSource = 0;

contrib/devtools/bitcoin-tidy/example_logprintf.cpp

@@ -0,0 +1,108 @@
+// Copyright (c) 2023 Bitcoin Developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <string>
+
+// Test for bitcoin-unterminated-logprintf
+
+enum LogFlags {
+    NONE
+};
+
+enum Level {
+    None
+};
+
+template <typename... Args>
+static inline void LogPrintf_(const std::string& logging_function, const std::string& source_file, const int source_line, const LogFlags flag, const Level level, const char* fmt, const Args&... args)
+{
+}
+
+#define LogPrintLevel_(category, level, ...) LogPrintf_(__func__, __FILE__, __LINE__, category, level, __VA_ARGS__)
+#define LogPrintf(...) LogPrintLevel_(LogFlags::NONE, Level::None, __VA_ARGS__)
+
+#define LogPrint(category, ...) \
+    do {                        \
+        LogPrintf(__VA_ARGS__); \
+    } while (0)
+
+
+class CWallet
+{
+    std::string GetDisplayName() const
+    {
+        return "default wallet";
+    }
+
+public:
+    template <typename... Params>
+    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 " + std::string{fmt}).c_str(), GetDisplayName(), parameters...);
+    };
+};
+
+void good_func()
+{
+    LogPrintf("hello world!\n");
+}
+void good_func2()
+{
+    CWallet wallet;
+    wallet.WalletLogPrintf("hi\n");
+    ScriptPubKeyMan spkm;
+    spkm.WalletLogPrintf("hi\n");
+
+    const CWallet& walletref = wallet;
+    walletref.WalletLogPrintf("hi\n");
+
+    auto* walletptr = new CWallet();
+    walletptr->WalletLogPrintf("hi\n");
+    delete walletptr;
+}
+void bad_func()
+{
+    LogPrintf("hello world!");
+}
+void bad_func2()
+{
+    LogPrintf("");
+}
+void bad_func3()
+{
+    // Ending in "..." has no special meaning.
+    LogPrintf("hello world!...");
+}
+void bad_func4_ignored()
+{
+    LogPrintf("hello world!"); // NOLINT(bitcoin-unterminated-logprintf)
+}
+void bad_func5()
+{
+    CWallet wallet;
+    wallet.WalletLogPrintf("hi");
+    ScriptPubKeyMan spkm;
+    spkm.WalletLogPrintf("hi");
+
+    const CWallet& walletref = wallet;
+    walletref.WalletLogPrintf("hi");
+
+    auto* walletptr = new CWallet();
+    walletptr->WalletLogPrintf("hi");
+    delete walletptr;
+}

contrib/devtools/bitcoin-tidy/logprintf.cpp

@@ -0,0 +1,60 @@
+// Copyright (c) 2023 Bitcoin Developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include "logprintf.h"
+
+#include <clang/AST/ASTContext.h>
+#include <clang/ASTMatchers/ASTMatchFinder.h>
+
+
+namespace {
+AST_MATCHER(clang::StringLiteral, unterminated)
+{
+    size_t len = Node.getLength();
+    if (len > 0 && Node.getCodeUnit(len - 1) == '\n') {
+        return false;
+    }
+    return true;
+}
+} // namespace
+
+namespace bitcoin {
+
+void LogPrintfCheck::registerMatchers(clang::ast_matchers::MatchFinder* finder)
+{
+    using namespace clang::ast_matchers;
+
+    /*
+      Logprintf(..., ..., ..., ..., ..., "foo", ...)
+    */
+
+    finder->addMatcher(
+        callExpr(
+            callee(functionDecl(hasName("LogPrintf_"))),
+            hasArgument(5, stringLiteral(unterminated()).bind("logstring"))),
+        this);
+
+    /*
+      auto walletptr = &wallet;
+      wallet.WalletLogPrintf("foo");
+      wallet->WalletLogPrintf("foo");
+    */
+    finder->addMatcher(
+        cxxMemberCallExpr(
+            callee(cxxMethodDecl(hasName("WalletLogPrintf"))),
+            hasArgument(0, stringLiteral(unterminated()).bind("logstring"))),
+        this);
+}
+
+void LogPrintfCheck::check(const clang::ast_matchers::MatchFinder::MatchResult& Result)
+{
+    if (const clang::StringLiteral* lit = Result.Nodes.getNodeAs<clang::StringLiteral>("logstring")) {
+        const clang::ASTContext& ctx = *Result.Context;
+        const auto user_diag = diag(lit->getEndLoc(), "Unterminated format string used with LogPrintf");
+        const auto& loc = lit->getLocationOfByte(lit->getByteLength(), *Result.SourceManager, ctx.getLangOpts(), ctx.getTargetInfo());
+        user_diag << clang::FixItHint::CreateInsertion(loc, "\\n");
+    }
+}
+
+} // namespace bitcoin

contrib/devtools/bitcoin-tidy/logprintf.h

@@ -0,0 +1,29 @@
+// Copyright (c) 2023 Bitcoin Developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef LOGPRINTF_CHECK_H
+#define LOGPRINTF_CHECK_H
+
+#include <clang-tidy/ClangTidyCheck.h>
+
+namespace bitcoin {
+
+// Warn about any use of LogPrintf that does not end with a newline.
+class LogPrintfCheck final : public clang::tidy::ClangTidyCheck
+{
+public:
+    LogPrintfCheck(clang::StringRef Name, clang::tidy::ClangTidyContext* Context)
+        : clang::tidy::ClangTidyCheck(Name, Context) {}
+
+    bool isLanguageVersionSupported(const clang::LangOptions& LangOpts) const override
+    {
+        return LangOpts.CPlusPlus;
+    }
+    void registerMatchers(clang::ast_matchers::MatchFinder* Finder) override;
+    void check(const clang::ast_matchers::MatchFinder::MatchResult& Result) override;
+};
+
+} // namespace bitcoin
+
+#endif // LOGPRINTF_CHECK_H

src/.clang-tidy

@@ -1,5 +1,6 @@
 Checks: '
 -*,
+bitcoin-*,
 bugprone-argument-comment,
 bugprone-use-after-move,
 bugprone-lambda-function-name,

src/bench/mempool_eviction.cpp

@@ -13,11 +13,12 @@ static void AddTx(const CTransactionRef& tx, const CAmount& nFee, CTxMemPool& po
 {
     int64_t nTime = 0;
     unsigned int nHeight = 1;
+    uint64_t sequence = 0;
     bool spendsCoinbase = false;
     unsigned int sigOps = 1;
     LockPoints lp;
     pool.addUnchecked(CTxMemPoolEntry(
-        tx, nFee, nTime, nHeight,
+        tx, nFee, nTime, nHeight, sequence,
         spendsCoinbase, sigOps, lp));
 }
 

src/bench/mempool_stress.cpp

@@ -16,10 +16,11 @@ static void AddTx(const CTransactionRef& tx, CTxMemPool& pool) EXCLUSIVE_LOCKS_R
 {
     int64_t nTime = 0;
     unsigned int nHeight = 1;
+    uint64_t sequence = 0;
     bool spendsCoinbase = false;
     unsigned int sigOpCost = 4;
     LockPoints lp;
-    pool.addUnchecked(CTxMemPoolEntry(tx, 1000, nTime, nHeight, spendsCoinbase, sigOpCost, lp));
+    pool.addUnchecked(CTxMemPoolEntry(tx, 1000, nTime, nHeight, sequence, spendsCoinbase, sigOpCost, lp));
 }
 
 struct Available {

src/bench/rpc_mempool.cpp

@@ -16,7 +16,7 @@
 static void AddTx(const CTransactionRef& tx, const CAmount& fee, CTxMemPool& pool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, pool.cs)
 {
     LockPoints lp;
-    pool.addUnchecked(CTxMemPoolEntry(tx, fee, /*time=*/0, /*entry_height=*/1, /*spends_coinbase=*/false, /*sigops_count=*/1, lp));
+    pool.addUnchecked(CTxMemPoolEntry(tx, fee, /*time=*/0, /*entry_height=*/1, /*entry_sequence=*/0, /*spends_coinbase=*/false, /*sigops_count=*/1, lp));
 }
 
 static void RpcMempool(benchmark::Bench& bench)

src/chainlock/signing.cpp

@@ -149,7 +149,7 @@ void ChainLockSigner::TrySignChainTip()
 
             for (const auto& txid : *txids) {
                 if (!m_clhandler.IsTxSafeForMining(txid) && !m_isman.IsLocked(txid)) {
-                    LogPrint(BCLog::CHAINLOCKS, /* Continued */
+                    LogPrint(BCLog::CHAINLOCKS,
                              "%s -- not signing block %s due to TX %s not being islocked and not old enough.\n",
                              __func__, pindexWalk->GetBlockHash().ToString(), txid.ToString());
                     return;