fix: resolve or sidestep compiler warnings, re-enable -Werror for Clang, merge bitcoin#13633

ci/test/00_setup_env_mac.sh

@@ -15,4 +15,3 @@ export RUN_UNIT_TESTS=false
 export RUN_FUNCTIONAL_TESTS=false
 export GOAL="all deploy"
 export BITCOIN_CONFIG="--with-gui --enable-reduce-exports --disable-miner --with-boost-process"
-export NO_WERROR=1

ci/test/00_setup_env_native_fuzz.sh

@@ -16,4 +16,3 @@ export RUN_FUNCTIONAL_TESTS=false
 export RUN_FUZZ_TESTS=true
 export GOAL="install"
 export BITCOIN_CONFIG="--enable-zmq --disable-ccache --enable-fuzz --with-sanitizers=fuzzer,address,undefined,integer CC='clang-18 -ftrivial-auto-var-init=pattern' CXX='clang++-18 -ftrivial-auto-var-init=pattern' --with-boost-process"
-export NO_WERROR=1

ci/test/00_setup_env_native_multiprocess.sh

@@ -17,4 +17,3 @@ export BITCOIN_CONFIG="--with-boost-process --enable-debug CC=clang-18 CXX=clang
 # Additional flags for RUN_TIDY
 export BITCOIN_CONFIG="${BITCOIN_CONFIG} --disable-hardening CFLAGS='-O0 -g0' CXXFLAGS='-O0 -g0 -Wno-error=documentation'"
 export BITCOIND=dash-node  # Used in functional tests
-export NO_WERROR=1

ci/test/00_setup_env_native_tsan.sh

@@ -15,4 +15,3 @@ export GOAL="install"
 export BITCOIN_CONFIG="--enable-zmq --with-sanitizers=thread CC=clang-18 CXX=clang++-18 CXXFLAGS='-g' --with-boost-process"
 export CPPFLAGS="-DARENA_DEBUG -DDEBUG_LOCKORDER -DDEBUG_LOCKCONTENTION"
 export PYZMQ=true
-export NO_WERROR=1

ci/test/00_setup_env_native_ubsan.sh

@@ -13,4 +13,3 @@ export DEP_OPTS="NO_UPNP=1 DEBUG=1"
 export GOAL="install"
 export BITCOIN_CONFIG="--enable-zmq --enable-reduce-exports --enable-crash-hooks --with-sanitizers=undefined CC=clang-18 CXX=clang++-18"
 export PYZMQ=true
-export NO_WERROR=1

ci/test/00_setup_env_native_valgrind.sh

@@ -12,4 +12,3 @@ export NO_DEPENDS=1
 export TEST_RUNNER_EXTRA="--exclude rpc_bind,feature_bind_extra --timeout-factor=4"  # Excluded for now, see https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-602068547
 export GOAL="install"
 export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=no CC=clang-18 CXX=clang++-18"  # TODO enable GUI
-export NO_WERROR=1

src/coinjoin/client.cpp

@@ -105,7 +105,7 @@ PeerMsgRet CCoinJoinClientQueueManager::ProcessDSQueue(const CNode& peer, CConnm
 
         // if the queue is ready, submit if we can
         if (dsq.fReady &&
-            m_walletman.ForAnyCJClientMan([this, &connman, &dmn](std::unique_ptr<CCoinJoinClientManager>& clientman) {
+            m_walletman.ForAnyCJClientMan([&connman, &dmn](std::unique_ptr<CCoinJoinClientManager>& clientman) {
                 return clientman->TrySubmitDenominate(dmn->proTxHash, connman);
             })) {
             LogPrint(BCLog::COINJOIN, "DSQUEUE -- CoinJoin queue is ready, masternode=%s, queue=%s\n", dmn->proTxHash.ToString(), dsq.ToString());
@@ -161,14 +161,12 @@ void CCoinJoinClientManager::ProcessMessage(CNode& peer, CChainState& active_cha
     }
 }
 
-CCoinJoinClientSession::CCoinJoinClientSession(const std::shared_ptr<CWallet>& wallet, CoinJoinWalletManager& walletman,
-                                               CCoinJoinClientManager& clientman, CDeterministicMNManager& dmnman,
-                                               CMasternodeMetaMan& mn_metaman, const CMasternodeSync& mn_sync,
-                                               const llmq::CInstantSendManager& isman,
+CCoinJoinClientSession::CCoinJoinClientSession(const std::shared_ptr<CWallet>& wallet, CCoinJoinClientManager& clientman,
+                                               CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman,
+                                               const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman,
                                                const std::unique_ptr<CCoinJoinClientQueueManager>& queueman,
                                                bool is_masternode) :
     m_wallet(wallet),
-    m_walletman(walletman),
     m_clientman(clientman),
     m_dmnman(dmnman),
     m_mn_metaman(mn_metaman),
@@ -998,8 +996,7 @@ bool CCoinJoinClientManager::DoAutomaticDenominating(ChainstateManager& chainman
     AssertLockNotHeld(cs_deqsessions);
     LOCK(cs_deqsessions);
     if (int(deqSessions.size()) < CCoinJoinClientOptions::GetSessions()) {
-        deqSessions.emplace_back(m_wallet, m_walletman, *this, m_dmnman, m_mn_metaman, m_mn_sync, m_isman, m_queueman,
-                                 m_is_masternode);
+        deqSessions.emplace_back(m_wallet, *this, m_dmnman, m_mn_metaman, m_mn_sync, m_isman, m_queueman, m_is_masternode);
     }
     for (auto& session : deqSessions) {
         if (!CheckAutomaticBackup()) return false;
@@ -1905,9 +1902,8 @@ void CoinJoinWalletManager::Add(const std::shared_ptr<CWallet>& wallet)
 {
     LOCK(cs_wallet_manager_map);
     m_wallet_manager_map.try_emplace(wallet->GetName(),
-                                     std::make_unique<CCoinJoinClientManager>(wallet, *this, m_dmnman, m_mn_metaman,
-                                                                              m_mn_sync, m_isman, m_queueman,
-                                                                              m_is_masternode));
+                                     std::make_unique<CCoinJoinClientManager>(wallet, m_dmnman, m_mn_metaman, m_mn_sync,
+                                                                              m_isman, m_queueman, m_is_masternode));
 }
 
 void CoinJoinWalletManager::DoMaintenance(CConnman& connman)

src/coinjoin/client.h

@@ -139,7 +139,6 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession
 {
 private:
     const std::shared_ptr<CWallet> m_wallet;
-    CoinJoinWalletManager& m_walletman;
     CCoinJoinClientManager& m_clientman;
     CDeterministicMNManager& m_dmnman;
     CMasternodeMetaMan& m_mn_metaman;
@@ -201,10 +200,9 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession
     void SetNull() override EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin);
 
 public:
-    explicit CCoinJoinClientSession(const std::shared_ptr<CWallet>& wallet, CoinJoinWalletManager& walletman,
-                                    CCoinJoinClientManager& clientman, CDeterministicMNManager& dmnman,
-                                    CMasternodeMetaMan& mn_metaman, const CMasternodeSync& mn_sync,
-                                    const llmq::CInstantSendManager& isman,
+    explicit CCoinJoinClientSession(const std::shared_ptr<CWallet>& wallet, CCoinJoinClientManager& clientman,
+                                    CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman,
+                                    const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman,
                                     const std::unique_ptr<CCoinJoinClientQueueManager>& queueman, bool is_masternode);
 
     void ProcessMessage(CNode& peer, CChainState& active_chainstate, CConnman& connman, const CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv);
@@ -266,7 +264,6 @@ class CCoinJoinClientManager
 {
 private:
     const std::shared_ptr<CWallet> m_wallet;
-    CoinJoinWalletManager& m_walletman;
     CDeterministicMNManager& m_dmnman;
     CMasternodeMetaMan& m_mn_metaman;
     const CMasternodeSync& m_mn_sync;
@@ -305,12 +302,11 @@ class CCoinJoinClientManager
     CCoinJoinClientManager(CCoinJoinClientManager const&) = delete;
     CCoinJoinClientManager& operator=(CCoinJoinClientManager const&) = delete;
 
-    explicit CCoinJoinClientManager(const std::shared_ptr<CWallet>& wallet, CoinJoinWalletManager& walletman,
-                                    CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman,
-                                    const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman,
+    explicit CCoinJoinClientManager(const std::shared_ptr<CWallet>& wallet, CDeterministicMNManager& dmnman,
+                                    CMasternodeMetaMan& mn_metaman, const CMasternodeSync& mn_sync,
+                                    const llmq::CInstantSendManager& isman,
                                     const std::unique_ptr<CCoinJoinClientQueueManager>& queueman, bool is_masternode) :
         m_wallet(wallet),
-        m_walletman(walletman),
         m_dmnman(dmnman),
         m_mn_metaman(mn_metaman),
         m_mn_sync(mn_sync),

src/coinjoin/coinjoin.h

@@ -317,6 +317,7 @@ class CCoinJoinBaseSession
     int nSessionDenom{0}; // Users must submit a denom matching this
 
     CCoinJoinBaseSession() = default;
+    virtual ~CCoinJoinBaseSession() = default;
 
     int GetState() const { return nState; }
     std::string GetStateString() const;

src/evo/creditpool.cpp

@@ -235,10 +235,10 @@ CCreditPoolManager::CCreditPoolManager(CEvoDB& _evoDb)
 {
 }
 
-CCreditPoolDiff::CCreditPoolDiff(CCreditPool starter, const CBlockIndex *pindexPrev, const Consensus::Params& consensusParams, const CAmount blockSubsidy) :
+CCreditPoolDiff::CCreditPoolDiff(CCreditPool starter, const CBlockIndex* pindexPrev,
+                                 const Consensus::Params& consensusParams, const CAmount blockSubsidy) :
     pool(std::move(starter)),
-    pindexPrev(pindexPrev),
-    params(consensusParams)
+    pindexPrev(pindexPrev)
 {
     assert(pindexPrev);
 

src/evo/creditpool.h

@@ -75,7 +75,7 @@ class CCreditPoolDiff {
     CAmount platformReward{0};
 
     const CBlockIndex *pindexPrev{nullptr};
-    const Consensus::Params& params;
+
 public:
     explicit CCreditPoolDiff(CCreditPool starter, const CBlockIndex *pindexPrev,
                              const Consensus::Params& consensusParams,

src/evo/deterministicmns.cpp

@@ -1187,32 +1187,6 @@ void CDeterministicMNManager::CleanupCache(int nHeight)
 
 }
 
-[[nodiscard]] static bool EraseOldDBData(CDBWrapper& db, const std::vector<std::string>& db_key_prefixes)
-{
-    bool erased{false};
-    for(const auto& db_key_prefix : db_key_prefixes) {
-        CDBBatch batch{db};
-        std::unique_ptr<CDBIterator> it{db.NewIterator()};
-        std::pair firstKey{db_key_prefix, uint256()};
-        it->Seek(firstKey);
-        while (it->Valid()) {
-            decltype(firstKey) curKey;
-            if (!it->GetKey(curKey) || std::get<0>(curKey) != db_key_prefix) {
-                break;
-            }
-            batch.Erase(curKey);
-            erased = true;
-            it->Next();
-        }
-        if (erased) {
-            LogPrintf("CDeterministicMNManager::%s -- updating db...\n", __func__);
-            db.WriteBatch(batch);
-            LogPrintf("CDeterministicMNManager::%s -- done cleaning old data for %s\n", __func__, db_key_prefix);
-        }
-    }
-    return erased;
-}
-
 template <typename ProTx>
 static bool CheckService(const ProTx& proTx, TxValidationState& state)
 {

src/evo/deterministicmns.h

@@ -563,7 +563,6 @@ class CDeterministicMNManager
     // Main thread has indicated we should perform cleanup up to this height
     std::atomic<int> to_cleanup {0};
 
-    CChainState& m_chainstate;
     CEvoDB& m_evoDb;
 
     std::unordered_map<uint256, CDeterministicMNList, StaticSaltedHasher> mnListsCache GUARDED_BY(cs);
@@ -572,8 +571,7 @@ class CDeterministicMNManager
     const CBlockIndex* m_initial_snapshot_index GUARDED_BY(cs) {nullptr};
 
 public:
-    explicit CDeterministicMNManager(CChainState& chainstate, CEvoDB& evoDb) :
-        m_chainstate(chainstate),
+    explicit CDeterministicMNManager(CEvoDB& evoDb) :
         m_evoDb(evoDb)
     {
     }

src/node/chainstate.cpp

@@ -213,7 +213,7 @@ void DashChainstateSetup(ChainstateManager& chainman,
 {
     // Same logic as pblocktree
     dmnman.reset();
-    dmnman = std::make_unique<CDeterministicMNManager>(chainman.ActiveChainstate(), *evodb);
+    dmnman = std::make_unique<CDeterministicMNManager>(*evodb);
 
     cpoolman.reset();
     cpoolman = std::make_unique<CCreditPoolManager>(*evodb);

src/node/coinstats.cpp

@@ -91,19 +91,6 @@ static void ApplyStats(CCoinsStats& stats, const uint256& hash, const std::map<u
     }
 }
 
-static void ApplyStats(CCoinsStats& stats, std::nullptr_t, const uint256& hash, const std::map<uint32_t, Coin>& outputs)
-{
-    assert(!outputs.empty());
-    stats.nTransactions++;
-    for (const auto& output : outputs) {
-        stats.nTransactionOutputs++;
-        if (stats.total_amount.has_value()) {
-            stats.total_amount = CheckedAdd(*stats.total_amount, output.second.out.nValue);
-        }
-        stats.nBogoSize += GetBogoSize(output.second.out.scriptPubKey);
-    }
-}
-
 //! Calculate statistics about the unspent transaction output set
 template <typename T>
 static bool GetUTXOStats(CCoinsView* view, BlockManager& blockman, CCoinsStats& stats, T hash_obj, const std::function<void()>& interruption_point, const CBlockIndex* pindex)

src/qt/networkstyle.cpp

@@ -24,9 +24,9 @@ static const struct {
     const std::string titleAddText;
 } network_styles[] = {
     {"main", QAPP_APP_NAME_DEFAULT, 0, 0, ""},
-    {"test", QAPP_APP_NAME_TESTNET, 190, 20},
+    {"test", QAPP_APP_NAME_TESTNET, 190, 20, ""},
     {"devnet", QAPP_APP_NAME_DEVNET, 190, 20, "[devnet: %s]"},
-    {"regtest", QAPP_APP_NAME_REGTEST, 160, 30}
+    {"regtest", QAPP_APP_NAME_REGTEST, 160, 30, ""}
 };
 
 void NetworkStyle::rotateColor(QColor& col, const int iconColorHueShift, const int iconColorSaturationReduction)

src/rpc/evo.cpp

@@ -180,6 +180,19 @@ static RPCArg GetRpcArg(const std::string& strParamName)
     return it->second;
 }
 
+static CBLSSecretKey ParseBLSSecretKey(const std::string& hexKey, const std::string& paramName)
+{
+    CBLSSecretKey secKey;
+
+    // Actually, bool flag for bls::PrivateKey has other meaning (modOrder)
+    if (!secKey.SetHexStr(hexKey, false)) {
+        throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("%s must be a valid BLS secret key", paramName));
+    }
+    return secKey;
+}
+
+#ifdef ENABLE_WALLET
+
 static CKeyID ParsePubKeyIDFromAddress(const std::string& strAddress, const std::string& paramName)
 {
     CTxDestination dest = DecodeDestination(strAddress);
@@ -199,24 +212,11 @@ static CBLSPublicKey ParseBLSPubKey(const std::string& hexKey, const std::string
     return pubKey;
 }
 
-static CBLSSecretKey ParseBLSSecretKey(const std::string& hexKey, const std::string& paramName)
-{
-    CBLSSecretKey secKey;
-
-    // Actually, bool flag for bls::PrivateKey has other meaning (modOrder)
-    if (!secKey.SetHexStr(hexKey, false)) {
-        throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("%s must be a valid BLS secret key", paramName));
-    }
-    return secKey;
-}
-
 static bool ValidatePlatformPort(const int32_t port)
 {
     return port >= 1 && port <= std::numeric_limits<uint16_t>::max();
 }
 
-#ifdef ENABLE_WALLET
-
 template <typename SpecialTxPayload>
 static void FundSpecialTx(CWallet& wallet, CMutableTransaction& tx, const SpecialTxPayload& payload,
                           const CTxDestination& fundDest) EXCLUSIVE_LOCKS_REQUIRED(!wallet.cs_wallet)

src/script/bitcoinconsensus.cpp

@@ -48,7 +48,7 @@ class TxInputStream
         return *this;
     }
 
-    int GetVersion() const { return m_version; }
+    [[maybe_unused]] int GetVersion() const { return m_version; }
 private:
     const int m_version;
     const unsigned char* m_data;

src/script/interpreter.cpp

@@ -1521,7 +1521,7 @@ template void PrecomputedTransactionData::Init(const CMutableTransaction& txTo,
 template PrecomputedTransactionData::PrecomputedTransactionData(const CTransaction& txTo);
 template PrecomputedTransactionData::PrecomputedTransactionData(const CMutableTransaction& txTo);
 
-static bool HandleMissingData(MissingDataBehavior mdb)
+[[maybe_unused]] static bool HandleMissingData(MissingDataBehavior mdb)
 {
     switch (mdb) {
     case MissingDataBehavior::ASSERT_FAIL:

src/script/sign.cpp

@@ -239,17 +239,11 @@ struct Stacks
 {
     std::vector<valtype> script;
 
-    Stacks() {}
-    explicit Stacks(const std::vector<valtype>& scriptSigStack_) : script(scriptSigStack_) {}
+    Stacks() = delete;
+    Stacks(const Stacks&) = delete;
     explicit Stacks(const SignatureData& data) {
         EvalScript(script, data.scriptSig, SCRIPT_VERIFY_STRICTENC, BaseSignatureChecker(), SigVersion::BASE);
     }
-
-    SignatureData Output() const {
-        SignatureData result;
-        result.scriptSig = PushAll(script);
-        return result;
-    }
 };
 }
 

src/test/script_tests.cpp

@@ -282,19 +282,19 @@ class TestBuilder
         return *this;
     }
 
-    TestBuilder& Push(const std::string& hex)
+    [[maybe_unused]] TestBuilder& Push(const std::string& hex)
     {
         DoPush(ParseHex(hex));
         return *this;
     }
 
-    TestBuilder& Push(const uint256& hash)
+    [[maybe_unused]] TestBuilder& Push(const uint256& hash)
     {
         DoPush(ToByteVector(hash));
         return *this;
     }
 
-    TestBuilder& Push(const CScript& script)
+    [[maybe_unused]] TestBuilder& Push(const CScript& script)
     {
          DoPush(std::vector<unsigned char>(script.begin(), script.end()));
         return *this;

src/versionbits.h

@@ -107,13 +107,16 @@ class AbstractEHFManager
 public:
     using Signals = std::unordered_map<uint8_t, int>;
 
+public:
+    AbstractEHFManager() = default;
+    virtual ~AbstractEHFManager() = default;
+
     /**
      * getInstance() is used in versionbit because it is non-trivial
      * to get access to NodeContext from all usages of VersionBits* methods
      * For simplification of interface this methods static/global variable is used
      * to get access to EHF data
      */
-public:
     [[nodiscard]] static gsl::not_null<AbstractEHFManager*> getInstance() {
         return globalInstance;
     };
@@ -127,7 +130,6 @@ class AbstractEHFManager
      */
     virtual Signals GetSignalsStage(const CBlockIndex* const pindexPrev) = 0;
 
-
 protected:
     static AbstractEHFManager* globalInstance;
 };

src/wallet/hdchain.h

@@ -122,7 +122,7 @@ class CHDPubKey
 {
 private:
     static const int CURRENT_VERSION = 1;
-    int nVersion{CHDPubKey::CURRENT_VERSION};
+    [[maybe_unused]] int nVersion{CHDPubKey::CURRENT_VERSION};
 
 public:
     CExtPubKey extPubKey{};