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/bitcoind.cpp b/src/bitcoind.cpp index ceb3c99410c..a4373dafdf2 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -132,11 +132,16 @@ static bool ParseArgs(NodeContext& node, int argc, char* argv[]) return true; } -static bool ProcessInitCommands(ArgsManager& args) +static bool ProcessInitCommands(interfaces::Init& init, ArgsManager& args) { // Process help and version before taking care about datadir if (HelpRequested(args) || args.GetBoolArg("-version", false)) { - std::string strUsage = CLIENT_NAME " daemon version " + FormatFullVersion() + "\n"; + std::string strUsage = CLIENT_NAME " daemon version " + FormatFullVersion(); + if (const char* exe_name{init.exeName()}) { + strUsage += " "; + strUsage += exe_name; + } + strUsage += "\n"; if (args.GetBoolArg("-version", false)) { strUsage += FormatParagraph(LicenseInfo()); @@ -277,7 +282,7 @@ MAIN_FUNCTION ArgsManager& args = *Assert(node.args); if (!ParseArgs(node, argc, argv)) return EXIT_FAILURE; // Process early info return commands such as -help or -version - if (ProcessInitCommands(args)) return EXIT_SUCCESS; + if (ProcessInitCommands(*init, args)) return EXIT_SUCCESS; // Start application if (!AppInit(node) || !Assert(node.shutdown_signal)->wait()) { 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/src/init/bitcoin-gui.cpp b/src/init/bitcoin-gui.cpp index eae30bc995a..aca3fbe1c44 100644 --- a/src/init/bitcoin-gui.cpp +++ b/src/init/bitcoin-gui.cpp @@ -39,6 +39,7 @@ public: // 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 m_ipc; }; diff --git a/src/init/bitcoin-node.cpp b/src/init/bitcoin-node.cpp index 3f8c50b8d66..e5f1411fdb5 100644 --- a/src/init/bitcoin-node.cpp +++ b/src/init/bitcoin-node.cpp @@ -38,6 +38,7 @@ public: std::unique_ptr makeEcho() override { return interfaces::MakeEcho(); } interfaces::Ipc* ipc() override { return m_ipc.get(); } bool canListenIpc() override { return true; } + const char* exeName() override { return EXE_NAME; } node::NodeContext& m_node; std::unique_ptr m_ipc; }; diff --git a/src/init/bitcoin-qt.cpp b/src/init/bitcoin-qt.cpp index 5209c729731..05f3bc32d7d 100644 --- a/src/init/bitcoin-qt.cpp +++ b/src/init/bitcoin-qt.cpp @@ -16,6 +16,8 @@ namespace init { namespace { +const char* EXE_NAME = "bitcoin-qt"; + class BitcoinQtInit : public interfaces::Init { public: @@ -32,6 +34,7 @@ public: return MakeWalletLoader(chain, *Assert(m_node.args)); } std::unique_ptr makeEcho() override { return interfaces::MakeEcho(); } + const char* exeName() override { return EXE_NAME; } node::NodeContext m_node; }; } // namespace diff --git a/src/init/bitcoind.cpp b/src/init/bitcoind.cpp index 48be8831d25..0b0ab3f8fa8 100644 --- a/src/init/bitcoind.cpp +++ b/src/init/bitcoind.cpp @@ -18,6 +18,8 @@ using node::NodeContext; namespace init { namespace { +const char* EXE_NAME = "bitcoind"; + class BitcoindInit : public interfaces::Init { public: @@ -34,6 +36,7 @@ public: return MakeWalletLoader(chain, *Assert(m_node.args)); } std::unique_ptr makeEcho() override { return interfaces::MakeEcho(); } + const char* exeName() override { return EXE_NAME; } NodeContext& m_node; }; } // namespace diff --git a/src/interfaces/init.h b/src/interfaces/init.h index b496ada05f4..030ce306c00 100644 --- a/src/interfaces/init.h +++ b/src/interfaces/init.h @@ -38,6 +38,7 @@ public: virtual std::unique_ptr makeEcho() { return nullptr; } virtual Ipc* ipc() { return nullptr; } virtual bool canListenIpc() { return false; } + virtual const char* exeName() { return nullptr; } }; //! Return implementation of Init interface for the node process. If the argv diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 633d9448e62..41c2aaa04fc 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -337,6 +337,7 @@ BASE_SCRIPTS = [ 'p2p_tx_privacy.py', 'rpc_getdescriptoractivity.py', 'rpc_scanblocks.py', + 'tool_bitcoin.py', 'p2p_sendtxrcncl.py', 'rpc_scantxoutset.py', 'feature_unsupported_utxo_db.py', diff --git a/test/functional/tool_bitcoin.py b/test/functional/tool_bitcoin.py new file mode 100755 index 00000000000..1112e02e090 --- /dev/null +++ b/test/functional/tool_bitcoin.py @@ -0,0 +1,111 @@ +#!/usr/bin/env python3 +# Copyright (c) The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test the bitcoin wrapper tool.""" +from test_framework.test_framework import ( + BitcoinTestFramework, + SkipTest, +) +from test_framework.util import ( + append_config, + assert_equal, +) + +import platform +import re + + +class ToolBitcoinTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 1 + + def skip_test_if_missing_module(self): + # Skip test on windows because currently when `bitcoin node -version` is + # run on windows, python doesn't capture output from the child + # `bitcoind` and `bitcoin-node` process started with _wexecvp, and + # stdout/stderr are always empty. See + # https://github.com/bitcoin/bitcoin/pull/33229#issuecomment-3265524908 + if platform.system() == "Windows": + raise SkipTest("Test does not currently work on windows") + + def setup_network(self): + """Set up nodes normally, but save a copy of their arguments before starting them.""" + self.add_nodes(self.num_nodes, self.extra_args) + node_argv = self.get_binaries().node_argv() + self.node_options = [node.args[len(node_argv):] for node in self.nodes] + assert all(node.args[:len(node_argv)] == node_argv for node in self.nodes) + + def set_cmd_args(self, node, args): + """Set up node so it will be started through bitcoin wrapper command with specified arguments.""" + node.args = [self.binary_paths.bitcoin_bin] + args + ["node"] + self.node_options[node.index] + + def test_args(self, cmd_args, node_args, expect_exe=None, expect_error=None): + node = self.nodes[0] + self.set_cmd_args(node, cmd_args) + extra_args = node_args + ["-version"] + if expect_error is not None: + node.assert_start_raises_init_error(expected_msg=expect_error, extra_args=extra_args) + else: + assert expect_exe + node.start(extra_args=extra_args) + ret, out, err = get_node_output(node) + try: + assert_equal(get_exe_name(out), expect_exe.encode()) + assert_equal(err, b"") + except Exception as e: + 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 -M invokes bitcoind") + self.test_args(["-M"], [], expect_exe="bitcoind") + + self.log.info("Ensure bitcoin -M does not accept -ipcbind") + self.test_args(["-M"], ["-ipcbind=unix"], expect_error='Error: Error parsing command line arguments: Invalid parameter -ipcbind=unix') + + if self.is_ipc_compiled(): + self.log.info("Ensure bitcoin -m invokes bitcoin-node") + self.test_args(["-m"], [], expect_exe="bitcoin-node") + + 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) + node.stdout.seek(0) + node.stderr.seek(0) + out = node.stdout.read() + err = node.stderr.read() + node.stdout.close() + node.stderr.close() + + # Clean up TestNode state + node.running = False + node.process = None + node.rpc_connected = False + node.rpc = None + + return ret, out, err + + +def get_exe_name(version_str): + """Get exe name from last word of first line of version string.""" + return re.match(rb".*?(\S+)\s*?(?:\n|$)", version_str.strip()).group(1) + + +if __name__ == '__main__': + ToolBitcoinTest(__file__).main() 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():