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, 40 insertions, 59 deletions

diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp
index f3e635967d..30d0acb7ef 100644
--- a/src/qt/bitcoin.cpp
+++ b/src/qt/bitcoin.cpp
@@ -27,6 +27,8 @@
 #endif
 
 #include <init.h>
+#include <interface/handler.h>
+#include <interface/node.h>
 #include <rpc/server.h>
 #include <ui_interface.h>
 #include <uint256.h>
@@ -35,7 +37,6 @@
 
 #ifdef ENABLE_WALLET
 #include <wallet/init.h>
-#include <wallet/wallet.h>
 #endif
 #include <walletinitinterface.h>
 
@@ -181,11 +182,7 @@ class BitcoinCore: public QObject
 {
     Q_OBJECT
 public:
-    explicit BitcoinCore();
-    /** Basic initialization, before starting initialization/shutdown thread.
-     * Return true on success.
-     */
-    static bool baseInitialize();
+    explicit BitcoinCore(interface::Node& node);
 
 public Q_SLOTS:
     void initialize();
@@ -197,9 +194,10 @@ Q_SIGNALS:
     void runawayException(const QString &message);
 
 private:
-
     /// Pass fatal exception message to UI thread
     void handleRunawayException(const std::exception *e);
+
+    interface::Node& m_node;
 };
 
 /** Main Bitcoin application object */
@@ -207,7 +205,7 @@ class BitcoinApplication: public QApplication
 {
     Q_OBJECT
 public:
-    explicit BitcoinApplication(int &argc, char **argv);
+    explicit BitcoinApplication(interface::Node& node, int &argc, char **argv);
     ~BitcoinApplication();
 
 #ifdef ENABLE_WALLET
@@ -248,6 +246,7 @@ Q_SIGNALS:
 
 private:
     QThread *coreThread;
+    interface::Node& m_node;
     OptionsModel *optionsModel;
     ClientModel *clientModel;
     BitcoinGUI *window;
@@ -265,36 +264,15 @@ private:
 
 #include <qt/bitcoin.moc>
 
-BitcoinCore::BitcoinCore():
-    QObject()
+BitcoinCore::BitcoinCore(interface::Node& node) :
+    QObject(), m_node(node)
 {
 }
 
 void BitcoinCore::handleRunawayException(const std::exception *e)
 {
     PrintExceptionContinue(e, "Runaway exception");
-    Q_EMIT runawayException(QString::fromStdString(GetWarnings("gui")));
-}
-
-bool BitcoinCore::baseInitialize()
-{
-    if (!AppInitBasicSetup())
-    {
-        return false;
-    }
-    if (!AppInitParameterInteraction())
-    {
-        return false;
-    }
-    if (!AppInitSanityChecks())
-    {
-        return false;
-    }
-    if (!AppInitLockDataDirectory())
-    {
-        return false;
-    }
-    return true;
+    Q_EMIT runawayException(QString::fromStdString(m_node.getWarnings("gui")));
 }
 
 void BitcoinCore::initialize()
@@ -302,7 +280,7 @@ void BitcoinCore::initialize()
     try
     {
         qDebug() << __func__ << ": Running initialization in thread";
-        bool rv = AppInitMain();
+        bool rv = m_node.appInitMain();
         Q_EMIT initializeResult(rv);
     } catch (const std::exception& e) {
         handleRunawayException(&e);
@@ -316,8 +294,7 @@ void BitcoinCore::shutdown()
     try
     {
         qDebug() << __func__ << ": Running Shutdown in thread";
-        Interrupt();
-        Shutdown();
+        m_node.appShutdown();
         qDebug() << __func__ << ": Shutdown finished";
         Q_EMIT shutdownResult();
     } catch (const std::exception& e) {
@@ -327,9 +304,10 @@ void BitcoinCore::shutdown()
     }
 }
 
-BitcoinApplication::BitcoinApplication(int &argc, char **argv):
+BitcoinApplication::BitcoinApplication(interface::Node& node, int &argc, char **argv):
     QApplication(argc, argv),
     coreThread(0),
+    m_node(node),
     optionsModel(0),
     clientModel(0),
     window(0),
@@ -384,12 +362,12 @@ void BitcoinApplication::createPaymentServer()
 
 void BitcoinApplication::createOptionsModel(bool resetSettings)
 {
-    optionsModel = new OptionsModel(nullptr, resetSettings);
+    optionsModel = new OptionsModel(m_node, nullptr, resetSettings);
 }
 
 void BitcoinApplication::createWindow(const NetworkStyle *networkStyle)
 {
-    window = new BitcoinGUI(platformStyle, networkStyle, 0);
+    window = new BitcoinGUI(m_node, platformStyle, networkStyle, 0);
 
     pollShutdownTimer = new QTimer(window);
     connect(pollShutdownTimer, SIGNAL(timeout()), window, SLOT(detectShutdown()));
@@ -397,7 +375,7 @@ void BitcoinApplication::createWindow(const NetworkStyle *networkStyle)
 
 void BitcoinApplication::createSplashScreen(const NetworkStyle *networkStyle)
 {
-    SplashScreen *splash = new SplashScreen(0, networkStyle);
+    SplashScreen *splash = new SplashScreen(m_node, 0, networkStyle);
     // We don't hold a direct pointer to the splash screen after creation, but the splash
     // screen will take care of deleting itself when slotFinish happens.
     splash->show();
@@ -410,7 +388,7 @@ void BitcoinApplication::startThread()
     if(coreThread)
         return;
     coreThread = new QThread(this);
-    BitcoinCore *executor = new BitcoinCore();
+    BitcoinCore *executor = new BitcoinCore(m_node);
     executor->moveToThread(coreThread);
 
     /*  communication to and from thread */
@@ -428,8 +406,8 @@ void BitcoinApplication::startThread()
 
 void BitcoinApplication::parameterSetup()
 {
-    InitLogging();
-    InitParameterInteraction();
+    m_node.initLogging();
+    m_node.initParameterInteraction();
 }
 
 void BitcoinApplication::requestInitialize()
@@ -462,7 +440,7 @@ void BitcoinApplication::requestShutdown()
     delete clientModel;
     clientModel = 0;
 
-    StartShutdown();
+    m_node.startShutdown();
 
     // Request shutdown from core thread
     Q_EMIT requestedShutdown();
@@ -482,13 +460,14 @@ void BitcoinApplication::initializeResult(bool success)
         paymentServer->setOptionsModel(optionsModel);
 #endif
 
-        clientModel = new ClientModel(optionsModel);
+        clientModel = new ClientModel(m_node, optionsModel);
         window->setClientModel(clientModel);
 
 #ifdef ENABLE_WALLET
         bool fFirstWallet = true;
-        for (CWalletRef pwallet : vpwallets) {
-            WalletModel * const walletModel = new WalletModel(platformStyle, pwallet, optionsModel);
+        auto wallets = m_node.getWallets();
+        for (auto& wallet : wallets) {
+            WalletModel * const walletModel = new WalletModel(std::move(wallet), m_node, platformStyle, optionsModel);
 
             window->addWallet(walletModel);
             if (fFirstWallet) {
@@ -496,8 +475,8 @@ void BitcoinApplication::initializeResult(bool success)
                 fFirstWallet = false;
             }
 
-            connect(walletModel, SIGNAL(coinsSent(CWallet*,SendCoinsRecipient,QByteArray)),
-                             paymentServer, SLOT(fetchPaymentACK(CWallet*,const SendCoinsRecipient&,QByteArray)));
+            connect(walletModel, SIGNAL(coinsSent(WalletModel*,SendCoinsRecipient,QByteArray)),
+                             paymentServer, SLOT(fetchPaymentACK(WalletModel*,const SendCoinsRecipient&,QByteArray)));
 
             m_wallet_models.push_back(walletModel);
         }
@@ -556,9 +535,11 @@ int main(int argc, char *argv[])
 {
     SetupEnvironment();
 
+    std::unique_ptr<interface::Node> node = interface::MakeNode();
+
     /// 1. Parse command-line options. These take precedence over anything else.
     // Command-line options take precedence:
-    gArgs.ParseParameters(argc, argv);
+    node->parseParameters(argc, argv);
 
     // Do not refer to data directory yet, this can be overridden by Intro::pickDataDirectory
 
@@ -572,7 +553,7 @@ int main(int argc, char *argv[])
     Q_INIT_RESOURCE(bitcoin);
     Q_INIT_RESOURCE(bitcoin_locale);
 
-    BitcoinApplication app(argc, argv);
+    BitcoinApplication app(*node, argc, argv);
 #if QT_VERSION > 0x050100
     // Generate high-dpi pixmaps
     QApplication::setAttribute(Qt::AA_UseHighDpiPixmaps);
@@ -615,14 +596,14 @@ int main(int argc, char *argv[])
     // Show help message immediately after parsing command-line options (for "-lang") and setting locale,
     // but before showing splash screen.
     if (HelpRequested(gArgs) || gArgs.IsArgSet("-version")) {
-        HelpMessageDialog help(nullptr, gArgs.IsArgSet("-version"));
+        HelpMessageDialog help(*node, nullptr, gArgs.IsArgSet("-version"));
         help.showOrPrint();
         return EXIT_SUCCESS;
     }
 
     /// 5. Now that settings and translations are available, ask user for data directory
     // User language is set up: pick a data directory
-    if (!Intro::pickDataDirectory())
+    if (!Intro::pickDataDirectory(*node))
         return EXIT_SUCCESS;
 
     /// 6. Determine availability of data and blocks directory and parse bitcoin.conf
@@ -634,7 +615,7 @@ int main(int argc, char *argv[])
         return EXIT_FAILURE;
     }
     try {
-        gArgs.ReadConfigFile(gArgs.GetArg("-conf", BITCOIN_CONF_FILENAME));
+        node->readConfigFile(gArgs.GetArg("-conf", BITCOIN_CONF_FILENAME));
     } catch (const std::exception& e) {
         QMessageBox::critical(0, QObject::tr(PACKAGE_NAME),
                               QObject::tr("Error: Cannot parse configuration file: %1. Only use key=value syntax.").arg(e.what()));
@@ -649,14 +630,14 @@ int main(int argc, char *argv[])
 
     // Check for -testnet or -regtest parameter (Params() calls are only valid after this clause)
     try {
-        SelectParams(ChainNameFromCommandLine());
+        node->selectParams(ChainNameFromCommandLine());
     } catch(std::exception &e) {
         QMessageBox::critical(0, QObject::tr(PACKAGE_NAME), QObject::tr("Error: %1").arg(e.what()));
         return EXIT_FAILURE;
     }
 #ifdef ENABLE_WALLET
     // Parse URIs on command line -- this can affect Params()
-    PaymentServer::ipcParseCommandLine(argc, argv);
+    PaymentServer::ipcParseCommandLine(*node, argc, argv);
 #endif
 
     QScopedPointer<const NetworkStyle> networkStyle(NetworkStyle::instantiate(QString::fromStdString(Params().NetworkIDString())));
@@ -706,7 +687,7 @@ int main(int argc, char *argv[])
     app.createOptionsModel(gArgs.GetBoolArg("-resetguisettings", false));
 
     // Subscribe to global signals from core
-    uiInterface.InitMessage.connect(InitMessage);
+    std::unique_ptr<interface::Handler> handler = node->handleInitMessage(InitMessage);
 
     if (gArgs.GetBoolArg("-splash", DEFAULT_SPLASHSCREEN) && !gArgs.GetBoolArg("-min", false))
         app.createSplashScreen(networkStyle.data());
@@ -718,7 +699,7 @@ int main(int argc, char *argv[])
         // Perform base initialization before spinning up initialization/shutdown thread
         // This is acceptable because this function only contains steps that are quick to execute,
         // so the GUI thread won't be held up.
-        if (BitcoinCore::baseInitialize()) {
+        if (node->baseInitialize()) {
             app.requestInitialize();
 #if defined(Q_OS_WIN) && QT_VERSION >= 0x050000
             WinShutdownMonitor::registerShutdownBlockReason(QObject::tr("%1 didn't yet exit safely...").arg(QObject::tr(PACKAGE_NAME)), (HWND)app.getMainWinId());
@@ -733,10 +714,10 @@ int main(int argc, char *argv[])
         }
     } catch (const std::exception& e) {
         PrintExceptionContinue(&e, "Runaway exception");
-        app.handleRunawayException(QString::fromStdString(GetWarnings("gui")));
+        app.handleRunawayException(QString::fromStdString(node->getWarnings("gui")));
     } catch (...) {
         PrintExceptionContinue(nullptr, "Runaway exception");
-        app.handleRunawayException(QString::fromStdString(GetWarnings("gui")));
+        app.handleRunawayException(QString::fromStdString(node->getWarnings("gui")));
     }
     return rv;
 }