Merge #15101: gui: Add WalletController

0dd9bdefa gui: Refactor to use WalletController (João Barbosa)
8fa271f08 gui: Add WalletController (João Barbosa)
cefb399e2 gui: Use AutoConnection for WalletModel::unload signal (João Barbosa)

Pull request description:

  This PR is a subset of the work done in the context of #13100. This change consists in extracting from the application class the code that manages the wallet models.

  The role of the `WalletController` instance is to coordinate wallet operations and the window.

Tree-SHA512: 6a824054376730eb7d16c643dd2003f5f60778e8ad3af707b82bc12c48438db179ca4446316b28fb17b206f4b9aba8998419aab8c5dd1f7c32467015732b5094

8 files changed, 202 insertions, 63 deletions

diff --git a/src/Makefile.qt.include b/src/Makefile.qt.include
index f4f84e2a99..ba6523d7c2 100644
--- a/src/Makefile.qt.include
+++ b/src/Makefile.qt.include
@@ -157,6 +157,7 @@ QT_MOC_CPP = \
   qt/moc_transactiontablemodel.cpp \
   qt/moc_transactionview.cpp \
   qt/moc_utilitydialog.cpp \
+  qt/moc_walletcontroller.cpp \
   qt/moc_walletframe.cpp \
   qt/moc_walletmodel.cpp \
   qt/moc_walletview.cpp
@@ -237,6 +238,7 @@ BITCOIN_QT_H = \
   qt/transactiontablemodel.h \
   qt/transactionview.h \
   qt/utilitydialog.h \
+  qt/walletcontroller.h \
   qt/walletframe.h \
   qt/walletmodel.h \
   qt/walletmodeltransaction.h \
@@ -350,6 +352,7 @@ BITCOIN_QT_WALLET_CPP = \
   qt/transactionrecord.cpp \
   qt/transactiontablemodel.cpp \
   qt/transactionview.cpp \
+  qt/walletcontroller.cpp \
   qt/walletframe.cpp \
   qt/walletmodel.cpp \
   qt/walletmodeltransaction.cpp \
diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp
index fb433c2957..ca26131b95 100644
--- a/src/qt/bitcoin.cpp
+++ b/src/qt/bitcoin.cpp
@@ -24,7 +24,7 @@
 
 #ifdef ENABLE_WALLET
 #include <qt/paymentserver.h>
-#include <qt/walletmodel.h>
+#include <qt/walletcontroller.h>
 #endif
 
 #include <interfaces/handler.h>
@@ -184,10 +184,6 @@ BitcoinApplication::BitcoinApplication(interfaces::Node& node, int &argc, char *
     clientModel(nullptr),
     window(nullptr),
     pollShutdownTimer(nullptr),
-#ifdef ENABLE_WALLET
-    paymentServer(nullptr),
-    m_wallet_models(),
-#endif
     returnValue(0),
     platformStyle(nullptr)
 {
@@ -315,11 +311,8 @@ void BitcoinApplication::requestShutdown()
     pollShutdownTimer->stop();
 
 #ifdef ENABLE_WALLET
-    window->removeAllWallets();
-    for (const WalletModel* walletModel : m_wallet_models) {
-        delete walletModel;
-    }
-    m_wallet_models.clear();
+    delete m_wallet_controller;
+    m_wallet_controller = nullptr;
 #endif
     delete clientModel;
     clientModel = nullptr;
@@ -330,35 +323,6 @@ void BitcoinApplication::requestShutdown()
     Q_EMIT requestedShutdown();
 }
 
-void BitcoinApplication::addWallet(WalletModel* walletModel)
-{
-#ifdef ENABLE_WALLET
-    window->addWallet(walletModel);
-
-    if (m_wallet_models.empty()) {
-        window->setCurrentWallet(walletModel);
-    }
-
-#ifdef ENABLE_BIP70
-    connect(walletModel, &WalletModel::coinsSent,
-        paymentServer, &PaymentServer::fetchPaymentACK);
-#endif
-    connect(walletModel, &WalletModel::unload, this, &BitcoinApplication::removeWallet);
-
-    m_wallet_models.push_back(walletModel);
-#endif
-}
-
-void BitcoinApplication::removeWallet()
-{
-#ifdef ENABLE_WALLET
-    WalletModel* walletModel = static_cast<WalletModel*>(sender());
-    m_wallet_models.erase(std::find(m_wallet_models.begin(), m_wallet_models.end(), walletModel));
-    window->removeWallet(walletModel);
-    walletModel->deleteLater();
-#endif
-}
-
 void BitcoinApplication::initializeResult(bool success)
 {
     qDebug() << __func__ << ": Initialization result: " << success;
@@ -369,26 +333,22 @@ void BitcoinApplication::initializeResult(bool success)
         // Log this only after AppInitMain finishes, as then logging setup is guaranteed complete
         qWarning() << "Platform customization:" << platformStyle->getName();
 #ifdef ENABLE_WALLET
+        m_wallet_controller = new WalletController(m_node, platformStyle, optionsModel, this);
 #ifdef ENABLE_BIP70
         PaymentServer::LoadRootCAs();
 #endif
-        if (paymentServer) paymentServer->setOptionsModel(optionsModel);
+        if (paymentServer) {
+            paymentServer->setOptionsModel(optionsModel);
+#ifdef ENABLE_BIP70
+            connect(m_wallet_controller, &WalletController::coinsSent, paymentServer, &PaymentServer::fetchPaymentACK);
+#endif
+        }
 #endif
 
         clientModel = new ClientModel(m_node, optionsModel);
         window->setClientModel(clientModel);
-
 #ifdef ENABLE_WALLET
-        m_handler_load_wallet = m_node.handleLoadWallet([this](std::unique_ptr<interfaces::Wallet> wallet) {
-            WalletModel* wallet_model = new WalletModel(std::move(wallet), m_node, platformStyle, optionsModel, nullptr);
-            // Fix wallet model thread affinity.
-            wallet_model->moveToThread(thread());
-            QMetaObject::invokeMethod(this, "addWallet", Qt::QueuedConnection, Q_ARG(WalletModel*, wallet_model));
-        });
-
-        for (auto& wallet : m_node.getWallets()) {
-            addWallet(new WalletModel(std::move(wallet), m_node, platformStyle, optionsModel));
-        }
+        window->setWalletController(m_wallet_controller);
 #endif
 
         // If -min option passed, start window minimized (iconified) or minimized to tray
@@ -492,9 +452,6 @@ int GuiMain(int argc, char* argv[])
     //   IMPORTANT if it is no longer a typedef use the normal variant above
     qRegisterMetaType< CAmount >("CAmount");
     qRegisterMetaType< std::function<void()> >("std::function<void()>");
-#ifdef ENABLE_WALLET
-    qRegisterMetaType<WalletModel*>("WalletModel*");
-#endif
 
     /// 2. Parse command-line options. We do this after qt in order to show an error if there are problems parsing these
     // Command-line options take precedence:
diff --git a/src/qt/bitcoin.h b/src/qt/bitcoin.h
index ffed45236c..370712d953 100644
--- a/src/qt/bitcoin.h
+++ b/src/qt/bitcoin.h
@@ -19,6 +19,7 @@ class NetworkStyle;
 class OptionsModel;
 class PaymentServer;
 class PlatformStyle;
+class WalletController;
 class WalletModel;
 
 namespace interfaces {
@@ -93,8 +94,6 @@ public Q_SLOTS:
     void shutdownResult();
     /// Handle runaway exceptions. Shows a message box with the problem and quits the program.
     void handleRunawayException(const QString &message);
-    void addWallet(WalletModel* walletModel);
-    void removeWallet();
 
 Q_SIGNALS:
     void requestedInitialize();
@@ -110,9 +109,8 @@ private:
     BitcoinGUI *window;
     QTimer *pollShutdownTimer;
 #ifdef ENABLE_WALLET
-    PaymentServer* paymentServer;
-    std::vector<WalletModel*> m_wallet_models;
-    std::unique_ptr<interfaces::Handler> m_handler_load_wallet;
+    PaymentServer* paymentServer{nullptr};
+    WalletController* m_wallet_controller{nullptr};
 #endif
     int returnValue;
     const PlatformStyle *platformStyle;
diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp
index 4417e48fb1..d424c1ef2d 100644
--- a/src/qt/bitcoingui.cpp
+++ b/src/qt/bitcoingui.cpp
@@ -19,6 +19,7 @@
 #include <qt/utilitydialog.h>
 
 #ifdef ENABLE_WALLET
+#include <qt/walletcontroller.h>
 #include <qt/walletframe.h>
 #include <qt/walletmodel.h>
 #include <qt/walletview.h>
@@ -565,18 +566,33 @@ void BitcoinGUI::setClientModel(ClientModel *_clientModel)
 }
 
 #ifdef ENABLE_WALLET
+void BitcoinGUI::setWalletController(WalletController* wallet_controller)
+{
+    assert(!m_wallet_controller);
+    assert(wallet_controller);
+
+    m_wallet_controller = wallet_controller;
+
+    connect(wallet_controller, &WalletController::walletAdded, this, &BitcoinGUI::addWallet);
+    connect(wallet_controller, &WalletController::walletRemoved, this, &BitcoinGUI::removeWallet);
+
+    for (WalletModel* wallet_model : m_wallet_controller->getWallets()) {
+        addWallet(wallet_model);
+    }
+}
+
 void BitcoinGUI::addWallet(WalletModel* walletModel)
 {
     if (!walletFrame) return;
     const QString display_name = walletModel->getDisplayName();
     setWalletActionsEnabled(true);
+    rpcConsole->addWallet(walletModel);
+    walletFrame->addWallet(walletModel);
     m_wallet_selector->addItem(display_name, QVariant::fromValue(walletModel));
     if (m_wallet_selector->count() == 2) {
         m_wallet_selector_label_action->setVisible(true);
         m_wallet_selector_action->setVisible(true);
     }
-    rpcConsole->addWallet(walletModel);
-    walletFrame->addWallet(walletModel);
 }
 
 void BitcoinGUI::removeWallet(WalletModel* walletModel)
@@ -599,13 +615,19 @@ void BitcoinGUI::setCurrentWallet(WalletModel* wallet_model)
 {
     if (!walletFrame) return;
     walletFrame->setCurrentWallet(wallet_model);
+    for (int index = 0; index < m_wallet_selector->count(); ++index) {
+        if (m_wallet_selector->itemData(index).value<WalletModel*>() == wallet_model) {
+            m_wallet_selector->setCurrentIndex(index);
+            break;
+        }
+    }
     updateWindowTitle();
 }
 
 void BitcoinGUI::setCurrentWalletBySelectorIndex(int index)
 {
     WalletModel* wallet_model = m_wallet_selector->itemData(index).value<WalletModel*>();
-    setCurrentWallet(wallet_model);
+    if (wallet_model) setCurrentWallet(wallet_model);
 }
 
 void BitcoinGUI::removeAllWallets()
diff --git a/src/qt/bitcoingui.h b/src/qt/bitcoingui.h
index 4eb5e43f5e..f1b76a6b64 100644
--- a/src/qt/bitcoingui.h
+++ b/src/qt/bitcoingui.h
@@ -33,6 +33,7 @@ class PlatformStyle;
 class RPCConsole;
 class SendCoinsRecipient;
 class UnitDisplayStatusBarControl;
+class WalletController;
 class WalletFrame;
 class WalletModel;
 class HelpMessageDialog;
@@ -74,6 +75,9 @@ public:
         The client model represents the part of the core that communicates with the P2P network, and is wallet-agnostic.
     */
     void setClientModel(ClientModel *clientModel);
+#ifdef ENABLE_WALLET
+    void setWalletController(WalletController* wallet_controller);
+#endif
 
 #ifdef ENABLE_WALLET
     /** Set the wallet model.
@@ -101,6 +105,7 @@ protected:
 
 private:
     interfaces::Node& m_node;
+    WalletController* m_wallet_controller{nullptr};
     std::unique_ptr<interfaces::Handler> m_handler_message_box;
     std::unique_ptr<interfaces::Handler> m_handler_question;
     ClientModel* clientModel = nullptr;
diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp
new file mode 100644
index 0000000000..df2b7a3f9b
--- /dev/null
+++ b/src/qt/walletcontroller.cpp
@@ -0,0 +1,95 @@
+// Copyright (c) 2019 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <qt/walletcontroller.h>
+
+#include <interfaces/handler.h>
+#include <interfaces/node.h>
+
+#include <algorithm>
+
+#include <QMutexLocker>
+#include <QThread>
+
+WalletController::WalletController(interfaces::Node& node, const PlatformStyle* platform_style, OptionsModel* options_model, QObject* parent)
+    : QObject(parent)
+    , m_node(node)
+    , m_platform_style(platform_style)
+    , m_options_model(options_model)
+{
+    m_handler_load_wallet = m_node.handleLoadWallet([this](std::unique_ptr<interfaces::Wallet> wallet) {
+        getOrCreateWallet(std::move(wallet));
+    });
+
+    for (std::unique_ptr<interfaces::Wallet>& wallet : m_node.getWallets()) {
+        getOrCreateWallet(std::move(wallet));
+    }
+}
+
+// Not using the default destructor because not all member types definitions are
+// available in the header, just forward declared.
+WalletController::~WalletController() {}
+
+std::vector<WalletModel*> WalletController::getWallets() const
+{
+    QMutexLocker locker(&m_mutex);
+    return m_wallets;
+}
+
+WalletModel* WalletController::getOrCreateWallet(std::unique_ptr<interfaces::Wallet> wallet)
+{
+    QMutexLocker locker(&m_mutex);
+
+    // Return model instance if exists.
+    if (!m_wallets.empty()) {
+        std::string name = wallet->getWalletName();
+        for (WalletModel* wallet_model : m_wallets) {
+            if (wallet_model->wallet().getWalletName() == name) {
+                return wallet_model;
+            }
+        }
+    }
+
+    // Instantiate model and register it.
+    WalletModel* wallet_model = new WalletModel(std::move(wallet), m_node, m_platform_style, m_options_model, nullptr);
+    m_wallets.push_back(wallet_model);
+
+    connect(wallet_model, &WalletModel::unload, [this, wallet_model] {
+        removeAndDeleteWallet(wallet_model);
+    });
+
+    // Re-emit coinsSent signal from wallet model.
+    connect(wallet_model, &WalletModel::coinsSent, this, &WalletController::coinsSent);
+
+    // Notify walletAdded signal on the GUI thread.
+    if (QThread::currentThread() == thread()) {
+        addWallet(wallet_model);
+    } else {
+        // Handler callback runs in a different thread so fix wallet model thread affinity.
+        wallet_model->moveToThread(thread());
+        QMetaObject::invokeMethod(this, "addWallet", Qt::QueuedConnection, Q_ARG(WalletModel*, wallet_model));
+    }
+
+    return wallet_model;
+}
+
+void WalletController::addWallet(WalletModel* wallet_model)
+{
+    // Take ownership of the wallet model and register it.
+    wallet_model->setParent(this);
+    Q_EMIT walletAdded(wallet_model);
+}
+
+void WalletController::removeAndDeleteWallet(WalletModel* wallet_model)
+{
+    // Unregister wallet model.
+    {
+        QMutexLocker locker(&m_mutex);
+        m_wallets.erase(std::remove(m_wallets.begin(), m_wallets.end(), wallet_model));
+    }
+    Q_EMIT walletRemoved(wallet_model);
+    // Currently this can trigger the unload since the model can hold the last
+    // CWallet shared pointer.
+    delete wallet_model;
+}
diff --git a/src/qt/walletcontroller.h b/src/qt/walletcontroller.h
new file mode 100644
index 0000000000..22b71b07ff
--- /dev/null
+++ b/src/qt/walletcontroller.h
@@ -0,0 +1,59 @@
+// Copyright (c) 2019 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_QT_WALLETCONTROLLER_H
+#define BITCOIN_QT_WALLETCONTROLLER_H
+
+#include <qt/walletmodel.h>
+#include <sync.h>
+
+#include <list>
+#include <memory>
+#include <vector>
+
+#include <QMutex>
+
+class OptionsModel;
+class PlatformStyle;
+
+namespace interfaces {
+class Handler;
+class Node;
+} // namespace interfaces
+
+/**
+ * Controller between interfaces::Node, WalletModel instances and the GUI.
+ */
+class WalletController : public QObject
+{
+    Q_OBJECT
+
+    WalletModel* getOrCreateWallet(std::unique_ptr<interfaces::Wallet> wallet);
+    void removeAndDeleteWallet(WalletModel* wallet_model);
+
+public:
+    WalletController(interfaces::Node& node, const PlatformStyle* platform_style, OptionsModel* options_model, QObject* parent);
+    ~WalletController();
+
+    std::vector<WalletModel*> getWallets() const;
+
+private Q_SLOTS:
+    void addWallet(WalletModel* wallet_model);
+
+Q_SIGNALS:
+    void walletAdded(WalletModel* wallet_model);
+    void walletRemoved(WalletModel* wallet_model);
+
+    void coinsSent(WalletModel* wallet_model, SendCoinsRecipient recipient, QByteArray transaction);
+
+private:
+    interfaces::Node& m_node;
+    const PlatformStyle* const m_platform_style;
+    OptionsModel* const m_options_model;
+    mutable QMutex m_mutex;
+    std::vector<WalletModel*> m_wallets;
+    std::unique_ptr<interfaces::Handler> m_handler_load_wallet;
+};
+
+#endif // BITCOIN_QT_WALLETCONTROLLER_H
diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
index e1fb4819f1..f139152042 100644
--- a/src/qt/walletmodel.cpp
+++ b/src/qt/walletmodel.cpp
@@ -376,7 +376,7 @@ bool WalletModel::changePassphrase(const SecureString &oldPass, const SecureStri
 static void NotifyUnload(WalletModel* walletModel)
 {
     qDebug() << "NotifyUnload";
-    QMetaObject::invokeMethod(walletModel, "unload", Qt::QueuedConnection);
+    QMetaObject::invokeMethod(walletModel, "unload");
 }
 
 static void NotifyKeyStoreStatusChanged(WalletModel *walletmodel)