From 453b0fa286e5dce0af682b7b73684dd6415a50de Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 20 Aug 2025 15:52:21 -0400 Subject: [PATCH] bitcoin: Make wrapper not require -m Choose the right binary by default if an IPC option is specified --- src/CMakeLists.txt | 2 +- src/bitcoin.cpp | 32 +++++++++++++++++++++++++++++--- src/common/args.cpp | 8 +++++++- src/common/args.h | 8 +++++++- test/functional/tool_bitcoin.py | 17 +++++++++++++---- test/lint/check-doc.py | 2 +- 6 files changed, 58 insertions(+), 11 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 9abbf9586b8..f9934bb5906 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -292,7 +292,7 @@ if(BUILD_BITCOIN_BIN) add_executable(bitcoin bitcoin.cpp) add_windows_resources(bitcoin bitcoin-res.rc) add_windows_application_manifest(bitcoin) - target_link_libraries(bitcoin core_interface bitcoin_util) + target_link_libraries(bitcoin core_interface bitcoin_common bitcoin_util) install_binary_component(bitcoin HAS_MANPAGE) endif() diff --git a/src/bitcoin.cpp b/src/bitcoin.cpp index c3278274153..c1a5fce33af 100644 --- a/src/bitcoin.cpp +++ b/src/bitcoin.cpp @@ -5,6 +5,7 @@ #include // IWYU pragma: keep #include +#include #include #include #include @@ -47,7 +48,7 @@ Run '%s help' to see additional commands (e.g. for testing and debugging). )"; struct CommandLine { - bool use_multiprocess{false}; + std::optional use_multiprocess; bool show_version{false}; bool show_help{false}; std::string_view command; @@ -55,6 +56,7 @@ struct CommandLine { }; CommandLine ParseCommandLine(int argc, char* argv[]); +bool UseMultiprocess(const CommandLine& cmd); static void ExecCommand(const std::vector& args, std::string_view argv0); int main(int argc, char* argv[]) @@ -78,9 +80,9 @@ int main(int argc, char* argv[]) return EXIT_FAILURE; } } else if (cmd.command == "gui") { - args.emplace_back(cmd.use_multiprocess ? "bitcoin-gui" : "bitcoin-qt"); + args.emplace_back(UseMultiprocess(cmd) ? "bitcoin-gui" : "bitcoin-qt"); } else if (cmd.command == "node") { - args.emplace_back(cmd.use_multiprocess ? "bitcoin-node" : "bitcoind"); + args.emplace_back(UseMultiprocess(cmd) ? "bitcoin-node" : "bitcoind"); } else if (cmd.command == "rpc") { args.emplace_back("bitcoin-cli"); // Since "bitcoin rpc" is a new interface that doesn't need to be @@ -143,6 +145,30 @@ CommandLine ParseCommandLine(int argc, char* argv[]) return cmd; } +bool UseMultiprocess(const CommandLine& cmd) +{ + // If -m or -M options were explicitly specified, there is no need to + // further parse arguments to determine which to use. + if (cmd.use_multiprocess) return *cmd.use_multiprocess; + + ArgsManager args; + args.SetDefaultFlags(ArgsManager::ALLOW_ANY); + std::string error_message; + auto argv{cmd.args}; + argv.insert(argv.begin(), nullptr); + if (!args.ParseParameters(argv.size(), argv.data(), error_message)) { + tfm::format(std::cerr, "Warning: failed to parse subcommand command line options: %s\n", error_message); + } + if (!args.ReadConfigFiles(error_message, true)) { + tfm::format(std::cerr, "Warning: failed to parse subcommand config: %s\n", error_message); + } + args.SelectConfigNetwork(args.GetChainTypeString()); + + // If any -ipc* options are set these need to be processed by a + // multiprocess-capable binary. + return args.IsArgSet("-ipcbind") || args.IsArgSet("-ipcconnect") || args.IsArgSet("-ipcfd"); +} + //! Execute the specified bitcoind, bitcoin-qt or other command line in `args` //! using src, bin and libexec directory paths relative to this executable, where //! the path to this executable is specified in `wrapper_argv0`. diff --git a/src/common/args.cpp b/src/common/args.cpp index ff30ec5b8c1..d44cd4319b6 100644 --- a/src/common/args.cpp +++ b/src/common/args.cpp @@ -266,7 +266,13 @@ std::optional ArgsManager::GetArgFlags(const std::string& name) co return search->second.m_flags; } } - return std::nullopt; + return m_default_flags; +} + +void ArgsManager::SetDefaultFlags(std::optional flags) +{ + LOCK(cs_args); + m_default_flags = flags; } fs::path ArgsManager::GetPathArg(std::string arg, const fs::path& default_value) const diff --git a/src/common/args.h b/src/common/args.h index da19cbda66f..d907ad7663d 100644 --- a/src/common/args.h +++ b/src/common/args.h @@ -137,6 +137,7 @@ protected: std::string m_network GUARDED_BY(cs_args); std::set m_network_only_args GUARDED_BY(cs_args); std::map> m_available_args GUARDED_BY(cs_args); + std::optional m_default_flags GUARDED_BY(cs_args){}; bool m_accept_any_command GUARDED_BY(cs_args){true}; std::list m_config_sections GUARDED_BY(cs_args); std::optional m_config_path GUARDED_BY(cs_args); @@ -375,10 +376,15 @@ protected: /** * Return Flags for known arg. - * Return nullopt for unknown arg. + * Return default flags for unknown arg. */ std::optional GetArgFlags(const std::string& name) const; + /** + * Set default flags to return for an unknown arg. + */ + void SetDefaultFlags(std::optional); + /** * Get settings file path, or return false if read-write settings were * disabled with -nosettings. diff --git a/test/functional/tool_bitcoin.py b/test/functional/tool_bitcoin.py index dcfdb2d774b..1112e02e090 100755 --- a/test/functional/tool_bitcoin.py +++ b/test/functional/tool_bitcoin.py @@ -7,7 +7,10 @@ from test_framework.test_framework import ( BitcoinTestFramework, SkipTest, ) -from test_framework.util import assert_equal +from test_framework.util import ( + append_config, + assert_equal, +) import platform import re @@ -55,12 +58,11 @@ class ToolBitcoinTest(BitcoinTestFramework): raise RuntimeError(f"Unexpected output from {node.args + extra_args}: {out=!r} {err=!r} {ret=!r}") from e def run_test(self): + node = self.nodes[0] + self.log.info("Ensure bitcoin node command invokes bitcoind by default") self.test_args([], [], expect_exe="bitcoind") - self.log.info("Ensure bitcoin command does not accept -ipcbind by default") - self.test_args(["-M"], ["-ipcbind=unix"], expect_error='Error: Error parsing command line arguments: Invalid parameter -ipcbind=unix') - self.log.info("Ensure bitcoin -M invokes bitcoind") self.test_args(["-M"], [], expect_exe="bitcoind") @@ -74,6 +76,13 @@ class ToolBitcoinTest(BitcoinTestFramework): self.log.info("Ensure bitcoin -m does accept -ipcbind") self.test_args(["-m"], ["-ipcbind=unix"], expect_exe="bitcoin-node") + self.log.info("Ensure bitcoin accepts -ipcbind by default") + self.test_args([], ["-ipcbind=unix"], expect_exe="bitcoin-node") + + self.log.info("Ensure bitcoin recognizes -ipcbind in config file") + append_config(node.datadir_path, ["ipcbind=unix"]) + self.test_args([], [], expect_exe="bitcoin-node") + def get_node_output(node): ret = node.process.wait(timeout=60) diff --git a/test/lint/check-doc.py b/test/lint/check-doc.py index 3e9e5ba230a..e47ff30f31c 100755 --- a/test/lint/check-doc.py +++ b/test/lint/check-doc.py @@ -23,7 +23,7 @@ CMD_GREP_WALLET_ARGS = r"git grep --function-context 'void WalletInit::AddWallet CMD_GREP_WALLET_HIDDEN_ARGS = r"git grep --function-context 'void DummyWalletInit::AddWalletOptions' -- {}".format(CMD_ROOT_DIR) CMD_GREP_DOCS = r"git grep --perl-regexp '{}' {}".format(REGEX_DOC, CMD_ROOT_DIR) # list unsupported, deprecated and duplicate args as they need no documentation -SET_DOC_OPTIONAL = set(['-h', '-?', '-dbcrashratio', '-forcecompactdb']) +SET_DOC_OPTIONAL = set(['-h', '-?', '-dbcrashratio', '-forcecompactdb', '-ipcconnect', '-ipcfd']) def lint_missing_argument_documentation():