Merge #18587: gui: Avoid wallet tryGetBalances calls in WalletModel::pollBalanceChanged

d3a56be77a9d112cde4baef4314882170b9f228f Revert "gui: Avoid Wallet::GetBalance in WalletModel::pollBalanceChanged" (Russell Yanofsky)
bf0a510981ddc28c754881ca21c50ab18e5f2b59 gui: Avoid wallet tryGetBalances calls before TransactionChanged or BlockTip notifications (Russell Yanofsky)
2bc9b92ed8b7736ad67876398a0bb8287f57e9b3 Cancel wallet balance timer when shutdown requested (Russell Yanofsky)
83f69fab3a1ae97c5cff8ba1e6fd191b0fa264bb Switch transaction table to use wallet height not node height (Russell Yanofsky)

Pull request description:

  Main commit `gui: Avoid wallet tryGetBalances calls` is one-line change to `WalletModel::pollBalanceChanged` that returns early if there hasn't been a new `TransactionChanged` or `BlockTip` notification since the previous poll call. This is the same behavior that was implemented in #18160, now implemented in a simpler way.

  The other commits are a straight revert of #18160, and two tweaks to avoid relying on `WalletModel::m_client_model` lifetime which were causing travis failures with earlier versions of this PR.

  Motivation for this change is to be able to revert #18160 and cut down on unnecessary cross-process calls that happen when #18160 is combined with #10102

  This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).

ACKs for top commit:
  jonasschnelli:
    utACK d3a56be77a9d112cde4baef4314882170b9f228f

Tree-SHA512: 3cd31ca515e77c3bd7160d3f1ea0dce5050d4038b2aa441b6f66b8599bd413d81ca5542a197806e773d6092dd1d26830932b1cecbc95298b1f1ab41099e2f12f

6 files changed, 32 insertions, 20 deletions

diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp
index 13b034936b..349dce0247 100644
--- a/src/interfaces/wallet.cpp
+++ b/src/interfaces/wallet.cpp
@@ -351,14 +351,13 @@ public:
         }
         return result;
     }
-    bool tryGetBalances(WalletBalances& balances, int& num_blocks, bool force, int cached_num_blocks) override
+    bool tryGetBalances(WalletBalances& balances, int& num_blocks) override
     {
         TRY_LOCK(m_wallet->cs_wallet, locked_wallet);
         if (!locked_wallet) {
             return false;
         }
         num_blocks = m_wallet->GetLastBlockHeight();
-        if (!force && num_blocks == cached_num_blocks) return false;
         balances = getBalances();
         return true;
     }
diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h
index f35335c69f..421d35af15 100644
--- a/src/interfaces/wallet.h
+++ b/src/interfaces/wallet.h
@@ -202,11 +202,8 @@ public:
     //! Get balances.
     virtual WalletBalances getBalances() = 0;
 
-    //! Get balances if possible without waiting for chain and wallet locks.
-    virtual bool tryGetBalances(WalletBalances& balances,
-        int& num_blocks,
-        bool force,
-        int cached_num_blocks) = 0;
+    //! Get balances if possible without blocking.
+    virtual bool tryGetBalances(WalletBalances& balances, int& num_blocks) = 0;
 
     //! Get balance.
     virtual CAmount getBalance() = 0;
diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp
index 18554aef1f..7a15503228 100644
--- a/src/qt/transactiontablemodel.cpp
+++ b/src/qt/transactiontablemodel.cpp
@@ -664,7 +664,7 @@ QVariant TransactionTableModel::headerData(int section, Qt::Orientation orientat
 QModelIndex TransactionTableModel::index(int row, int column, const QModelIndex &parent) const
 {
     Q_UNUSED(parent);
-    TransactionRecord *data = priv->index(walletModel->wallet(), walletModel->clientModel().getNumBlocks(), row);
+    TransactionRecord *data = priv->index(walletModel->wallet(), walletModel->getNumBlocks(), row);
     if(data)
     {
         return createIndex(row, column, data);
diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
index 70ee7f4917..1084ec9725 100644
--- a/src/qt/walletmodel.cpp
+++ b/src/qt/walletmodel.cpp
@@ -39,14 +39,15 @@
 WalletModel::WalletModel(std::unique_ptr<interfaces::Wallet> wallet, ClientModel& client_model, const PlatformStyle *platformStyle, QObject *parent) :
     QObject(parent),
     m_wallet(std::move(wallet)),
-    m_client_model(client_model),
+    m_client_model(&client_model),
     m_node(client_model.node()),
     optionsModel(client_model.getOptionsModel()),
     addressTableModel(nullptr),
     transactionTableModel(nullptr),
     recentRequestsTableModel(nullptr),
     cachedEncryptionStatus(Unencrypted),
-    cachedNumBlocks(0)
+    cachedNumBlocks(0),
+    timer(new QTimer(this))
 {
     fHaveWatchOnly = m_wallet->haveWatchOnly();
     addressTableModel = new AddressTableModel(this);
@@ -64,11 +65,16 @@ WalletModel::~WalletModel()
 void WalletModel::startPollBalance()
 {
     // This timer will be fired repeatedly to update the balance
-    QTimer* timer = new QTimer(this);
     connect(timer, &QTimer::timeout, this, &WalletModel::pollBalanceChanged);
     timer->start(MODEL_UPDATE_DELAY);
 }
 
+void WalletModel::setClientModel(ClientModel* client_model)
+{
+    m_client_model = client_model;
+    if (!m_client_model) timer->stop();
+}
+
 void WalletModel::updateStatus()
 {
     EncryptionStatus newEncryptionStatus = getEncryptionStatus();
@@ -80,24 +86,31 @@ void WalletModel::updateStatus()
 
 void WalletModel::pollBalanceChanged()
 {
+    // Avoid recomputing wallet balances unless a TransactionChanged or
+    // BlockTip notification was received.
+    if (!fForceCheckBalanceChanged && cachedNumBlocks == m_client_model->getNumBlocks()) return;
+
     // Try to get balances and return early if locks can't be acquired. This
     // avoids the GUI from getting stuck on periodical polls if the core is
     // holding the locks for a longer time - for example, during a wallet
     // rescan.
     interfaces::WalletBalances new_balances;
     int numBlocks = -1;
-    if (!m_wallet->tryGetBalances(new_balances, numBlocks, fForceCheckBalanceChanged, cachedNumBlocks)) {
+    if (!m_wallet->tryGetBalances(new_balances, numBlocks)) {
         return;
     }
 
-    fForceCheckBalanceChanged = false;
+    if(fForceCheckBalanceChanged || numBlocks != cachedNumBlocks)
+    {
+        fForceCheckBalanceChanged = false;
 
-    // Balance and number of transactions might have changed
-    cachedNumBlocks = numBlocks;
+        // Balance and number of transactions might have changed
+        cachedNumBlocks = numBlocks;
 
-    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/qt/walletmodel.h b/src/qt/walletmodel.h
index 07004b7c6b..23232ec66b 100644
--- a/src/qt/walletmodel.h
+++ b/src/qt/walletmodel.h
@@ -144,7 +144,8 @@ public:
 
     interfaces::Node& node() const { return m_node; }
     interfaces::Wallet& wallet() const { return *m_wallet; }
-    ClientModel& clientModel() const { return m_client_model; }
+    void setClientModel(ClientModel* client_model);
+    int getNumBlocks() const { return cachedNumBlocks; }
 
     QString getWalletName() const;
     QString getDisplayName() const;
@@ -161,7 +162,7 @@ private:
     std::unique_ptr<interfaces::Handler> m_handler_show_progress;
     std::unique_ptr<interfaces::Handler> m_handler_watch_only_changed;
     std::unique_ptr<interfaces::Handler> m_handler_can_get_addrs_changed;
-    ClientModel& m_client_model;
+    ClientModel* m_client_model;
     interfaces::Node& m_node;
 
     bool fHaveWatchOnly;
@@ -179,6 +180,7 @@ private:
     interfaces::WalletBalances m_cached_balances;
     EncryptionStatus cachedEncryptionStatus;
     int cachedNumBlocks;
+    QTimer* timer;
 
     void subscribeToCoreSignals();
     void unsubscribeFromCoreSignals();
diff --git a/src/qt/walletview.cpp b/src/qt/walletview.cpp
index 5d9b420df7..c4e607990a 100644
--- a/src/qt/walletview.cpp
+++ b/src/qt/walletview.cpp
@@ -97,6 +97,7 @@ void WalletView::setClientModel(ClientModel *_clientModel)
 
     overviewPage->setClientModel(_clientModel);
     sendCoinsPage->setClientModel(_clientModel);
+    if (walletModel) walletModel->setClientModel(_clientModel);
 }
 
 void WalletView::setWalletModel(WalletModel *_walletModel)