Merge bitcoin#22219: multiprocess: Start using init makeNode, makeCha…

…in, etc methods

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

src/Makefile.qt.include

@@ -416,14 +416,14 @@ bitcoin_qt_libtoolflags = $(AM_LIBTOOLFLAGS) --tag CXX
 
 qt_dash_qt_CPPFLAGS = $(bitcoin_qt_cppflags)
 qt_dash_qt_CXXFLAGS = $(bitcoin_qt_cxxflags)
-qt_dash_qt_SOURCES = $(bitcoin_qt_sources)
+qt_dash_qt_SOURCES = $(bitcoin_qt_sources) init/bitcoind.cpp
 qt_dash_qt_LDADD = $(bitcoin_qt_ldadd)
 qt_dash_qt_LDFLAGS = $(bitcoin_qt_ldflags)
 qt_dash_qt_LIBTOOLFLAGS = $(bitcoin_qt_libtoolflags)
 
 dash_gui_CPPFLAGS = $(bitcoin_qt_cppflags)
 dash_gui_CXXFLAGS = $(bitcoin_qt_cxxflags)
-dash_gui_SOURCES = $(bitcoin_qt_sources)
+dash_gui_SOURCES = $(bitcoin_qt_sources) init/bitcoind.cpp
 dash_gui_LDADD = $(bitcoin_qt_ldadd)
 dash_gui_LDFLAGS = $(bitcoin_qt_ldflags)
 dash_gui_LIBTOOLFLAGS = $(bitcoin_qt_libtoolflags)

src/Makefile.qttest.include

@@ -33,6 +33,7 @@ qt_test_test_dash_qt_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(BITCOIN_QT_
   $(QT_INCLUDES) $(QT_TEST_INCLUDES)
 
 qt_test_test_dash_qt_SOURCES = \
+  init/bitcoind.cpp \
   qt/test/apptests.cpp \
   qt/test/optiontests.cpp \
   qt/test/rpcnestedtests.cpp \

src/dummywallet.cpp

@@ -6,12 +6,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 {
@@ -80,4 +82,9 @@ std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet, const
     throw std::logic_error("Wallet function called in non-wallet build.");
 }
 
+std::unique_ptr<WalletClient> MakeWalletLoader(Chain& chain, ArgsManager& args)
+{
+    throw std::logic_error("Wallet function called in non-wallet build.");
+}
+
 } // namespace interfaces

src/init.cpp

@@ -29,6 +29,7 @@
 #include <index/blockfilterindex.h>
 #include <index/coinstatsindex.h>
 #include <index/txindex.h>
+#include <interfaces/init.h>
 #include <interfaces/node.h>
 #include <mapport.h>
 #include <node/miner.h>
@@ -1431,7 +1432,7 @@ bool AppInitLockDataDirectory()
 
 bool AppInitInterfaces(NodeContext& node)
 {
-    node.chain = interfaces::MakeChain(node);
+    node.chain = node.init->makeChain();
     return true;
 }
 

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::WalletLoader> makeWalletLoader(interfaces::Chain& chain, const std::unique_ptr<interfaces::CoinJoin::Loader>& loader) override
+    {
+        return MakeWalletLoader(chain, loader, *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;

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::WalletLoader> makeWalletLoader(interfaces::Chain& chain, const std::unique_ptr<interfaces::CoinJoin::Loader>& loader) override
+    {
+        return MakeWalletLoader(chain, loader, *Assert(m_node.args));
+    }
+    std::unique_ptr<interfaces::Echo> makeEcho() override { return interfaces::MakeEcho(); }
     NodeContext& m_node;
 };
 } // namespace

src/interfaces/init.cpp

@@ -11,7 +11,7 @@
 namespace interfaces {
 std::unique_ptr<Node> Init::makeNode() { return {}; }
 std::unique_ptr<Chain> Init::makeChain() { return {}; }
-std::unique_ptr<WalletLoader> Init::makeWalletLoader(Chain& chain) { return {}; }
+std::unique_ptr<WalletLoader> Init::makeWalletLoader(Chain& chain, const std::unique_ptr<CoinJoin::Loader>&) { return {}; }
 std::unique_ptr<Echo> Init::makeEcho() { return {}; }
 Ipc* Init::ipc() { return nullptr; }
 } // namespace interfaces

src/interfaces/init.h

@@ -16,6 +16,11 @@ class Ipc;
 class Node;
 class WalletLoader;
 
+namespace CoinJoin
+{
+    class Loader;
+}
+
 //! Initial interface created when a process is first started, and used to give
 //! and get access to other interfaces (Node, Chain, Wallet, etc).
 //!
@@ -29,7 +34,7 @@ class Init
     virtual ~Init() = default;
     virtual std::unique_ptr<Node> makeNode();
     virtual std::unique_ptr<Chain> makeChain();
-    virtual std::unique_ptr<WalletLoader> makeWalletLoader(Chain& chain);
+    virtual std::unique_ptr<interfaces::WalletLoader> makeWalletLoader(interfaces::Chain&, const std::unique_ptr<CoinJoin::Loader>&);
     virtual std::unique_ptr<Echo> makeEcho();
     virtual Ipc* ipc();
 };

src/interfaces/node.h

@@ -362,7 +362,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 {

src/node/interfaces.cpp

@@ -305,7 +305,7 @@ class NodeImpl : public Node
     MasternodeSyncImpl m_masternodeSync;
     CoinJoinOptionsImpl m_coinjoin;
 
-    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); }
@@ -1026,6 +1026,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& node) { return std::make_unique<node::ChainImpl>(node); }
 } // namespace interfaces

src/qt/bitcoin.cpp

@@ -13,6 +13,7 @@
 #include <fs.h>
 #include <init.h>
 #include <interfaces/handler.h>
+#include <interfaces/init.h>
 #include <interfaces/node.h>
 #include <net.h>
 #include <node/context.h>
@@ -269,10 +270,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);
 }
@@ -478,11 +479,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);
@@ -504,7 +507,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;
@@ -729,7 +731,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

src/qt/bitcoin.h

@@ -26,6 +26,9 @@ class PaymentServer;
 class SplashScreen;
 class WalletController;
 class WalletModel;
+namespace interfaces {
+class Init;
+} // namespace interfaces
 
 
 /** Main Bitcoin application object */
@@ -50,6 +53,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();
 
@@ -65,7 +70,6 @@ class BitcoinApplication: public QApplication
     WId getMainWinId() const;
 
     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);
@@ -102,7 +106,7 @@ public Q_SLOTS:
     int returnValue;
     std::unique_ptr<QWidget> shutdownWindow;
     SplashScreen* m_splash = nullptr;
-    interfaces::Node* m_node = nullptr;
+    std::unique_ptr<interfaces::Node> m_node;
 
     void startThread();
 };

src/qt/test/test_main.cpp

@@ -7,6 +7,7 @@
 #include <config/bitcoin-config.h>
 #endif
 
+#include <interfaces/init.h>
 #include <interfaces/node.h>
 #include <qt/bitcoin.h>
 #include <qt/test/apptests.h>
@@ -62,7 +63,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");
@@ -81,10 +83,8 @@ int main(int argc, char* argv[])
     #endif
 
     BitcoinApplication app;
-    app.setNode(*node);
     app.setApplicationName("Dash-Qt-test");
-
-    app.node().context()->args = &gArgs;     // Make gArgs available in the NodeContext
+    app.createNode(*init);
 
     int num_test_failures{0};
 

src/qt/test/wallettests.cpp

@@ -108,6 +108,8 @@ void TestGUI(interfaces::Node& node)
     for (int i = 0; i < 5; ++i) {
         test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey()));
     }
+    auto wallet_loader = interfaces::MakeWalletLoader(*test.m_node.chain, test.m_node.coinjoin_loader, *Assert(test.m_node.args));
+    test.m_node.wallet_loader = wallet_loader.get();
     node.setContext(&test.m_node);
     const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), node.context()->coinjoin_loader.get(), "", CreateMockWalletDatabase());
     AddWallet(wallet);

src/qt/transactionrecord.cpp

@@ -27,7 +27,7 @@ bool TransactionRecord::showTransaction()
 /*
  * Decompose CWallet transaction to model transaction records.
  */
-QList<TransactionRecord> TransactionRecord::decomposeTransaction(interfaces::Wallet& wallet, const interfaces::WalletTx& wtx)
+QList<TransactionRecord> TransactionRecord::decomposeTransaction(interfaces::Node& node, interfaces::Wallet& wallet, const interfaces::WalletTx& wtx)
 {
     QList<TransactionRecord> parts;
     int64_t nTime = wtx.time;
@@ -36,8 +36,7 @@ QList<TransactionRecord> TransactionRecord::decomposeTransaction(interfaces::Wal
     CAmount nNet = nCredit - nDebit;
     uint256 hash = wtx.tx->GetHash();
     std::map<std::string, std::string> mapValue = wtx.value_map;
-    auto node = interfaces::MakeNode();
-    auto& coinJoinOptions = node->coinJoinOptions();
+    auto& coinJoinOptions = node.coinJoinOptions();
 
     if (nNet > 0 || wtx.is_coinbase || wtx.is_platform_transfer)
     {

src/qt/transactionrecord.h

@@ -129,7 +129,7 @@ class TransactionRecord
     /** Decompose CWallet transaction to model transaction records.
      */
     static bool showTransaction();
-    static QList<TransactionRecord> decomposeTransaction(interfaces::Wallet& wallet, const interfaces::WalletTx& wtx);
+    static QList<TransactionRecord> decomposeTransaction(interfaces::Node& node, interfaces::Wallet& wallet, const interfaces::WalletTx& wtx);
 
     /** @name Immutable transaction attributes
       @{*/

src/qt/transactiontablemodel.cpp

@@ -114,7 +114,7 @@ class TransactionTablePriv
         try {
             for (const auto& wtx : wallet.getWalletTxs()) {
                 if (TransactionRecord::showTransaction()) {
-                    cachedWallet.append(TransactionRecord::decomposeTransaction(wallet, wtx));
+                    cachedWallet.append(TransactionRecord::decomposeTransaction(parent->walletModel->node(), wallet, wtx));
                 }
             }
         } catch(const std::exception& e) {
@@ -174,7 +174,7 @@ class TransactionTablePriv
                 }
                 // Added -- insert at the right position
                 QList<TransactionRecord> toInsert =
-                        TransactionRecord::decomposeTransaction(wallet, wtx);
+                        TransactionRecord::decomposeTransaction(parent->walletModel->node(), wallet, wtx);
                 if(!toInsert.isEmpty()) /* only if something to insert */
                 {
                     parent->beginInsertRows(QModelIndex(), lowerIndex, lowerIndex+toInsert.size()-1);

src/rpc/misc.cpp

@@ -1380,8 +1380,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
@@ -1399,7 +1400,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());
         },

src/test/util/setup_common.cpp

@@ -192,10 +192,6 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::ve
                                                m_node.args->GetIntArg("-checkaddrman", 0));
     m_node.connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); // Deterministic randomness for tests.
 
-    // while g_wallet_init_interface is init here at very early stage
-    // we can't get rid of unique_ptr from wallet/context.h
-    // TODO: remove unique_ptr from wallet/context.h after bitcoin/bitcoin#22219
-    g_wallet_init_interface.Construct(m_node);
     fCheckBlockIndex = true;
     m_node.evodb = std::make_unique<CEvoDB>(1 << 20, true, true);
     m_node.mnhf_manager = std::make_unique<CMNHFManager>(*m_node.evodb);

src/wallet/init.cpp

@@ -6,6 +6,7 @@
 #include <chainparams.h>
 #include <init.h>
 #include <interfaces/chain.h>
+#include <interfaces/init.h>
 #include <interfaces/wallet.h>
 #include <net.h>
 #include <node/context.h>
@@ -187,7 +188,7 @@ void WalletInit::Construct(NodeContext& node) const
         LogPrintf("Wallet disabled!\n");
         return;
     }
-    auto wallet_loader = interfaces::MakeWalletLoader(*node.chain, node.coinjoin_loader, args);
+    auto wallet_loader = node.init->makeWalletLoader(*node.chain, node.coinjoin_loader);
     node.wallet_loader = wallet_loader.get();
     node.chain_clients.emplace_back(std::move(wallet_loader));
 }