Merge pull request #1452

944b6079 Wallet API: Do not refresh while daemon is syncing + fixed fast refresh when creating wallet offline + improved close wallet logic (make sure refresh thread is stopped) (Jaquee)

3 files changed, 65 insertions, 22 deletions

diff --git a/src/wallet/api/wallet.cpp b/src/wallet/api/wallet.cpp
index 6d8e48deb..3a4493ec3 100644
--- a/src/wallet/api/wallet.cpp
+++ b/src/wallet/api/wallet.cpp
@@ -204,6 +204,7 @@ WalletImpl::WalletImpl(bool testnet)
     , m_recoveringFromSeed(false)
     , m_synchronized(false)
     , m_rebuildWalletCache(false)
+    , m_is_connected(false)
 {
     m_wallet = new tools::wallet2(testnet);
     m_history = new TransactionHistoryImpl(this);
@@ -224,11 +225,19 @@ WalletImpl::WalletImpl(bool testnet)
 
 WalletImpl::~WalletImpl()
 {
+
+    LOG_PRINT_L1(__FUNCTION__);
+    // Pause refresh thread - prevents refresh from starting again
+    pauseRefresh();
+    // Close wallet - stores cache and stops ongoing refresh operation 
+    close();
+    // Stop refresh thread
     stopRefresh();
+    delete m_wallet2Callback;
     delete m_history;
     delete m_addressBook;
     delete m_wallet;
-    delete m_wallet2Callback;
+    LOG_PRINT_L1(__FUNCTION__ << " finished");
 }
 
 bool WalletImpl::create(const std::string &path, const std::string &password, const std::string &language)
@@ -329,7 +338,7 @@ bool WalletImpl::close()
 {
 
     bool result = false;
-    LOG_PRINT_L3("closing wallet...");
+    LOG_PRINT_L1("closing wallet...");
     try {
         // Do not store wallet with invalid status
         // Status Critical refers to errors on opening or creating wallets.
@@ -337,10 +346,10 @@ bool WalletImpl::close()
             m_wallet->store();
         else
             LOG_PRINT_L3("Status_Critical - not storing wallet");
-        LOG_PRINT_L3("wallet::store done");
-        LOG_PRINT_L3("Calling wallet::stop...");
+        LOG_PRINT_L1("wallet::store done");
+        LOG_PRINT_L1("Calling wallet::stop...");
         m_wallet->stop();
-        LOG_PRINT_L3("wallet::stop done");
+        LOG_PRINT_L1("wallet::stop done");
         result = true;
         clearStatus();
     } catch (const std::exception &e) {
@@ -487,6 +496,8 @@ uint64_t WalletImpl::approximateBlockChainHeight() const
 }
 uint64_t WalletImpl::daemonBlockChainHeight() const
 {
+    if (!m_is_connected)
+        return 0;
     std::string err;
     uint64_t result = m_wallet->get_daemon_blockchain_height(err);
     if (!err.empty()) {
@@ -504,6 +515,8 @@ uint64_t WalletImpl::daemonBlockChainHeight() const
 
 uint64_t WalletImpl::daemonBlockChainTargetHeight() const
 {
+    if (!m_is_connected)
+        return 0;
     std::string err;
     uint64_t result = m_wallet->get_daemon_blockchain_target_height(err);
     if (!err.empty()) {
@@ -516,9 +529,20 @@ uint64_t WalletImpl::daemonBlockChainTargetHeight() const
         m_status = Status_Ok;
         m_errorString = "";
     }
+    // Target height can be 0 when daemon is synced. Use blockchain height instead. 
+    if(result == 0)
+        result = daemonBlockChainHeight();
     return result;
 }
 
+bool WalletImpl::daemonSynced() const
+{   
+    if(connected() == Wallet::ConnectionStatus_Disconnected)
+        return false;
+    uint64_t blockChainHeight = daemonBlockChainHeight();
+    return (blockChainHeight >= daemonBlockChainTargetHeight() && blockChainHeight > 1);
+}
+
 bool WalletImpl::synchronized() const
 {
     return m_synchronized;
@@ -924,8 +948,8 @@ bool WalletImpl::connectToDaemon()
 Wallet::ConnectionStatus WalletImpl::connected() const
 {
     uint32_t version = 0;
-    bool is_connected = m_wallet->check_connection(&version);
-    if (!is_connected)
+    m_is_connected = m_wallet->check_connection(&version);
+    if (!m_is_connected)
         return Wallet::ConnectionStatus_Disconnected;
     if ((version >> 16) != CORE_RPC_VERSION_MAJOR)
         return Wallet::ConnectionStatus_WrongVersion;
@@ -970,7 +994,7 @@ void WalletImpl::refreshThreadFunc()
         LOG_PRINT_L3(__FUNCTION__ << ": refresh lock acquired...");
         LOG_PRINT_L3(__FUNCTION__ << ": m_refreshEnabled: " << m_refreshEnabled);
         LOG_PRINT_L3(__FUNCTION__ << ": m_status: " << m_status);
-        if (m_refreshEnabled /*&& m_status == Status_Ok*/) {
+        if (m_refreshEnabled) {
             LOG_PRINT_L3(__FUNCTION__ << ": refreshing...");
             doRefresh();
         }
@@ -983,16 +1007,23 @@ void WalletImpl::doRefresh()
     // synchronizing async and sync refresh calls
     boost::lock_guard<boost::mutex> guarg(m_refreshMutex2);
     try {
-        m_wallet->refresh();
-        if (!m_synchronized) {
-            m_synchronized = true;
-        }
-        // assuming if we have empty history, it wasn't initialized yet
-        // for futher history changes client need to update history in
-        // "on_money_received" and "on_money_sent" callbacks
-        if (m_history->count() == 0) {
-            m_history->refresh();
-        }
+        // Syncing daemon and refreshing wallet simultaneously is very resource intensive.
+        // Disable refresh if wallet is disconnected or daemon isn't synced.
+        if (daemonSynced()) {
+            // Use fast refresh for new wallets
+            if (isNewWallet())
+                m_wallet->set_refresh_from_block_height(daemonBlockChainHeight());        
+            m_wallet->refresh();
+            if (!m_synchronized) {
+                m_synchronized = true;
+            }
+            // assuming if we have empty history, it wasn't initialized yet
+            // for futher history changes client need to update history in
+            // "on_money_received" and "on_money_sent" callbacks
+            if (m_history->count() == 0) {
+                m_history->refresh();
+            }
+        } 
     } catch (const std::exception &e) {
         m_status = Status_Error;
         m_errorString = e.what();
diff --git a/src/wallet/api/wallet.h b/src/wallet/api/wallet.h
index 5046fbf0f..d245a78d0 100644
--- a/src/wallet/api/wallet.h
+++ b/src/wallet/api/wallet.h
@@ -107,14 +107,15 @@ public:
     virtual std::string getTxKey(const std::string &txid) const;
     virtual std::string signMessage(const std::string &message);
     virtual bool verifySignedMessage(const std::string &message, const std::string &address, const std::string &signature) const;
+    virtual void startRefresh();
+    virtual void pauseRefresh();
 
 private:
     void clearStatus();
     void refreshThreadFunc();
     void doRefresh();
-    void startRefresh();
+    bool daemonSynced() const;
     void stopRefresh();
-    void pauseRefresh();
     bool isNewWallet() const;
     void doInit(const std::string &daemon_address, uint64_t upper_transaction_size_limit);
 
@@ -148,9 +149,11 @@ private:
     // flag indicating wallet is recovering from seed
     // so it shouldn't be considered as new and pull blocks (slow-refresh)
     // instead of pulling hashes (fast-refresh)
-    bool                m_recoveringFromSeed;
+    std::atomic<bool>   m_recoveringFromSeed;
     std::atomic<bool>   m_synchronized;
-    bool                m_rebuildWalletCache;
+    std::atomic<bool>   m_rebuildWalletCache;
+    // cache connection status to avoid unnecessary RPC calls
+    mutable std::atomic<bool>   m_is_connected;
 };
 
 
diff --git a/src/wallet/wallet2_api.h b/src/wallet/wallet2_api.h
index cd2e7230a..625cddfd2 100644
--- a/src/wallet/wallet2_api.h
+++ b/src/wallet/wallet2_api.h
@@ -364,6 +364,15 @@ struct Wallet
     static std::string paymentIdFromAddress(const std::string &str, bool testnet);
     static uint64_t maximumAllowedAmount();
 
+   /**
+    * @brief StartRefresh - Start/resume refresh thread (refresh every 10 seconds)
+    */
+    virtual void startRefresh() = 0;
+   /**
+    * @brief pauseRefresh - pause refresh thread
+    */
+    virtual void pauseRefresh() = 0;
+
     /**
      * @brief refresh - refreshes the wallet, updating transactions from daemon
      * @return - true if refreshed successfully;