Fine-grained UI updates

Gets rid of `MainFrameRepaint` in favor of specific update functions that tell the UI exactly what changed.

This improves the efficiency of various handlers. Also fixes problems with mined transactions not showing up until restart.

The following notifications were added:

- `NotifyBlocksChanged`: Block chain changed
- `NotifyKeyStoreStatusChanged`: Wallet status (encrypted, locked) changed.
- `NotifyAddressBookChanged`: Address book entry changed.
- `NotifyTransactionChanged`: Wallet transaction added, removed or updated.
- `NotifyNumConnectionsChanged`: Number of connections changed.
- `NotifyAlertChanged`: New, updated or cancelled alert. As this finally makes it possible for the UI to know when a new alert arrived, it can be shown as OS notification.

These notifications could also be useful for RPC clients. However, currently, they are ignored in bitcoind (in noui.cpp).

Also brings back polling with timer for numBlocks in ClientModel. This value updates so frequently during initial download that the number of signals clogs the UI thread and causes heavy CPU usage. And after initial block download, the value changes so rarely that a delay of half a second until the UI updates is unnoticable.

src/keystore.cpp

@@ -5,6 +5,7 @@
 
 #include "keystore.h"
 #include "script.h"
+#include "ui_interface.h"
 
 bool CKeyStore::GetPubKey(const CBitcoinAddress &address, std::vector<unsigned char> &vchPubKeyOut) const
 {
@@ -73,6 +74,20 @@ bool CCryptoKeyStore::SetCrypted()
     return true;
 }
 
+bool CCryptoKeyStore::Lock()
+{
+    if (!SetCrypted())
+        return false;
+
+    {
+        LOCK(cs_KeyStore);
+        vMasterKey.clear();
+    }
+
+    NotifyKeyStoreStatusChanged(this);
+    return true;
+}
+
 bool CCryptoKeyStore::Unlock(const CKeyingMaterial& vMasterKeyIn)
 {
     {
@@ -99,6 +114,7 @@ bool CCryptoKeyStore::Unlock(const CKeyingMaterial& vMasterKeyIn)
         }
         vMasterKey = vMasterKeyIn;
     }
+    NotifyKeyStoreStatusChanged(this);
     return true;
 }
 

src/keystore.h

@@ -143,18 +143,7 @@ class CCryptoKeyStore : public CBasicKeyStore
         return result;
     }
 
-    bool Lock()
-    {
-        if (!SetCrypted())
-            return false;
-
-        {
-            LOCK(cs_KeyStore);
-            vMasterKey.clear();
-        }
-
-        return true;
-    }
+    bool Lock();
 
     virtual bool AddCryptedKey(const std::vector<unsigned char> &vchPubKey, const std::vector<unsigned char> &vchCryptedSecret);
     bool AddKey(const CKey& key);

src/main.cpp

@@ -946,7 +946,7 @@ void static InvalidChainFound(CBlockIndex* pindexNew)
     {
         bnBestInvalidWork = pindexNew->bnChainWork;
         CTxDB().WriteBestInvalidWork(bnBestInvalidWork);
-        MainFrameRepaint();
+        NotifyBlocksChanged();
     }
     printf("InvalidChainFound: invalid block=%s  height=%d  work=%s\n", pindexNew->GetBlockHash().ToString().substr(0,20).c_str(), pindexNew->nHeight, pindexNew->bnChainWork.ToString().c_str());
     printf("InvalidChainFound:  current best=%s  height=%d  work=%s\n", hashBestChain.ToString().substr(0,20).c_str(), nBestHeight, bnBestChainWork.ToString().c_str());
@@ -1647,7 +1647,7 @@ bool CBlock::AddToBlockIndex(unsigned int nFile, unsigned int nBlockPos)
         hashPrevBestCoinBase = vtx[0].GetHash();
     }
 
-    MainFrameRepaint();
+    NotifyBlocksChanged();
     return true;
 }
 
@@ -2176,6 +2176,18 @@ string GetWarnings(string strFor)
     return "error";
 }
 
+CAlert CAlert::getAlertByHash(const uint256 &hash)
+{
+    CAlert retval;
+    {
+        LOCK(cs_mapAlerts);
+        map<uint256, CAlert>::iterator mi = mapAlerts.find(hash);
+        if(mi != mapAlerts.end())
+            retval = mi->second;
+    }
+    return retval;
+}
+
 bool CAlert::ProcessAlert()
 {
     if (!CheckSignature())
@@ -2192,11 +2204,13 @@ bool CAlert::ProcessAlert()
             if (Cancels(alert))
             {
                 printf("cancelling alert %d\n", alert.nID);
+                NotifyAlertChanged((*mi).first, CT_DELETED);
                 mapAlerts.erase(mi++);
             }
             else if (!alert.IsInEffect())
             {
                 printf("expiring alert %d\n", alert.nID);
+                NotifyAlertChanged((*mi).first, CT_DELETED);
                 mapAlerts.erase(mi++);
             }
             else
@@ -2216,10 +2230,12 @@ bool CAlert::ProcessAlert()
 
         // Add to mapAlerts
         mapAlerts.insert(make_pair(GetHash(), *this));
+        // Notify UI if it applies to me
+        if(AppliesToMe())
+            NotifyAlertChanged(GetHash(), CT_NEW);
     }
 
     printf("accepted alert %d, AppliesToMe()=%d\n", nID, AppliesToMe());
-    MainFrameRepaint();
     return true;
 }
 

src/main.h

@@ -1574,6 +1574,11 @@ class CAlert : public CUnsignedAlert
     }
 
     bool ProcessAlert();
+
+    /*
+     * Get copy of (active) alert object by hash. Returns a null alert if it is not found.
+     */
+    static CAlert getAlertByHash(const uint256 &hash);
 };
 
 class CTxMemPool

src/net.cpp

@@ -705,7 +705,7 @@ void ThreadSocketHandler2(void* parg)
         if (vNodes.size() != nPrevNodeCount)
         {
             nPrevNodeCount = vNodes.size();
-            MainFrameRepaint();
+            NotifyNumConnectionsChanged(vNodes.size());
         }
 
 

src/noui.cpp

@@ -19,14 +19,6 @@ bool ThreadSafeAskFee(int64 nFeeRequired, const std::string& strCaption)
     return true;
 }
 
-void MainFrameRepaint()
-{
-}
-
-void AddressBookRepaint()
-{
-}
-
 void InitMessage(const std::string &message)
 {
 }
@@ -42,3 +34,26 @@ void QueueShutdown()
     CreateThread(Shutdown, NULL);
 }
 
+void NotifyBlocksChanged()
+{
+}
+
+void NotifyKeyStoreStatusChanged(CBasicKeyStore *wallet)
+{
+}
+
+void NotifyAddressBookChanged(CWallet *wallet, const std::string &address, const std::string &label, EntryStatus status)
+{
+}
+
+void NotifyTransactionChanged(CWallet *wallet, const uint256 &hashTx, EntryStatus status)
+{
+}
+
+void NotifyNumConnectionsChanged(int newNumConnections)
+{
+}
+
+void NotifyAlertChanged(const uint256 &hash, EntryStatus status)
+{
+}

src/qt/addresstablemodel.cpp

@@ -232,9 +232,10 @@ QModelIndex AddressTableModel::index(int row, int column, const QModelIndex & pa
     }
 }
 
-void AddressTableModel::update()
+void AddressTableModel::updateEntry(const QString &address, const QString &label, int status)
 {
     // Update address book model from Bitcoin core
+    // TODO: use address, label, status to update only the specified entry (like in WalletModel)
     beginResetModel();
     priv->refreshAddressTable();
     endResetModel();

src/qt/addresstablemodel.h

@@ -78,9 +78,9 @@ class AddressTableModel : public QAbstractTableModel
     void defaultAddressChanged(const QString &address);
 
 public slots:
-    /* Update address list from core. Invalidates any indices.
+    /* Update address list from core.
      */
-    void update();
+    void updateEntry(const QString &address, const QString &label, int status);
 };
 
 #endif // ADDRESSTABLEMODEL_H

src/qt/bitcoin.cpp

@@ -84,20 +84,6 @@ void ThreadSafeHandleURI(const std::string& strURI)
                                Q_ARG(QString, QString::fromStdString(strURI)));
 }
 
-void MainFrameRepaint()
-{
-    if(clientmodel)
-        QMetaObject::invokeMethod(clientmodel, "update", Qt::QueuedConnection);
-    if(walletmodel)
-        QMetaObject::invokeMethod(walletmodel, "update", Qt::QueuedConnection);
-}
-
-void AddressBookRepaint()
-{
-    if(walletmodel)
-        QMetaObject::invokeMethod(walletmodel, "updateAddressList", Qt::QueuedConnection);
-}
-
 void InitMessage(const std::string &message)
 {
     if(splashref)
@@ -120,6 +106,61 @@ std::string _(const char* psz)
     return QCoreApplication::translate("bitcoin-core", psz).toStdString();
 }
 
+void NotifyBlocksChanged()
+{
+    // This notification is too frequent. Don't trigger a signal.
+    // Don't remove it, though, as it might be useful later.
+}
+
+void NotifyKeyStoreStatusChanged(CBasicKeyStore *wallet)
+{
+    // This currently ignores the wallet argument. When multiple wallet support is implemented, this
+    // parameter should be mapped to a specific WalletModel for that wallet.
+    OutputDebugStringF("NotifyKeyStoreStatusChanged\n");
+    if(walletmodel)
+        QMetaObject::invokeMethod(walletmodel, "updateStatus", Qt::QueuedConnection);
+}
+
+void NotifyAddressBookChanged(CWallet *wallet, const std::string &address, const std::string &label, ChangeType status)
+{
+    // This currently ignores the wallet argument. When multiple wallet support is implemented, this
+    // parameter should be mapped to a specific WalletModel for that wallet.
+    OutputDebugStringF("NotifyAddressBookChanged %s %s status=%i\n", address.c_str(), label.c_str(), status);
+    if(walletmodel)
+        QMetaObject::invokeMethod(walletmodel, "updateAddressBook", Qt::QueuedConnection,
+                                  Q_ARG(QString, QString::fromStdString(address)),
+                                  Q_ARG(QString, QString::fromStdString(label)),
+                                  Q_ARG(int, status));
+}
+
+void NotifyTransactionChanged(CWallet *wallet, const uint256 &hash, ChangeType status)
+{
+    // This currently ignores the wallet argument. When multiple wallet support is implemented, this
+    // parameter should be mapped to a specific WalletModel for that wallet.
+    OutputDebugStringF("NotifyTransactionChanged %s status=%i\n", hash.GetHex().c_str(), status);
+    if(walletmodel)
+        QMetaObject::invokeMethod(walletmodel, "updateTransaction", Qt::QueuedConnection,
+                                  Q_ARG(QString, QString::fromStdString(hash.GetHex())),
+                                  Q_ARG(int, status));
+}
+
+void NotifyNumConnectionsChanged(int newNumConnections)
+{
+    // Too noisy: OutputDebugStringF("NotifyNumConnectionsChanged %i\n", newNumConnections);
+    if(clientmodel)
+        QMetaObject::invokeMethod(clientmodel, "updateNumConnections", Qt::QueuedConnection,
+                                  Q_ARG(int, newNumConnections));
+}
+
+void NotifyAlertChanged(const uint256 &hash, ChangeType status)
+{
+    OutputDebugStringF("NotifyAlertChanged %s status=%i\n", hash.GetHex().c_str(), status);
+    if(clientmodel)
+        QMetaObject::invokeMethod(clientmodel, "updateAlert", Qt::QueuedConnection,
+                                  Q_ARG(QString, QString::fromStdString(hash.GetHex())),
+                                  Q_ARG(int, status));
+}
+
 /* Handle runaway exceptions. Shows a message box with the problem and quits the program.
  */
 static void handleRunawayException(std::exception *e)

src/qt/bitcoingui.cpp

@@ -350,11 +350,11 @@ void BitcoinGUI::setClientModel(ClientModel *clientModel)
         setNumConnections(clientModel->getNumConnections());
         connect(clientModel, SIGNAL(numConnectionsChanged(int)), this, SLOT(setNumConnections(int)));
 
-        setNumBlocks(clientModel->getNumBlocks());
-        connect(clientModel, SIGNAL(numBlocksChanged(int)), this, SLOT(setNumBlocks(int)));
+        setNumBlocks(clientModel->getNumBlocks(), clientModel->getNumBlocksOfPeers());
+        connect(clientModel, SIGNAL(numBlocksChanged(int,int)), this, SLOT(setNumBlocks(int,int)));
 
         // Report errors from network/worker thread
-        connect(clientModel, SIGNAL(error(QString,QString, bool)), this, SLOT(error(QString,QString,bool)));
+        connect(clientModel, SIGNAL(error(QString,QString,bool)), this, SLOT(error(QString,QString,bool)));
 
         rpcConsole->setClientModel(clientModel);
     }
@@ -493,7 +493,7 @@ void BitcoinGUI::setNumConnections(int count)
     labelConnectionsIcon->setToolTip(tr("%n active connection(s) to Bitcoin network", "", count));
 }
 
-void BitcoinGUI::setNumBlocks(int count)
+void BitcoinGUI::setNumBlocks(int count, int nTotalBlocks)
 {
     // don't show / hide progressBar and it's label if we have no connection(s) to the network
     if (!clientModel || clientModel->getNumConnections() == 0)
@@ -504,7 +504,6 @@ void BitcoinGUI::setNumBlocks(int count)
         return;
     }
 
-    int nTotalBlocks = clientModel->getNumBlocksOfPeers();
     QString tooltip;
 
     if(count < nTotalBlocks)

src/qt/bitcoingui.h

@@ -111,7 +111,7 @@ public slots:
     /** Set number of connections shown in the UI */
     void setNumConnections(int count);
     /** Set number of blocks shown in the UI */
-    void setNumBlocks(int count);
+    void setNumBlocks(int count, int countOfPeers);
     /** Set the encryption status as shown in the UI.
        @param[in] status            current encryption status
        @see WalletModel::EncryptionStatus