Static analysis fixes (dashpay#4316)

* Make constructors explicit

Signed-off-by: pasta <pasta@dashboost.org>

* static analysis fixes

Signed-off-by: pasta <pasta@dashboost.org>

* Make pFrom nullptr check it's own

Signed-off-by: pasta <pasta@dashboost.org>

* revert std thread changes and hasOperatorKey

Signed-off-by: pasta <pasta@dashboost.org>

src/bls/bls.h

@@ -98,7 +98,7 @@ class CBLSWrapper
 
     void Reset()
     {
-        *((C*)this) = C(fLegacy);
+        *(static_cast<C*>(this)) = C(fLegacy);
     }
 
     void SetByteVector(const std::vector<uint8_t>& vecBytes)

src/cachemap.h

@@ -72,7 +72,7 @@ class CacheMap
           mapIndex()
     {}
 
-    CacheMap(const CacheMap<K,V>& other)
+    explicit CacheMap(const CacheMap<K,V>& other)
         : nMaxSize(other.nMaxSize),
           listItems(other.listItems),
           mapIndex()

src/cachemultimap.h

@@ -51,13 +51,13 @@ class CacheMultiMap
     map_t mapIndex;
 
 public:
-    CacheMultiMap(size_type nMaxSizeIn = 0)
+    explicit CacheMultiMap(size_type nMaxSizeIn = 0)
         : nMaxSize(nMaxSizeIn),
           listItems(),
           mapIndex()
     {}
 
-    CacheMultiMap(const CacheMap<K,V>& other)
+    explicit CacheMultiMap(const CacheMap<K,V>& other)
         : nMaxSize(other.nMaxSize),
           listItems(other.listItems),
           mapIndex()

src/coinjoin/coinjoin-client.cpp

@@ -322,7 +322,7 @@ std::string CCoinJoinClientSession::GetStatus(bool fWaitForBlock) const
             strSuffix = ".";
         else if (nStatusMessageProgress % 70 <= 50)
             strSuffix = "..";
-        else if (nStatusMessageProgress % 70 <= 70)
+        else
             strSuffix = "...";
         return strprintf(_("Submitted to masternode, waiting in queue %s"), strSuffix);
     case POOL_STATE_ACCEPTING_ENTRIES:
@@ -334,7 +334,7 @@ std::string CCoinJoinClientSession::GetStatus(bool fWaitForBlock) const
             strSuffix = ".";
         else if (nStatusMessageProgress % 70 <= 60)
             strSuffix = "..";
-        else if (nStatusMessageProgress % 70 <= 70)
+        else
             strSuffix = "...";
         return strprintf(_("Found enough users, signing ( waiting %s )"), strSuffix);
     case POOL_STATE_ERROR:

src/coinjoin/coinjoin-server.cpp

@@ -600,8 +600,8 @@ bool CCoinJoinServer::AddEntry(CConnman& connman, const CCoinJoinEntry& entry, P
     for (const auto& txin : entry.vecTxDSIn) {
         LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- txin=%s\n", __func__, txin.ToString());
 
-        for (const auto& entry : vecEntries) {
-            for (const auto& txdsin : entry.vecTxDSIn) {
+        for (const auto& inner_entry : vecEntries) {
+            for (const auto& txdsin : inner_entry.vecTxDSIn) {
                 if (txdsin.prevout == txin.prevout) {
                     LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- ERROR: already have this txin in entries\n", __func__);
                     nMessageIDRet = ERR_ALREADY_HAVE;

src/coinjoin/coinjoin-util.h

@@ -16,7 +16,7 @@ class CKeyHolder
     CPubKey pubKey;
 
 public:
-    CKeyHolder(CWallet* pwalletIn);
+    explicit CKeyHolder(CWallet* pwalletIn);
     CKeyHolder(CKeyHolder&&) = delete;
     CKeyHolder& operator=(CKeyHolder&&) = delete;
     void KeepKey();

src/dbwrapper.h

@@ -567,7 +567,7 @@ class CDBTransaction {
 
     struct ValueHolder {
         size_t memoryUsage;
-        ValueHolder(size_t _memoryUsage) : memoryUsage(_memoryUsage) {}
+        explicit ValueHolder(size_t _memoryUsage) : memoryUsage(_memoryUsage) {}
         virtual ~ValueHolder() = default;
         virtual void Write(const CDataStream& ssKey, CommitTarget &parent) = 0;
     };

src/evo/cbtx.cpp

@@ -55,8 +55,6 @@ bool CheckCbTxMerkleRoots(const CBlock& block, const CBlockIndex* pindex, CValid
     }
 
     static int64_t nTimePayload = 0;
-    static int64_t nTimeMerkleMNL = 0;
-    static int64_t nTimeMerkleQuorum = 0;
 
     int64_t nTime1 = GetTimeMicros();
 
@@ -69,6 +67,9 @@ bool CheckCbTxMerkleRoots(const CBlock& block, const CBlockIndex* pindex, CValid
     LogPrint(BCLog::BENCHMARK, "          - GetTxPayload: %.2fms [%.2fs]\n", 0.001 * (nTime2 - nTime1), nTimePayload * 0.000001);
 
     if (pindex) {
+        static int64_t nTimeMerkleMNL = 0;
+        static int64_t nTimeMerkleQuorum = 0;
+
         uint256 calculatedMerkleRoot;
         if (!CalcCbTxMerkleRootMNList(block, pindex->pprev, calculatedMerkleRoot, state, view)) {
             // pass the state returned by the function above
@@ -103,13 +104,13 @@ bool CalcCbTxMerkleRootMNList(const CBlock& block, const CBlockIndex* pindexPrev
 {
     LOCK(deterministicMNManager->cs);
 
-    static int64_t nTimeDMN = 0;
-    static int64_t nTimeSMNL = 0;
-    static int64_t nTimeMerkle = 0;
+    try {
+        static int64_t nTimeDMN = 0;
+        static int64_t nTimeSMNL = 0;
+        static int64_t nTimeMerkle = 0;
 
-    int64_t nTime1 = GetTimeMicros();
+        int64_t nTime1 = GetTimeMicros();
 
-    try {
         CDeterministicMNList tmpMNList;
         if (!deterministicMNManager->BuildNewListFromBlock(block, pindexPrev, state, view, tmpMNList, false)) {
             // pass the state returned by the function above
@@ -232,9 +233,8 @@ bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPre
     qcHashesVec.reserve(hashCount);
 
     for (const auto& p : qcHashes) {
-        for (const auto& h : p.second) {
-            qcHashesVec.emplace_back(h);
-        }
+        // Copy p.second into qcHashesVec
+        std::copy(p.second.begin(), p.second.end(), std::back_inserter(qcHashesVec));
     }
     std::sort(qcHashesVec.begin(), qcHashesVec.end());
 

src/evo/deterministicmns.cpp

@@ -918,7 +918,7 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, const C
     return true;
 }
 
-void CDeterministicMNManager::HandleQuorumCommitment(llmq::CFinalCommitment& qc, const CBlockIndex* pindexQuorum, CDeterministicMNList& mnList, bool debugLogs)
+void CDeterministicMNManager::HandleQuorumCommitment(const llmq::CFinalCommitment& qc, const CBlockIndex* pindexQuorum, CDeterministicMNList& mnList, bool debugLogs)
 {
     // The commitment has already been validated at this point, so it's safe to use members of it
 

src/evo/deterministicmns.h

@@ -677,7 +677,7 @@ class CDeterministicMNManager
 
     // the returned list will not contain the correct block hash (we can't know it yet as the coinbase TX is not updated yet)
     bool BuildNewListFromBlock(const CBlock& block, const CBlockIndex* pindexPrev, CValidationState& state, const CCoinsViewCache& view, CDeterministicMNList& mnListRet, bool debugLogs);
-    static void HandleQuorumCommitment(llmq::CFinalCommitment& qc, const CBlockIndex* pindexQuorum, CDeterministicMNList& mnList, bool debugLogs);
+    static void HandleQuorumCommitment(const llmq::CFinalCommitment& qc, const CBlockIndex* pindexQuorum, CDeterministicMNList& mnList, bool debugLogs);
     static void DecreasePoSePenalties(CDeterministicMNList& mnList);
 
     CDeterministicMNList GetListForBlock(const CBlockIndex* pindex);

src/evo/specialtx.cpp

@@ -91,12 +91,12 @@ bool UndoSpecialTx(const CTransaction& tx, const CBlockIndex* pindex)
 
 bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, CValidationState& state, const CCoinsViewCache& view, bool fJustCheck, bool fCheckCbTxMerleRoots)
 {
-    static int64_t nTimeLoop = 0;
-    static int64_t nTimeQuorum = 0;
-    static int64_t nTimeDMN = 0;
-    static int64_t nTimeMerkle = 0;
-
     try {
+        static int64_t nTimeLoop = 0;
+        static int64_t nTimeQuorum = 0;
+        static int64_t nTimeDMN = 0;
+        static int64_t nTimeMerkle = 0;
+
         int64_t nTime1 = GetTimeMicros();
 
         for (const auto& ptr_tx : block.vtx) {

src/governance/governance-classes.cpp

@@ -606,8 +606,6 @@ bool CSuperblock::IsValid(const CTransaction& txNew, int nBlockHeight, CAmount b
         return false;
     }
 
-    std::string strPayeesPossible;
-
     // CONFIGURE SUPERBLOCK OUTPUTS
 
     int nOutputs = txNew.vout.size();

src/governance/governance-vote.cpp

@@ -168,14 +168,14 @@ bool CGovernanceVote::Sign(const CKey& key, const CKeyID& keyID)
 
     // Harden Spork6 so that it is active on testnet and no other networks
     if (Params().NetworkIDString() == CBaseChainParams::TESTNET) {
-        uint256 hash = GetSignatureHash();
+        uint256 signatureHash = GetSignatureHash();
 
-        if (!CHashSigner::SignHash(hash, key, vchSig)) {
+        if (!CHashSigner::SignHash(signatureHash, key, vchSig)) {
             LogPrintf("CGovernanceVote::Sign -- SignHash() failed\n");
             return false;
         }
 
-        if (!CHashSigner::VerifyHash(hash, keyID, vchSig, strError)) {
+        if (!CHashSigner::VerifyHash(signatureHash, keyID, vchSig, strError)) {
             LogPrintf("CGovernanceVote::Sign -- VerifyHash() failed, error: %s\n", strError);
             return false;
         }
@@ -203,9 +203,7 @@ bool CGovernanceVote::CheckSignature(const CKeyID& keyID) const
 
     // Harden Spork6 so that it is active on testnet and no other networks
     if (Params().NetworkIDString() == CBaseChainParams::TESTNET) {
-        uint256 hash = GetSignatureHash();
-
-        if (!CHashSigner::VerifyHash(hash, keyID, vchSig, strError)) {
+        if (!CHashSigner::VerifyHash(GetSignatureHash(), keyID, vchSig, strError)) {
             LogPrint(BCLog::GOBJECT, "CGovernanceVote::IsValid -- VerifyHash() failed, error: %s\n", strError);
             return false;
         }
@@ -226,8 +224,7 @@ bool CGovernanceVote::CheckSignature(const CKeyID& keyID) const
 
 bool CGovernanceVote::Sign(const CBLSSecretKey& key)
 {
-    uint256 hash = GetSignatureHash();
-    CBLSSignature sig = key.Sign(hash);
+    CBLSSignature sig = key.Sign(GetSignatureHash());
     if (!sig.IsValid()) {
         return false;
     }

src/llmq/quorums.cpp

@@ -381,11 +381,15 @@ bool CQuorumManager::HasQuorum(Consensus::LLMQType llmqType, const uint256& quor
 
 bool CQuorumManager::RequestQuorumData(CNode* pFrom, Consensus::LLMQType llmqType, const CBlockIndex* pQuorumIndex, uint16_t nDataMask, const uint256& proTxHash) const
 {
+    if (pFrom == nullptr) {
+        LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- Invalid pFrom: nullptr\n", __func__);
+        return false;
+    }
     if (pFrom->nVersion < LLMQ_DATA_MESSAGES_VERSION) {
         LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- Version must be %d or greater.\n", __func__, LLMQ_DATA_MESSAGES_VERSION);
         return false;
     }
-    if (pFrom == nullptr || (pFrom->GetVerifiedProRegTxHash().IsNull() && !pFrom->qwatch)) {
+    if (pFrom->GetVerifiedProRegTxHash().IsNull() && !pFrom->qwatch) {
         LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- pFrom is neither a verified masternode nor a qwatch connection\n", __func__);
         return false;
     }
@@ -448,7 +452,8 @@ std::vector<CQuorumCPtr> CQuorumManager::ScanQuorums(Consensus::LLMQType llmqTyp
             }
             // If we have more cached than requested return only a subvector
             if (vecResultQuorums.size() > nCountRequested) {
-                return {vecResultQuorums.begin(), vecResultQuorums.begin() + nCountRequested};
+                const std::vector<CQuorumCPtr>& ret = {vecResultQuorums.begin(), vecResultQuorums.begin() + nCountRequested};
+                return ret;
             }
             // If we have cached quorums but not enough, subtract what we have from the count and the set correct index where to start
             // scanning for the rests
@@ -462,7 +467,7 @@ std::vector<CQuorumCPtr> CQuorumManager::ScanQuorums(Consensus::LLMQType llmqTyp
         }
     }
     // Get the block indexes of the mined commitments to build the required quorums from
-    auto quorumIndexes = quorumBlockProcessor->GetMinedCommitmentsUntilBlock(llmqType, (const CBlockIndex*)pIndexScanCommitments, nScanCommitments);
+    auto quorumIndexes = quorumBlockProcessor->GetMinedCommitmentsUntilBlock(llmqType, static_cast<const CBlockIndex*>(pIndexScanCommitments), nScanCommitments);
     vecResultQuorums.reserve(vecResultQuorums.size() + quorumIndexes.size());
 
     for (auto& quorumIndex : quorumIndexes) {
@@ -482,7 +487,8 @@ std::vector<CQuorumCPtr> CQuorumManager::ScanQuorums(Consensus::LLMQType llmqTyp
     }
     // Don't return more than nCountRequested elements
     size_t nResultEndIndex = std::min(nCountResult, nCountRequested);
-    return {vecResultQuorums.begin(), vecResultQuorums.begin() + nResultEndIndex};
+    const std::vector<CQuorumCPtr>& ret = {vecResultQuorums.begin(), vecResultQuorums.begin() + nResultEndIndex};
+    return ret;
 }
 
 CQuorumCPtr CQuorumManager::GetQuorum(Consensus::LLMQType llmqType, const uint256& quorumHash) const

src/llmq/quorums_blockprocessor.cpp

@@ -45,10 +45,9 @@ void CQuorumBlockProcessor::ProcessMessage(CNode* pfrom, const std::string& strC
         CFinalCommitment qc;
         vRecv >> qc;
 
-        auto hash = ::SerializeHash(qc);
         {
             LOCK(cs_main);
-            EraseObjectRequest(pfrom->GetId(), CInv(MSG_QUORUM_FINAL_COMMITMENT, hash));
+            EraseObjectRequest(pfrom->GetId(), CInv(MSG_QUORUM_FINAL_COMMITMENT, ::SerializeHash(qc)));
         }
 
         if (qc.IsNull()) {
@@ -337,7 +336,7 @@ bool CQuorumBlockProcessor::GetCommitmentsFromBlock(const CBlock& block, const C
 {
     AssertLockHeld(cs_main);
 
-    auto& consensus = Params().GetConsensus();
+    const auto& consensus = Params().GetConsensus();
     bool fDIP0003Active = pindex->nHeight >= consensus.DIP0003Height;
 
     ret.clear();
@@ -485,9 +484,7 @@ std::map<Consensus::LLMQType, std::vector<const CBlockIndex*>> CQuorumBlockProce
         auto& v = ret[p.second.type];
         v.reserve(p.second.signingActiveQuorumCount);
         auto commitments = GetMinedCommitmentsUntilBlock(p.second.type, pindex, p.second.signingActiveQuorumCount);
-        for (auto& c : commitments) {
-            v.emplace_back(c);
-        }
+        std::copy(commitments.begin(), commitments.end(), std::back_inserter(v));
     }
 
     return ret;

src/llmq/quorums_chainlocks.cpp

@@ -100,9 +100,7 @@ void CChainLocksHandler::ProcessMessage(CNode* pfrom, const std::string& strComm
         CChainLockSig clsig;
         vRecv >> clsig;
 
-        auto hash = ::SerializeHash(clsig);
-
-        ProcessNewChainLock(pfrom->GetId(), clsig, hash);
+        ProcessNewChainLock(pfrom->GetId(), clsig, ::SerializeHash(clsig));
     }
 }
 

src/llmq/quorums_chainlocks.h

@@ -101,7 +101,7 @@ class CChainLocksHandler : public CRecoveredSigsListener
     void CheckActiveState();
     void TrySignChainTip();
     void EnforceBestChainLock();
-    virtual void HandleNewRecoveredSig(const CRecoveredSig& recoveredSig);
+    void HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override;
 
     bool HasChainLock(int nHeight, const uint256& blockHash);
     bool HasConflictingChainLock(int nHeight, const uint256& blockHash);

src/llmq/quorums_instantsend.h

@@ -181,7 +181,7 @@ class CInstantSendManager : public CRecoveredSigsListener
     // TXs which are neither IS locked nor ChainLocked. We use this to determine for which TXs we need to retry IS locking
     // of child TXs
     struct NonLockedTxInfo {
-        const CBlockIndex* pindexMined{nullptr};
+        const CBlockIndex* pindexMined;
         CTransactionRef tx;
         std::unordered_set<uint256, StaticSaltedHasher> children;
     };
@@ -234,7 +234,7 @@ class CInstantSendManager : public CRecoveredSigsListener
     bool IsLocked(const uint256& txHash) const;
     CInstantSendLockPtr GetConflictingLock(const CTransaction& tx) const;
 
-    virtual void HandleNewRecoveredSig(const CRecoveredSig& recoveredSig);
+    void HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override;
 
     void ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vRecv);
 

src/llmq/quorums_signing_shares.cpp

@@ -631,7 +631,7 @@ void CSigSharesManager::CollectPendingSigSharesToVerify(
     }
 }
 
-bool CSigSharesManager::ProcessPendingSigShares(CConnman& connman)
+bool CSigSharesManager::ProcessPendingSigShares(const CConnman& connman)
 {
     std::unordered_map<NodeId, std::vector<CSigShare>> sigSharesByNodes;
     std::unordered_map<std::pair<Consensus::LLMQType, uint256>, CQuorumCPtr, StaticSaltedHasher> quorums;
@@ -708,7 +708,7 @@ bool CSigSharesManager::ProcessPendingSigShares(CConnman& connman)
 // It's ensured that no duplicates are passed to this method
 void CSigSharesManager::ProcessPendingSigShares(const std::vector<CSigShare>& sigShares,
         const std::unordered_map<std::pair<Consensus::LLMQType, uint256>, CQuorumCPtr, StaticSaltedHasher>& quorums,
-        CConnman& connman)
+        const CConnman& connman)
 {
     cxxtimer::Timer t(true);
     for (auto& sigShare : sigShares) {
@@ -722,7 +722,7 @@ void CSigSharesManager::ProcessPendingSigShares(const std::vector<CSigShare>& si
 }
 
 // sig shares are already verified when entering this method
-void CSigSharesManager::ProcessSigShare(const CSigShare& sigShare, CConnman& connman, const CQuorumCPtr& quorum)
+void CSigSharesManager::ProcessSigShare(const CSigShare& sigShare, const CConnman& connman, const CQuorumCPtr& quorum)
 {
     auto llmqType = quorum->params.type;
 
@@ -1065,7 +1065,7 @@ void CSigSharesManager::CollectSigSharesToAnnounce(std::unordered_map<NodeId, st
 
     sigSharesQueuedToAnnounce.ForEach([&](const SigShareKey& sigShareKey, bool) {
         AssertLockHeld(cs);
-        auto& signHash = sigShareKey.first;
+        const auto& signHash = sigShareKey.first;
         auto quorumMember = sigShareKey.second;
         const CSigShare* sigShare = sigShares.Get(sigShareKey);
         if (!sigShare) {
@@ -1287,7 +1287,7 @@ bool CSigSharesManager::GetSessionInfoByRecvId(NodeId nodeId, uint32_t sessionId
 CSigShare CSigSharesManager::RebuildSigShare(const CSigSharesNodeState::SessionInfo& session, const CBatchedSigShares& batchedSigShares, size_t idx)
 {
     assert(idx < batchedSigShares.sigShares.size());
-    auto& s = batchedSigShares.sigShares[idx];
+    const auto& s = batchedSigShares.sigShares[idx];
     CSigShare sigShare;
     sigShare.llmqType = session.llmqType;
     sigShare.quorumHash = session.quorumHash;
@@ -1454,7 +1454,6 @@ void CSigSharesManager::RemoveBannedNodeStates()
     // Called regularly to cleanup local node states for banned nodes
 
     LOCK2(cs_main, cs);
-    std::unordered_set<NodeId> toRemove;
     for (auto it = nodeStates.begin(); it != nodeStates.end();) {
         if (IsBanned(it->first)) {
             // re-request sigshares from other nodes