Merge #10244: Refactor: separate gui from wallet and node

9960137 Add developer notes about blocking GUI code (Russell Yanofsky)
9a61eed Use WalletBalances struct in Qt (Russell Yanofsky)
56f33ca Remove direct bitcoin calls from qt/sendcoinsdialog.cpp (Russell Yanofsky)
e872c93 Remove direct bitcoin access from qt/guiutil.cpp (Russell Yanofsky)
5884558 Remove direct bitcoin calls from qt transaction table files (Russell Yanofsky)
3cab2ce Remove direct bitcoin calls from qt/paymentserver.cpp (Russell Yanofsky)
3ec2ebc Remove direct bitcoin calls from qt/addresstablemodel.cpp (Russell Yanofsky)
827de03 Remove direct bitcoin calls from qt/coincontroldialog.cpp (Russell Yanofsky)
a0704a8 Remove most direct bitcoin calls from qt/walletmodel.cpp (Russell Yanofsky)
90d4640 Remove direct bitcoin calls from qt/optionsdialog.cpp (Russell Yanofsky)
582daf6 Remove direct bitcoin calls from qt/rpcconsole.cpp (Russell Yanofsky)
3034a46 Remove direct bitcoin calls from qt/bantablemodel.cpp (Russell Yanofsky)
e0b66a3 Remove direct bitcoin calls from qt/peertablemodel.cpp (Russell Yanofsky)
d7c2c95 Remove direct bitcoin calls from qt/intro.cpp (Russell Yanofsky)
fe6f27e Remove direct bitcoin calls from qt/clientmodel.cpp (Russell Yanofsky)
5fba3af Remove direct bitcoin calls from qt/splashscreen.cpp (Russell Yanofsky)
c2f672f Remove direct bitcoin calls from qt/utilitydialog.cpp (Russell Yanofsky)
3d619e9 Remove direct bitcoin calls from qt/bitcoingui.cpp (Russell Yanofsky)
c0f2756 Remove direct bitcoin calls from qt/optionsmodel.cpp (Russell Yanofsky)
71e0d90 Remove direct bitcoin calls from qt/bitcoin.cpp (Russell Yanofsky)
ea73b84 Add src/interface/README.md (Russell Yanofsky)

Pull request description:

  This is a refactoring PR that does not change behavior in any way. This change:

  1. Creates abstract [`Node`](https://github.com/ryanofsky/bitcoin/blob/pr/ipc-local/src/interface/node.h) and [`Wallet`](https://github.com/ryanofsky/bitcoin/blob/pr/ipc-local/src/interface/wallet.h) interfaces in [`src/interface/`](https://github.com/ryanofsky/bitcoin/tree/pr/ipc-local/src/interface)
  1. Updates Qt code to call the new interfaces. This largely consists of diffs of the form:

  ```diff
  -    InitLogging();
  -    InitParameterInteraction();
  +    node.initLogging();
  +    node.initParameterInteraction();
  ```

  This change allows followup PR #10102 (makes `bitcoin-qt` control `bitcoind` over an IPC socket) to work without any significant updates to Qt code. Additionally:

  * It provides a single place to describe the interface between GUI and daemon code.
  * It can make better GUI testing possible, because Node and Wallet objects have virtual methods that can be overloaded for mocking.
  * It can be used to help make the GUI more responsive (see https://github.com/bitcoin/bitcoin/issues/10504)

  Other notes:

  * I used python scripts [hide-globals.py](https://github.com/ryanofsky/home/blob/master/src/2017/hide-globals/hide-globals.py) and [replace-syms.py](https://github.com/ryanofsky/home/blob/master/src/2017/hide-globals/replace-syms.py) to identify all the places where Qt code was accessing libbitcoin global variables and calling functions accessing those global variables.
  * These changes were originally part of #10102. Thanks to @JeremyRubin for the suggestion of splitting them out.

  Commits:

  - [`ea73b84d2d` Add src/interface/README.md](https://github.com/bitcoin/bitcoin/pull/10244/commits/ea73b84d2ddde22487dee0f71d7a619051e067f2)
  - [`71e0d90876` Remove direct bitcoin calls from qt/bitcoin.cpp](https://github.com/bitcoin/bitcoin/pull/10244/commits/71e0d90876efd11e2a4aeb8f3f806c5a1fd54b42)
  - [`c0f2756be5` Remove direct bitcoin calls from qt/optionsmodel.cpp](https://github.com/bitcoin/bitcoin/pull/10244/commits/c0f2756be517feddacd7c6b89b9faa888b3fef8e)
  - [`3d619e9d36` Remove direct bitcoin calls from qt/bitcoingui.cpp](https://github.com/bitcoin/bitcoin/pull/10244/commits/3d619e9d3658e36cba4a19a5bed33e5538317b27)
  - [`c2f672fb19` Remove direct bitcoin calls from qt/utilitydialog.cpp](https://github.com/bitcoin/bitcoin/pull/10244/commits/c2f672fb1960399389dea9cdd8f76d7156c2c88b)
  - [`5fba3af21e` Remove direct bitcoin calls from qt/splashscreen.cpp](https://github.com/bitcoin/bitcoin/pull/10244/commits/5fba3af21e44ab7552c57782de430c1f4cfd6697)
  - [`fe6f27e6ea` Remove direct bitcoin calls from qt/clientmodel.cpp](https://github.com/bitcoin/bitcoin/pull/10244/commits/fe6f27e6ea68a139d3a98b30a53706008ef8b132)
  - [`d7c2c95948` Remove direct bitcoin calls from qt/intro.cpp](https://github.com/bitcoin/bitcoin/pull/10244/commits/d7c2c9594897c39df6739b92610dfb5a7a1cb3ec)
  - [`e0b66a3b7c` Remove direct bitcoin calls from qt/peertablemodel.cpp](https://github.com/bitcoin/bitcoin/pull/10244/commits/e0b66a3b7c5d3a079636d61fcf611bb6b36c7bc1)
  - [`3034a462a5` Remove direct bitcoin calls from qt/bantablemodel.cpp](https://github.com/bitcoin/bitcoin/pull/10244/commits/3034a462a5d30144cf0ec801d07f0c8c36d560f3)
  - [`582daf6d22` Remove direct bitcoin calls from qt/rpcconsole.cpp](https://github.com/bitcoin/bitcoin/pull/10244/commits/582daf6d22da5394d02a12003b9542d9f5865ae2)
  - [`90d4640b7e` Remove direct bitcoin calls from qt/optionsdialog.cpp](https://github.com/bitcoin/bitcoin/pull/10244/commits/90d4640b7eff3154a0750c5acb52d39bd41e0bbb)
  - [`a0704a8996` Remove most direct bitcoin calls from qt/walletmodel.cpp](https://github.com/bitcoin/bitcoin/pull/10244/commits/a0704a8996bb950ae3c4d5b5a30e9dfe34cde1d3)
  - [`827de038ab` Remove direct bitcoin calls from qt/coincontroldialog.cpp](https://github.com/bitcoin/bitcoin/pull/10244/commits/827de038ab6fa58aa3d46151eb2f8dc6add7743e)
  - [`3ec2ebcd9b` Remove direct bitcoin calls from qt/addresstablemodel.cpp](https://github.com/bitcoin/bitcoin/pull/10244/commits/3ec2ebcd9b4beb4277f1f4791c6acbc538784f70)
  - [`3cab2ce5f9` Remove direct bitcoin calls from qt/paymentserver.cpp](https://github.com/bitcoin/bitcoin/pull/10244/commits/3cab2ce5f9e159ad5a2e9ed682f28121b5248580)
  - [`58845587e1` Remove direct bitcoin calls from qt transaction table files](https://github.com/bitcoin/bitcoin/pull/10244/commits/58845587e11140e81f087a74c3db76a4d1fc3a1a)
  - [`e872c93ee8` Remove direct bitcoin access from qt/guiutil.cpp](https://github.com/bitcoin/bitcoin/pull/10244/commits/e872c93ee87477130fb877da1a536b4c693bbab9)
  - [`56f33ca349` Remove direct bitcoin calls from qt/sendcoinsdialog.cpp](https://github.com/bitcoin/bitcoin/pull/10244/commits/56f33ca349b3721a15fce3bf0b6d4fd7fd788970)
  - [`9a61eed1fc` Use WalletBalances struct in Qt](https://github.com/bitcoin/bitcoin/pull/10244/commits/9a61eed1fcc16ddcedc315045f470e1958b0760b)
  - [`9960137697` Add developer notes about blocking GUI code](https://github.com/bitcoin/bitcoin/pull/10244/commits/996013769711bd507cdcd6dde88cbd59fcd4fbad)

Tree-SHA512: 7b9eff2f37d4ea21972d7cc6a3dbe144248595d6c330524396d867f3cd2841d666cdc040fd3605af559dab51b075812402f61d628d16cf13719335c1d8bf8ed3

1 files changed, 38 insertions, 62 deletions

diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp
index 84800125fe..2148ff1728 100644
--- a/src/qt/transactiontablemodel.cpp
+++ b/src/qt/transactiontablemodel.cpp
@@ -14,11 +14,12 @@
 #include <qt/walletmodel.h>
 
 #include <core_io.h>
+#include <interface/handler.h>
+#include <interface/node.h>
 #include <validation.h>
 #include <sync.h>
 #include <uint256.h>
 #include <util.h>
-#include <wallet/wallet.h>
 
 #include <QColor>
 #include <QDateTime>
@@ -57,13 +58,11 @@ struct TxLessThan
 class TransactionTablePriv
 {
 public:
-    TransactionTablePriv(CWallet *_wallet, TransactionTableModel *_parent) :
-        wallet(_wallet),
+    TransactionTablePriv(TransactionTableModel *_parent) :
         parent(_parent)
     {
     }
 
-    CWallet *wallet;
     TransactionTableModel *parent;
 
     /* Local cache of wallet.
@@ -74,16 +73,15 @@ public:
 
     /* Query entire wallet anew from core.
      */
-    void refreshWallet()
+    void refreshWallet(interface::Wallet& wallet)
     {
         qDebug() << "TransactionTablePriv::refreshWallet";
         cachedWallet.clear();
         {
-            LOCK2(cs_main, wallet->cs_wallet);
-            for (const auto& entry : wallet->mapWallet)
-            {
-                if (TransactionRecord::showTransaction(entry.second))
-                    cachedWallet.append(TransactionRecord::decomposeTransaction(wallet, entry.second));
+            for (const auto& wtx : wallet.getWalletTxs()) {
+                if (TransactionRecord::showTransaction()) {
+                    cachedWallet.append(TransactionRecord::decomposeTransaction(wtx));
+                }
             }
         }
     }
@@ -93,7 +91,7 @@ public:
 
        Call with transaction that was added, removed or changed.
      */
-    void updateWallet(const uint256 &hash, int status, bool showTransaction)
+    void updateWallet(interface::Wallet& wallet, const uint256 &hash, int status, bool showTransaction)
     {
         qDebug() << "TransactionTablePriv::updateWallet: " + QString::fromStdString(hash.ToString()) + " " + QString::number(status);
 
@@ -128,17 +126,16 @@ public:
             }
             if(showTransaction)
             {
-                LOCK2(cs_main, wallet->cs_wallet);
                 // Find transaction in wallet
-                std::map<uint256, CWalletTx>::iterator mi = wallet->mapWallet.find(hash);
-                if(mi == wallet->mapWallet.end())
+                interface::WalletTx wtx = wallet.getWalletTx(hash);
+                if(!wtx.tx)
                 {
                     qWarning() << "TransactionTablePriv::updateWallet: Warning: Got CT_NEW, but transaction is not in wallet";
                     break;
                 }
                 // Added -- insert at the right position
                 QList<TransactionRecord> toInsert =
-                        TransactionRecord::decomposeTransaction(wallet, mi->second);
+                        TransactionRecord::decomposeTransaction(wtx);
                 if(!toInsert.isEmpty()) /* only if something to insert */
                 {
                     parent->beginInsertRows(QModelIndex(), lowerIndex, lowerIndex+toInsert.size()-1);
@@ -179,7 +176,7 @@ public:
         return cachedWallet.size();
     }
 
-    TransactionRecord *index(int idx)
+    TransactionRecord *index(interface::Wallet& wallet, int idx)
     {
         if(idx >= 0 && idx < cachedWallet.size())
         {
@@ -192,61 +189,42 @@ public:
             // If a status update is needed (blocks came in since last check),
             //  update the status of this transaction from the wallet. Otherwise,
             // simply re-use the cached status.
-            TRY_LOCK(cs_main, lockMain);
-            if(lockMain)
-            {
-                TRY_LOCK(wallet->cs_wallet, lockWallet);
-                if(lockWallet && rec->statusUpdateNeeded())
-                {
-                    std::map<uint256, CWalletTx>::iterator mi = wallet->mapWallet.find(rec->hash);
-
-                    if(mi != wallet->mapWallet.end())
-                    {
-                        rec->updateStatus(mi->second);
-                    }
-                }
+            interface::WalletTxStatus wtx;
+            int numBlocks;
+            int64_t adjustedTime;
+            if (wallet.tryGetTxStatus(rec->hash, wtx, numBlocks, adjustedTime) && rec->statusUpdateNeeded(numBlocks)) {
+                rec->updateStatus(wtx, numBlocks, adjustedTime);
             }
             return rec;
         }
         return 0;
     }
 
-    QString describe(TransactionRecord *rec, int unit)
+    QString describe(interface::Node& node, interface::Wallet& wallet, TransactionRecord *rec, int unit)
     {
-        {
-            LOCK2(cs_main, wallet->cs_wallet);
-            std::map<uint256, CWalletTx>::iterator mi = wallet->mapWallet.find(rec->hash);
-            if(mi != wallet->mapWallet.end())
-            {
-                return TransactionDesc::toHTML(wallet, mi->second, rec, unit);
-            }
-        }
-        return QString();
+        return TransactionDesc::toHTML(node, wallet, rec, unit);
     }
 
-    QString getTxHex(TransactionRecord *rec)
+    QString getTxHex(interface::Wallet& wallet, TransactionRecord *rec)
     {
-        LOCK2(cs_main, wallet->cs_wallet);
-        std::map<uint256, CWalletTx>::iterator mi = wallet->mapWallet.find(rec->hash);
-        if(mi != wallet->mapWallet.end())
-        {
-            std::string strHex = EncodeHexTx(*mi->second.tx);
+        auto tx = wallet.getTx(rec->hash);
+        if (tx) {
+            std::string strHex = EncodeHexTx(*tx);
             return QString::fromStdString(strHex);
         }
         return QString();
     }
 };
 
-TransactionTableModel::TransactionTableModel(const PlatformStyle *_platformStyle, CWallet* _wallet, WalletModel *parent):
+TransactionTableModel::TransactionTableModel(const PlatformStyle *_platformStyle, WalletModel *parent):
         QAbstractTableModel(parent),
-        wallet(_wallet),
         walletModel(parent),
-        priv(new TransactionTablePriv(_wallet, this)),
+        priv(new TransactionTablePriv(this)),
         fProcessingQueuedTransactions(false),
         platformStyle(_platformStyle)
 {
     columns << QString() << QString() << tr("Date") << tr("Type") << tr("Label") << BitcoinUnits::getAmountColumnTitle(walletModel->getOptionsModel()->getDisplayUnit());
-    priv->refreshWallet();
+    priv->refreshWallet(walletModel->wallet());
 
     connect(walletModel->getOptionsModel(), SIGNAL(displayUnitChanged(int)), this, SLOT(updateDisplayUnit()));
 
@@ -271,7 +249,7 @@ void TransactionTableModel::updateTransaction(const QString &hash, int status, b
     uint256 updated;
     updated.SetHex(hash.toStdString());
 
-    priv->updateWallet(updated, status, showTransaction);
+    priv->updateWallet(walletModel->wallet(), updated, status, showTransaction);
 }
 
 void TransactionTableModel::updateConfirmations()
@@ -608,7 +586,7 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const
     case WatchonlyDecorationRole:
         return txWatchonlyDecoration(rec);
     case LongDescriptionRole:
-        return priv->describe(rec, walletModel->getOptionsModel()->getDisplayUnit());
+        return priv->describe(walletModel->node(), walletModel->wallet(), rec, walletModel->getOptionsModel()->getDisplayUnit());
     case AddressRole:
         return QString::fromStdString(rec->address);
     case LabelRole:
@@ -618,7 +596,7 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const
     case TxHashRole:
         return rec->getTxHash();
     case TxHexRole:
-        return priv->getTxHex(rec);
+        return priv->getTxHex(walletModel->wallet(), rec);
     case TxPlainTextRole:
         {
             QString details;
@@ -694,10 +672,10 @@ 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(row);
+    TransactionRecord *data = priv->index(walletModel->wallet(), row);
     if(data)
     {
-        return createIndex(row, column, priv->index(row));
+        return createIndex(row, column, priv->index(walletModel->wallet(), row));
     }
     return QModelIndex();
 }
@@ -735,13 +713,11 @@ private:
 static bool fQueueNotifications = false;
 static std::vector< TransactionNotification > vQueueNotifications;
 
-static void NotifyTransactionChanged(TransactionTableModel *ttm, CWallet *wallet, const uint256 &hash, ChangeType status)
+static void NotifyTransactionChanged(TransactionTableModel *ttm, const uint256 &hash, ChangeType status)
 {
     // Find transaction in wallet
-    std::map<uint256, CWalletTx>::iterator mi = wallet->mapWallet.find(hash);
     // Determine whether to show transaction or not (determine this here so that no relocking is needed in GUI thread)
-    bool inWallet = mi != wallet->mapWallet.end();
-    bool showTransaction = (inWallet && TransactionRecord::showTransaction(mi->second));
+    bool showTransaction = TransactionRecord::showTransaction();
 
     TransactionNotification notification(hash, status, showTransaction);
 
@@ -777,13 +753,13 @@ static void ShowProgress(TransactionTableModel *ttm, const std::string &title, i
 void TransactionTableModel::subscribeToCoreSignals()
 {
     // Connect signals to wallet
-    wallet->NotifyTransactionChanged.connect(boost::bind(NotifyTransactionChanged, this, _1, _2, _3));
-    wallet->ShowProgress.connect(boost::bind(ShowProgress, this, _1, _2));
+    m_handler_transaction_changed = walletModel->wallet().handleTransactionChanged(boost::bind(NotifyTransactionChanged, this, _1, _2));
+    m_handler_show_progress = walletModel->wallet().handleShowProgress(boost::bind(ShowProgress, this, _1, _2));
 }
 
 void TransactionTableModel::unsubscribeFromCoreSignals()
 {
     // Disconnect signals from wallet
-    wallet->NotifyTransactionChanged.disconnect(boost::bind(NotifyTransactionChanged, this, _1, _2, _3));
-    wallet->ShowProgress.disconnect(boost::bind(ShowProgress, this, _1, _2));
+    m_handler_transaction_changed->disconnect();
+    m_handler_show_progress->disconnect();
 }