[backport#15288] Remove use of uiInterface.LoadWallet in wallet code

Summary: This also changes the uiInterface.LoadWallet signal argument type from shared_ptr<CWallet> to unique_ptr<interfaces::Wallet> because CWallet is an internal wallet class that shouldn't be used in non-wallet code (and also can't be passed across process boundaries). This commit does not change behavior. 1106a6fde4bfde31a16de45e4cc84ed5da05c5a4 (Russell Yanofksi) --- Depends on D5867 This is a partial backport of Core [[bitcoin/bitcoin#15288 | PR15288]] Test Plan: cmake .. -GNinja -DENABLE_WERROR=ON -DCMAKE_BUILD_TYPE=Debug -DBUILD_BITCOIN_WALLET=OFF ninja check-all cmake .. -GNinja -DENABLE_WERROR=ON -DCMAKE_BUILD_TYPE=Debug ninja check-all Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Subscribers: deadalnix Differential Revision: https://reviews.bitcoinabc.org/D5868
jonspock · Aug 27, 2020 · 6c266ac · 6c266ac
1 parent f5e8669
commit 6c266ac
Show file tree

Hide file tree

Showing 8 changed files with 87 additions and 11 deletions.
diff --git a/src/init.cpp b/src/init.cpp
@@ -2399,7 +2399,7 @@ bool AppInitMain(Config &config, RPCServer& rpcServer,
 
     // Step 9: load wallet
     for (const auto &client : interfaces.chain_clients) {
-      if (!client->open(chainparams, walletPassphrase, words, use_bls)) {
+      if (!client->load(chainparams, walletPassphrase, words, use_bls)) {
         return false;
       }
     }

diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp
@@ -4,6 +4,15 @@
 
 #include <interfaces/chain.h>
 
+#include <chain.h>
+#include <chainparams.h>
+#include <interfaces/wallet.h>
+#include <net.h>
+//#include <policy/mempool.h>
+#include <primitives/block.h>
+//#include <primitives/blockhash.h>
+#include <primitives/transaction.h>
+#include <protocol.h>
 #include <sync.h>
 #include <util/system.h>
 #include <validation.h>
@@ -36,6 +45,9 @@ namespace {
         std::unique_ptr<Chain::Lock> assumeLocked() override {
             return std::make_unique<LockImpl>();
         }
+        void loadWallet(std::unique_ptr<Wallet> wallet) override {
+            ::uiInterface.LoadWallet(wallet);
+        }
     };
 
 } // namespace

diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h
@@ -15,6 +15,8 @@ class CScheduler;
 
 namespace interfaces {
 
+class Wallet;
+
 //! Interface for giving wallet processes access to blockchain state.
 class Chain {
 public:
@@ -38,6 +40,9 @@ class Chain {
     //! method is temporary and is only used in a few places to avoid changing
     //! behavior while code is transitioned to use the Chain::Lock interface.
     virtual std::unique_ptr<Lock> assumeLocked() = 0;
+
+    //! Send wallet load notification to the GUI.
+    virtual void loadWallet(std::unique_ptr<Wallet> wallet) = 0;
 };
 
 //! Interface to let node manage chain clients (wallets, or maybe tools for
@@ -53,7 +58,7 @@ class ChainClient {
     virtual bool verify(const CChainParams &chainParams) = 0;
 
     /** Open wallets*/    
-    virtual bool open(const CChainParams &chainParams, const SecureString& walletPassphrase,
+    virtual bool load(const CChainParams &chainParams, const SecureString& walletPassphrase,
                       const std::vector<std::string>& words, bool use_bls) = 0;
 
     //! Start client execution and provide a scheduler.

diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp
@@ -293,10 +293,10 @@ namespace {
             return MakeHandler(::uiInterface.ShowProgress.connect(fn));
         }
         std::unique_ptr<Handler> handleLoadWallet(LoadWalletFn fn) override {
-            CHECK_WALLET(return MakeHandler(::uiInterface.LoadWallet.connect(
-                [fn](std::shared_ptr<CWallet> wallet) {
-                    fn(MakeWallet(wallet));
-                })));
+            return MakeHandler(::uiInterface.LoadWallet_connect(
+                [fn](std::unique_ptr<Wallet> &wallet) {
+                    fn(std::move(wallet));
+                }));
         }
         std::unique_ptr<Handler> handleNotifyNumConnectionsChanged(
             NotifyNumConnectionsChangedFn fn) override {

diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp
@@ -507,7 +507,7 @@ namespace {
             StartWallets(scheduler);
         }
 
-        bool open(const CChainParams &chainParams, const SecureString& walletPassphrase,
+        bool load(const CChainParams &chainParams, const SecureString& walletPassphrase,
                   const std::vector<std::string>& words, bool use_bls) override {
           return LoadWallets(chainParams, m_chain, m_wallet_filenames,
                              walletPassphrase,words,use_bls);

diff --git a/src/ui_interface.cpp b/src/ui_interface.cpp
@@ -7,6 +7,52 @@
 
 CClientUIInterface uiInterface;
 
+
+struct UISignals {
+  /*
+    boost::signals2::signal<CClientUIInterface::ThreadSafeMessageBoxSig,
+                            boost::signals2::last_value<bool>>
+        ThreadSafeMessageBox;
+    boost::signals2::signal<CClientUIInterface::ThreadSafeQuestionSig,
+                            boost::signals2::last_value<bool>>
+        ThreadSafeQuestion;
+    boost::signals2::signal<CClientUIInterface::InitMessageSig> InitMessage;
+    boost::signals2::signal<CClientUIInterface::NotifyNumConnectionsChangedSig>
+        NotifyNumConnectionsChanged;
+    boost::signals2::signal<CClientUIInterface::NotifyNetworkActiveChangedSig>
+        NotifyNetworkActiveChanged;
+    boost::signals2::signal<CClientUIInterface::NotifyAlertChangedSig>
+        NotifyAlertChanged;
+  */
+    boost::signals2::signal<CClientUIInterface::LoadWalletSig> LoadWallet;
+  /*
+    boost::signals2::signal<CClientUIInterface::ShowProgressSig> ShowProgress;
+    boost::signals2::signal<CClientUIInterface::NotifyBlockTipSig>
+        NotifyBlockTip;
+    boost::signals2::signal<CClientUIInterface::NotifyHeaderTipSig>
+        NotifyHeaderTip;
+    boost::signals2::signal<CClientUIInterface::BannedListChangedSig>
+        BannedListChanged;
+  */
+} g_ui_signals;
+
+#define ADD_SIGNALS_IMPL_WRAPPER(signal_name)                                  \
+    boost::signals2::connection CClientUIInterface::signal_name##_connect(     \
+        std::function<signal_name##Sig> fn) {                                  \
+        return g_ui_signals.signal_name.connect(fn);                           \
+    }                                                                          \
+    void CClientUIInterface::signal_name##_disconnect(                         \
+        std::function<signal_name##Sig> fn) {                                  \
+        return g_ui_signals.signal_name.disconnect(&fn);                       \
+    }
+
+ADD_SIGNALS_IMPL_WRAPPER(LoadWallet);
+
+void CClientUIInterface::LoadWallet(
+    std::unique_ptr<interfaces::Wallet> &wallet) {
+    return g_ui_signals.LoadWallet(wallet);
+}
+
 bool InitError(const std::string &str) {
     uiInterface.ThreadSafeMessageBox(str, "", CClientUIInterface::MSG_ERROR);
     return false;

diff --git a/src/ui_interface.h b/src/ui_interface.h
@@ -13,9 +13,12 @@
 #include <memory>
 #include <string>
 
-class CWallet;
 class CBlockIndex;
 
+namespace interfaces {
+class Wallet;
+} // namespace interfaces
+
 /** General change type (added, updated, removed). */
 enum ChangeType { CT_NEW, CT_UPDATED, CT_DELETED };
 
@@ -73,6 +76,13 @@ class CClientUIInterface {
         MSG_ERROR = (ICON_ERROR | BTN_OK | MODAL)
     };
 
+#define ADD_SIGNALS_DECL_WRAPPER(signal_name, rtype, args...)                  \
+    rtype signal_name(args);                                                   \
+    using signal_name##Sig = rtype(args);                                      \
+    boost::signals2::connection signal_name##_connect(                         \
+        std::function<signal_name##Sig> fn);                                   \
+    void signal_name##_disconnect(std::function<signal_name##Sig> fn);
+
     /** Show message box. */
     boost::signals2::signal<bool(const std::string &message,
                                  const std::string &caption,
@@ -109,7 +119,8 @@ class CClientUIInterface {
     boost::signals2::signal<void()> NotifyAlertChanged;
 
     /** A wallet has been loaded. */
-    boost::signals2::signal<void(std::shared_ptr<CWallet> wallet)> LoadWallet;
+    ADD_SIGNALS_DECL_WRAPPER(LoadWallet, void,
+                             std::unique_ptr<interfaces::Wallet> &wallet);
 
     /**
      * Show progress e.g. for verifychain.

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
@@ -18,6 +18,7 @@
 #include <init.h>
 #include <fs.h>
 #include <interfaces/chain.h>
+#include <interfaces/wallet.h>
 #include <key.h>
 #include <keystore.h>
 #include <net.h>
@@ -5397,8 +5398,9 @@ CWallet::CreateWalletFromFile(const CChainParams &chainParams,
         }
     }
 
-    uiInterface.LoadWallet(walletInstance);
-
+          // was uiInterface.LoadWallet(walletInstance);
+    chain.loadWallet(interfaces::MakeWallet(walletInstance));
+
     // Register with the validation interface. It's ok to do this after rescan
     // since we're still holding cs_main.
     RegisterValidationInterface(walletInstance.get());