Merge #2943: [Cleanup] Declare single-argument (non-converting) const…

…ructors "explicit"

1cc9a80 CI: Add extended lint job for cppcheck (Fuzzbawls)
43b75ca Declare single-argument (non-converting) constructors "explicit" (Fuzzbawls)

Pull request description:

  Declare single-argument (non-converting) constructors `explicit`.

  In order to avoid unintended implicit conversions.

  Coming from bitcoin#10969

  Added a new CI lint job that uses `cppcheck` to indicate any regressions or new introductions. This lint framework can be expanded on to catch other static analysis warnings in future PRs.

ACKs for top commit: 1cc9a80
  panleone:
    utACK 1cc9a80
  Liquid369:
    utACK 1cc9a80

Tree-SHA512: 612997f2799d2527d3ecfff023740add6ea4f5d67bce3e378880b27e596e5b6ab4e33c39492e846ae071b9c5da1fe6938fc8d030d3bab2894b7ad0b406697765

.github/workflows/build-and-test.yml

@@ -72,6 +72,64 @@ jobs:
             test/lint/commit-script-check.sh $COMMIT_RANGE
           fi
 
+  # Extended lint using cppcheck. Other jobs do NOT require this to pass.
+  extended-lint:
+    env:
+      CPPCHECK_VERSION: 2.14.0
+      CPPCHECK_BIN_DIR: ${{ github.workspace }}/.cppcheck
+      CPPCHECK_BUILD_DIR: ${{ github.workspace }}/.ci-cppcheck
+      LC_ALL: C
+    runs-on: ubuntu-20.04
+    defaults:
+      run:
+        shell: bash
+    steps:
+      - name: Checkout Repo
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+
+      - name: Initialize Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: '3.8'
+
+      - name: cppcheck cache files
+        uses: actions/cache@v4
+        with:
+          path: |
+            .cppcheck
+            .ci-cppcheck
+          key: ci-cppcheck
+          restore-keys: ci-cppcheck
+
+      - name: Install Dependencies
+        run: |
+          python -m pip install --upgrade pip
+
+          mkdir -p ${CPPCHECK_BIN_DIR}
+          curl -s https://codeload.github.com/danmar/cppcheck/tar.gz/${CPPCHECK_VERSION} | tar -zxf - --directory ${CPPCHECK_BIN_DIR}/
+          cd ${CPPCHECK_BIN_DIR}/cppcheck-${CPPCHECK_VERSION}/
+          make -j 3 MATCHCOMPILER=yes FILESDIR=${CPPCHECK_BIN_DIR}/cppcheck-${CPPCHECK_VERSION}/cfg/
+
+      - name: Set TRAVIS_BRANCH workaround env variable
+        if: github.event_name == 'pull_request'
+        run: echo "TRAVIS_BRANCH=${{ github.base_ref }}" >> $GITHUB_ENV
+
+      - name: Build cppcheck cache
+        run: |
+          export PATH="${CPPCHECK_BIN_DIR}/cppcheck-${CPPCHECK_VERSION}:${PATH}"
+          git checkout -qf -B master refs/remotes/origin/master
+          git checkout -qf $GITHUB_SHA
+
+          # Build or rebuild the cppcheck cache.
+          test/lint/cppcheck/lint-all.sh --setup
+
+      - name: Lint
+        run: |
+          # Run the remainder of lint tests in `test/lint/`.
+          test/lint/cppcheck/lint-all.sh
+
   # CMake build jobs to ensure building with CMake remains viable.
   # Currently this only supports native linux and macOS.
   cmake:

src/addrdb.h

@@ -39,7 +39,7 @@ class CBanEntry
         SetNull();
     }
 
-    CBanEntry(int64_t nCreateTimeIn)
+    explicit CBanEntry(int64_t nCreateTimeIn)
     {
         SetNull();
         nCreateTime = nCreateTimeIn;

src/blockassembler.h

@@ -34,7 +34,7 @@ struct CBlockTemplate
 // Container for tracking updates to ancestor feerate as we include (parent)
 // transactions in a block
 struct CTxMemPoolModifiedEntry {
-    CTxMemPoolModifiedEntry(CTxMemPool::txiter entry)
+    explicit CTxMemPoolModifiedEntry(CTxMemPool::txiter entry)
     {
         iter = entry;
         nSizeWithAncestors = entry->GetSizeWithAncestors();
@@ -117,7 +117,7 @@ typedef indexed_modified_transaction_set::index<ancestor_score>::type::iterator
 
 struct update_for_parent_inclusion
 {
-    update_for_parent_inclusion(CTxMemPool::txiter it) : iter(it) {}
+    explicit update_for_parent_inclusion(CTxMemPool::txiter it) : iter(it) {}
 
     void operator() (CTxMemPoolModifiedEntry &e)
     {

src/bls/bls_worker.h

@@ -156,7 +156,7 @@ class CBLSWorkerCache
     std::map<uint256, std::shared_future<CBLSPublicKey> > publicKeyShareCache;
 
 public:
-    CBLSWorkerCache(CBLSWorker& _worker) :
+    explicit CBLSWorkerCache(CBLSWorker& _worker) :
         worker(_worker) {}
 
     BLSVerificationVectorPtr BuildQuorumVerificationVector(const uint256& cacheKey, const std::vector<BLSVerificationVectorPtr>& vvecs)

src/chain.h

@@ -205,7 +205,7 @@ class CBlockIndex
     unsigned int nTimeMax{0};
 
     CBlockIndex() {}
-    CBlockIndex(const CBlock& block);
+    explicit CBlockIndex(const CBlock& block);
 
     std::string ToString() const;
 

src/chainparams.h

@@ -20,7 +20,7 @@
 struct CDNSSeedData {
     std::string host;
     bool supportsServiceBitsFiltering;
-    CDNSSeedData(const std::string& strHost, bool supportsServiceBitsFilteringIn = false) : host(strHost), supportsServiceBitsFiltering(supportsServiceBitsFilteringIn) {}
+    explicit CDNSSeedData(const std::string& strHost, bool supportsServiceBitsFilteringIn = false) : host(strHost), supportsServiceBitsFiltering(supportsServiceBitsFilteringIn) {}
 };
 
 typedef std::map<int, uint256> MapCheckpoints;

src/checkqueue.h

@@ -175,7 +175,7 @@ class CCheckQueueControl
     bool fDone;
 
 public:
-    CCheckQueueControl(CCheckQueue<T>* pqueueIn) : pqueue(pqueueIn), fDone(false)
+    explicit CCheckQueueControl(CCheckQueue<T>* pqueueIn) : pqueue(pqueueIn), fDone(false)
     {
         // passed queue is supposed to be unused, or nullptr
         if (pqueue != nullptr) {

src/crypto/aes.h

@@ -22,7 +22,7 @@ class AES128Encrypt
     AES128_ctx ctx;
 
 public:
-    AES128Encrypt(const unsigned char key[16]);
+    explicit AES128Encrypt(const unsigned char key[16]);
     ~AES128Encrypt();
     void Encrypt(unsigned char ciphertext[16], const unsigned char plaintext[16]) const;
 };
@@ -34,7 +34,7 @@ class AES128Decrypt
     AES128_ctx ctx;
 
 public:
-    AES128Decrypt(const unsigned char key[16]);
+    explicit AES128Decrypt(const unsigned char key[16]);
     ~AES128Decrypt();
     void Decrypt(unsigned char plaintext[16], const unsigned char ciphertext[16]) const;
 };
@@ -46,7 +46,7 @@ class AES256Encrypt
     AES256_ctx ctx;
 
 public:
-    AES256Encrypt(const unsigned char key[32]);
+    explicit AES256Encrypt(const unsigned char key[32]);
     ~AES256Encrypt();
     void Encrypt(unsigned char ciphertext[16], const unsigned char plaintext[16]) const;
 };
@@ -58,7 +58,7 @@ class AES256Decrypt
     AES256_ctx ctx;
 
 public:
-    AES256Decrypt(const unsigned char key[32]);
+    explicit AES256Decrypt(const unsigned char key[32]);
     ~AES256Decrypt();
     void Decrypt(unsigned char plaintext[16], const unsigned char ciphertext[16]) const;
 };

src/ctpl_stl.h

@@ -73,7 +73,7 @@ namespace ctpl {
     public:
 
         thread_pool() { this->init(); }
-        thread_pool(int nThreads) { this->init(); this->resize(nThreads); }
+        explicit thread_pool(int nThreads) { this->init(); this->resize(nThreads); }
 
         // the destructor waits for all the functions in the queue to be finished
         ~thread_pool() {

src/cxxtimer.h

@@ -45,7 +45,7 @@ class Timer {
      *          If true, the timer is started just after construction.
      *          Otherwise, it will not be automatically started.
      */
-    Timer(bool start = false);
+    explicit Timer(bool start = false);
 
     /**
      * Copy constructor.

src/dbwrapper.h

@@ -25,7 +25,7 @@ static const size_t DBWRAPPER_PREALLOC_VALUE_SIZE = 1024;
 class dbwrapper_error : public std::runtime_error
 {
 public:
-    dbwrapper_error(const std::string& msg) : std::runtime_error(msg) {}
+    explicit dbwrapper_error(const std::string& msg) : std::runtime_error(msg) {}
 };
 
 class CDBWrapper;

src/evo/deterministicmns.h

@@ -184,7 +184,7 @@ class CDeterministicMN
 
 public:
     CDeterministicMN() = delete; // no default constructor, must specify internalId
-    CDeterministicMN(uint64_t _internalId) : internalId(_internalId)
+    explicit CDeterministicMN(uint64_t _internalId) : internalId(_internalId)
     {
         // only non-initial values
         assert(_internalId != std::numeric_limits<uint64_t>::max());

src/hash.h

@@ -257,7 +257,7 @@ class CHashVerifier : public CHashWriter
     Source* source;
 
 public:
-    CHashVerifier(Source* source_) : CHashWriter(source_->GetType(), source_->GetVersion()), source(source_) {}
+    explicit CHashVerifier(Source* source_) : CHashWriter(source_->GetType(), source_->GetVersion()), source(source_) {}
 
     void read(char* pch, size_t nSize)
     {

src/httprpc.cpp

@@ -40,7 +40,7 @@ class HTTPRPCTimer : public RPCTimerBase
 class HTTPRPCTimerInterface : public RPCTimerInterface
 {
 public:
-    HTTPRPCTimerInterface(struct event_base* base) : base(base)
+    explicit HTTPRPCTimerInterface(struct event_base* base) : base(base)
     {
     }
     const char* Name()

src/httpserver.h

@@ -62,7 +62,7 @@ class HTTPRequest
     bool replySent;
 
 public:
-    HTTPRequest(struct evhttp_request* req);
+    explicit HTTPRequest(struct evhttp_request* req);
     ~HTTPRequest();
 
     enum RequestMethod {

src/init.cpp

@@ -160,7 +160,7 @@ NODISCARD static bool CreatePidFile()
 class CCoinsViewErrorCatcher : public CCoinsViewBacked
 {
 public:
-    CCoinsViewErrorCatcher(CCoinsView* view) : CCoinsViewBacked(view) {}
+    explicit CCoinsViewErrorCatcher(CCoinsView* view) : CCoinsViewBacked(view) {}
     bool GetCoin(const COutPoint& outpoint, Coin& coin) const override
     {
         try {

src/key_io.cpp

@@ -22,7 +22,7 @@ namespace
         const CChainParams::Base58Type m_addrType;
 
     public:
-        DestinationEncoder(const CChainParams& params, const CChainParams::Base58Type _addrType = CChainParams::PUBKEY_ADDRESS) : m_params(params), m_addrType(_addrType) {}
+        explicit DestinationEncoder(const CChainParams& params, const CChainParams::Base58Type _addrType = CChainParams::PUBKEY_ADDRESS) : m_params(params), m_addrType(_addrType) {}
 
         std::string operator()(const CKeyID& id) const
         {

src/libzerocoin/Coin.h

@@ -30,7 +30,7 @@ namespace libzerocoin
     class InvalidSerialException : public std::exception {
     public:
         std::string message;
-        InvalidSerialException(const std::string &message) : message(message) {}
+        explicit InvalidSerialException(const std::string &message) : message(message) {}
     };
 
     int ExtractVersionFromSerial(const CBigNum& bnSerial);
@@ -54,7 +54,7 @@ class PublicCoin
         strm >> *this;
     }
 
-    PublicCoin(const ZerocoinParams* p);
+    explicit PublicCoin(const ZerocoinParams* p);
 
     /**Generates a public coin
      *

src/libzerocoin/CoinRandomnessSchnorrSignature.h

@@ -19,7 +19,7 @@ namespace libzerocoin {
 class CoinRandomnessSchnorrSignature {
 public:
     CoinRandomnessSchnorrSignature() {};
-    template <typename Stream> CoinRandomnessSchnorrSignature(Stream& strm) {strm >> *this;}
+    template <typename Stream> explicit CoinRandomnessSchnorrSignature(Stream& strm) {strm >> *this;}
 
     /** Creates a Schnorr signature object using the randomness of a public coin as private key sk.
      *

src/libzerocoin/CoinSpend.h

@@ -80,7 +80,7 @@ class CoinSpend
 public:
 
     CoinSpend() {};
-    CoinSpend(CDataStream& strm) { strm >> *this; }
+    explicit CoinSpend(CDataStream& strm) { strm >> *this; }
     virtual ~CoinSpend(){};
 
     const CBigNum& getCoinSerialNumber() const { return this->coinSerialNumber; }

src/libzerocoin/Params.h

@@ -156,8 +156,7 @@ class ZerocoinParams {
 	* compromised. The integer "N" must be a MINIMUM of 1024
 	* in length. 3072 bits is strongly recommended.
 	**/
-	ZerocoinParams(CBigNum accumulatorModulus,
-	       uint32_t securityLevel = ZEROCOIN_DEFAULT_SECURITYLEVEL);
+        explicit ZerocoinParams(CBigNum accumulatorModulus, uint32_t securityLevel = ZEROCOIN_DEFAULT_SECURITYLEVEL);
 
 	bool initialized;
 

src/limitedmap.h

@@ -28,7 +28,7 @@ class limitedmap
     size_type nMaxSize;
 
 public:
-    limitedmap(size_type nMaxSizeIn = 0) { nMaxSize = nMaxSizeIn; }
+    explicit limitedmap(size_type nMaxSizeIn = 0) { nMaxSize = nMaxSizeIn; }
     const_iterator begin() const { return map.begin(); }
     const_iterator end() const { return map.end(); }
     size_type size() const { return map.size(); }

src/llmq/quorums_chainlocks.h

@@ -62,7 +62,7 @@ class CChainLocksHandler : public CRecoveredSigsListener
     int64_t lastCleanupTime{0};
 
 public:
-    CChainLocksHandler(CScheduler* _scheduler);
+    explicit CChainLocksHandler(CScheduler* _scheduler);
     ~CChainLocksHandler();
     void Start();
     void Stop();

src/llmq/quorums_dkgsession.h

@@ -95,7 +95,7 @@ class CDKGComplaint
 
 public:
     CDKGComplaint() {}
-    CDKGComplaint(const Consensus::LLMQParams& params);
+    explicit CDKGComplaint(const Consensus::LLMQParams& params);
 
     SERIALIZE_METHODS(CDKGComplaint, obj)
     {
@@ -163,7 +163,7 @@ class CDKGPrematureCommitment
 
 public:
     CDKGPrematureCommitment() {}
-    CDKGPrematureCommitment(const Consensus::LLMQParams& params);
+    explicit CDKGPrematureCommitment(const Consensus::LLMQParams& params);
 
     int CountValidMembers() const
     {

src/llmq/quorums_dkgsessionhandler.h

@@ -45,7 +45,7 @@ class CDKGPendingMessages
     std::set<uint256> seenMessages;
 
 public:
-    CDKGPendingMessages(size_t _maxMessagesPerNode);
+    explicit CDKGPendingMessages(size_t _maxMessagesPerNode);
 
     void PushPendingMessage(NodeId from, CDataStream& vRecv, int invType);
     std::list<BinaryMessage> PopPendingMessages(size_t maxCount);

src/llmq/quorums_signing.h

@@ -65,7 +65,7 @@ class CRecoveredSigsDb
     unordered_lru_cache<uint256, bool, StaticSaltedHasher, 30000> hasSigForHashCache;
 
 public:
-    CRecoveredSigsDb(CDBWrapper& _db);
+    explicit CRecoveredSigsDb(CDBWrapper& _db);
 
     void ConvertInvalidTimeKeys();
     void AddVoteTimeKeys();

src/masternode-payments.h

@@ -95,7 +95,7 @@ class CMasternodeBlockPayees
         nBlockHeight = 0;
         vecPayments.clear();
     }
-    CMasternodeBlockPayees(int nBlockHeightIn)
+    explicit CMasternodeBlockPayees(int nBlockHeightIn)
     {
         nBlockHeight = nBlockHeightIn;
         vecPayments.clear();

src/masternode.h

@@ -244,7 +244,7 @@ class CMasternodeBroadcast : public CMasternode
 public:
     CMasternodeBroadcast();
     CMasternodeBroadcast(CService newAddr, CTxIn newVin, CPubKey newPubkey, CPubKey newPubkey2, int protocolVersionIn, const CMasternodePing& _lastPing);
-    CMasternodeBroadcast(const CMasternode& mn);
+    explicit CMasternodeBroadcast(const CMasternode& mn);
 
     bool CheckAndUpdate(int& nDoS);
 

src/net_processing.cpp

@@ -2361,7 +2361,7 @@ class CompareInvMempoolOrder
 {
     CTxMemPool *mp;
 public:
-    CompareInvMempoolOrder(CTxMemPool *_mempool)
+    explicit CompareInvMempoolOrder(CTxMemPool *_mempool)
     {
         mp = _mempool;
     }

src/net_processing.h

@@ -37,7 +37,7 @@ class PeerLogicValidation : public CValidationInterface, public NetEventsInterfa
     CConnman* connman;
 
 public:
-    PeerLogicValidation(CConnman* connman);
+    explicit PeerLogicValidation(CConnman* connman);
     ~PeerLogicValidation() = default;
 
     void BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindex) override;