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, 30 insertions, 24 deletions

diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp
index c41e19f6f5..745055b944 100644
--- a/src/qt/rpcconsole.cpp
+++ b/src/qt/rpcconsole.cpp
@@ -14,6 +14,7 @@
 #include <qt/platformstyle.h>
 #include <qt/walletmodel.h>
 #include <chainparams.h>
+#include <interface/node.h>
 #include <netbase.h>
 #include <rpc/server.h>
 #include <rpc/client.h>
@@ -83,12 +84,17 @@ const QStringList historyFilter = QStringList()
 class RPCExecutor : public QObject
 {
     Q_OBJECT
+public:
+    RPCExecutor(interface::Node& node) : m_node(node) {}
 
 public Q_SLOTS:
     void request(const QString &command, const QString &walletID);
 
 Q_SIGNALS:
     void reply(int category, const QString &command);
+
+private:
+    interface::Node& m_node;
 };
 
 /** Class for handling RPC timers
@@ -140,13 +146,14 @@ public:
  *   - Within double quotes, only escape \c " and backslashes before a \c " or another backslash
  *   - Within single quotes, no escaping is possible and no special interpretation takes place
  *
+ * @param[in]    node    optional node to execute command on
  * @param[out]   result      stringified Result from the executed command(chain)
  * @param[in]    strCommand  Command line to split
  * @param[in]    fExecute    set true if you want the command to be executed
  * @param[out]   pstrFilteredOut  Command line, filtered to remove any sensitive data
  */
 
-bool RPCConsole::RPCParseCommandLine(std::string &strResult, const std::string &strCommand, const bool fExecute, std::string * const pstrFilteredOut, const std::string *walletID)
+bool RPCConsole::RPCParseCommandLine(interface::Node* node, std::string &strResult, const std::string &strCommand, const bool fExecute, std::string * const pstrFilteredOut, const std::string *walletID)
 {
     std::vector< std::vector<std::string> > stack;
     stack.push_back(std::vector<std::string>());
@@ -300,16 +307,17 @@ bool RPCConsole::RPCParseCommandLine(std::string &strResult, const std::string &
                         if (fExecute) {
                             // Convert argument list to JSON objects in method-dependent way,
                             // and pass it along with the method name to the dispatcher.
-                            JSONRPCRequest req;
-                            req.params = RPCConvertValues(stack.back()[0], std::vector<std::string>(stack.back().begin() + 1, stack.back().end()));
-                            req.strMethod = stack.back()[0];
+                            UniValue params = RPCConvertValues(stack.back()[0], std::vector<std::string>(stack.back().begin() + 1, stack.back().end()));
+                            std::string method = stack.back()[0];
+                            std::string uri;
 #ifdef ENABLE_WALLET
                             if (walletID && !walletID->empty()) {
                                 QByteArray encodedName = QUrl::toPercentEncoding(QString::fromStdString(*walletID));
-                                req.URI = "/wallet/"+std::string(encodedName.constData(), encodedName.length());
+                                uri = "/wallet/"+std::string(encodedName.constData(), encodedName.length());
                             }
 #endif
-                            lastResult = tableRPC.execute(req);
+                            assert(node);
+                            lastResult = node->executeRpc(method, params, uri);
                         }
 
                         state = STATE_COMMAND_EXECUTED;
@@ -416,7 +424,7 @@ void RPCExecutor::request(const QString &command, const QString &walletID)
             return;
         }
         std::string wallet_id = walletID.toStdString();
-        if(!RPCConsole::RPCExecuteCommandLine(result, executableCommand, nullptr, &wallet_id))
+        if(!RPCConsole::RPCExecuteCommandLine(m_node, result, executableCommand, nullptr, &wallet_id))
         {
             Q_EMIT reply(RPCConsole::CMD_ERROR, QString("Parse error: unbalanced ' or \""));
             return;
@@ -443,8 +451,9 @@ void RPCExecutor::request(const QString &command, const QString &walletID)
     }
 }
 
-RPCConsole::RPCConsole(const PlatformStyle *_platformStyle, QWidget *parent) :
+RPCConsole::RPCConsole(interface::Node& node, const PlatformStyle *_platformStyle, QWidget *parent) :
     QWidget(parent),
+    m_node(node),
     ui(new Ui::RPCConsole),
     clientModel(0),
     historyPtr(0),
@@ -493,7 +502,7 @@ RPCConsole::RPCConsole(const PlatformStyle *_platformStyle, QWidget *parent) :
     rpcTimerInterface = new QtRPCTimerInterface();
     // avoid accidentally overwriting an existing, non QTThread
     // based timer interface
-    RPCSetTimerInterfaceIfUnset(rpcTimerInterface);
+    m_node.rpcSetTimerInterfaceIfUnset(rpcTimerInterface);
 
     setTrafficGraphRange(INITIAL_TRAFFIC_GRAPH_MINS);
 
@@ -508,7 +517,7 @@ RPCConsole::~RPCConsole()
 {
     QSettings settings;
     settings.setValue("RPCConsoleWindowGeometry", saveGeometry());
-    RPCUnsetTimerInterface(rpcTimerInterface);
+    m_node.rpcUnsetTimerInterface(rpcTimerInterface);
     delete rpcTimerInterface;
     delete ui;
 }
@@ -566,13 +575,14 @@ void RPCConsole::setClientModel(ClientModel *model)
         setNumConnections(model->getNumConnections());
         connect(model, SIGNAL(numConnectionsChanged(int)), this, SLOT(setNumConnections(int)));
 
-        setNumBlocks(model->getNumBlocks(), model->getLastBlockDate(), model->getVerificationProgress(nullptr), false);
+        interface::Node& node = clientModel->node();
+        setNumBlocks(node.getNumBlocks(), QDateTime::fromTime_t(node.getLastBlockTime()), node.getVerificationProgress(), false);
         connect(model, SIGNAL(numBlocksChanged(int,QDateTime,double,bool)), this, SLOT(setNumBlocks(int,QDateTime,double,bool)));
 
         updateNetworkState();
         connect(model, SIGNAL(networkActiveChanged(bool)), this, SLOT(setNetworkActive(bool)));
 
-        updateTrafficStats(model->getTotalBytesRecv(), model->getTotalBytesSent());
+        updateTrafficStats(node.getTotalBytesRecv(), node.getTotalBytesSent());
         connect(model, SIGNAL(bytesChanged(quint64,quint64)), this, SLOT(updateTrafficStats(quint64, quint64)));
 
         connect(model, SIGNAL(mempoolSizeChanged(long,size_t)), this, SLOT(setMempoolSize(long,size_t)));
@@ -667,7 +677,7 @@ void RPCConsole::setClientModel(ClientModel *model)
 
         //Setup autocomplete and attach it
         QStringList wordList;
-        std::vector<std::string> commandList = tableRPC.listCommands();
+        std::vector<std::string> commandList = m_node.listRpcCommands();
         for (size_t i = 0; i < commandList.size(); ++i)
         {
             wordList << commandList[i].c_str();
@@ -835,7 +845,7 @@ void RPCConsole::updateNetworkState()
     connections += tr("In:") + " " + QString::number(clientModel->getNumConnections(CONNECTIONS_IN)) + " / ";
     connections += tr("Out:") + " " + QString::number(clientModel->getNumConnections(CONNECTIONS_OUT)) + ")";
 
-    if(!clientModel->getNetworkActive()) {
+    if(!clientModel->node().getNetworkActive()) {
         connections += " (" + tr("Network activity disabled") + ")";
     }
 
@@ -882,7 +892,7 @@ void RPCConsole::on_lineEdit_returnPressed()
         std::string strFilteredCmd;
         try {
             std::string dummy;
-            if (!RPCParseCommandLine(dummy, cmd.toStdString(), false, &strFilteredCmd)) {
+            if (!RPCParseCommandLine(nullptr, dummy, cmd.toStdString(), false, &strFilteredCmd)) {
                 // Failed to parse command, so we cannot even filter it for the history
                 throw std::runtime_error("Invalid command line");
             }
@@ -955,7 +965,7 @@ void RPCConsole::browseHistory(int offset)
 
 void RPCConsole::startExecutor()
 {
-    RPCExecutor *executor = new RPCExecutor();
+    RPCExecutor *executor = new RPCExecutor(m_node);
     executor->moveToThread(&thread);
 
     // Replies from executor object must go to this object
@@ -1181,9 +1191,6 @@ void RPCConsole::showBanTableContextMenu(const QPoint& point)
 
 void RPCConsole::disconnectSelectedNode()
 {
-    if(!g_connman)
-        return;
-
     // Get selected peer addresses
     QList<QModelIndex> nodes = GUIUtil::getEntryData(ui->peerWidget, PeerTableModel::NetNodeId);
     for(int i = 0; i < nodes.count(); i++)
@@ -1191,14 +1198,14 @@ void RPCConsole::disconnectSelectedNode()
         // Get currently selected peer address
         NodeId id = nodes.at(i).data().toLongLong();
         // Find the node, disconnect it and clear the selected node
-        if(g_connman->DisconnectNode(id))
+        if(m_node.disconnect(id))
             clearSelectedNode();
     }
 }
 
 void RPCConsole::banSelectedNode(int bantime)
 {
-    if (!clientModel || !g_connman)
+    if (!clientModel)
         return;
 
     // Get selected peer addresses
@@ -1216,7 +1223,7 @@ void RPCConsole::banSelectedNode(int bantime)
 	// Find possible nodes, ban it and clear the selected node
 	const CNodeCombinedStats *stats = clientModel->getPeerTableModel()->getNodeStats(detailNodeRow);
 	if(stats) {
-	    g_connman->Ban(stats->nodeStats.addr, BanReasonManuallyAdded, bantime);
+            m_node.ban(stats->nodeStats.addr, BanReasonManuallyAdded, bantime);
 	}
     }
     clearSelectedNode();
@@ -1237,9 +1244,8 @@ void RPCConsole::unbanSelectedNode()
         CSubNet possibleSubnet;
 
         LookupSubNet(strNode.toStdString().c_str(), possibleSubnet);
-        if (possibleSubnet.IsValid() && g_connman)
+        if (possibleSubnet.IsValid() && m_node.unban(possibleSubnet))
         {
-            g_connman->Unban(possibleSubnet);
             clientModel->getBanTableModel()->refresh();
         }
     }