Merge pull request #237 from AxVultis/sonarcloud-tests

Bug fixes, restore `tests`, code clean-up and fix warnings.

.github/workflows/check.yml

@@ -130,6 +130,41 @@ jobs:
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 
+  build-ubuntu20-clang:
+    name: Ubuntu 20.04 clang
+    runs-on: ubuntu-20.04
+    steps:
+      - uses: actions/checkout@master
+
+      - name: Build
+        id: build
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y libboost-all-dev clang
+          build_folder="build/debug"
+          ccx_version=${GITHUB_SHA::7}
+          ccx_ver_folder=$(echo $ccx_version | sed 's/\.//g')
+          release_name=ccx-cli-ubuntu-2004-clang-dev"$ccx_version"
+          mkdir -p "$build_folder"
+          cd "$build_folder"
+          cmake ../.. -DCMAKE_BUILD_TYPE=Debug
+          make -j2
+          mkdir -p "$release_name"
+          exeFiles=()
+          for f in src/*; do [[ -x $f && -f $f ]] && exeFiles+=( "$f" ); done
+          strip "${exeFiles[@]}"
+          cp "${exeFiles[@]}" "$release_name/"
+          echo "::set-output name=release_name::${release_name}.zip"
+          echo "::set-output name=artifact_path::$build_folder/$release_name"
+
+      - name: Upload To GH Artifacts
+        uses: actions/upload-artifact@v1.0.0
+        with:
+          name: ${{ steps.build.outputs.release_name }}
+          path: ${{ steps.build.outputs.artifact_path }}
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+
   build-macos:
     name: macOS
     runs-on: macos-latest

CMakeLists.txt

@@ -271,6 +271,7 @@ endif()
 
 if(BUILD_TESTS)
   add_subdirectory(tests)
+  add_subdirectory(external/gtest)
   enable_testing()
 endif()
 

src/Common/FileMappedVector.h

@@ -759,7 +759,7 @@ void FileMappedVector<T>::rename(const std::string& newPath) {
 template<class T>
 template<class F>
 void FileMappedVector<T>::atomicUpdate(F&& func) {
-  atomicUpdate0(capacity(), prefixSize(), suffixSize(), std::move(func));
+  atomicUpdate0(capacity(), prefixSize(), suffixSize(), std::forward(func));
 }
 
 template<class T>

src/Common/Varint.h

@@ -59,6 +59,6 @@ namespace Tools {
 
     template<typename InputIt, typename T>
     int read_varint(InputIt &&first, InputIt &&last, T &i) {
-        return read_varint<std::numeric_limits<T>::digits, InputIt, T>(std::move(first), std::move(last), i);
+        return read_varint<std::numeric_limits<T>::digits, InputIt, T>(std::forward<InputIt>(first), std::forward<InputIt>(last), i);
     }
 }

src/CryptoNoteCore/BlockIndex.cpp

@@ -43,19 +43,23 @@ namespace CryptoNote {
     return false;
   }
 
-  std::vector<Crypto::Hash> BlockIndex::buildSparseChain(const Crypto::Hash& startBlockId) const {
+  std::vector<Crypto::Hash> BlockIndex::buildSparseChain(const Crypto::Hash &startBlockId) const
+  {
     assert(m_index.count(startBlockId) > 0);
 
     uint32_t startBlockHeight;
-    getBlockHeight(startBlockId, startBlockHeight);
-
     std::vector<Crypto::Hash> result;
-    size_t sparseChainEnd = static_cast<size_t>(startBlockHeight + 1);
-    for (size_t i = 1; i <= sparseChainEnd; i *= 2) {
-      result.emplace_back(m_container[sparseChainEnd - i]);
+    if (getBlockHeight(startBlockId, startBlockHeight))
+    {
+      size_t sparseChainEnd = static_cast<size_t>(startBlockHeight + 1);
+      for (size_t i = 1; i <= sparseChainEnd; i *= 2)
+      {
+        result.emplace_back(m_container[sparseChainEnd - i]);
+      }
     }
 
-    if (result.back() != m_container[0]) {
+    if (result.back() != m_container[0])
+    {
       result.emplace_back(m_container[0]);
     }
 

src/CryptoNoteCore/Blockchain.cpp

@@ -664,11 +664,11 @@ namespace CryptoNote
       Crypto::Hash blockHash = get_block_hash(block.bl);
       m_blockIndex.push(blockHash);
       uint64_t interest = 0;
-      for (uint16_t t = 0; t < block.transactions.size(); ++t)
+      for (uint32_t t = 0; t < block.transactions.size(); ++t)
       {
         const TransactionEntry &transaction = block.transactions[t];
         Crypto::Hash transactionHash = getObjectHash(transaction.tx);
-        TransactionIndex transactionIndex = {b, t};
+        TransactionIndex transactionIndex = {b, static_cast<uint16_t>(t)};
         m_transactionMap.insert(std::make_pair(transactionHash, transactionIndex));
 
         // process inputs
@@ -686,7 +686,7 @@ namespace CryptoNote
         }
 
         // process outputs
-        for (uint16_t o = 0; o < transaction.tx.outputs.size(); ++o)
+        for (uint32_t o = 0; o < transaction.tx.outputs.size(); ++o)
         {
           const auto &out = transaction.tx.outputs[o];
           if (out.target.type() == typeid(KeyOutput))
@@ -695,7 +695,7 @@ namespace CryptoNote
           }
           else if (out.target.type() == typeid(MultisignatureOutput))
           {
-            MultisignatureOutputUsage usage = {transactionIndex, o, false};
+            MultisignatureOutputUsage usage = {transactionIndex, static_cast<uint16_t>(o), false};
             m_multisignatureOutputs[out.amount].push_back(usage);
           }
         }
@@ -2740,7 +2740,7 @@ namespace CryptoNote
     }
 
     transaction.m_global_output_indexes.resize(transaction.tx.outputs.size());
-    for (uint16_t output = 0; output < transaction.tx.outputs.size(); ++output)
+    for (uint32_t output = 0; output < transaction.tx.outputs.size(); ++output)
     {
       if (transaction.tx.outputs[output].target.type() == typeid(KeyOutput))
       {
@@ -2752,7 +2752,7 @@ namespace CryptoNote
       {
         auto &amountOutputs = m_multisignatureOutputs[transaction.tx.outputs[output].amount];
         transaction.m_global_output_indexes[output] = static_cast<uint32_t>(amountOutputs.size());
-        MultisignatureOutputUsage outputUsage = {transactionIndex, output, false};
+        MultisignatureOutputUsage outputUsage = {transactionIndex, static_cast<uint16_t>(output), false};
         amountOutputs.push_back(outputUsage);
       }
     }
@@ -3156,7 +3156,7 @@ namespace CryptoNote
         const BlockEntry &block = m_blocks[b];
         m_timestampIndex.add(block.bl.timestamp, get_block_hash(block.bl));
         m_generatedTransactionsIndex.add(block.bl);
-        for (uint16_t t = 0; t < block.transactions.size(); ++t)
+        for (size_t t = 0; t < block.transactions.size(); ++t)
         {
           const TransactionEntry &transaction = block.transactions[t];
           m_paymentIdIndex.add(transaction.tx);

src/CryptoNoteCore/Core.h

@@ -34,7 +34,7 @@ namespace CryptoNote {
 
   class core : public ICore, public IMinerHandler, public IBlockchainStorageObserver, public ITxPoolObserver {
    public:
-     core(const Currency &currency, i_cryptonote_protocol *pprotocol, Logging::ILogger &logger, bool blockchainIndexesEnabled, bool blockchainAutosaveEnabled);
+     core(const Currency &currency, i_cryptonote_protocol *pprotocol, Logging::ILogger &logger, bool blockchainIndexesEnabled = false, bool blockchainAutosaveEnabled = false);
      ~core();
 
      bool on_idle() override;

src/CryptoNoteCore/Currency.cpp

@@ -1092,17 +1092,22 @@ namespace CryptoNote
 
     uint64_t T = 120; // target solvetime seconds
     uint64_t N = 60;  //  N=45, 60, and 90 for T=600, 120, 60.
-    uint64_t L(0), ST, sum_3_ST(0), next_D, prev_D, this_timestamp, previous_timestamp;
+    uint64_t L = 0;
+    uint64_t sum_3_ST = 0;
+    uint64_t next_D = 0;
+    uint64_t prev_D;
+    uint64_t this_timestamp;
+    uint64_t previous_timestamp;
 
     // Make sure timestamps & CD vectors are not bigger than they are supposed to be.
     // assert(timestamps.size() == cumulative_difficulties.size() &&
     //        timestamps.size() <= N + 1); // fails at block 104200
 
     // If it's a new coin, do startup code.
     // Increase difficulty_guess if it needs to be much higher, but guess lower than lowest guess.
-    uint64_t difficulty_guess = 100;
     if (timestamps.size() <= 10)
     {
+      uint64_t difficulty_guess = 100;
       return difficulty_guess;
     }
     // Use "if" instead of "else if" in case vectors are incorrectly N all the time instead of N+1.
@@ -1130,7 +1135,7 @@ namespace CryptoNote
         this_timestamp = previous_timestamp + 1;
       }
       // Limit solvetime ST to 6*T to prevent large drop in difficulty that could cause oscillations.
-      ST = std::min(6 * T, this_timestamp - previous_timestamp);
+      uint64_t ST = std::min(6 * T, this_timestamp - previous_timestamp);
       previous_timestamp = this_timestamp;
       L += ST * i; // give linearly higher weight to more recent solvetimes
                    // delete the following line if you do not want the "jump rule"

src/CryptoNoteCore/TransactionPool.cpp

@@ -447,18 +447,47 @@ namespace CryptoNote
 
       ss << "blobSize: " << txd.blobSize << std::endl
          << "fee: " << m_currency.formatAmount(txd.fee) << std::endl
-         << "received: " << std::ctime(&txd.receiveTime);
+         << "received: ";
+
+      char receivedTimeStr[32];
+      struct tm receivedTimeTm;
+#ifdef _WIN32
+      gmtime_s(&receivedTimeTm, &txd.receiveTime);
+#else
+      gmtime_r(&txd.receiveTime, &receivedTimeTm);
+#endif
+      if (std::strftime(receivedTimeStr, sizeof(receivedTimeStr), "%c", &receivedTimeTm))
+      {
+        ss << receivedTimeStr << " UTC";
+      }
+      else
+      {
+        ss << "unable to get time";
+      }
+      ss << std::endl;
 
       auto ttlIt = m_ttlIndex.find(txd.id);
       if (ttlIt != m_ttlIndex.end())
       {
-        // ctime() returns string that ends with new line
-        ss << "TTL: " << std::ctime(reinterpret_cast<const time_t *>(&ttlIt->second));
+        char ttlTimeStr[32];
+        struct tm ttlTimeTm;
+        time_t timestamp = reinterpret_cast<time_t>(&ttlIt->second);
+#ifdef _WIN32
+        gmtime_s(&ttlTimeTm, &timestamp);
+#else
+        gmtime_r(&timestamp, &ttlTimeTm);
+#endif
+        if (std::strftime(ttlTimeStr, sizeof(ttlTimeStr), "%c", &ttlTimeTm))
+        {
+          ss << "TTL: " << ttlTimeStr << " UTC";
+        }
+        else
+        {
+          ss << "TTL failed";
+        }
+        ss << std::endl;
       }
-
-      ss << std::endl;
     }
-
     return ss.str();
   }
   //---------------------------------------------------------------------------------

src/CryptoNoteCore/UpgradeDetector.h

@@ -123,13 +123,21 @@ namespace CryptoNote {
 
           if (m_blockchain.size() % (60 * 60 / m_currency.difficultyTarget()) == 0) {
             auto interval = m_currency.difficultyTarget() * (upgradeHeight() - m_blockchain.size() + 2);
+            char upgradeTimeStr[32];
+            struct tm upgradeTimeTm;
             time_t upgradeTimestamp = time(nullptr) + static_cast<time_t>(interval);
-            struct tm* upgradeTime = localtime(&upgradeTimestamp);;
-            char upgradeTimeStr[40];
-            strftime(upgradeTimeStr, 40, "%H:%M:%S %Y.%m.%d", upgradeTime);
+#ifdef _WIN32
+            gmtime_s(&upgradeTimeTm, &upgradeTimestamp);
+#else
+            gmtime_r(&upgradeTimestamp, &upgradeTimeTm);
+#endif
+            if (!std::strftime(upgradeTimeStr, sizeof(upgradeTimeStr), "%c", &upgradeTimeTm))
+            {
+              throw std::runtime_error("time buffer is too small");
+            }
 
             logger(Logging::TRACE, Logging::BRIGHT_GREEN) << "###### UPGRADE is going to happen after block index " << upgradeHeight() << " at about " <<
-              upgradeTimeStr << " (in " << Common::timeIntervalToString(interval) << ")! Current last block index " << (m_blockchain.size() - 1) <<
+              upgradeTimeStr << " UTC" << " (in " << Common::timeIntervalToString(interval) << ")! Current last block index " << (m_blockchain.size() - 1) <<
               ", hash " << get_block_hash(m_blockchain.back().bl);
           }
         } else if (m_blockchain.size() == upgradeHeight() + 1) {
@@ -198,7 +206,7 @@ namespace CryptoNote {
       assert(m_currency.upgradeVotingThreshold() > 0 && m_currency.upgradeVotingThreshold() <= 100);
 
       size_t voteCounter = getNumberOfVotes(height);
-      return m_currency.upgradeVotingThreshold() * m_currency.upgradeVotingWindow() <= 100 * voteCounter;
+      return (size_t) m_currency.upgradeVotingThreshold() * m_currency.upgradeVotingWindow() <= 100 * voteCounter;
     }
 
   private:

src/HTTP/HttpParser.cpp

@@ -26,17 +26,30 @@ void throwIfNotGood(std::istream& stream) {
 
 namespace CryptoNote {
 
-HttpResponse::HTTP_STATUS HttpParser::parseResponseStatusFromString(const std::string& status) {
-  if (status == "200 OK" || status == "200 Ok") return CryptoNote::HttpResponse::STATUS_200;
-  else if (status.substr(0, 4) == "401 ") return CryptoNote::HttpResponse::STATUS_401;
-  else if (status == "404 Not Found") return CryptoNote::HttpResponse::STATUS_404;
-  else if (status == "500 Internal Server Error") return CryptoNote::HttpResponse::STATUS_500;
-  else throw std::system_error(make_error_code(CryptoNote::error::HttpParserErrorCodes::UNEXPECTED_SYMBOL),
-      "Unknown HTTP status code is given");
-
-  return CryptoNote::HttpResponse::STATUS_200; //unaccessible
-}
-
+  HttpResponse::HTTP_STATUS HttpParser::parseResponseStatusFromString(const std::string &status)
+  {
+    if (status == "200 OK" || status == "200 Ok")
+    {
+      return CryptoNote::HttpResponse::STATUS_200;
+    }
+    else if (status.substr(0, 4) == "401 ")
+    {
+      return CryptoNote::HttpResponse::STATUS_401;
+    }
+    else if (status == "404 Not Found")
+    {
+      return CryptoNote::HttpResponse::STATUS_404;
+    }
+    else if (status == "500 Internal Server Error")
+    {
+      return CryptoNote::HttpResponse::STATUS_500;
+    }
+    else
+    {
+      throw std::system_error(make_error_code(CryptoNote::error::HttpParserErrorCodes::UNEXPECTED_SYMBOL),
+                              "Unknown HTTP status code is given");
+    }
+  }
 
 void HttpParser::receiveRequest(std::istream& stream, HttpRequest& request) {
   readWord(stream, request.method);

src/HTTP/HttpResponse.cpp

@@ -10,38 +10,38 @@
 
 namespace {
 
-const char* getStatusString(CryptoNote::HttpResponse::HTTP_STATUS status) {
-  switch (status) {
-  case CryptoNote::HttpResponse::STATUS_200:
-    return "200 OK";
-  case CryptoNote::HttpResponse::STATUS_401:
-    return "401 Unauthorized";
-  case CryptoNote::HttpResponse::STATUS_404:
-    return "404 Not Found";
-  case CryptoNote::HttpResponse::STATUS_500:
-    return "500 Internal Server Error";
-  default:
-    throw std::runtime_error("Unknown HTTP status code is given");
+  const char *getStatusString(CryptoNote::HttpResponse::HTTP_STATUS status)
+  {
+    switch (status)
+    {
+    case CryptoNote::HttpResponse::STATUS_200:
+      return "200 OK";
+    case CryptoNote::HttpResponse::STATUS_401:
+      return "401 Unauthorized";
+    case CryptoNote::HttpResponse::STATUS_404:
+      return "404 Not Found";
+    case CryptoNote::HttpResponse::STATUS_500:
+      return "500 Internal Server Error";
+    default:
+      throw std::runtime_error("Unknown HTTP status code is given");
+    }
   }
 
-  return ""; //unaccessible
-}
-
-const char* getErrorBody(CryptoNote::HttpResponse::HTTP_STATUS status) {
-  switch (status) {
-  case CryptoNote::HttpResponse::STATUS_401:
-    return "Authorization required\n";
-  case CryptoNote::HttpResponse::STATUS_404:
-    return "Requested url is not found\n";
-  case CryptoNote::HttpResponse::STATUS_500:
-    return "Internal server error is occurred\n";
-  default:
-    throw std::runtime_error("Error body for given status is not available");
+  const char *getErrorBody(CryptoNote::HttpResponse::HTTP_STATUS status)
+  {
+    switch (status)
+    {
+    case CryptoNote::HttpResponse::STATUS_401:
+      return "Authorization required\n";
+    case CryptoNote::HttpResponse::STATUS_404:
+      return "Requested url is not found\n";
+    case CryptoNote::HttpResponse::STATUS_500:
+      return "Internal server error is occurred\n";
+    default:
+      throw std::runtime_error("Error body for given status is not available");
+    }
   }
 
-  return ""; //unaccessible
-}
-
 } //namespace
 
 namespace CryptoNote {