Merge bitcoin#22219: multiprocess: Start using init makeNode, makeChain, etc methods

fanquake · fanquake · commit 528e08119fd8 · 2021-09-16T08:47:38.000+08:00
e4709c7 Start using init makeNode, makeChain, etc methods (Russell Yanofsky) Pull request description: Use `interfaces::Init::make*` methods instead of `interfaces::Make*` functions, so interfaces can be constructed differently in different executable without having to change any code. (So for example `bitcoin-gui` can make an `interfaces::Node` pointer that communicates with a `bitcoin-node` subprocess, while `bitcoin-qt` can make an `interfaces::Node` pointer that controls node code in the same process.) --- This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10). The commit was first part of larger PR bitcoin#10102. ACKs for top commit: jamesob: reACK bitcoin@e4709c7 achow101: ACK e4709c7 benthecarman: utACK e4709c7 Tree-SHA512: 580c1979dbb2ef444157c8e53041e70d15ddeee77e5cbdb34f70b6d228cc2d2fe3843825f172da84e506200c58f7e0932f7cd4c006bb5058c1f4e43259394834
diff --git a/build_msvc/bitcoin-qt/bitcoin-qt.vcxproj b/build_msvc/bitcoin-qt/bitcoin-qt.vcxproj
@@ -9,6 +9,7 @@
   </PropertyGroup>
   <ItemGroup>
     <ClCompile Include="..\..\src\qt\main.cpp" />
+    <ClCompile Include="..\..\src\init\bitcoind.cpp" />
     <ResourceCompile Include="..\..\src\qt\res\bitcoin-qt-res.rc" />
   </ItemGroup>
   <ItemGroup>
diff --git a/build_msvc/test_bitcoin-qt/test_bitcoin-qt.vcxproj b/build_msvc/test_bitcoin-qt/test_bitcoin-qt.vcxproj
@@ -8,6 +8,7 @@
     <OutDir>$(SolutionDir)$(Platform)\$(Configuration)\</OutDir>
   </PropertyGroup>
   <ItemGroup>
+    <ClCompile Include="..\..\src\init\bitcoind.cpp" />
     <ClCompile Include="..\..\src\test\util\setup_common.cpp" />
     <ClCompile Include="..\..\src\qt\test\addressbooktests.cpp" />
     <ClCompile Include="..\..\src\qt\test\apptests.cpp" />
diff --git a/src/Makefile.qt.include b/src/Makefile.qt.include
@@ -338,14 +338,14 @@ bitcoin_qt_libtoolflags = $(AM_LIBTOOLFLAGS) --tag CXX
 
 qt_bitcoin_qt_CPPFLAGS = $(bitcoin_qt_cppflags)
 qt_bitcoin_qt_CXXFLAGS = $(bitcoin_qt_cxxflags)
-qt_bitcoin_qt_SOURCES = $(bitcoin_qt_sources)
+qt_bitcoin_qt_SOURCES = $(bitcoin_qt_sources) init/bitcoind.cpp
 qt_bitcoin_qt_LDADD = $(bitcoin_qt_ldadd)
 qt_bitcoin_qt_LDFLAGS = $(bitcoin_qt_ldflags)
 qt_bitcoin_qt_LIBTOOLFLAGS = $(bitcoin_qt_libtoolflags)
 
 bitcoin_gui_CPPFLAGS = $(bitcoin_qt_cppflags)
 bitcoin_gui_CXXFLAGS = $(bitcoin_qt_cxxflags)
-bitcoin_gui_SOURCES = $(bitcoin_qt_sources)
+bitcoin_gui_SOURCES = $(bitcoin_qt_sources) init/bitcoind.cpp
 bitcoin_gui_LDADD = $(bitcoin_qt_ldadd)
 bitcoin_gui_LDFLAGS = $(bitcoin_qt_ldflags)
 bitcoin_gui_LIBTOOLFLAGS = $(bitcoin_qt_libtoolflags)
diff --git a/src/Makefile.qttest.include b/src/Makefile.qttest.include
@@ -28,6 +28,7 @@ qt_test_test_bitcoin_qt_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(BITCOIN_
   $(QT_INCLUDES) $(QT_TEST_INCLUDES)
 
 qt_test_test_bitcoin_qt_SOURCES = \
+  init/bitcoind.cpp \
   qt/test/apptests.cpp \
   qt/test/rpcnestedtests.cpp \
   qt/test/test_main.cpp \
diff --git a/src/dummywallet.cpp b/src/dummywallet.cpp
@@ -5,12 +5,14 @@
 #include <util/system.h>
 #include <walletinitinterface.h>
 
+class ArgsManager;
 class CWallet;
 
 namespace interfaces {
 class Chain;
 class Handler;
 class Wallet;
+class WalletClient;
 }
 
 class DummyWalletInit : public WalletInitInterface {
@@ -64,4 +66,9 @@ std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet)
     throw std::logic_error("Wallet function called in non-wallet build.");
 }
 
+std::unique_ptr<WalletClient> MakeWalletClient(Chain& chain, ArgsManager& args)
+{
+    throw std::logic_error("Wallet function called in non-wallet build.");
+}
+
 } // namespace interfaces
diff --git a/src/init.cpp b/src/init.cpp
@@ -26,6 +26,7 @@
 #include <index/txindex.h>
 #include <init/common.h>
 #include <interfaces/chain.h>
+#include <interfaces/init.h>
 #include <interfaces/node.h>
 #include <mapport.h>
 #include <miner.h>
@@ -1063,7 +1064,7 @@ bool AppInitLockDataDirectory()
 
 bool AppInitInterfaces(NodeContext& node)
 {
-    node.chain = interfaces::MakeChain(node);
+    node.chain = node.init->makeChain();
     // 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
diff --git a/src/init/bitcoin-node.cpp b/src/init/bitcoin-node.cpp
@@ -2,9 +2,12 @@
 // 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 <interfaces/echo.h>
 #include <interfaces/init.h>
 #include <interfaces/ipc.h>
+#include <interfaces/node.h>
+#include <interfaces/wallet.h>
 #include <node/context.h>
 #include <util/system.h>
 
@@ -24,6 +27,12 @@ class BitcoinNodeInit : public interfaces::Init
         m_node.args = &gArgs;
         m_node.init = this;
     }
+    std::unique_ptr<interfaces::Node> makeNode() override { return interfaces::MakeNode(m_node); }
+    std::unique_ptr<interfaces::Chain> makeChain() override { return interfaces::MakeChain(m_node); }
+    std::unique_ptr<interfaces::WalletClient> makeWalletClient(interfaces::Chain& chain) override
+    {
+        return MakeWalletClient(chain, *Assert(m_node.args));
+    }
     std::unique_ptr<interfaces::Echo> makeEcho() override { return interfaces::MakeEcho(); }
     interfaces::Ipc* ipc() override { return m_ipc.get(); }
     NodeContext& m_node;
diff --git a/src/init/bitcoind.cpp b/src/init/bitcoind.cpp
@@ -2,7 +2,11 @@
 // 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 <interfaces/echo.h>
 #include <interfaces/init.h>
+#include <interfaces/node.h>
+#include <interfaces/wallet.h>
 #include <node/context.h>
 #include <util/system.h>
 
@@ -18,6 +22,13 @@ class BitcoindInit : public interfaces::Init
         m_node.args = &gArgs;
         m_node.init = this;
     }
+    std::unique_ptr<interfaces::Node> makeNode() override { return interfaces::MakeNode(m_node); }
+    std::unique_ptr<interfaces::Chain> makeChain() override { return interfaces::MakeChain(m_node); }
+    std::unique_ptr<interfaces::WalletClient> makeWalletClient(interfaces::Chain& chain) override
+    {
+        return MakeWalletClient(chain, *Assert(m_node.args));
+    }
+    std::unique_ptr<interfaces::Echo> makeEcho() override { return interfaces::MakeEcho(); }
     NodeContext& m_node;
 };
 } // namespace
diff --git a/src/interfaces/node.h b/src/interfaces/node.h
@@ -230,7 +230,7 @@ class Node
 };
 
 //! Return implementation of Node interface.
-std::unique_ptr<Node> MakeNode(NodeContext* context = nullptr);
+std::unique_ptr<Node> MakeNode(NodeContext& context);
 
 //! Block tip (could be a header or not, depends on the subscribed signal).
 struct BlockTip {
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
@@ -72,7 +72,7 @@ class NodeImpl : public Node
 private:
     ChainstateManager& chainman() { return *Assert(m_context->chainman); }
 public:
-    explicit NodeImpl(NodeContext* context) { setContext(context); }
+    explicit NodeImpl(NodeContext& context) { setContext(&context); }
     void initLogging() override { InitLogging(*Assert(m_context->args)); }
     void initParameterInteraction() override { InitParameterInteraction(*Assert(m_context->args)); }
     bilingual_str getWarnings() override { return GetWarnings(true); }
@@ -710,6 +710,6 @@ class ChainImpl : public Chain
 } // namespace node
 
 namespace interfaces {
-std::unique_ptr<Node> MakeNode(NodeContext* context) { return std::make_unique<node::NodeImpl>(context); }
+std::unique_ptr<Node> MakeNode(NodeContext& context) { return std::make_unique<node::NodeImpl>(context); }
 std::unique_ptr<Chain> MakeChain(NodeContext& context) { return std::make_unique<node::ChainImpl>(context); }
 } // namespace interfaces
diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp
@@ -11,6 +11,7 @@
 #include <chainparams.h>
 #include <init.h>
 #include <interfaces/handler.h>
+#include <interfaces/init.h>
 #include <interfaces/node.h>
 #include <node/context.h>
 #include <node/ui_interface.h>
@@ -275,10 +276,10 @@ void BitcoinApplication::createSplashScreen(const NetworkStyle *networkStyle)
     connect(this, &BitcoinApplication::requestedShutdown, m_splash, &QWidget::close);
 }
 
-void BitcoinApplication::setNode(interfaces::Node& node)
+void BitcoinApplication::createNode(interfaces::Init& init)
 {
     assert(!m_node);
-    m_node = &node;
+    m_node = init.makeNode();
     if (optionsModel) optionsModel->setNode(*m_node);
     if (m_splash) m_splash->setNode(*m_node);
 }
@@ -460,11 +461,13 @@ int GuiMain(int argc, char* argv[])
     util::WinCmdLineArgs winArgs;
     std::tie(argc, argv) = winArgs.get();
 #endif
-    SetupEnvironment();
-    util::ThreadSetInternalName("main");
 
     NodeContext node_context;
-    std::unique_ptr<interfaces::Node> node = interfaces::MakeNode(&node_context);
+    int unused_exit_status;
+    std::unique_ptr<interfaces::Init> init = interfaces::MakeNodeInit(node_context, argc, argv, unused_exit_status);
+
+    SetupEnvironment();
+    util::ThreadSetInternalName("main");
 
     // Subscribe to global signals from core
     boost::signals2::scoped_connection handler_message_box = ::uiInterface.ThreadSafeMessageBox_connect(noui_ThreadSafeMessageBox);
@@ -492,7 +495,6 @@ int GuiMain(int argc, char* argv[])
 
     /// 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:
-    node_context.args = &gArgs;
     SetupServerArgs(gArgs);
     SetupUIArgs(gArgs);
     std::string error;
@@ -623,7 +625,7 @@ int GuiMain(int argc, char* argv[])
     if (gArgs.GetBoolArg("-splash", DEFAULT_SPLASHSCREEN) && !gArgs.GetBoolArg("-min", false))
         app.createSplashScreen(networkStyle.data());
 
-    app.setNode(*node);
+    app.createNode(*init);
 
     int rv = EXIT_SUCCESS;
     try
diff --git a/src/qt/bitcoin.h b/src/qt/bitcoin.h
@@ -27,6 +27,9 @@ class PlatformStyle;
 class SplashScreen;
 class WalletController;
 class WalletModel;
+namespace interfaces {
+class Init;
+} // namespace interfaces
 
 
 /** Main Bitcoin application object */
@@ -51,6 +54,8 @@ class BitcoinApplication: public QApplication
     void createWindow(const NetworkStyle *networkStyle);
     /// Create splash screen
     void createSplashScreen(const NetworkStyle *networkStyle);
+    /// Create or spawn node
+    void createNode(interfaces::Init& init);
     /// Basic initialization, before starting initialization/shutdown thread. Return true on success.
     bool baseInitialize();
 
@@ -69,7 +74,6 @@ class BitcoinApplication: public QApplication
     void setupPlatformStyle();
 
     interfaces::Node& node() const { assert(m_node); return *m_node; }
-    void setNode(interfaces::Node& node);
 
 public Q_SLOTS:
     void initializeResult(bool success, interfaces::BlockAndHeaderTipInfo tip_info);
@@ -103,7 +107,7 @@ public Q_SLOTS:
     const PlatformStyle *platformStyle;
     std::unique_ptr<QWidget> shutdownWindow;
     SplashScreen* m_splash = nullptr;
-    interfaces::Node* m_node = nullptr;
+    std::unique_ptr<interfaces::Node> m_node;
 
     void startThread();
 };
diff --git a/src/qt/test/test_main.cpp b/src/qt/test/test_main.cpp
@@ -6,6 +6,7 @@
 #include <config/bitcoin-config.h>
 #endif
 
+#include <interfaces/init.h>
 #include <interfaces/node.h>
 #include <qt/bitcoin.h>
 #include <qt/initexecutor.h>
@@ -53,7 +54,8 @@ int main(int argc, char* argv[])
     }
 
     NodeContext node_context;
-    std::unique_ptr<interfaces::Node> node = interfaces::MakeNode(&node_context);
+    int unused_exit_status;
+    std::unique_ptr<interfaces::Init> init = interfaces::MakeNodeInit(node_context, argc, argv, unused_exit_status);
     gArgs.ForceSetArg("-listen", "0");
     gArgs.ForceSetArg("-listenonion", "0");
     gArgs.ForceSetArg("-discover", "0");
@@ -76,10 +78,9 @@ int main(int argc, char* argv[])
     // Don't remove this, it's needed to access
     // QApplication:: and QCoreApplication:: in the tests
     BitcoinApplication app;
-    app.setNode(*node);
     app.setApplicationName("Bitcoin-Qt-test");
+    app.createNode(*init);
 
-    app.node().context()->args = &gArgs;     // Make gArgs available in the NodeContext
     AppTests app_tests(app);
     if (QTest::qExec(&app_tests) != 0) {
         fInvalid = true;
diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp
@@ -138,6 +138,8 @@ void TestGUI(interfaces::Node& node)
     for (int i = 0; i < 5; ++i) {
         test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey()));
     }
+    auto wallet_client = interfaces::MakeWalletClient(*test.m_node.chain, *Assert(test.m_node.args));
+    test.m_node.wallet_client = wallet_client.get();
     node.setContext(&test.m_node);
     std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), "", CreateMockWalletDatabase());
     wallet->LoadWallet();
diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp
@@ -670,8 +670,9 @@ static RPCHelpMan echoipc()
         RPCExamples{HelpExampleCli("echo", "\"Hello world\"") +
                     HelpExampleRpc("echo", "\"Hello world\"")},
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue {
+            interfaces::Init& local_init = *EnsureAnyNodeContext(request.context).init;
             std::unique_ptr<interfaces::Echo> echo;
-            if (interfaces::Ipc* ipc = Assert(EnsureAnyNodeContext(request.context).init)->ipc()) {
+            if (interfaces::Ipc* ipc = local_init.ipc()) {
                 // Spawn a new bitcoin-node process and call makeEcho to get a
                 // client pointer to a interfaces::Echo instance running in
                 // that process. This is just for testing. A slightly more
@@ -689,7 +690,7 @@ static RPCHelpMan echoipc()
                 // interfaces::Echo object and return it so the `echoipc` RPC
                 // method will work, and the python test calling `echoipc`
                 // can expect the same result.
-                echo = interfaces::MakeEcho();
+                echo = local_init.makeEcho();
             }
             return echo->echo(request.params[0].get_str());
         },
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
@@ -114,7 +114,6 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::ve
     InitSignatureCache();
     InitScriptExecutionCache();
     m_node.chain = interfaces::MakeChain(m_node);
-    g_wallet_init_interface.Construct(m_node);
     fCheckBlockIndex = true;
     static bool noui_connected = false;
     if (!noui_connected) {
diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp
@@ -5,6 +5,7 @@
 
 #include <init.h>
 #include <interfaces/chain.h>
+#include <interfaces/init.h>
 #include <interfaces/wallet.h>
 #include <net.h>
 #include <node/context.h>
@@ -130,7 +131,7 @@ void WalletInit::Construct(NodeContext& node) const
         LogPrintf("Wallet disabled!\n");
         return;
     }
-    auto wallet_client = interfaces::MakeWalletClient(*node.chain, args);
+    auto wallet_client = node.init->makeWalletClient(*node.chain);
     node.wallet_client = wallet_client.get();
     node.chain_clients.emplace_back(std::move(wallet_client));
 }
