Merge bitcoin-core/gui#121: Early subscribe core signals in transacti…

…on table model

cafef08 qt: Refactor to remove unnecessary block in DispatchNotifications (João Barbosa)
57785fb qt: Early subscribe core signals in transaction table model (João Barbosa)
c6cbdf1 qt: Refactor ShowProgress to DispatchNotifications (João Barbosa)
3bccd50 qt: Set flag after inital load on transaction table model (João Barbosa)

Pull request description:

  This fixes the case where transaction notifications arrive between `getWalletTxs` and `subscribeToCoreSignals`. Basically notifications are queued until `getWalletTxs` and wallet rescan complete.

  This is also a requirement to call `getWalletTxs` in a background thread.

  Motivated by bitcoin#20241.

ACKs for top commit:
  jonatack:
    tACK cafef08
  ryanofsky:
    Code review ACK cafef08. Only change since last review is splitting commits and replacing m_progress with m_loading.
  meshcollider:
    Code review ACK cafef08

Tree-SHA512: 003caab2f2ae3522619711c8d02d521d2b8f7f280a467f6c3d08abf37ca81cc66b4b9fa10acfdf34e5fe250da7b696cfeec435f72b53c1ea97ccda96d8b4be33

src/qt/transactiontablemodel.cpp

@@ -95,21 +95,23 @@ class TransactionTablePriv
      */
     QList<TransactionRecord> cachedWallet;
 
-    bool fQueueNotifications = false;
+    /** True when model finishes loading all wallet transactions on start */
+    bool m_loaded = false;
+    /** True when transactions are being notified, for instance when scanning */
+    bool m_loading = false;
     std::vector< TransactionNotification > vQueueNotifications;
 
     void NotifyTransactionChanged(const uint256 &hash, ChangeType status);
     void NotifyAddressBookChanged(const CTxDestination &address, const std::string &label, bool isMine, const std::string &purpose, ChangeType status);
-    void ShowProgress(const std::string &title, int nProgress);
+    void DispatchNotifications();
 
     /* Query entire wallet anew from core.
      */
     void refreshWallet(interfaces::Wallet& wallet)
     {
-        qDebug() << "TransactionTablePriv::refreshWallet";
         parent->beginResetModel();
+        assert(!m_loaded);
         try {
-            cachedWallet.clear();
             for (const auto& wtx : wallet.getWalletTxs()) {
                 if (TransactionRecord::showTransaction()) {
                     cachedWallet.append(TransactionRecord::decomposeTransaction(wallet, wtx));
@@ -119,6 +121,8 @@ class TransactionTablePriv
             QMessageBox::critical(nullptr, PACKAGE_NAME, QString("Failed to refresh wallet table: ") + QString::fromStdString(e.what()));
         }
         parent->endResetModel();
+        m_loaded = true;
+        DispatchNotifications();
     }
 
     /* Update our model of the wallet incrementally, to synchronize our model of the wallet
@@ -267,12 +271,12 @@ TransactionTableModel::TransactionTableModel(WalletModel *parent):
         fProcessingQueuedTransactions(false),
         cachedChainLockHeight(-1)
 {
+    subscribeToCoreSignals();
+
     columns << QString() << QString() << tr("Date") << tr("Type") << tr("Address / Label") << BitcoinUnits::getAmountColumnTitle(walletModel->getOptionsModel()->getDisplayUnit());
     priv->refreshWallet(walletModel->wallet());
 
     connect(walletModel->getOptionsModel(), &OptionsModel::displayUnitChanged, this, &TransactionTableModel::updateDisplayUnit);
-
-    subscribeToCoreSignals();
 }
 
 TransactionTableModel::~TransactionTableModel()
@@ -793,7 +797,7 @@ void TransactionTablePriv::NotifyTransactionChanged(const uint256 &hash, ChangeT
 
     TransactionNotification notification(hash, status, showTransaction);
 
-    if (fQueueNotifications)
+    if (!m_loaded || m_loading)
     {
         vQueueNotifications.push_back(notification);
         return;
@@ -812,43 +816,41 @@ void TransactionTablePriv::NotifyAddressBookChanged(const CTxDestination &addres
     assert(invoked);
 }
 
-void TransactionTablePriv::ShowProgress(const std::string &title, int nProgress)
+void TransactionTablePriv::DispatchNotifications()
 {
-    if (nProgress == 0)
-        fQueueNotifications = true;
+    if (!m_loaded || m_loading) return;
 
-    if (nProgress == 100)
-    {
-        fQueueNotifications = false;
-        if (vQueueNotifications.size() < 10000) {
-            if (vQueueNotifications.size() > 10) { // prevent balloon spam, show maximum 10 balloons
-                bool invoked = QMetaObject::invokeMethod(parent, "setProcessingQueuedTransactions", Qt::QueuedConnection, Q_ARG(bool, true));
+    if (vQueueNotifications.size() < 10000) {
+        if (vQueueNotifications.size() > 10) { // prevent balloon spam, show maximum 10 balloons
+            bool invoked = QMetaObject::invokeMethod(parent, "setProcessingQueuedTransactions", Qt::QueuedConnection, Q_ARG(bool, true));
+            assert(invoked);
+        }
+        for (unsigned int i = 0; i < vQueueNotifications.size(); ++i)
+        {
+            if (vQueueNotifications.size() - i <= 10) {
+                bool invoked = QMetaObject::invokeMethod(parent, "setProcessingQueuedTransactions", Qt::QueuedConnection, Q_ARG(bool, false));
                 assert(invoked);
             }
-            for (unsigned int i = 0; i < vQueueNotifications.size(); ++i)
-            {
-                if (vQueueNotifications.size() - i <= 10) {
-                    bool invoked = QMetaObject::invokeMethod(parent, "setProcessingQueuedTransactions", Qt::QueuedConnection, Q_ARG(bool, false));
-                    assert(invoked);
-                }
 
-                vQueueNotifications[i].invoke(parent);
-            }
-        } else {
-            // it's much faster to just refresh the whole thing instead
-            bool invoked = QMetaObject::invokeMethod(parent, "refreshWallet", Qt::QueuedConnection);
-            assert(invoked);
+            vQueueNotifications[i].invoke(parent);
         }
-        vQueueNotifications.clear();
+    } else {
+        // it's much faster to just refresh the whole thing instead
+        bool invoked = QMetaObject::invokeMethod(parent, "refreshWallet", Qt::QueuedConnection);
+        assert(invoked);
     }
+    vQueueNotifications.clear();
 }
 
 void TransactionTableModel::subscribeToCoreSignals()
 {
     // Connect signals to wallet
     m_handler_transaction_changed = walletModel->wallet().handleTransactionChanged(std::bind(&TransactionTablePriv::NotifyTransactionChanged, priv, std::placeholders::_1, std::placeholders::_2));
     m_handler_address_book_changed = walletModel->wallet().handleAddressBookChanged(std::bind(&TransactionTablePriv::NotifyAddressBookChanged, priv, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, std::placeholders::_4, std::placeholders::_5));
-    m_handler_show_progress = walletModel->wallet().handleShowProgress(std::bind(&TransactionTablePriv::ShowProgress, priv, std::placeholders::_1, std::placeholders::_2));
+    m_handler_show_progress = walletModel->wallet().handleShowProgress([this](const std::string&, int progress) {
+        priv->m_loading = progress < 100;
+        priv->DispatchNotifications();
+    });
 }
 
 void TransactionTableModel::unsubscribeFromCoreSignals()