From 9a84f7ef76dc01f6de15c1e64ba5ccac01d022dd Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@protonmail.com>
Date: Tue, 31 Mar 2020 13:48:52 +0200
Subject: [PATCH] Merge #18160: gui: Avoid Wallet::GetBalance in
 WalletModel::pollBalanceChanged
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

0933a37078e1ce3a3d70983c3e7f4b3ac6c3fa37 gui: Avoid Wallet::GetBalance in WalletModel::pollBalanceChanged (João Barbosa)

Pull request description:

  Each 250ms the slot `WalletModel::pollBalanceChanged` is called which, at worst case, calls `Wallet::GetBalance`. This is a waste of resources since most of the time there aren't new transactions or new blocks. Fix this by early checking if cache is dirty or not.

  The actual balance computation can still hang the GUI thread but that is tracked in #16874 and should be fixed with a solution similar to #17135.

ACKs for top commit:
  hebasto:
    ACK 0933a37078e1ce3a3d70983c3e7f4b3ac6c3fa37, I have not tested the code, but I have reviewed it and it looks OK, I agree it can be merged.
  jonasschnelli:
    utACK 0933a37078e1ce3a3d70983c3e7f4b3ac6c3fa37
  instagibbs:
    ACK 0933a37078e1ce3a3d70983c3e7f4b3ac6c3fa37
  ryanofsky:
    Code review ACK 0933a37078e1ce3a3d70983c3e7f4b3ac6c3fa37, but I would prefer (not strongly) for #17905 to be merged first. This PR can be simpler if it is based on #17905, so tryGetBalances can just be left alone instead of changing into to a more complicated tryGetBalancesIfNeeded function, and then getting changed back later when we want to optimize it out.
  jonatack:
    ACK 0933a37078e based primarily on code review, despite a lot of manual testing with a large 177MB wallet.

Tree-SHA512: 18db35bf33a7577666658c8cb0b57308c8474baa5ea95bf1468cd8531a69857d8915584f6ac505874717aa6aabeb1b506ac77630f8acdb6651afab89275e38a1
---
 src/interfaces/wallet.h   |  7 +++++--
 src/qt/walletmodel.cpp    | 19 ++++++++-----------
 src/wallet/interfaces.cpp |  5 +++--
 3 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h
index eeef9a68de..8d922dc42c 100644
--- a/src/interfaces/wallet.h
+++ b/src/interfaces/wallet.h
@@ -229,8 +229,11 @@ public:
     //! Get balances.
     virtual WalletBalances getBalances() = 0;
 
-    //! Get balances if possible without blocking.
-    virtual bool tryGetBalances(WalletBalances& balances, uint256& block_hash) = 0;
+    //! Get balances if possible without waiting for chain and wallet locks.
+    virtual bool tryGetBalances(WalletBalances& balances,
+        uint256& block_hash,
+        bool force,
+        const uint256& cached_last_update_tip) = 0;
 
     //! Get balance.
     virtual CAmount getBalance() = 0;
diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
index faadbdaaaf..831fb68f87 100644
--- a/src/qt/walletmodel.cpp
+++ b/src/qt/walletmodel.cpp
@@ -101,22 +101,19 @@ void WalletModel::pollBalanceChanged()
     // rescan.
     interfaces::WalletBalances new_balances;
     uint256 block_hash;
-    if (!m_wallet->tryGetBalances(new_balances, block_hash)) {
+    if (!m_wallet->tryGetBalances(new_balances, block_hash, fForceCheckBalanceChanged, m_cached_last_update_tip)) {
         return;
     }
 
-    if (fForceCheckBalanceChanged || block_hash != m_cached_last_update_tip || node().coinJoinOptions().getRounds() != cachedCoinJoinRounds)
-    {
-        fForceCheckBalanceChanged = false;
+    fForceCheckBalanceChanged = false;
 
-        // Balance and number of transactions might have changed
-        m_cached_last_update_tip = block_hash;
-        cachedCoinJoinRounds = node().coinJoinOptions().getRounds();
+    // Balance and number of transactions might have changed
+    m_cached_last_update_tip = block_hash;
+    cachedCoinJoinRounds = node().coinJoinOptions().getRounds();
 
-        checkBalanceChanged(new_balances);
-        if(transactionTableModel)
-            transactionTableModel->updateConfirmations();
-    }
+    checkBalanceChanged(new_balances);
+    if(transactionTableModel)
+        transactionTableModel->updateConfirmations();
 }
 
 void WalletModel::checkBalanceChanged(const interfaces::WalletBalances& new_balances)
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
index 200bf3e831..1fbb75ce74 100644
--- a/src/wallet/interfaces.cpp
+++ b/src/wallet/interfaces.cpp
@@ -434,13 +434,14 @@ public:
         }
         return result;
     }
-    bool tryGetBalances(WalletBalances& balances, uint256& block_hash) override
+    bool tryGetBalances(WalletBalances& balances, uint256& block_hash, bool force, const uint256& cached_last_update_tip) override
     {
+        block_hash = m_wallet->GetLastBlockHash();
+        if (!force && block_hash == cached_last_update_tip) return false;
         TRY_LOCK(m_wallet->cs_wallet, locked_wallet);
         if (!locked_wallet) {
             return false;
         }
-        block_hash = m_wallet->GetLastBlockHash();
         balances = getBalances();
         return true;
     }