Returning topblock data, bug fixes

include/WalletTypes.h

@@ -503,6 +503,20 @@ namespace WalletTypes
         uint64_t height;
     };
 
+    inline void to_json(nlohmann::json &j, const TopBlock &t)
+    {
+        j = {
+            {"hash", t.hash},
+            {"height", t.height}
+        };
+    }
+
+    inline void from_json(const nlohmann::json &j, TopBlock &t)
+    {
+        t.hash = j.at("hash").get<Crypto::Hash>();
+        t.height = j.at("height").get<uint64_t>();
+    }
+
     inline void to_json(nlohmann::json &j, const WalletBlockInfo &w)
     {
         j = {

src/CMakeLists.txt

@@ -17,6 +17,7 @@ include_directories(${CMAKE_SOURCE_DIR}/external/lz4)
 # can find the new file
 file(GLOB_RECURSE BlockchainExplorer BlockchainExplorer/*)
 file(GLOB_RECURSE Common Common/*)
+file(GLOB_RECURSE Config config/*)
 file(GLOB_RECURSE Crypto crypto/*)
 file(GLOB_RECURSE CryptoNoteCore CryptoNoteCore/* CryptoNoteConfig.h)
 file(GLOB_RECURSE CryptoNoteProtocol CryptoNoteProtocol/*)
@@ -53,11 +54,12 @@ else()
 endif()
 
 # Group the files together in IDEs
-source_group("" FILES $${Common} ${Crypto} ${CryptoNoteCore} ${CryptoNoteProtocol} ${TurtleCoind} ${JsonRpcServer} ${Http} ${Logging} ${Logger} ${miner} ${Mnemonics} ${Nigel} ${NodeRpcProxy} ${P2p} ${Rpc} ${Serialization} ${System} ${Transfers} ${Wallet} ${WalletApi} ${WalletBackend} ${WalletService} ${zedwallet} ${zedwallet++} ${CryptoTest} ${Errors} ${Utilities} ${SubWallets})
+source_group("" FILES $${Common} ${Config} ${Crypto} ${CryptoNoteCore} ${CryptoNoteProtocol} ${TurtleCoind} ${JsonRpcServer} ${Http} ${Logging} ${Logger} ${miner} ${Mnemonics} ${Nigel} ${NodeRpcProxy} ${P2p} ${Rpc} ${Serialization} ${System} ${Transfers} ${Wallet} ${WalletApi} ${WalletBackend} ${WalletService} ${zedwallet} ${zedwallet++} ${CryptoTest} ${Errors} ${Utilities} ${SubWallets})
 
 # Define a group of files as a library to link against
 add_library(BlockchainExplorer STATIC ${BlockchainExplorer})
 add_library(Common STATIC ${Common})
+add_library(Config STATIC ${Config})
 add_library(Crypto STATIC ${Crypto})
 add_library(CryptoNoteCore STATIC ${CryptoNoteCore})
 add_library(Errors STATIC ${Errors})
@@ -150,7 +152,7 @@ target_link_libraries(Transfers CryptoNoteCore)
 target_link_libraries(Utilities Common Errors Wallet)
 target_link_libraries(Wallet NodeRpcProxy Transfers CryptoNoteCore Common ${Boost_LIBRARIES})
 target_link_libraries(WalletApi WalletBackend)
-target_link_libraries(WalletBackend Serialization Mnemonics Nigel cryptopp-static __filesystem Utilities SubWallets Logger)
+target_link_libraries(WalletBackend Serialization Mnemonics Nigel cryptopp-static __filesystem Utilities SubWallets Logger Config)
 target_link_libraries(WalletService JsonRpcServer Wallet Mnemonics Errors)
 target_link_libraries(zedwallet Mnemonics Wallet Utilities Errors)
 target_link_libraries(zedwallet++ WalletBackend)

src/CryptoNoteCore/BlockchainCache.cpp

@@ -619,7 +619,7 @@ std::vector<RawBlock> BlockchainCache::getNonEmptyBlocks(
     {
         auto block = storage->getBlockByIndex(i);
 
-        i++
+        i++;
 
         if (block.transactions.empty())
         {

src/CryptoNoteCore/Core.cpp

@@ -617,6 +617,7 @@ bool Core::getWalletSyncData(
            synced. */
         if (startTimestamp != 0 && !success)
         {
+            topBlockInfo = WalletTypes::TopBlock({ currentHash, currentIndex });
             return true;
         }
 
@@ -665,6 +666,7 @@ bool Core::getWalletSyncData(
            current block. */
         if (currentIndex < startIndex)
         {
+            topBlockInfo = WalletTypes::TopBlock({ currentHash, currentIndex });
             return true;
         }
 
@@ -712,8 +714,7 @@ bool Core::getWalletSyncData(
 
         if (walletBlocks.empty())
         {
-            topBlockInfo->height = currentIndex;
-            topBlockInfo->hash = currentHash;
+            topBlockInfo = WalletTypes::TopBlock({ currentHash, currentIndex });
         }
 
         return true;

src/CryptoNoteCore/DatabaseBlockchainCache.cpp

@@ -1650,39 +1650,35 @@ std::vector<RawBlock> DatabaseBlockchainCache::getNonEmptyBlocks(
 {
     std::vector<RawBlock> orderedBlocks;
 
-    /* Lets try taking the amount we need *2, to try and balance not needing
-       multiple DB requests to get the amount we need of non empty blocks, with
-       not taking too many */
-    const uint64_t endHeight = startHeight + (blockCount * 2);
-
     const uint32_t storageBlockCount = getBlockCount();
 
-    auto blockBatch = BlockchainReadBatch().requestRawBlocks(startHeight, endHeight);
-
-    auto rawBlocks = readDatabase(blockBatch).getRawBlocks();
-
     uint64_t height = startHeight;
 
-    while (orderedBlocks.size() < blockCount && orderedBlocks.size() < rawBlocks.size())
+    while (orderedBlocks.size() < blockCount && height < storageBlockCount)
     {
-        const auto block = rawBlocks.at(height);
-
-        height++;
+        uint64_t startHeight = height;
 
-        if (block.transactions.empty())
-        {
-            continue;
-        }
+        /* Lets try taking the amount we need *2, to try and balance not needing
+           multiple DB requests to get the amount we need of non empty blocks, with
+           not taking too many */
+        uint64_t endHeight = startHeight + (blockCount * 2);
 
-        orderedBlocks.push_back(block);
-    }
+        auto blockBatch = BlockchainReadBatch().requestRawBlocks(startHeight, endHeight);
+        const auto rawBlocks = readDatabase(blockBatch).getRawBlocks();
 
-    /* Not got as many blocks as we wanted. Go for another batch. */
-    if (orderedBlocks.size() < blockCount && height <= storageBlockCount)
-    {
-        const auto moreBlocks = getNonEmptyBlocks(height, blockCount - orderedBlocks.size());
+        while (orderedBlocks.size() < blockCount && height < startHeight + rawBlocks.size())
+        {
+            const auto block = rawBlocks.at(height);
+
+            height++;
+
+            if (block.transactions.empty())
+            {
+                continue;
+            }
 
-        std::copy(moreBlocks.begin(), moreBlocks.end(), std::back_inserter(orderedBlocks));
+            orderedBlocks.push_back(block);
+        }
     }
 
     return orderedBlocks;

src/Nigel/Nigel.cpp

@@ -107,10 +107,16 @@ void Nigel::resetRequestedBlockCount()
     m_blockCount = CryptoNote::BLOCKS_SYNCHRONIZING_DEFAULT_COUNT;
 }
 
-std::tuple<bool, std::vector<WalletTypes::WalletBlockInfo>> Nigel::getWalletSyncData(
+std::tuple<
+    bool,
+    std::vector<WalletTypes::WalletBlockInfo>,
+    std::optional<WalletTypes::TopBlock>
+> Nigel::getWalletSyncData(
+
     const std::vector<Crypto::Hash> blockHashCheckpoints,
-    uint64_t startHeight,
-    uint64_t startTimestamp) const
+    const uint64_t startHeight,
+    const uint64_t startTimestamp,
+    const bool skipCoinbaseTransactions) const
 {
     Logger::logger.log(
         "Fetching blocks from the daemon",
@@ -122,7 +128,8 @@ std::tuple<bool, std::vector<WalletTypes::WalletBlockInfo>> Nigel::getWalletSync
         {"blockHashCheckpoints", blockHashCheckpoints},
         {"startHeight", startHeight},
         {"startTimestamp", startTimestamp},
-        {"blockCount", m_blockCount.load()}
+        {"blockCount", m_blockCount.load()},
+        {"skipCoinbaseTransactions", skipCoinbaseTransactions}
     };
 
     auto res = m_nodeClient->Post(
@@ -137,12 +144,23 @@ std::tuple<bool, std::vector<WalletTypes::WalletBlockInfo>> Nigel::getWalletSync
 
             if (j.at("status").get<std::string>() != "OK")
             {
-                return {false, {}};
+                return {false, {}, std::nullopt};
             }
 
             const auto items = j.at("items").get<std::vector<WalletTypes::WalletBlockInfo>>();
 
-            return {true, items};
+            if (j.find("synced") != j.end() 
+             && j.find("topBlock") != j.end()
+             && j.at("synced").get<bool>())
+            {
+                return {
+                    true,
+                    items,
+                    j.at("topBlock").get<WalletTypes::TopBlock>()
+                };
+            }
+
+            return {true, items, std::nullopt};
         }
         catch (const json::exception &e)
         {
@@ -154,7 +172,7 @@ std::tuple<bool, std::vector<WalletTypes::WalletBlockInfo>> Nigel::getWalletSync
         }
     }
 
-    return {false, {}};
+    return {false, {}, std::nullopt};
 }
 
 void Nigel::stop()

src/Nigel/Nigel.h

@@ -68,10 +68,15 @@ class Nigel
 
         std::tuple<std::string, uint16_t, bool> nodeAddress() const;
 
-        std::tuple<bool, std::vector<WalletTypes::WalletBlockInfo>> getWalletSyncData(
+        std::tuple<
+            bool,
+            std::vector<WalletTypes::WalletBlockInfo>,
+            std::optional<WalletTypes::TopBlock>
+        > getWalletSyncData(
             const std::vector<Crypto::Hash> blockHashCheckpoints,
-            uint64_t startHeight,
-            uint64_t startTimestamp) const;
+            const uint64_t startHeight,
+            const uint64_t startTimestamp,
+            const bool skipCoinbaseTransactions) const;
 
         /* Returns a bool on success or not */
         bool getTransactionsStatus(

src/Rpc/CoreRpcServerCommandsDefinitions.h

@@ -844,10 +844,11 @@ struct COMMAND_RPC_GET_WALLET_SYNC_DATA {
         {
             KV_MEMBER(status)
             KV_MEMBER(items);
+            KV_MEMBER(synced);
 
             if (topBlock)
             {
-                KV_MEMBER(*topBlock);
+                s(*topBlock, "topBlock");
             }
         }
     };

src/Rpc/RpcServer.cpp

@@ -106,8 +106,8 @@ void serialize(WalletTypes::KeyOutput &keyOutput, ISerializer &s)
 
 void serialize(WalletTypes::TopBlock &topBlock, ISerializer &s)
 {
-    KV_MEMBER(topBlock.hash);
-    KV_MEMBER(topBlock.height);
+    s(topBlock.hash, "hash");
+    s(topBlock.height, "height");
 }
 
 namespace {

src/WalletBackend/BlockDownloader.cpp

@@ -11,9 +11,12 @@
 #include <Logger/Logger.h>
 
 #include <Utilities/FormatTools.h>
+#include <Utilities/Utilities.h>
 
 #include <WalletBackend/Constants.h>
 
+#include <config/Config.h>
+
 /* Constructor */
 BlockDownloader::BlockDownloader(
     const std::shared_ptr<Nigel> daemon,
@@ -111,8 +114,11 @@ void BlockDownloader::downloader()
 
         while (shouldFetchMoreBlocks() && !m_shouldStop)
         {
-            if (!downloadBlocks())
+            const bool blocksDownloaded = downloadBlocks();
+
+            if (!blocksDownloaded)
             {
+                Utilities::sleepUnlessStopping(std::chrono::seconds(5), m_shouldStop);
                 break;
             }
         }
@@ -249,7 +255,7 @@ bool BlockDownloader::downloadBlocks()
         std::stringstream stream;
 
         stream << "First checkpoint: " << blockCheckpoints.front()
-                  << "\nLast checkpoint: " << blockCheckpoints.back();
+               << "\nLast checkpoint: " << blockCheckpoints.back();
 
         Logger::logger.log(
             stream.str(),
@@ -258,14 +264,24 @@ bool BlockDownloader::downloadBlocks()
         );
     }
 
-    const auto [success, blocks] = m_daemon->getWalletSyncData(
-        blockCheckpoints, m_startHeight, m_startTimestamp
+    const auto [success, blocks, topBlock] = m_daemon->getWalletSyncData(
+        blockCheckpoints,
+        m_startHeight,
+        m_startTimestamp,
+        Config::config.wallet.skipCoinbaseTransactions
     );
 
+    /* Synced, store the top block so sync status displayes correctly if
+       we are not scanning coinbase tx only blocks */
+    if (success && blocks.empty() && topBlock)
+    {
+        m_synchronizationStatus.storeBlockHash(topBlock->hash, topBlock->height);
+        return false;
+    }
     /* If we get no blocks, we are fully synced.
        (Or timed out/failed to get blocks)
        Sleep a bit so we don't spam the daemon. */
-    if (!success || blocks.empty())
+    else if (!success || blocks.empty())
     {
         /* We may have also failed because we requested
            more data than could be returned in a reasonable
@@ -295,39 +311,6 @@ bool BlockDownloader::downloadBlocks()
         m_subWallets->convertSyncTimestampToHeight(m_startTimestamp, m_startHeight);
     }
 
-    /* If checkpoints are empty, this is the first sync request. */
-    if (blockCheckpoints.empty())
-    {
-        /* Only check if a timestamp isn't given */
-        if (m_startTimestamp == 0)
-        {
-            /* Loop through the blocks we got back and make sure that
-               we were given data for the start block we were looking for */
-            const auto it = std::find_if(blocks.begin(), blocks.end(), [this](const auto &block) {
-                return block.blockHeight == m_startHeight;
-            });
-
-            /* If we weren't given a block with the startHeight we were
-               looking for then we don't need to store this data */
-            if (it == blocks.end())
-            {
-                std::stringstream stream;
-
-                stream << "Received unexpected block height from daemon. "
-                       << "Expected " << m_startHeight << ", but did not "
-                       << "receive that block. Not storing any blocks.";
-
-                Logger::logger.log(
-                    stream.str(),
-                    Logger::WARNING,
-                    {Logger::SYNC, Logger::DAEMON}
-                );
-
-                return false;
-            }
-        }
-    }
-
     std::stringstream stream;
 
     stream << "Downloaded " << blocks.size() << " blocks from daemon, ["

src/WalletBackend/SynchronizationStatus.cpp

@@ -23,10 +23,17 @@ void SynchronizationStatus::storeBlockHash(
 {
     m_lastKnownBlockHeight = height;
 
+    /* Already added this hash */
+    if (!m_lastKnownBlockHashes.empty() && m_lastKnownBlockHashes.back() == hash)
+    {
+        return;
+    }
+
     /* If we're at a checkpoint height, add the hash to the infrequent
        checkpoints (at the beginning of the queue) */
-    if (height % Constants::BLOCK_HASH_CHECKPOINTS_INTERVAL == 0)
+    if (m_lastSavedCheckpointAt + Constants::BLOCK_HASH_CHECKPOINTS_INTERVAL < height)
     {
+        m_lastSavedCheckpointAt = height;
         m_blockHashCheckpoints.push_front(hash);
     }
 

src/WalletBackend/SynchronizationStatus.h

@@ -61,4 +61,8 @@ class SynchronizationStatus
 
         /* The last block height we are aware of */
         uint64_t m_lastKnownBlockHeight = 0;
+
+        /* The last height we saved a block checkpoint at. Can't do every 5k
+           since we skip blocks with coinbase tx scanning of. */
+        uint64_t m_lastSavedCheckpointAt = 0;
 };