From b266b3e0bf29d0f3d5deaeec62d57c5025b35525 Mon Sep 17 00:00:00 2001
From: Russell Yanofsky <russ@yanofsky.org>
Date: Fri, 29 May 2020 00:07:18 -0400
Subject: [PATCH 1/3] refactor: Create interfaces earlier during initialization

Add AppInitInterfaces function so wallet chain and chain client interfaces are
created earlier during initialization. This is needed in the next commit to
allow the gui splash screen to be able to register for wallet events through a
dedicated WalletClient interface instead managing wallets indirectly through
the Node interface. This only works if the wallet client interface is created
before the splash screen needs to use it.
---
 src/bitcoind.cpp        |  3 +--
 src/init.cpp            | 17 +++++++++++------
 src/init.h              |  4 ++++
 src/interfaces/node.cpp |  3 +--
 4 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp
index 227626f40fa..02074f820aa 100644
--- a/src/bitcoind.cpp
+++ b/src/bitcoind.cpp
@@ -44,7 +44,6 @@ static void WaitForShutdown(NodeContext& node)
 static bool AppInit(int argc, char* argv[])
 {
     NodeContext node;
-    node.chain = interfaces::MakeChain(node);
 
     bool fRet = false;
 
@@ -144,7 +143,7 @@ static bool AppInit(int argc, char* argv[])
             // If locking the data directory failed, exit immediately
             return false;
         }
-        fRet = AppInitMain(context, node);
+        fRet = AppInitInterfaces(node) && AppInitMain(context, node);
     }
     catch (const std::exception& e) {
         PrintExceptionContinue(&e, "AppInit()");
diff --git a/src/init.cpp b/src/init.cpp
index ecd57960adc..4fc2c6211ac 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1229,6 +1229,17 @@ bool AppInitLockDataDirectory()
     return true;
 }
 
+bool AppInitInterfaces(NodeContext& node)
+{
+    node.chain = interfaces::MakeChain(node);
+    // Create client interfaces for wallets that are supposed to be loaded
+    // according to -wallet and -disablewallet options. This only constructs
+    // the interfaces, it doesn't load wallet data. Wallets actually get loaded
+    // when load() and start() interface methods are called below.
+    g_wallet_init_interface.Construct(node);
+    return true;
+}
+
 bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
 {
     const ArgsManager& args = *Assert(node.args);
@@ -1318,12 +1329,6 @@ bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockA
 
     GetMainSignals().RegisterBackgroundSignalScheduler(*node.scheduler);
 
-    // Create client interfaces for wallets that are supposed to be loaded
-    // according to -wallet and -disablewallet options. This only constructs
-    // the interfaces, it doesn't load wallet data. Wallets actually get loaded
-    // when load() and start() interface methods are called below.
-    g_wallet_init_interface.Construct(node);
-
     /* Register RPC commands regardless of -server setting so they will be
      * available in the GUI RPC console even if external calls are disabled.
      */
diff --git a/src/init.h b/src/init.h
index ce12a80dc71..679e875da1e 100644
--- a/src/init.h
+++ b/src/init.h
@@ -52,6 +52,10 @@ bool AppInitSanityChecks();
  * @pre Parameters should be parsed and config file should be read, AppInitSanityChecks should have been called.
  */
 bool AppInitLockDataDirectory();
+/**
+ * Initialize node and wallet interface pointers. Has no prerequisites or side effects besides allocating memory.
+ */
+bool AppInitInterfaces(NodeContext& node);
 /**
  * Bitcoin core main initialization.
  * @note This should only be done after daemonization. Call Shutdown() if this function fails.
diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp
index 206262eb03e..73171686eb5 100644
--- a/src/interfaces/node.cpp
+++ b/src/interfaces/node.cpp
@@ -64,11 +64,10 @@ public:
     bool baseInitialize() override
     {
         return AppInitBasicSetup(gArgs) && AppInitParameterInteraction(gArgs) && AppInitSanityChecks() &&
-               AppInitLockDataDirectory();
+               AppInitLockDataDirectory() && AppInitInterfaces(*m_context);
     }
     bool appInitMain(interfaces::BlockAndHeaderTipInfo* tip_info) override
     {
-        m_context->chain = MakeChain(*m_context);
         return AppInitMain(m_context_ref, *m_context, tip_info);
     }
     void appShutdown() override

From e4f435047121886edb6e6a6c4e4998e44ed2e36a Mon Sep 17 00:00:00 2001
From: Russell Yanofsky <russ@yanofsky.org>
Date: Thu, 28 May 2020 09:48:30 -0400
Subject: [PATCH 2/3] refactor: Move wallet methods out of chain.h and node.h

Add WalletClient interface so node interface is cleaner and don't need
wallet-specific methods.

The new NodeContext::wallet_client pointer will also be needed to eliminate
global wallet variables like ::vpwallets, because createWallet(), loadWallet(),
getWallets(), etc methods called by the GUI need a way to get a reference to
the list of open wallets if it is no longer a global variable.

Also tweaks splash screen registration for load wallet events to be delayed
until after wallet client is created.
---
 src/dummywallet.cpp                     | 34 -------------------
 src/interfaces/chain.h                  | 13 -------
 src/interfaces/node.cpp                 | 45 ++-----------------------
 src/interfaces/node.h                   | 25 ++------------
 src/interfaces/wallet.cpp               | 36 ++++++++++++++++++--
 src/interfaces/wallet.h                 | 35 +++++++++++++++++++
 src/node/context.h                      |  5 +++
 src/qt/bitcoin.cpp                      |  1 +
 src/qt/bitcoingui.cpp                   |  2 +-
 src/qt/splashscreen.cpp                 |  6 +++-
 src/qt/splashscreen.h                   |  3 ++
 src/qt/walletcontroller.cpp             | 10 +++---
 src/qt/walletmodel.cpp                  |  2 +-
 src/wallet/init.cpp                     |  5 ++-
 src/wallet/test/init_test_fixture.cpp   |  2 +-
 src/wallet/test/init_test_fixture.h     |  3 +-
 src/wallet/test/init_tests.cpp          | 14 ++++----
 src/wallet/test/wallet_test_fixture.cpp |  2 +-
 src/wallet/test/wallet_test_fixture.h   |  2 +-
 src/wallet/wallet.cpp                   |  5 ---
 20 files changed, 111 insertions(+), 139 deletions(-)

diff --git a/src/dummywallet.cpp b/src/dummywallet.cpp
index 380d4eb8ac6..e54c2daaeb4 100644
--- a/src/dummywallet.cpp
+++ b/src/dummywallet.cpp
@@ -4,11 +4,8 @@
 
 #include <util/system.h>
 #include <walletinitinterface.h>
-#include <support/allocators/secure.h>
 
 class CWallet;
-enum class WalletCreationStatus;
-struct bilingual_str;
 
 namespace interfaces {
 class Chain;
@@ -59,37 +56,6 @@ void DummyWalletInit::AddWalletOptions(ArgsManager& argsman) const
 
 const WalletInitInterface& g_wallet_init_interface = DummyWalletInit();
 
-fs::path GetWalletDir()
-{
-    throw std::logic_error("Wallet function called in non-wallet build.");
-}
-
-std::vector<fs::path> ListWalletDir()
-{
-    throw std::logic_error("Wallet function called in non-wallet build.");
-}
-
-std::vector<std::shared_ptr<CWallet>> GetWallets()
-{
-    throw std::logic_error("Wallet function called in non-wallet build.");
-}
-
-std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings)
-{
-    throw std::logic_error("Wallet function called in non-wallet build.");
-}
-
-WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, std::shared_ptr<CWallet>& result)
-{
-    throw std::logic_error("Wallet function called in non-wallet build.");
-}
-
-using LoadWalletFn = std::function<void(std::unique_ptr<interfaces::Wallet> wallet)>;
-std::unique_ptr<interfaces::Handler> HandleLoadWallet(LoadWalletFn load_wallet)
-{
-    throw std::logic_error("Wallet function called in non-wallet build.");
-}
-
 namespace interfaces {
 
 std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet)
diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h
index 053d40335f2..6e50ccb27a3 100644
--- a/src/interfaces/chain.h
+++ b/src/interfaces/chain.h
@@ -314,24 +314,11 @@ public:
 
     //! Set mock time.
     virtual void setMockTime(int64_t time) = 0;
-
-    //! Return interfaces for accessing wallets (if any).
-    virtual std::vector<std::unique_ptr<Wallet>> getWallets() = 0;
 };
 
 //! Return implementation of Chain interface.
 std::unique_ptr<Chain> MakeChain(NodeContext& node);
 
-//! Return implementation of ChainClient interface for a wallet client. This
-//! function will be undefined in builds where ENABLE_WALLET is false.
-//!
-//! Currently, wallets are the only chain clients. But in the future, other
-//! types of chain clients could be added, such as tools for monitoring,
-//! analysis, or fee estimation. These clients need to expose their own
-//! MakeXXXClient functions returning their implementations of the ChainClient
-//! interface.
-std::unique_ptr<ChainClient> MakeWalletClient(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames);
-
 } // namespace interfaces
 
 #endif // BITCOIN_INTERFACES_CHAIN_H
diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp
index 73171686eb5..2c5f8627e61 100644
--- a/src/interfaces/node.cpp
+++ b/src/interfaces/node.cpp
@@ -27,6 +27,7 @@
 #include <support/allocators/secure.h>
 #include <sync.h>
 #include <txmempool.h>
+#include <util/check.h>
 #include <util/ref.h>
 #include <util/system.h>
 #include <util/translation.h>
@@ -41,16 +42,7 @@
 
 #include <boost/signals2/signal.hpp>
 
-class CWallet;
-fs::path GetWalletDir();
-std::vector<fs::path> ListWalletDir();
-std::vector<std::shared_ptr<CWallet>> GetWallets();
-std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings);
-WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, std::shared_ptr<CWallet>& result);
-std::unique_ptr<interfaces::Handler> HandleLoadWallet(interfaces::Node::LoadWalletFn load_wallet);
-
 namespace interfaces {
-
 namespace {
 
 class NodeImpl : public Node
@@ -239,36 +231,9 @@ public:
         LOCK(::cs_main);
         return ::ChainstateActive().CoinsTip().GetCoin(output, coin);
     }
-    std::string getWalletDir() override
+    WalletClient& walletClient() override
     {
-        return GetWalletDir().string();
-    }
-    std::vector<std::string> listWalletDir() override
-    {
-        std::vector<std::string> paths;
-        for (auto& path : ListWalletDir()) {
-            paths.push_back(path.string());
-        }
-        return paths;
-    }
-    std::vector<std::unique_ptr<Wallet>> getWallets() override
-    {
-        std::vector<std::unique_ptr<Wallet>> wallets;
-        for (auto& client : m_context->chain_clients) {
-            auto client_wallets = client->getWallets();
-            std::move(client_wallets.begin(), client_wallets.end(), std::back_inserter(wallets));
-        }
-        return wallets;
-    }
-    std::unique_ptr<Wallet> loadWallet(const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) override
-    {
-        return MakeWallet(LoadWallet(*m_context->chain, name, error, warnings));
-    }
-    std::unique_ptr<Wallet> createWallet(const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, WalletCreationStatus& status) override
-    {
-        std::shared_ptr<CWallet> wallet;
-        status = CreateWallet(*m_context->chain, passphrase, wallet_creation_flags, name, error, warnings, wallet);
-        return MakeWallet(wallet);
+        return *Assert(m_context->wallet_client);
     }
     std::unique_ptr<Handler> handleInitMessage(InitMessageFn fn) override
     {
@@ -286,10 +251,6 @@ public:
     {
         return MakeHandler(::uiInterface.ShowProgress_connect(fn));
     }
-    std::unique_ptr<Handler> handleLoadWallet(LoadWalletFn fn) override
-    {
-        return HandleLoadWallet(std::move(fn));
-    }
     std::unique_ptr<Handler> handleNotifyNumConnectionsChanged(NotifyNumConnectionsChangedFn fn) override
     {
         return MakeHandler(::uiInterface.NotifyNumConnectionsChanged_connect(fn));
diff --git a/src/interfaces/node.h b/src/interfaces/node.h
index 0cff7ae3a10..5079be038eb 100644
--- a/src/interfaces/node.h
+++ b/src/interfaces/node.h
@@ -29,14 +29,13 @@ class RPCTimerInterface;
 class UniValue;
 class proxyType;
 enum class SynchronizationState;
-enum class WalletCreationStatus;
 struct CNodeStateStats;
 struct NodeContext;
 struct bilingual_str;
 
 namespace interfaces {
 class Handler;
-class Wallet;
+class WalletClient;
 struct BlockTip;
 
 //! Block and header tip information
@@ -173,22 +172,8 @@ public:
     //! Get unspent outputs associated with a transaction.
     virtual bool getUnspentOutput(const COutPoint& output, Coin& coin) = 0;
 
-    //! Return default wallet directory.
-    virtual std::string getWalletDir() = 0;
-
-    //! Return available wallets in wallet directory.
-    virtual std::vector<std::string> listWalletDir() = 0;
-
-    //! Return interfaces for accessing wallets (if any).
-    virtual std::vector<std::unique_ptr<Wallet>> getWallets() = 0;
-
-    //! Attempts to load a wallet from file or directory.
-    //! The loaded wallet is also notified to handlers previously registered
-    //! with handleLoadWallet.
-    virtual std::unique_ptr<Wallet> loadWallet(const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0;
-
-    //! Create a wallet from file
-    virtual std::unique_ptr<Wallet> createWallet(const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, WalletCreationStatus& status) = 0;
+    //! Get wallet client.
+    virtual WalletClient& walletClient() = 0;
 
     //! Register handler for init messages.
     using InitMessageFn = std::function<void(const std::string& message)>;
@@ -210,10 +195,6 @@ public:
     using ShowProgressFn = std::function<void(const std::string& title, int progress, bool resume_possible)>;
     virtual std::unique_ptr<Handler> handleShowProgress(ShowProgressFn fn) = 0;
 
-    //! Register handler for load wallet messages.
-    using LoadWalletFn = std::function<void(std::unique_ptr<Wallet> wallet)>;
-    virtual std::unique_ptr<Handler> handleLoadWallet(LoadWalletFn fn) = 0;
-
     //! Register handler for number of connections changed messages.
     using NotifyNumConnectionsChangedFn = std::function<void(int new_num_connections)>;
     virtual std::unique_ptr<Handler> handleNotifyNumConnectionsChanged(NotifyNumConnectionsChangedFn fn) = 0;
diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp
index 937e602fb0a..63c109658ea 100644
--- a/src/interfaces/wallet.cpp
+++ b/src/interfaces/wallet.cpp
@@ -485,7 +485,7 @@ public:
     std::shared_ptr<CWallet> m_wallet;
 };
 
-class WalletClientImpl : public ChainClient
+class WalletClientImpl : public WalletClient
 {
 public:
     WalletClientImpl(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames)
@@ -494,6 +494,9 @@ public:
         m_context.chain = &chain;
         m_context.args = &args;
     }
+    ~WalletClientImpl() override { UnloadWallets(); }
+
+    //! ChainClient methods
     void registerRpcs() override
     {
         for (const CRPCCommand& command : GetWalletRPCCommands()) {
@@ -509,6 +512,30 @@ public:
     void flush() override { return FlushWallets(); }
     void stop() override { return StopWallets(); }
     void setMockTime(int64_t time) override { return SetMockTime(time); }
+
+    //! WalletClient methods
+    std::unique_ptr<Wallet> createWallet(const std::string& name, const SecureString& passphrase, uint64_t wallet_creation_flags, WalletCreationStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings) override
+    {
+        std::shared_ptr<CWallet> wallet;
+        status = CreateWallet(*m_context.chain, passphrase, wallet_creation_flags, name, error, warnings, wallet);
+        return MakeWallet(std::move(wallet));
+    }
+    std::unique_ptr<Wallet> loadWallet(const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) override
+    {
+        return MakeWallet(LoadWallet(*m_context.chain, WalletLocation(name), error, warnings));
+    }
+    std::string getWalletDir() override
+    {
+        return GetWalletDir().string();
+    }
+    std::vector<std::string> listWalletDir() override
+    {
+        std::vector<std::string> paths;
+        for (auto& path : ListWalletDir()) {
+            paths.push_back(path.string());
+        }
+        return paths;
+    }
     std::vector<std::unique_ptr<Wallet>> getWallets() override
     {
         std::vector<std::unique_ptr<Wallet>> wallets;
@@ -517,7 +544,10 @@ public:
         }
         return wallets;
     }
-    ~WalletClientImpl() override { UnloadWallets(); }
+    std::unique_ptr<Handler> handleLoadWallet(LoadWalletFn fn) override
+    {
+        return HandleLoadWallet(std::move(fn));
+    }
 
     WalletContext m_context;
     const std::vector<std::string> m_wallet_filenames;
@@ -529,7 +559,7 @@ public:
 
 std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet) { return wallet ? MakeUnique<WalletImpl>(wallet) : nullptr; }
 
-std::unique_ptr<ChainClient> MakeWalletClient(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames)
+std::unique_ptr<WalletClient> MakeWalletClient(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames)
 {
     return MakeUnique<WalletClientImpl>(chain, args, std::move(wallet_filenames));
 }
diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h
index 3cdadbc72ec..186f5d81a5d 100644
--- a/src/interfaces/wallet.h
+++ b/src/interfaces/wallet.h
@@ -6,6 +6,7 @@
 #define BITCOIN_INTERFACES_WALLET_H
 
 #include <amount.h>                    // For CAmount
+#include <interfaces/chain.h>          // For ChainClient
 #include <pubkey.h>                    // For CKeyID and CScriptID (definitions needed in CTxDestination instantiation)
 #include <script/standard.h>           // For CTxDestination
 #include <support/allocators/secure.h> // For SecureString
@@ -28,9 +29,11 @@ class CWallet;
 enum class FeeReason;
 enum class OutputType;
 enum class TransactionError;
+enum class WalletCreationStatus;
 enum isminetype : unsigned int;
 struct CRecipient;
 struct PartiallySignedTransaction;
+struct WalletContext;
 struct bilingual_str;
 typedef uint8_t isminefilter;
 
@@ -301,6 +304,34 @@ public:
     virtual CWallet* wallet() { return nullptr; }
 };
 
+//! Wallet chain client that in addition to having chain client methods for
+//! starting up, shutting down, and registering RPCs, also has additional
+//! methods (called by the GUI) to load and create wallets.
+class WalletClient : public ChainClient
+{
+public:
+    //! Create new wallet.
+    virtual std::unique_ptr<Wallet> createWallet(const std::string& name, const SecureString& passphrase, uint64_t wallet_creation_flags, WalletCreationStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0;
+
+   //! Load existing wallet.
+   virtual std::unique_ptr<Wallet> loadWallet(const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0;
+
+   //! Return default wallet directory.
+   virtual std::string getWalletDir() = 0;
+
+   //! Return available wallets in wallet directory.
+   virtual std::vector<std::string> listWalletDir() = 0;
+
+   //! Return interfaces for accessing wallets (if any).
+   virtual std::vector<std::unique_ptr<Wallet>> getWallets() = 0;
+
+   //! Register handler for load wallet messages. This callback is triggered by
+   //! createWallet and loadWallet above, and also triggered when wallets are
+   //! loaded at startup or by RPC.
+   using LoadWalletFn = std::function<void(std::unique_ptr<Wallet> wallet)>;
+   virtual std::unique_ptr<Handler> handleLoadWallet(LoadWalletFn fn) = 0;
+};
+
 //! Information about one wallet address.
 struct WalletAddress
 {
@@ -379,6 +410,10 @@ struct WalletTxOut
 //! dummywallet.cpp and throws if the wallet component is not compiled.
 std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet);
 
+//! Return implementation of ChainClient interface for a wallet client. This
+//! function will be undefined in builds where ENABLE_WALLET is false.
+std::unique_ptr<WalletClient> MakeWalletClient(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames);
+
 } // namespace interfaces
 
 #endif // BITCOIN_INTERFACES_WALLET_H
diff --git a/src/node/context.h b/src/node/context.h
index be568cba365..793c9dfc343 100644
--- a/src/node/context.h
+++ b/src/node/context.h
@@ -20,6 +20,7 @@ class PeerLogicValidation;
 namespace interfaces {
 class Chain;
 class ChainClient;
+class WalletClient;
 } // namespace interfaces
 
 //! NodeContext struct containing references to chain state and connection
@@ -40,7 +41,11 @@ struct NodeContext {
     std::unique_ptr<BanMan> banman;
     ArgsManager* args{nullptr}; // Currently a raw pointer because the memory is not managed by this struct
     std::unique_ptr<interfaces::Chain> chain;
+    //! List of all chain clients (wallet processes or other client) connected to node.
     std::vector<std::unique_ptr<interfaces::ChainClient>> chain_clients;
+    //! Reference to chain client that should used to load or create wallets
+    //! opened by the gui.
+    interfaces::WalletClient* wallet_client{nullptr};
     std::unique_ptr<CScheduler> scheduler;
     std::function<void()> rpc_interruption_point = [] {};
 
diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp
index e63ffdfb36f..c1fb8d2249f 100644
--- a/src/qt/bitcoin.cpp
+++ b/src/qt/bitcoin.cpp
@@ -266,6 +266,7 @@ void BitcoinApplication::createSplashScreen(const NetworkStyle *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 finish() happens.
     m_splash->show();
+    connect(this, &BitcoinApplication::requestedInitialize, m_splash, &SplashScreen::handleLoadWallet);
     connect(this, &BitcoinApplication::splashFinished, m_splash, &SplashScreen::finish);
     connect(this, &BitcoinApplication::requestedShutdown, m_splash, &QWidget::close);
 }
diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp
index 8935ff19bf9..aa58c0b10e7 100644
--- a/src/qt/bitcoingui.cpp
+++ b/src/qt/bitcoingui.cpp
@@ -1189,7 +1189,7 @@ void BitcoinGUI::incomingTransaction(const QString& date, int unit, const CAmoun
     // On new transaction, make an info balloon
     QString msg = tr("Date: %1\n").arg(date) +
                   tr("Amount: %1\n").arg(BitcoinUnits::formatWithUnit(unit, amount, true));
-    if (m_node.getWallets().size() > 1 && !walletName.isEmpty()) {
+    if (m_node.walletClient().getWallets().size() > 1 && !walletName.isEmpty()) {
         msg += tr("Wallet: %1\n").arg(walletName);
     }
     msg += tr("Type: %1\n").arg(type);
diff --git a/src/qt/splashscreen.cpp b/src/qt/splashscreen.cpp
index bd63d6e7fb6..d9861b79537 100644
--- a/src/qt/splashscreen.cpp
+++ b/src/qt/splashscreen.cpp
@@ -198,8 +198,12 @@ void SplashScreen::subscribeToCoreSignals()
     // Connect signals to client
     m_handler_init_message = m_node->handleInitMessage(std::bind(InitMessage, this, std::placeholders::_1));
     m_handler_show_progress = m_node->handleShowProgress(std::bind(ShowProgress, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3));
+}
+
+void SplashScreen::handleLoadWallet()
+{
 #ifdef ENABLE_WALLET
-    m_handler_load_wallet = m_node->handleLoadWallet([this](std::unique_ptr<interfaces::Wallet> wallet) { ConnectWallet(std::move(wallet)); });
+    m_handler_load_wallet = m_node->walletClient().handleLoadWallet([this](std::unique_ptr<interfaces::Wallet> wallet) { ConnectWallet(std::move(wallet)); });
 #endif
 }
 
diff --git a/src/qt/splashscreen.h b/src/qt/splashscreen.h
index 2213b02c55a..d4e3f6ece11 100644
--- a/src/qt/splashscreen.h
+++ b/src/qt/splashscreen.h
@@ -43,6 +43,9 @@ public Q_SLOTS:
     /** Show message and progress */
     void showMessage(const QString &message, int alignment, const QColor &color);
 
+    /** Handle wallet load notifications. */
+    void handleLoadWallet();
+
 protected:
     bool eventFilter(QObject * obj, QEvent * ev) override;
 
diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp
index 3aed98e0e87..828f84ffccd 100644
--- a/src/qt/walletcontroller.cpp
+++ b/src/qt/walletcontroller.cpp
@@ -35,11 +35,11 @@ WalletController::WalletController(ClientModel& client_model, const PlatformStyl
     , m_platform_style(platform_style)
     , m_options_model(client_model.getOptionsModel())
 {
-    m_handler_load_wallet = m_node.handleLoadWallet([this](std::unique_ptr<interfaces::Wallet> wallet) {
+    m_handler_load_wallet = m_node.walletClient().handleLoadWallet([this](std::unique_ptr<interfaces::Wallet> wallet) {
         getOrCreateWallet(std::move(wallet));
     });
 
-    for (std::unique_ptr<interfaces::Wallet>& wallet : m_node.getWallets()) {
+    for (std::unique_ptr<interfaces::Wallet>& wallet : m_node.walletClient().getWallets()) {
         getOrCreateWallet(std::move(wallet));
     }
 
@@ -66,7 +66,7 @@ std::map<std::string, bool> WalletController::listWalletDir() const
 {
     QMutexLocker locker(&m_mutex);
     std::map<std::string, bool> wallets;
-    for (const std::string& name : m_node.listWalletDir()) {
+    for (const std::string& name : m_node.walletClient().listWalletDir()) {
         wallets[name] = false;
     }
     for (WalletModel* wallet_model : m_wallets) {
@@ -250,7 +250,7 @@ void CreateWalletActivity::createWallet()
 
     QTimer::singleShot(500, worker(), [this, name, flags] {
         WalletCreationStatus status;
-        std::unique_ptr<interfaces::Wallet> wallet = node().createWallet(m_passphrase, flags, name, m_error_message, m_warning_message, status);
+        std::unique_ptr<interfaces::Wallet> wallet = node().walletClient().createWallet(name, m_passphrase, flags, status, m_error_message, m_warning_message);
 
         if (status == WalletCreationStatus::SUCCESS) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(wallet));
 
@@ -321,7 +321,7 @@ void OpenWalletActivity::open(const std::string& path)
     showProgressDialog(tr("Opening Wallet <b>%1</b>...").arg(name.toHtmlEscaped()));
 
     QTimer::singleShot(0, worker(), [this, path] {
-        std::unique_ptr<interfaces::Wallet> wallet = node().loadWallet(path, m_error_message, m_warning_message);
+        std::unique_ptr<interfaces::Wallet> wallet = node().walletClient().loadWallet(path, m_error_message, m_warning_message);
 
         if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(wallet));
 
diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
index e374dd191cf..0556895948d 100644
--- a/src/qt/walletmodel.cpp
+++ b/src/qt/walletmodel.cpp
@@ -581,7 +581,7 @@ QString WalletModel::getDisplayName() const
 
 bool WalletModel::isMultiwallet()
 {
-    return m_node.getWallets().size() > 1;
+    return m_node.walletClient().getWallets().size() > 1;
 }
 
 void WalletModel::refresh(bool pk_hash_only)
diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp
index bf05ef844af..1456e1595e0 100644
--- a/src/wallet/init.cpp
+++ b/src/wallet/init.cpp
@@ -5,6 +5,7 @@
 
 #include <init.h>
 #include <interfaces/chain.h>
+#include <interfaces/wallet.h>
 #include <net.h>
 #include <node/context.h>
 #include <node/ui_interface.h>
@@ -129,5 +130,7 @@ void WalletInit::Construct(NodeContext& node) const
             settings.rw_settings["wallet"] = wallets;
         });
     }
-    node.chain_clients.emplace_back(interfaces::MakeWalletClient(*node.chain, args, args.GetArgs("-wallet")));
+    auto wallet_client = interfaces::MakeWalletClient(*node.chain, args, args.GetArgs("-wallet"));
+    node.wallet_client = wallet_client.get();
+    node.chain_clients.emplace_back(std::move(wallet_client));
 }
diff --git a/src/wallet/test/init_test_fixture.cpp b/src/wallet/test/init_test_fixture.cpp
index 35bd9656732..c23dea1338b 100644
--- a/src/wallet/test/init_test_fixture.cpp
+++ b/src/wallet/test/init_test_fixture.cpp
@@ -10,7 +10,7 @@
 
 InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainName) : BasicTestingSetup(chainName)
 {
-    m_chain_client = MakeWalletClient(*m_chain, *Assert(m_node.args), {});
+    m_wallet_client = MakeWalletClient(*m_chain, *Assert(m_node.args), {});
 
     std::string sep;
     sep += fs::path::preferred_separator;
diff --git a/src/wallet/test/init_test_fixture.h b/src/wallet/test/init_test_fixture.h
index c95b4f1f6e7..f5bade77dfe 100644
--- a/src/wallet/test/init_test_fixture.h
+++ b/src/wallet/test/init_test_fixture.h
@@ -6,6 +6,7 @@
 #define BITCOIN_WALLET_TEST_INIT_TEST_FIXTURE_H
 
 #include <interfaces/chain.h>
+#include <interfaces/wallet.h>
 #include <node/context.h>
 #include <test/util/setup_common.h>
 
@@ -19,7 +20,7 @@ struct InitWalletDirTestingSetup: public BasicTestingSetup {
     fs::path m_cwd;
     std::map<std::string, fs::path> m_walletdir_path_cases;
     std::unique_ptr<interfaces::Chain> m_chain = interfaces::MakeChain(m_node);
-    std::unique_ptr<interfaces::ChainClient> m_chain_client;
+    std::unique_ptr<interfaces::WalletClient> m_wallet_client;
 };
 
 #endif // BITCOIN_WALLET_TEST_INIT_TEST_FIXTURE_H
diff --git a/src/wallet/test/init_tests.cpp b/src/wallet/test/init_tests.cpp
index c228e06009b..9b905569fc2 100644
--- a/src/wallet/test/init_tests.cpp
+++ b/src/wallet/test/init_tests.cpp
@@ -15,7 +15,7 @@ BOOST_FIXTURE_TEST_SUITE(init_tests, InitWalletDirTestingSetup)
 BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_default)
 {
     SetWalletDir(m_walletdir_path_cases["default"]);
-    bool result = m_chain_client->verify();
+    bool result = m_wallet_client->verify();
     BOOST_CHECK(result == true);
     fs::path walletdir = gArgs.GetArg("-walletdir", "");
     fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]);
@@ -25,7 +25,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_default)
 BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_custom)
 {
     SetWalletDir(m_walletdir_path_cases["custom"]);
-    bool result = m_chain_client->verify();
+    bool result = m_wallet_client->verify();
     BOOST_CHECK(result == true);
     fs::path walletdir = gArgs.GetArg("-walletdir", "");
     fs::path expected_path = fs::canonical(m_walletdir_path_cases["custom"]);
@@ -37,7 +37,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_does_not_exist)
     SetWalletDir(m_walletdir_path_cases["nonexistent"]);
     {
         ASSERT_DEBUG_LOG("does not exist");
-        bool result = m_chain_client->verify();
+        bool result = m_wallet_client->verify();
         BOOST_CHECK(result == false);
     }
 }
@@ -47,7 +47,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_is_not_directory)
     SetWalletDir(m_walletdir_path_cases["file"]);
     {
         ASSERT_DEBUG_LOG("is not a directory");
-        bool result = m_chain_client->verify();
+        bool result = m_wallet_client->verify();
         BOOST_CHECK(result == false);
     }
 }
@@ -57,7 +57,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_is_not_relative)
     SetWalletDir(m_walletdir_path_cases["relative"]);
     {
         ASSERT_DEBUG_LOG("is a relative path");
-        bool result = m_chain_client->verify();
+        bool result = m_wallet_client->verify();
         BOOST_CHECK(result == false);
     }
 }
@@ -65,7 +65,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_is_not_relative)
 BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing)
 {
     SetWalletDir(m_walletdir_path_cases["trailing"]);
-    bool result = m_chain_client->verify();
+    bool result = m_wallet_client->verify();
     BOOST_CHECK(result == true);
     fs::path walletdir = gArgs.GetArg("-walletdir", "");
     fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]);
@@ -75,7 +75,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing)
 BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing2)
 {
     SetWalletDir(m_walletdir_path_cases["trailing2"]);
-    bool result = m_chain_client->verify();
+    bool result = m_wallet_client->verify();
     BOOST_CHECK(result == true);
     fs::path walletdir = gArgs.GetArg("-walletdir", "");
     fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]);
diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp
index 44f9eb5ddd8..9241470745e 100644
--- a/src/wallet/test/wallet_test_fixture.cpp
+++ b/src/wallet/test/wallet_test_fixture.cpp
@@ -11,5 +11,5 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName)
     bool fFirstRun;
     m_wallet.LoadWallet(fFirstRun);
     m_chain_notifications_handler = m_chain->handleNotifications({ &m_wallet, [](CWallet*) {} });
-    m_chain_client->registerRpcs();
+    m_wallet_client->registerRpcs();
 }
diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h
index 99d7cfe921f..12ad13b49bc 100644
--- a/src/wallet/test/wallet_test_fixture.h
+++ b/src/wallet/test/wallet_test_fixture.h
@@ -21,7 +21,7 @@ struct WalletTestingSetup : public TestingSetup {
     explicit WalletTestingSetup(const std::string& chainName = CBaseChainParams::MAIN);
 
     std::unique_ptr<interfaces::Chain> m_chain = interfaces::MakeChain(m_node);
-    std::unique_ptr<interfaces::ChainClient> m_chain_client = interfaces::MakeWalletClient(*m_chain, *Assert(m_node.args), {});
+    std::unique_ptr<interfaces::WalletClient> m_wallet_client = interfaces::MakeWalletClient(*m_chain, *Assert(m_node.args), {});
     CWallet m_wallet;
     std::unique_ptr<interfaces::Handler> m_chain_notifications_handler;
 };
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index fa00d125518..3a03fa08ed4 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -183,11 +183,6 @@ std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const WalletLocati
     return wallet;
 }
 
-std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings)
-{
-    return LoadWallet(chain, WalletLocation(name), error, warnings);
-}
-
 WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, std::shared_ptr<CWallet>& result)
 {
     // Indicate that the wallet is actually supposed to be blank and not just blank to make it encrypted

From 24bf17602c620445f76c3b407937751c8a894d37 Mon Sep 17 00:00:00 2001
From: Russell Yanofsky <russ@yanofsky.org>
Date: Fri, 10 Jul 2020 11:23:47 -0400
Subject: [PATCH 3/3] gui refactor: Inline SplashScreen::ConnectWallet

Suggested https://github.com/bitcoin/bitcoin/pull/19099#discussion_r450522201
---
 src/qt/splashscreen.cpp | 12 ++++--------
 src/qt/splashscreen.h   |  2 --
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/src/qt/splashscreen.cpp b/src/qt/splashscreen.cpp
index d9861b79537..8e381861a0f 100644
--- a/src/qt/splashscreen.cpp
+++ b/src/qt/splashscreen.cpp
@@ -185,13 +185,6 @@ static void ShowProgress(SplashScreen *splash, const std::string &title, int nPr
                                 : _("press q to shutdown").translated) +
             strprintf("\n%d", nProgress) + "%");
 }
-#ifdef ENABLE_WALLET
-void SplashScreen::ConnectWallet(std::unique_ptr<interfaces::Wallet> wallet)
-{
-    m_connected_wallet_handlers.emplace_back(wallet->handleShowProgress(std::bind(ShowProgress, this, std::placeholders::_1, std::placeholders::_2, false)));
-    m_connected_wallets.emplace_back(std::move(wallet));
-}
-#endif
 
 void SplashScreen::subscribeToCoreSignals()
 {
@@ -203,7 +196,10 @@ void SplashScreen::subscribeToCoreSignals()
 void SplashScreen::handleLoadWallet()
 {
 #ifdef ENABLE_WALLET
-    m_handler_load_wallet = m_node->walletClient().handleLoadWallet([this](std::unique_ptr<interfaces::Wallet> wallet) { ConnectWallet(std::move(wallet)); });
+    m_handler_load_wallet = m_node->walletClient().handleLoadWallet([this](std::unique_ptr<interfaces::Wallet> wallet) {
+        m_connected_wallet_handlers.emplace_back(wallet->handleShowProgress(std::bind(ShowProgress, this, std::placeholders::_1, std::placeholders::_2, false)));
+        m_connected_wallets.emplace_back(std::move(wallet));
+    });
 #endif
 }
 
diff --git a/src/qt/splashscreen.h b/src/qt/splashscreen.h
index d4e3f6ece11..a0cd677d3dc 100644
--- a/src/qt/splashscreen.h
+++ b/src/qt/splashscreen.h
@@ -56,8 +56,6 @@ private:
     void unsubscribeFromCoreSignals();
     /** Initiate shutdown */
     void shutdown();
-    /** Connect wallet signals to splash screen */
-    void ConnectWallet(std::unique_ptr<interfaces::Wallet> wallet);
 
     QPixmap pixmap;
     QString curMessage;