backport: merge bitcoin#22903, #24753, #24831, #28240, #27012, #25713, #24971, #25047 (clang-tidy and IWYU)

ci/dash/build_src.sh

@@ -51,7 +51,12 @@ make distdir VERSION=$BUILD_TARGET
 cd dashcore-$BUILD_TARGET
 bash -c "./configure $BITCOIN_CONFIG_ALL $BITCOIN_CONFIG" || ( cat config.log && false)
 
-make $MAKEJOBS $GOAL || ( echo "Build failure. Verbose build follows." && make $GOAL V=1 ; false )
+if [ "${RUN_TIDY}" = "true" ]; then
+  MAYBE_BEAR="bear --config src/.bear-tidy-config"
+  MAYBE_TOKEN="--"
+fi
+
+bash -c "${MAYBE_BEAR} ${MAYBE_TOKEN} make ${MAKEJOBS} ${GOAL}" || ( echo "Build failure. Verbose build follows." && make $GOAL V=1 ; false )
 
 ccache --version | head -n 1 && ccache --show-stats
 
@@ -60,6 +65,24 @@ if [ -n "$USE_VALGRIND" ]; then
     ${BASE_ROOT_DIR}/ci/test/wrap-valgrind.sh
 fi
 
+if [ "${RUN_TIDY}" = "true" ]; then
+  set -eo pipefail
+  cd src
+  ( run-clang-tidy -quiet "${MAKEJOBS}" ) | grep -C5 "error"
+  cd ..
+  iwyu_tool.py \
+    "src/compat" \
+    "src/init" \
+    "src/rpc/fees.cpp" \
+    "src/rpc/signmessage.cpp" \
+    -p . "${MAKEJOBS}" \
+    -- -Xiwyu --cxx17ns -Xiwyu --mapping_file="${BASE_ROOT_DIR}/contrib/devtools/iwyu/bitcoin.core.imp" \
+    |& tee "/tmp/iwyu_ci.out"
+  cd src
+  fix_includes.py --nosafe_headers < /tmp/iwyu_ci.out
+  git --no-pager diff
+fi
+
 if [ "$RUN_SECURITY_TESTS" = "true" ]; then
   make test-security-check
 fi

ci/test/00_setup_env.sh

@@ -38,6 +38,7 @@ export USE_BUSY_BOX=${USE_BUSY_BOX:-false}
 
 export RUN_UNIT_TESTS=${RUN_UNIT_TESTS:-true}
 export RUN_FUNCTIONAL_TESTS=${RUN_FUNCTIONAL_TESTS:-true}
+export RUN_TIDY=${RUN_TIDY:-false}
 export RUN_SECURITY_TESTS=${RUN_SECURITY_TESTS:-false}
 # By how much to scale the test_runner timeouts (option --timeout-factor).
 # This is needed because some ci machines have slow CPU or disk, so sanitizers

ci/test/00_setup_env_native_multiprocess.sh

@@ -10,7 +10,10 @@ export CONTAINER_NAME=ci_native_multiprocess
 export HOST=x86_64-pc-linux-gnu
 export PACKAGES="cmake python3 llvm clang"
 export DEP_OPTS="MULTIPROCESS=1 CC=clang-18 CXX=clang++-18"
+export RUN_TIDY=true
 export GOAL="install"
 export TEST_RUNNER_EXTRA="--v2transport"
 export BITCOIN_CONFIG="--with-boost-process --enable-debug CC=clang-18 CXX=clang++-18" # Use clang to avoid OOM
+# Additional flags for RUN_TIDY
+export BITCOIN_CONFIG="${BITCOIN_CONFIG} --disable-hardening CFLAGS='-O0 -g0' CXXFLAGS='-O0 -g0'"
 export BITCOIND=dash-node  # Used in functional tests

configure.ac

@@ -1921,6 +1921,8 @@ AC_CONFIG_LINKS([contrib/devtools/test-security-check.py:contrib/devtools/test-s
 AC_CONFIG_LINKS([contrib/devtools/symbol-check.py:contrib/devtools/symbol-check.py])
 AC_CONFIG_LINKS([contrib/devtools/test-symbol-check.py:contrib/devtools/test-symbol-check.py])
 AC_CONFIG_LINKS([contrib/filter-lcov.py:contrib/filter-lcov.py])
+AC_CONFIG_LINKS([src/.bear-tidy-config:src/.bear-tidy-config])
+AC_CONFIG_LINKS([src/.clang-tidy:src/.clang-tidy])
 AC_CONFIG_LINKS([test/functional/test_runner.py:test/functional/test_runner.py])
 AC_CONFIG_LINKS([test/fuzz/test_runner.py:test/fuzz/test_runner.py])
 AC_CONFIG_LINKS([test/util/test_runner.py:test/util/test_runner.py])

contrib/containers/ci/Dockerfile

@@ -39,6 +39,7 @@ RUN set -ex; \
     autotools-dev \
     automake \
     autoconf \
+    bear \
     bison \
     build-essential \
     bsdmainutils \
@@ -83,12 +84,14 @@ RUN set -ex; \
     "clang-tidy-${LLVM_VERSION}" \
     "libc++-${LLVM_VERSION}-dev" \
     "libc++abi-${LLVM_VERSION}-dev" \
+    "libclang-${LLVM_VERSION}-dev" \
     "libclang-rt-${LLVM_VERSION}-dev" \
-    "lld-${LLVM_VERSION}"; \
+    "lld-${LLVM_VERSION}" \
+    "llvm-${LLVM_VERSION}-dev"; \
     rm -rf /var/lib/apt/lists/*; \
     echo "Setting defaults..."; \
     lldbUpdAltArgs="update-alternatives --install /usr/bin/llvm-config llvm-config /usr/bin/llvm-config-${LLVM_VERSION} 100"; \
-    for binName in clang clang++ clang-format clang-tidy clangd dsymutil lld lldb lldb-server llvm-ar llvm-cov llvm-nm llvm-objdump llvm-ranlib llvm-strip; do \
+    for binName in clang clang++ clang-apply-replacements clang-format clang-tidy clangd dsymutil lld lldb lldb-server llvm-ar llvm-cov llvm-nm llvm-objdump llvm-ranlib llvm-strip run-clang-tidy; do \
         lldbUpdAltArgs="${lldbUpdAltArgs} --slave /usr/bin/${binName} ${binName} /usr/bin/${binName}-${LLVM_VERSION}"; \
     done; \
     for binName in ld64.lld ld.lld lld-link wasm-ld; do \
@@ -140,6 +143,14 @@ RUN set -ex; \
     cd dash_hash && pip3 install -r requirements.txt .; \
     cd .. && rm -rf dash_hash
 
+RUN set -ex; \
+    git clone --depth=1 "https://github.com/include-what-you-use/include-what-you-use" -b "clang_${LLVM_VERSION}" /opt/iwyu; \
+    cd /opt/iwyu; \
+    mkdir build && cd build; \
+    cmake -G 'Unix Makefiles' -DCMAKE_PREFIX_PATH=/usr/lib/llvm-${LLVM_VERSION} ..; \
+    make install -j "$(( $(nproc) - 1 ))"; \
+    cd /opt && rm -rf /opt/iwyu;
+
 ARG SHELLCHECK_VERSION=v0.7.1
 RUN set -ex; \
     curl -fL "https://github.com/koalaman/shellcheck/releases/download/${SHELLCHECK_VERSION}/shellcheck-${SHELLCHECK_VERSION}.linux.x86_64.tar.xz" -o /tmp/shellcheck.tar.xz; \

contrib/devtools/iwyu/bitcoin.core.imp

@@ -0,0 +1,6 @@
+# Fixups / upstreamed changes
+[
+  { include: [ "<bits/termios-c_lflag.h>", private, "<termios.h>", public ] },
+  { include: [ "<bits/termios-struct.h>", private, "<termios.h>", public ] },
+  { include: [ "<bits/termios-tcflow.h>", private, "<termios.h>", public ] },
+]

src/.bear-tidy-config

@@ -0,0 +1,23 @@
+{
+  "output": {
+    "content": {
+      "include_only_existing_source": true,
+      "paths_to_include": [],
+      "paths_to_exclude": [
+        "src/crc32",
+        "src/crypto/x11",
+        "src/dashbls",
+        "src/gsl",
+        "src/immer",
+        "src/leveldb",
+        "src/minisketch",
+        "src/univalue",
+        "src/secp256k1"
+      ]
+    },
+    "format": {
+      "command_as_array": true,
+      "drop_output_field": false
+    }
+  }
+}

src/.clang-tidy

@@ -0,0 +1,11 @@
+Checks: '
+-*,
+bugprone-argument-comment,
+modernize-use-nullptr,
+readability-redundant-declaration,
+'
+WarningsAsErrors: '
+bugprone-argument-comment,
+modernize-use-nullptr,
+readability-redundant-declaration,
+'

src/bench/coin_selection.cpp

@@ -51,7 +51,7 @@ static void CoinSelection(benchmark::Bench& bench)
     const CoinSelectionParams coin_selection_params(/* change_output_size= */ 34,
                                                     /* change_spend_size= */ 148, /* effective_feerate= */ CFeeRate(0),
                                                     /* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0),
-                                                    /* tx_no_inputs_size= */ 0, /* avoid_partial= */ false);
+                                                    /* tx_noinputs_size= */ 0, /* avoid_partial= */ false);
     bench.run([&] {
         std::set<CInputCoin> setCoinsRet;
         CAmount nValueRet;

src/compat/byteswap.h

@@ -9,7 +9,7 @@
 #include <config/bitcoin-config.h>
 #endif
 
-#include <stdint.h>
+#include <cstdint>
 
 #if defined(HAVE_BYTESWAP_H)
 #include <byteswap.h>

src/compat/compat.h

@@ -22,19 +22,18 @@
 #include <ws2tcpip.h>
 #include <cstdint>
 #else
-#include <fcntl.h>
-#include <sys/mman.h>
-#include <sys/select.h>
-#include <sys/socket.h>
-#include <sys/types.h>
-#include <net/if.h>
-#include <netinet/in.h>
-#include <netinet/tcp.h>
-#include <arpa/inet.h>
-#include <ifaddrs.h>
-#include <limits.h>
-#include <netdb.h>
-#include <unistd.h>
+#include <arpa/inet.h>   // IWYU pragma: export
+#include <fcntl.h>       // IWYU pragma: export
+#include <ifaddrs.h>     // IWYU pragma: export
+#include <net/if.h>      // IWYU pragma: export
+#include <netdb.h>       // IWYU pragma: export
+#include <netinet/in.h>  // IWYU pragma: export
+#include <netinet/tcp.h> // IWYU pragma: export
+#include <sys/mman.h>    // IWYU pragma: export
+#include <sys/select.h>  // IWYU pragma: export
+#include <sys/socket.h>  // IWYU pragma: export
+#include <sys/types.h>   // IWYU pragma: export
+#include <unistd.h>      // IWYU pragma: export
 #endif
 
 // We map Linux / BSD error functions and codes, to the equivalent

src/compat/cpuid.h

@@ -10,6 +10,8 @@
 
 #include <cpuid.h>
 
+#include <cstdint>
+
 // We can't use cpuid.h's __get_cpuid as it does not support subleafs.
 void static inline GetCPUID(uint32_t leaf, uint32_t subleaf, uint32_t& a, uint32_t& b, uint32_t& c, uint32_t& d)
 {

src/compat/endian.h

@@ -11,7 +11,7 @@
 
 #include <compat/byteswap.h>
 
-#include <stdint.h>
+#include <cstdint>
 
 #if defined(HAVE_ENDIAN_H)
 #include <endian.h>

src/compat/stdin.cpp

@@ -2,23 +2,19 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
-#if defined(HAVE_CONFIG_H)
-#include <config/bitcoin-config.h>
-#endif
+#include <compat/stdin.h>
 
-#include <cstdio>       // for fileno(), stdin
+#include <cstdio>
 
 #ifdef WIN32
-#include <windows.h>    // for SetStdinEcho()
-#include <io.h>         // for isatty()
+#include <windows.h>
+#include <io.h>
 #else
-#include <termios.h>    // for SetStdinEcho()
-#include <unistd.h>     // for SetStdinEcho(), isatty()
-#include <poll.h>       // for StdinReady()
+#include <termios.h>
+#include <unistd.h>
+#include <poll.h>
 #endif
 
-#include <compat/stdin.h>
-
 // https://stackoverflow.com/questions/1413445/reading-a-password-from-stdcin
 void SetStdinEcho(bool enable)
 {

src/governance/governance.cpp

@@ -1545,7 +1545,7 @@ void CGovernanceManager::UpdatedBlockTip(const CBlockIndex* pindex, CConnman& co
 
 void CGovernanceManager::RequestOrphanObjects(CConnman& connman)
 {
-    const CConnman::NodesSnapshot snap{connman, /* filter = */ CConnman::FullyConnectedOnly};
+    const CConnman::NodesSnapshot snap{connman, /* cond = */ CConnman::FullyConnectedOnly};
 
     std::vector<uint256> vecHashesFiltered;
     {

src/init/common.cpp

@@ -9,16 +9,20 @@
 #include <bls/bls.h>
 #include <clientversion.h>
 #include <crypto/sha256.h>
+#include <fs.h>
 #include <key.h>
 #include <logging.h>
 #include <node/interface_ui.h>
 #include <random.h>
+#include <tinyformat.h>
 #include <util/string.h>
 #include <util/system.h>
 #include <util/time.h>
 #include <util/translation.h>
 
-#include <memory>
+#include <algorithm>
+#include <string>
+#include <vector>
 
 namespace init {
 void SetGlobals()

src/llmq/signing_shares.cpp

@@ -1113,7 +1113,7 @@ bool CSigSharesManager::SendMessages(CConnman& connman)
         return session->sendSessionId;
     };
 
-    const CConnman::NodesSnapshot snap{connman, /* filter = */ CConnman::FullyConnectedOnly};
+    const CConnman::NodesSnapshot snap{connman, /* cond = */ CConnman::FullyConnectedOnly};
     {
         LOCK(cs);
         CollectSigSharesToRequest(sigSharesToRequest);

src/mapport.cpp

@@ -195,7 +195,7 @@ static bool ProcessUpnp()
         std::string strDesc = PACKAGE_NAME " " + FormatFullVersion();
 
         do {
-            r = UPNP_AddPortMapping(urls.controlURL, data.first.servicetype, port.c_str(), port.c_str(), lanaddr, strDesc.c_str(), "TCP", 0, "0");
+            r = UPNP_AddPortMapping(urls.controlURL, data.first.servicetype, port.c_str(), port.c_str(), lanaddr, strDesc.c_str(), "TCP", nullptr, "0");
 
             if (r != UPNPCOMMAND_SUCCESS) {
                 ret = false;
@@ -208,7 +208,7 @@ static bool ProcessUpnp()
         } while (g_mapport_interrupt.sleep_for(PORT_MAPPING_REANNOUNCE_PERIOD));
         g_mapport_interrupt.reset();
 
-        r = UPNP_DeletePortMapping(urls.controlURL, data.first.servicetype, port.c_str(), "TCP", 0);
+        r = UPNP_DeletePortMapping(urls.controlURL, data.first.servicetype, port.c_str(), "TCP", nullptr);
         LogPrintf("UPNP_DeletePortMapping() returned: %d\n", r);
         freeUPNPDevlist(devlist); devlist = nullptr;
         FreeUPNPUrls(&urls);

src/masternode/sync.cpp

@@ -139,7 +139,7 @@ void CMasternodeSync::ProcessTick(const PeerManager& peerman, const CGovernanceM
     }
 
     nTimeLastProcess = GetTime();
-    const CConnman::NodesSnapshot snap{connman, /* filter = */ CConnman::FullyConnectedOnly};
+    const CConnman::NodesSnapshot snap{connman, /* cond = */ CConnman::FullyConnectedOnly};
 
     // gradually request the rest of the votes after sync finished
     if(IsSynced()) {

src/net.cpp

@@ -134,9 +134,6 @@ static const uint64_t SELECT_TIMEOUT_MILLISECONDS = 500;
 
 const std::string NET_MESSAGE_TYPE_OTHER = "*other*";
 
-constexpr const CConnman::CFullyConnectedOnly CConnman::FullyConnectedOnly;
-constexpr const CConnman::CAllNodes CConnman::AllNodes;
-
 static const uint64_t RANDOMIZER_ID_NETGROUP = 0x6c0edd8036ef4036ULL; // SHA256("netgroup")[0:8]
 static const uint64_t RANDOMIZER_ID_LOCALHOSTNONCE = 0xd93e69e2bbfa5735ULL; // SHA256("localhostnonce")[0:8]
 static const uint64_t RANDOMIZER_ID_ADDRCACHE = 0x1cf2e4ddd306dda9ULL; // SHA256("addrcache")[0:8]
@@ -2336,7 +2333,7 @@ void CConnman::CalculateNumConnectionsChangedStats()
     }
     mapRecvBytesMsgStats[NET_MESSAGE_TYPE_OTHER] = 0;
     mapSentBytesMsgStats[NET_MESSAGE_TYPE_OTHER] = 0;
-    const NodesSnapshot snap{*this, /* filter = */ CConnman::FullyConnectedOnly};
+    const NodesSnapshot snap{*this, /* cond = */ CConnman::FullyConnectedOnly};
     for (auto pnode : snap.Nodes()) {
         WITH_LOCK(pnode->cs_vRecv, pnode->UpdateRecvMapWithStats(mapRecvBytesMsgStats));
         WITH_LOCK(pnode->cs_vSend, pnode->UpdateSentMapWithStats(mapSentBytesMsgStats));
@@ -2705,7 +2702,7 @@ void CConnman::SocketHandler(CMasternodeSync& mn_sync)
     }();
 
     {
-        const NodesSnapshot snap{*this, /* filter = */ CConnman::AllNodes, /* shuffle = */ false};
+        const NodesSnapshot snap{*this, /* cond = */ CConnman::AllNodes, /* shuffle = */ false};
 
         // Check for the readiness of the already connected sockets and the
         // listening sockets in one call ("readiness" as in poll(2) or
@@ -3977,7 +3974,7 @@ void CConnman::ThreadMessageHandler()
         // Randomize the order in which we process messages from/to our peers.
         // This prevents attacks in which an attacker exploits having multiple
         // consecutive connections in the m_nodes list.
-        const NodesSnapshot snap{*this, /* filter = */ CConnman::AllNodes, /* shuffle = */ true};
+        const NodesSnapshot snap{*this, /* cond = */ CConnman::AllNodes, /* shuffle = */ true};
 
         for (CNode* pnode : snap.Nodes()) {
             if (pnode->fDisconnect)

src/net_processing.cpp

@@ -4763,7 +4763,7 @@ void PeerManagerImpl::ProcessMessage(
             // the peer if the header turns out to be for an invalid block.
             // Note that if a peer tries to build on an invalid chain, that
             // will be detected and the peer will be disconnected/discouraged.
-            return ProcessHeadersMessage(pfrom, *peer, {cmpctblock.header}, /*punish_duplicate_invalid=*/true);
+            return ProcessHeadersMessage(pfrom, *peer, {cmpctblock.header}, /*via_compact_block=*/true);
         }
 
         if (fBlockReconstructed) {

src/netaddress.cpp

@@ -19,9 +19,6 @@
 #include <ios>
 #include <tuple>
 
-constexpr size_t CNetAddr::V1_SERIALIZATION_SIZE;
-constexpr size_t CNetAddr::MAX_ADDRV2_SIZE;
-
 CNetAddr::BIP155Network CNetAddr::GetBIP155Network() const
 {
     switch (m_net) {

src/netbase.cpp

@@ -658,7 +658,7 @@ bool ConnectThroughProxy(const Proxy& proxy, const std::string& strDest, uint16_
             return false;
         }
     } else {
-        if (!Socks5(strDest, port, 0, sock)) {
+        if (!Socks5(strDest, port, nullptr, sock)) {
             return false;
         }
     }