multiprocess: Add bitcoin-gui -ipcconnect option

contrib/devtools/circular-dependencies.py

@@ -15,7 +15,7 @@
 # define functions and variables declared in corresponding .h files is
 # incorrect.
 HEADER_MODULE_PATHS = [
-    'interfaces/'
+    'ipc/'
 ]
 
 def module_name(path):

doc/design/libraries.md

@@ -8,7 +8,7 @@
 | *libbitcoin_crypto*      | Hardware-optimized functions for data encryption, hashing, message authentication, and key derivation. |
 | *libbitcoin_kernel*      | Consensus engine and support library used for validation by *libbitcoin_node*. |
 | *libbitcoinqt*           | GUI functionality used by *bitcoin-qt* and *bitcoin-gui* executables. |
-| *libbitcoin_ipc*         | IPC functionality used by *bitcoin-node* and *bitcoin-gui* executables to communicate when [`-DENABLE_IPC=ON`](multiprocess.md) is used. |
+| *libbitcoin_ipc*         | IPC functionality used by *bitcoin-node*, *bitcoin-wallet*, *bitcoin-gui* executables to communicate when [`-DENABLE_IPC=ON`](multiprocess.md) is used. |
 | *libbitcoin_node*        | P2P and RPC server functionality used by *bitcoind* and *bitcoin-qt* executables. |
 | *libbitcoin_util*        | Home for common functionality shared by different executables and libraries. Similar to *libbitcoin_common*, but lower-level (see [Dependencies](#dependencies)). |
 | *libbitcoin_wallet*      | Wallet functionality used by *bitcoind* and *bitcoin-wallet* executables. |

doc/multiprocess.md

@@ -49,3 +49,9 @@ The multiprocess binaries currently function the same as the monolithic binaries
 
 In the future, after [#10102](https://github.com/bitcoin/bitcoin/pull/10102) they will have other differences. Specifically `bitcoin-gui` will spawn a `bitcoin-node` process to run P2P and RPC code, communicating with it across a socket pair, and `bitcoin-node` will spawn `bitcoin-wallet` to run wallet code, also communicating over a socket pair. This will let node, wallet, and GUI code run in separate address spaces for better isolation, and allow future improvements like being able to start and stop components independently on different machines and environments. [#19460](https://github.com/bitcoin/bitcoin/pull/19460) also adds a new `bitcoin-wallet -ipcconnect` option to allow new wallet processes to connect to an existing node process.
 And [#19461](https://github.com/bitcoin/bitcoin/pull/19461) adds a new `bitcoin-gui -ipcconnect` option to allow new GUI processes to connect to an existing node process.
+
+## Known issues
+
+- Unexpected socket disconnects aren't handled cleanly many places. Interface calls that used to never throw can now throw exceptions if a socket is disconnected (typically because a process on the other side of the connection has crashed or been killed), leading to errors.
+
+- Internally spawned bitcoin-node and bitcoin-wallet processes don't currently install signal handlers and so won't shut down cleanly if terminated with [CTRL-C](https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-595353238). Shutting down with `bitcoin-cli stop` should still shut down cleanly, and is a suggested alternative.

src/CMakeLists.txt

@@ -120,6 +120,7 @@ add_library(bitcoin_common STATIC EXCLUDE_FROM_ALL
   net_types.cpp
   netaddress.cpp
   netbase.cpp
+  node/interface_ui.cpp
   outputtype.cpp
   policy/feerate.cpp
   policy/policy.cpp
@@ -154,12 +155,18 @@ include(InstallBinaryComponent)
 if(ENABLE_WALLET)
   add_subdirectory(wallet)
 
-  if(BUILD_WALLET_TOOL)
+  if(BUILD_WALLET_TOOL OR ENABLE_IPC)
     add_executable(bitcoin-wallet
       bitcoin-wallet.cpp
-      init/bitcoin-wallet.cpp
       wallet/wallettool.cpp
     )
+    if(ENABLE_IPC)
+      # FIX: Dependency on kernel should be dropped. See BitcoinWalletInit constructor comment.
+      target_sources(bitcoin-wallet PRIVATE init/bitcoin-wallet-ipc.cpp kernel/context.cpp)
+      target_link_libraries(bitcoin-wallet bitcoin_ipc)
+    else()
+      target_sources(bitcoin-wallet PRIVATE init/bitcoin-wallet.cpp)
+    endif()
     add_windows_resources(bitcoin-wallet bitcoin-wallet-res.rc)
     add_windows_application_manifest(bitcoin-wallet)
     target_link_libraries(bitcoin-wallet
@@ -218,7 +225,6 @@ add_library(bitcoin_node STATIC EXCLUDE_FROM_ALL
   node/context.cpp
   node/database_args.cpp
   node/eviction.cpp
-  node/interface_ui.cpp
   node/interfaces.cpp
   node/kernel_notifications.cpp
   node/mempool_args.cpp

src/bitcoin-wallet.cpp

@@ -10,7 +10,9 @@
 #include <common/args.h>
 #include <common/system.h>
 #include <compat/compat.h>
+#include <interfaces/chain.h>
 #include <interfaces/init.h>
+#include <interfaces/ipc.h>
 #include <key.h>
 #include <logging.h>
 #include <pubkey.h>
@@ -28,7 +30,7 @@ using util::Join;
 
 const TranslateFn G_TRANSLATION_FUN{nullptr};
 
-static void SetupWalletToolArgs(ArgsManager& argsman)
+static void SetupWalletToolArgs(ArgsManager& argsman, bool can_connect_ipc)
 {
     SetupHelpOptions(argsman);
     SetupChainParamsBaseOptions(argsman);
@@ -39,6 +41,9 @@ static void SetupWalletToolArgs(ArgsManager& argsman)
     argsman.AddArg("-dumpfile=<file name>", "When used with 'dump', writes out the records to this file. When used with 'createfromdump', loads the records into a new wallet.", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::OPTIONS);
     argsman.AddArg("-debug=<category>", "Output debugging information (default: 0).", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
     argsman.AddArg("-printtoconsole", "Send trace/debug info to console (default: 1 when no -debug is true, 0 otherwise).", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
+    if (can_connect_ipc) {
+        argsman.AddArg("-ipcconnect=<address>", "Connect to bitcoin-node process in the background to perform online operations. Valid <address> values are 'auto' to try connecting to default socket in <datadir>/sockets/node.sock, but proceed offline if it isn't available, 'unix' to connect to the default socket and fail if it isn't available, 'unix:<socket path>' to connect to a socket at a nonstandard path, and -noipcconnect to not connect. Default value: auto", ArgsManager::ALLOW_ANY, OptionsCategory::IPC);
+    }
 
     argsman.AddCommand("info", "Get wallet info");
     argsman.AddCommand("create", "Create a new descriptor wallet file");
@@ -48,7 +53,6 @@ static void SetupWalletToolArgs(ArgsManager& argsman)
 
 static std::optional<int> WalletAppInit(ArgsManager& args, int argc, char* argv[])
 {
-    SetupWalletToolArgs(args);
     std::string error_message;
     if (!args.ParseParameters(argc, argv, error_message)) {
         tfm::format(std::cerr, "Error parsing command line arguments: %s\n", error_message);
@@ -108,6 +112,7 @@ MAIN_FUNCTION
     SetupEnvironment();
     RandomInit();
     try {
+        SetupWalletToolArgs(args, init->canConnectIpc());
         if (const auto maybe_exit{WalletAppInit(args, argc, argv)}) return *maybe_exit;
     } catch (const std::exception& e) {
         PrintExceptionContinue(&e, "WalletAppInit()");
@@ -128,7 +133,18 @@ MAIN_FUNCTION
     }
 
     ECC_Context ecc_context{};
-    if (!wallet::WalletTool::ExecuteWalletToolFunc(args, command->command)) {
+
+    std::unique_ptr<interfaces::Chain> chain;
+    if (interfaces::Ipc* ipc = init->ipc()) {
+        std::string address = args.GetArg("-ipcconnect", "auto");
+        if (auto init = ipc->connectAddress(address)) {
+            tfm::format(std::cout, "Connected to IPC address %s\n", address);
+            chain = init->makeChain();
+            ipc->addCleanup(*chain, [init = init.release()] { delete init; });
+        }
+    }
+
+    if (!wallet::WalletTool::ExecuteWalletToolFunc(args, chain.get(), command->command)) {
         return EXIT_FAILURE;
     }
     return EXIT_SUCCESS;

src/bitcoind.cpp

@@ -14,6 +14,7 @@
 #include <init.h>
 #include <interfaces/chain.h>
 #include <interfaces/init.h>
+#include <interfaces/ipc.h>
 #include <kernel/context.h>
 #include <node/context.h>
 #include <node/interface_ui.h>
@@ -113,7 +114,7 @@ static bool ParseArgs(NodeContext& node, int argc, char* argv[])
 {
     ArgsManager& args{*Assert(node.args)};
     // If Qt is used, parameters/bitcoin.conf are parsed in qt/bitcoin.cpp's main()
-    SetupServerArgs(args, node.init->canListenIpc());
+    SetupServerArgs(args, node.init->canConnectIpc(), node.init->canListenIpc());
     std::string error;
     if (!args.ParseParameters(argc, argv, error)) {
         return InitError(Untranslated(strprintf("Error parsing command line arguments: %s", error)));
@@ -183,7 +184,8 @@ static bool AppInit(NodeContext& node)
         // -server defaults to true for bitcoind but not for the GUI so do this here
         args.SoftSetBoolArg("-server", true);
         // Set this early so that parameter interactions go to console
-        InitLogging(args);
+        interfaces::Ipc* ipc = node.init->ipc();
+        InitLogging(args, ipc ? ipc->logSuffix() : nullptr);
         InitParameterInteraction(args);
         if (!AppInitBasicSetup(args, node.exit_status)) {
             // InitError will have been called with detailed error, which ends up on console

src/init.cpp

@@ -450,7 +450,7 @@ static void registerSignalHandler(int signal, void(*handler)(int))
 }
 #endif
 
-void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc)
+void SetupServerArgs(ArgsManager& argsman, bool can_connect_ipc, bool can_listen_ipc)
 {
     SetupHelpOptions(argsman);
     argsman.AddArg("-help-debug", "Print help message with debugging options and exit", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); // server-only for now
@@ -693,6 +693,9 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc)
     argsman.AddArg("-rpcwhitelistdefault", "Sets default behavior for rpc whitelisting. Unless rpcwhitelistdefault is set to 0, if any -rpcwhitelist is set, the rpc server acts as if all rpc users are subject to empty-unless-otherwise-specified whitelists. If rpcwhitelistdefault is set to 1 and no -rpcwhitelist is set, rpc server acts as if all rpc users are subject to empty whitelists.", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
     argsman.AddArg("-rpcworkqueue=<n>", strprintf("Set the maximum depth of the work queue to service RPC calls (default: %d)", DEFAULT_HTTP_WORKQUEUE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::RPC);
     argsman.AddArg("-server", "Accept command line and JSON-RPC commands", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
+    if (can_connect_ipc) {
+        argsman.AddArg("-ipcconnect=<address>", "Instead of starting a bitcoin-node process in the background, connect to the an existing process listening at the specified address. Valid <address> values are 'auto' to try connecting to default socket in <datadir>/sockets/node.sock, but start a bitcoin-node process if it isn't available, 'unix' to connect to the default socket and fail if it isn't available, 'unix:<socket path>' to connect to a socket at a nonstandard path, and -noipcconnect to not try to connect. Default value: auto", ArgsManager::ALLOW_ANY, OptionsCategory::IPC);
+    }
     if (can_listen_ipc) {
         argsman.AddArg("-ipcbind=<address>", "Bind to Unix socket address and listen for incoming connections. Valid address values are \"unix\" to listen on the default path, <datadir>/node.sock, or \"unix:/custom/path\" to specify a custom path. Can be specified multiple times to listen on multiple paths. Default behavior is not to listen on any path. If relative paths are specified, they are interpreted relative to the network data directory. If paths include any parent directory components and the parent directories do not exist, they will be created.", ArgsManager::ALLOW_ANY, OptionsCategory::IPC);
     }
@@ -825,9 +828,9 @@ void InitParameterInteraction(ArgsManager& args)
  * Note that this is called very early in the process lifetime, so you should be
  * careful about what global state you rely on here.
  */
-void InitLogging(const ArgsManager& args)
+void InitLogging(const ArgsManager& args, const char* log_suffix)
 {
-    init::SetLoggingOptions(args);
+    init::SetLoggingOptions(args, log_suffix);
     init::LogPackageVersion();
 }
 

src/init.h

@@ -33,7 +33,7 @@ bool ShutdownRequested(node::NodeContext& node);
 void Interrupt(node::NodeContext& node);
 void Shutdown(node::NodeContext& node);
 //!Initialize the logging infrastructure
-void InitLogging(const ArgsManager& args);
+void InitLogging(const ArgsManager& args, const char* log_suffix);
 //!Parameter interaction: change current parameters depending on various rules
 void InitParameterInteraction(ArgsManager& args);
 
@@ -74,7 +74,7 @@ bool AppInitMain(node::NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip
 /**
  * Register all arguments with the ArgsManager
  */
-void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc=false);
+void SetupServerArgs(ArgsManager& argsman, bool can_connect_ipc=false, bool can_listen_ipc=false);
 
 /** Validates requirements to run the indexes and spawns each index initial sync thread */
 bool StartIndexBackgroundSync(node::NodeContext& node);

src/init/bitcoin-gui.cpp

@@ -3,44 +3,37 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <init.h>
-#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/check.h>
 
 #include <memory>
 
+namespace ipc {
+namespace capnp {
+void SetupNodeClient(ipc::Context& context);
+} // namespace capnp
+} // namespace ipc
+
 namespace init {
 namespace {
 const char* EXE_NAME = "bitcoin-gui";
 
 class BitcoinGuiInit : public interfaces::Init
 {
 public:
-    BitcoinGuiInit(const char* arg0) : m_ipc(interfaces::MakeIpc(EXE_NAME, arg0, *this))
-    {
-        InitContext(m_node);
-        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) override
+    BitcoinGuiInit(const char* arg0) : m_ipc(interfaces::MakeIpc(EXE_NAME, ".gui", arg0, *this))
     {
-        return MakeWalletLoader(chain, *Assert(m_node.args));
+        ipc::capnp::SetupNodeClient(m_ipc->context());
     }
-    std::unique_ptr<interfaces::Echo> makeEcho() override { return interfaces::MakeEcho(); }
     interfaces::Ipc* ipc() override { return m_ipc.get(); }
+    bool canConnectIpc() override { return true; }
     // bitcoin-gui accepts -ipcbind option even though it does not use it
     // directly. It just returns true here to accept the option because
     // bitcoin-node accepts the option, and bitcoin-gui accepts all bitcoin-node
     // options and will start the node with those options.
     bool canListenIpc() override { return true; }
     const char* exeName() override { return EXE_NAME; }
-    node::NodeContext m_node;
     std::unique_ptr<interfaces::Ipc> m_ipc;
 };
 } // namespace

src/init/bitcoin-node.cpp

@@ -2,17 +2,29 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <chainparams.h>
 #include <init.h>
 #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 <ipc/context.h>
 #include <node/context.h>
 #include <util/check.h>
 
+#include <functional>
 #include <memory>
+#include <string>
+#include <utility>
+
+namespace ipc {
+namespace capnp {
+void SetupNodeServer(ipc::Context& context);
+std::string GlobalArgsNetwork();
+} // namespace capnp
+} // namespace ipc
 
 namespace init {
 namespace {
@@ -22,19 +34,22 @@ class BitcoinNodeInit : public interfaces::Init
 {
 public:
     BitcoinNodeInit(node::NodeContext& node, const char* arg0)
-        : m_node(node),
-          m_ipc(interfaces::MakeIpc(EXE_NAME, arg0, *this))
+        : m_node(node), m_ipc(interfaces::MakeIpc(EXE_NAME, "", arg0, *this))
     {
         InitContext(m_node);
         m_node.init = this;
+        // Extra initialization code that runs when a bitcoin-node process is
+        // spawned by a bitcoin-gui process, after the ArgsManager configuration
+        // is transferred from the parent process to the child process.
+        m_ipc->context().init_process = [this] {
+            InitLogging(*Assert(m_node.args), m_ipc->logSuffix());
+            InitParameterInteraction(*Assert(m_node.args));
+        };
+        ipc::capnp::SetupNodeServer(m_ipc->context());
     }
     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::Mining> makeMining() override { return interfaces::MakeMining(m_node); }
-    std::unique_ptr<interfaces::WalletLoader> makeWalletLoader(interfaces::Chain& chain) override
-    {
-        return MakeWalletLoader(chain, *Assert(m_node.args));
-    }
     std::unique_ptr<interfaces::Echo> makeEcho() override { return interfaces::MakeEcho(); }
     interfaces::Ipc* ipc() override { return m_ipc.get(); }
     bool canListenIpc() override { return true; }
@@ -49,9 +64,9 @@ namespace interfaces {
 std::unique_ptr<Init> MakeNodeInit(node::NodeContext& node, int argc, char* argv[], int& exit_status)
 {
     auto init = std::make_unique<init::BitcoinNodeInit>(node, argc > 0 ? argv[0] : "");
-    // Check if bitcoin-node is being invoked as an IPC server. If so, then
-    // bypass normal execution and just respond to requests over the IPC
-    // channel and return null.
+    // Check if bitcoin-node is being invoked as an IPC server by the gui. If
+    // so, then bypass normal execution and just respond to requests over the
+    // IPC channel and return null.
     if (init->m_ipc->startSpawnedProcess(argc, argv, exit_status)) {
         return nullptr;
     }