[backport#18587] gui: Avoid wallet tryGetBalances calls in WalletModel::pollBalanceChanged

Summary:
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).

---

Backport of Core [[bitcoin/bitcoin#18587 | PR18587]]

Test Plan:
  ninja all check-check-functional

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D9168

Author: jonasschnelli

src/interfaces/wallet.cpp

@@ -331,16 +331,13 @@ namespace {
             }
             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;
         }

src/interfaces/wallet.h

@@ -198,9 +198,8 @@ class Wallet {
     //! 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 Amount getBalance() = 0;

src/qt/transactiontablemodel.cpp

@@ -671,8 +671,8 @@ QVariant TransactionTableModel::headerData(int section,
 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);
     }

src/qt/walletmodel.cpp

@@ -31,10 +31,11 @@ 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_node(client_model.node()),
+      m_client_model(&client_model), m_node(client_model.node()),
       optionsModel(client_model.getOptionsModel()), addressTableModel(nullptr),
       transactionTableModel(nullptr), recentRequestsTableModel(nullptr),
-      cachedEncryptionStatus(Unencrypted), cachedNumBlocks(0) {
+      cachedEncryptionStatus(Unencrypted), cachedNumBlocks(0),
+      timer(new QTimer(this)) {
     fHaveWatchOnly = m_wallet->haveWatchOnly();
     addressTableModel = new AddressTableModel(this);
     transactionTableModel = new TransactionTableModel(platformStyle, this);
@@ -49,11 +50,17 @@ 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();
 
@@ -63,25 +70,33 @@ 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();
+        }
     }
 }
 

src/qt/walletmodel.h

@@ -148,7 +148,8 @@ class WalletModel : public QObject {
 
     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; }
 
     const CChainParams &getChainParams() const;
 
@@ -170,7 +171,7 @@ class WalletModel : public QObject {
     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;
@@ -188,6 +189,7 @@ class WalletModel : public QObject {
     interfaces::WalletBalances m_cached_balances;
     EncryptionStatus cachedEncryptionStatus;
     int cachedNumBlocks;
+    QTimer *timer;
 
     void subscribeToCoreSignals();
     void unsubscribeFromCoreSignals();

src/qt/walletview.cpp

@@ -111,6 +111,9 @@ void WalletView::setClientModel(ClientModel *_clientModel) {
 
     overviewPage->setClientModel(_clientModel);
     sendCoinsPage->setClientModel(_clientModel);
+    if (walletModel) {
+        walletModel->setClientModel(_clientModel);
+    }
 }
 
 void WalletView::setWalletModel(WalletModel *_walletModel) {