Merge #14437: Refactor: Start to separate wallet from node

081accb Pass chain locked variables where needed (Russell Yanofsky)
79d579f Remove uses of cs_main in wallet code (Russell Yanofsky)
ea961c3 Remove direct node->wallet calls in init.cpp (Russell Yanofsky)
8db11dd Pass chain and client variables where needed (Russell Yanofsky)
7e2e62c Add skeleton chain and client classes (Russell Yanofsky)

Pull request description:

  This creates an incomplete [`Chain`](https://github.com/ryanofsky/bitcoin/blob/pr/wipc-sep/src/interfaces/chain.h) interface in [`src/interfaces/`](https://github.com/ryanofsky/bitcoin/tree/pr/wipc-sep/src/interfaces) and begins to update wallet code to use it.

  #10973 builds on this, changing the wallet to use the new interface to access chain state, instead of using CBlockIndex pointers and global variables like `chainActive`.

Tree-SHA512: 6ef05a4d8ebf57f2ad71835e4d970c9c59e34057e39e48cee76b887492c2fee907e3f6a74a9861e5a9f97cdc6823f4865ebc41ec556ab371ebca1b664c20dbea

Author: MarcoFalke

build_msvc/msvc-autogen.py

@@ -16,10 +16,6 @@
 ]
 
 ignore_list = [
-    'rpc/net.cpp',
-    'interfaces/handler.cpp',
-    'interfaces/node.cpp',
-    'interfaces/wallet.cpp',
 ]
 
 lib_sources = {}
@@ -32,7 +28,9 @@ def parse_makefile(makefile):
             if current_lib:
                 source = line.split()[0]
                 if source.endswith('.cpp') and not source.startswith('$') and source not in ignore_list:
-                    lib_sources[current_lib].append(source.replace('/', '\\'))
+                    source_filename = source.replace('/', '\\')
+                    object_filename = source.replace('/', '_')[:-4] + ".obj"
+                    lib_sources[current_lib].append((source_filename, object_filename))
                 if not line.endswith('\\'):
                     current_lib = ''
                 continue
@@ -51,8 +49,10 @@ def main():
     for key, value in lib_sources.items():
         vcxproj_filename = os.path.abspath(os.path.join(os.path.dirname(__file__), key, key + '.vcxproj'))
         content = ''
-        for source_filename in value:
-            content += '    <ClCompile Include="..\\..\\src\\' + source_filename + '" />\n'
+        for source_filename, object_filename in value:
+            content += '    <ClCompile Include="..\\..\\src\\' + source_filename + '">\n'
+            content += '      <ObjectFileName>$(IntDir)' + object_filename + '</ObjectFileName>\n'
+            content += '    </ClCompile>\n'
         with open(vcxproj_filename + '.in', 'r', encoding='utf-8') as vcxproj_in_file:
             with open(vcxproj_filename, 'w', encoding='utf-8') as vcxproj_file:
                 vcxproj_file.write(vcxproj_in_file.read().replace(

src/Makefile.am

@@ -125,6 +125,7 @@ BITCOIN_CORE_H = \
   index/txindex.h \
   indirectmap.h \
   init.h \
+  interfaces/chain.h \
   interfaces/handler.h \
   interfaces/node.h \
   interfaces/wallet.h \
@@ -233,6 +234,7 @@ libbitcoin_server_a_SOURCES = \
   httpserver.cpp \
   index/base.cpp \
   index/txindex.cpp \
+  interfaces/chain.cpp \
   interfaces/handler.cpp \
   interfaces/node.cpp \
   init.cpp \
@@ -465,6 +467,7 @@ endif
 bitcoind_LDADD = \
   $(LIBBITCOIN_SERVER) \
   $(LIBBITCOIN_WALLET) \
+  $(LIBBITCOIN_SERVER) \
   $(LIBBITCOIN_COMMON) \
   $(LIBUNIVALUE) \
   $(LIBBITCOIN_UTIL) \

src/bench/coin_selection.cpp

@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <bench/bench.h>
+#include <interfaces/chain.h>
 #include <wallet/wallet.h>
 #include <wallet/coinselection.h>
 
@@ -33,7 +34,8 @@ static void addCoin(const CAmount& nValue, const CWallet& wallet, std::vector<Ou
 // (https://github.com/bitcoin/bitcoin/issues/7883#issuecomment-224807484)
 static void CoinSelection(benchmark::State& state)
 {
-    const CWallet wallet(WalletLocation(), WalletDatabase::CreateDummy());
+    auto chain = interfaces::MakeChain();
+    const CWallet wallet(*chain, WalletLocation(), WalletDatabase::CreateDummy());
     LOCK(wallet.cs_wallet);
 
     // Add coins.
@@ -57,7 +59,8 @@ static void CoinSelection(benchmark::State& state)
 }
 
 typedef std::set<CInputCoin> CoinSet;
-static const CWallet testWallet(WalletLocation(), WalletDatabase::CreateDummy());
+static auto testChain = interfaces::MakeChain();
+static const CWallet testWallet(*testChain, WalletLocation(), WalletDatabase::CreateDummy());
 std::vector<std::unique_ptr<CWalletTx>> wtxn;
 
 // Copied from src/wallet/test/coinselector_tests.cpp

src/bitcoind.cpp

@@ -11,6 +11,7 @@
 #include <clientversion.h>
 #include <compat.h>
 #include <fs.h>
+#include <interfaces/chain.h>
 #include <rpc/server.h>
 #include <init.h>
 #include <noui.h>
@@ -58,6 +59,9 @@ static void WaitForShutdown()
 //
 static bool AppInit(int argc, char* argv[])
 {
+    InitInterfaces interfaces;
+    interfaces.chain = interfaces::MakeChain();
+
     bool fRet = false;
 
     //
@@ -164,7 +168,7 @@ static bool AppInit(int argc, char* argv[])
             // If locking the data directory failed, exit immediately
             return false;
         }
-        fRet = AppInitMain();
+        fRet = AppInitMain(interfaces);
     }
     catch (const std::exception& e) {
         PrintExceptionContinue(&e, "AppInit()");
@@ -178,7 +182,7 @@ static bool AppInit(int argc, char* argv[])
     } else {
         WaitForShutdown();
     }
-    Shutdown();
+    Shutdown(interfaces);
 
     return fRet;
 }

src/dummywallet.cpp

@@ -14,13 +14,7 @@ class DummyWalletInit : public WalletInitInterface {
     bool HasWalletSupport() const override {return false;}
     void AddWalletOptions() const override;
     bool ParameterInteraction() const override {return true;}
-    void RegisterRPC(CRPCTable &) const override {}
-    bool Verify() const override {return true;}
-    bool Open() const override {LogPrintf("No wallet support compiled in!\n"); return true;}
-    void Start(CScheduler& scheduler) const override {}
-    void Flush() const override {}
-    void Stop() const override {}
-    void Close() const override {}
+    void Construct(InitInterfaces& interfaces) const override {LogPrintf("No wallet support compiled in!\n");}
 };
 
 void DummyWalletInit::AddWalletOptions() const

src/init.cpp

@@ -19,6 +19,7 @@
 #include <fs.h>
 #include <httpserver.h>
 #include <httprpc.h>
+#include <interfaces/chain.h>
 #include <index/txindex.h>
 #include <key.h>
 #include <validation.h>
@@ -32,6 +33,7 @@
 #include <rpc/server.h>
 #include <rpc/register.h>
 #include <rpc/blockchain.h>
+#include <rpc/util.h>
 #include <script/standard.h>
 #include <script/sigcache.h>
 #include <scheduler.h>
@@ -157,7 +159,7 @@ void Interrupt()
     }
 }
 
-void Shutdown()
+void Shutdown(InitInterfaces& interfaces)
 {
     LogPrintf("%s: In progress...\n", __func__);
     static CCriticalSection cs_Shutdown;
@@ -176,7 +178,9 @@ void Shutdown()
     StopREST();
     StopRPC();
     StopHTTPServer();
-    g_wallet_init_interface.Flush();
+    for (const auto& client : interfaces.chain_clients) {
+        client->flush();
+    }
     StopMapPort();
 
     // Because these depend on each-other, we make sure that neither can be
@@ -239,7 +243,9 @@ void Shutdown()
         pcoinsdbview.reset();
         pblocktree.reset();
     }
-    g_wallet_init_interface.Stop();
+    for (const auto& client : interfaces.chain_clients) {
+        client->stop();
+    }
 
 #if ENABLE_ZMQ
     if (g_zmq_notification_interface) {
@@ -259,7 +265,7 @@ void Shutdown()
     UnregisterAllValidationInterfaces();
     GetMainSignals().UnregisterBackgroundSignalScheduler();
     GetMainSignals().UnregisterWithMempoolSignals(mempool);
-    g_wallet_init_interface.Close();
+    interfaces.chain_clients.clear();
     globalVerifyHandle.reset();
     ECC_Stop();
     LogPrintf("%s: done\n", __func__);
@@ -1158,7 +1164,7 @@ bool AppInitLockDataDirectory()
     return true;
 }
 
-bool AppInitMain()
+bool AppInitMain(InitInterfaces& interfaces)
 {
     const CChainParams& chainparams = Params();
     // ********************************************************* Step 4a: application initialization
@@ -1221,11 +1227,20 @@ bool AppInitMain()
     GetMainSignals().RegisterBackgroundSignalScheduler(scheduler);
     GetMainSignals().RegisterWithMempoolSignals(mempool);
 
+    // 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(interfaces);
+
     /* Register RPC commands regardless of -server setting so they will be
      * available in the GUI RPC console even if external calls are disabled.
      */
     RegisterAllCoreRPCCommands(tableRPC);
-    g_wallet_init_interface.RegisterRPC(tableRPC);
+    for (const auto& client : interfaces.chain_clients) {
+        client->registerRpcs();
+    }
+    g_rpc_interfaces = &interfaces;
 #if ENABLE_ZMQ
     RegisterZMQRPCCommands(tableRPC);
 #endif
@@ -1243,7 +1258,11 @@ bool AppInitMain()
     }
 
     // ********************************************************* Step 5: verify wallet database integrity
-    if (!g_wallet_init_interface.Verify()) return false;
+    for (const auto& client : interfaces.chain_clients) {
+        if (!client->verify()) {
+            return false;
+        }
+    }
 
     // ********************************************************* Step 6: network initialization
     // Note that we absolutely cannot open any actual connections
@@ -1562,7 +1581,11 @@ bool AppInitMain()
     }
 
     // ********************************************************* Step 9: load wallet
-    if (!g_wallet_init_interface.Open()) return false;
+    for (const auto& client : interfaces.chain_clients) {
+        if (!client->load()) {
+            return false;
+        }
+    }
 
     // ********************************************************* Step 10: data directory maintenance
 
@@ -1708,7 +1731,9 @@ bool AppInitMain()
     SetRPCWarmupFinished();
     uiInterface.InitMessage(_("Done loading"));
 
-    g_wallet_init_interface.Start(scheduler);
+    for (const auto& client : interfaces.chain_clients) {
+        client->start(scheduler);
+    }
 
     return true;
 }

src/init.h

@@ -10,8 +10,17 @@
 #include <string>
 #include <util/system.h>
 
-class CScheduler;
-class CWallet;
+namespace interfaces {
+class Chain;
+class ChainClient;
+} // namespace interfaces
+
+//! Pointers to interfaces used during init and destroyed on shutdown.
+struct InitInterfaces
+{
+    std::unique_ptr<interfaces::Chain> chain;
+    std::vector<std::unique_ptr<interfaces::ChainClient>> chain_clients;
+};
 
 namespace boost
 {
@@ -20,7 +29,7 @@ class thread_group;
 
 /** Interrupt threads */
 void Interrupt();
-void Shutdown();
+void Shutdown(InitInterfaces& interfaces);
 //!Initialize the logging infrastructure
 void InitLogging();
 //!Parameter interaction: change current parameters depending on various rules
@@ -54,7 +63,7 @@ bool AppInitLockDataDirectory();
  * @note This should only be done after daemonization. Call Shutdown() if this function fails.
  * @pre Parameters should be parsed and config file should be read, AppInitLockDataDirectory should have been called.
  */
-bool AppInitMain();
+bool AppInitMain(InitInterfaces& interfaces);
 
 /**
  * Setup the arguments for gArgs

src/interfaces/README.md

@@ -4,7 +4,7 @@ The following interfaces are defined here:
 
 * [`Chain`](chain.h) — used by wallet to access blockchain and mempool state. Added in [#10973](https://github.com/bitcoin/bitcoin/pull/10973).
 
-* [`Chain::Client`](chain.h) — used by node to start & stop `Chain` clients. Added in [#10973](https://github.com/bitcoin/bitcoin/pull/10973).
+* [`ChainClient`](chain.h) — used by node to start & stop `Chain` clients. Added in [#10973](https://github.com/bitcoin/bitcoin/pull/10973).
 
 * [`Node`](node.h) — used by GUI to start & stop bitcoin node. Added in [#10244](https://github.com/bitcoin/bitcoin/pull/10244).
 

src/interfaces/chain.cpp

@@ -0,0 +1,44 @@
+// Copyright (c) 2018 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 <interfaces/chain.h>
+
+#include <sync.h>
+#include <util/system.h>
+#include <validation.h>
+
+#include <memory>
+#include <utility>
+
+namespace interfaces {
+namespace {
+
+class LockImpl : public Chain::Lock
+{
+};
+
+class LockingStateImpl : public LockImpl, public UniqueLock<CCriticalSection>
+{
+    using UniqueLock::UniqueLock;
+};
+
+class ChainImpl : public Chain
+{
+public:
+    std::unique_ptr<Chain::Lock> lock(bool try_lock) override
+    {
+        auto result = MakeUnique<LockingStateImpl>(::cs_main, "cs_main", __FILE__, __LINE__, try_lock);
+        if (try_lock && result && !*result) return {};
+        // std::move necessary on some compilers due to conversion from
+        // LockingStateImpl to Lock pointer
+        return std::move(result);
+    }
+    std::unique_ptr<Chain::Lock> assumeLocked() override { return MakeUnique<LockImpl>(); }
+};
+
+} // namespace
+
+std::unique_ptr<Chain> MakeChain() { return MakeUnique<ChainImpl>(); }
+
+} // namespace interfaces