Merge 80c0927769 into a50af6e4c4

src/bitcoin-wallet.cpp

@@ -36,19 +36,19 @@ static void SetupWalletToolArgs(ArgsManager& argsman)
     argsman.AddArg("-version", "Print version and exit", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-datadir=<dir>", "Specify data directory", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::OPTIONS);
     argsman.AddArg("-wallet=<wallet-name>", "Specify wallet name", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::OPTIONS);
-    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("-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::COMMAND_OPTIONS);
     argsman.AddArg("-debug=<category>", "Output debugging information (default: 0).", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
-    argsman.AddArg("-descriptors", "Create descriptors wallet. Only for 'create'", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    argsman.AddArg("-descriptors", "Create descriptors wallet.", ArgsManager::ALLOW_ANY, OptionsCategory::COMMAND_OPTIONS);
-    argsman.AddArg("-legacy", "Create legacy wallet. Only for 'create'", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    argsman.AddArg("-legacy", "Create legacy wallet.", ArgsManager::ALLOW_ANY, OptionsCategory::COMMAND_OPTIONS);
-    argsman.AddArg("-format=<format>", "The format of the wallet file to create. Either \"bdb\" or \"sqlite\". Only used with 'createfromdump'", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    argsman.AddArg("-format=<format>", "The format of the wallet file to create. Either \"bdb\" or \"sqlite\".", ArgsManager::ALLOW_ANY, OptionsCategory::COMMAND_OPTIONS);
     argsman.AddArg("-printtoconsole", "Send trace/debug info to console (default: 1 when no -debug is true, 0 otherwise).", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
     argsman.AddArg("-withinternalbdb", "Use the internal Berkeley DB parser when dumping a Berkeley DB wallet file (default: false)", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
 
     argsman.AddCommand("info", "Get wallet info");
-    argsman.AddCommand("create", "Create new wallet file");
+    argsman.AddCommand("create", "Create new wallet file", {"-legacy", "-descriptors"});
     argsman.AddCommand("salvage", "Attempt to recover private keys from a corrupt wallet. Warning: 'salvage' is experimental.");
-    argsman.AddCommand("dump", "Print out all of the wallet key-value records");
+    argsman.AddCommand("dump", "Print out all of the wallet key-value records", {"-dumpfile"});
-    argsman.AddCommand("createfromdump", "Create new wallet file from dumped records");
+    argsman.AddCommand("createfromdump", "Create new wallet file from dumped records", {"-dumpfile", "-format"});
 }
 
 static std::optional<int> WalletAppInit(ArgsManager& args, int argc, char* argv[])

src/common/args.cpp

@@ -359,6 +359,29 @@ std::optional<const ArgsManager::Command> ArgsManager::GetCommand() const
     return ret;
 }
 
+bool ArgsManager::CheckCommandOptions(const std::string& command, std::vector<std::string>* errors) const
+{
+    LOCK(cs_args);
+
+    auto command_options = m_available_args.find(OptionsCategory::COMMAND_OPTIONS);
+    if (command_options == m_available_args.end()) return true;
+
+    const std::set<std::string> dummy;
+    auto command_args = m_command_args.find(command);
+    const std::set<std::string>& valid_opts = (command_args == m_command_args.end() ? dummy : command_args->second);
+
+    bool ok = true;
+    for (const auto& opts : command_options->second) {
+        if (!IsArgSet(opts.first)) continue;
+        if (valid_opts.count(opts.first)) continue;
+        if (errors != nullptr) {
+            errors->emplace_back(strprintf("The %s option cannot be used with the '%s' command.", opts.first, command));
+            ok = false;
+        }
+    }
+    return ok;
+}
+
 std::vector<std::string> ArgsManager::GetArgs(const std::string& strArg) const
 {
     std::vector<std::string> result;
@@ -549,7 +572,7 @@ void ArgsManager::ForceSetArg(const std::string& strArg, const std::string& strV
     m_settings.forced_settings[SettingName(strArg)] = strValue;
 }
 
-void ArgsManager::AddCommand(const std::string& cmd, const std::string& help)
+void ArgsManager::AddCommand(const std::string& cmd, const std::string& help, std::set<std::string>&& options)
 {
     Assert(cmd.find('=') == std::string::npos);
     Assert(cmd.at(0) != '-');
@@ -558,6 +581,9 @@ void ArgsManager::AddCommand(const std::string& cmd, const std::string& help)
     m_accept_any_command = false; // latch to false
     std::map<std::string, Arg>& arg_map = m_available_args[OptionsCategory::COMMANDS];
     auto ret = arg_map.emplace(cmd, Arg{"", help, ArgsManager::COMMAND});
+    if (!options.empty()) {
+        m_command_args.try_emplace(cmd, std::move(options));
+    }
     Assert(ret.second); // Fail on duplicate commands
 }
 
@@ -606,14 +632,46 @@ void ArgsManager::CheckMultipleCLIArgs() const
     }
 }
 
+namespace {
+/** Helper class for iterating over COMMAND_OPTIONS applicable to a given command */
+template <typename T>
+class CommandOptionsGetter
+{
+private:
+    const typename T::const_iterator m_end;
+    const typename T::const_iterator m_iter;
+public:
+    CommandOptionsGetter(const T& available_args)
+    : m_end{available_args.end()},
+      m_iter{available_args.find(OptionsCategory::COMMAND_OPTIONS)}
+    {
+    }
+
+    template <typename Fn>
+    void Iterate(const std::set<std::string>& select, bool with_debug, Fn&& fn) const
+    {
+        if (select.empty()) return;
+        if (m_iter == m_end) return;
+        for (const auto& [cmdopt_name, cmdopt_info] : m_iter->second) {
+            if (!with_debug && (cmdopt_info.m_flags & ArgsManager::DEBUG_ONLY)) continue;
+            if (!select.count(cmdopt_name)) continue;
+            fn(cmdopt_name, cmdopt_info);
+        }
+    }
+};
+} // anonymous namespace
+
 std::string ArgsManager::GetHelpMessage() const
 {
     const bool show_debug = GetBoolArg("-help-debug", false);
 
     std::string usage;
     LOCK(cs_args);
-    for (const auto& arg_map : m_available_args) {
+
-        switch(arg_map.first) {
+    const auto command_options = CommandOptionsGetter(m_available_args);
+
+    for (const auto& [category, category_args] : m_available_args) {
+        switch(category) {
             case OptionsCategory::OPTIONS:
                 usage += HelpMessageGroup("Options:");
                 break;
@@ -659,22 +717,29 @@ std::string ArgsManager::GetHelpMessage() const
             case OptionsCategory::CLI_COMMANDS:
                 usage += HelpMessageGroup("CLI Commands:");
                 break;
+            case OptionsCategory::COMMAND_OPTIONS:
+                break;
             default:
                 break;
         }
 
-        // When we get to the hidden options, stop
+        if (category == OptionsCategory::COMMAND_OPTIONS) continue;
-        if (arg_map.first == OptionsCategory::HIDDEN) break;
 
-        for (const auto& arg : arg_map.second) {
+        // When we get to the hidden options, stop
-            if (show_debug || !(arg.second.m_flags & ArgsManager::DEBUG_ONLY)) {
+        if (category == OptionsCategory::HIDDEN) break;
-                std::string name;
+
-                if (arg.second.m_help_param.empty()) {
+        for (const auto& [arg_name, arg_info] : category_args) {
-                    name = arg.first;
+            if (show_debug || !(arg_info.m_flags & ArgsManager::DEBUG_ONLY)) {
-                } else {
+                usage += HelpMessageOpt(arg_name, arg_info.m_help_param, arg_info.m_help_text);
-                    name = arg.first + arg.second.m_help_param;
+
+                if (category == OptionsCategory::COMMANDS) {
+                    const auto cmd_args = m_command_args.find(arg_name);
+                    if (cmd_args != m_command_args.end()) {
+                        command_options.Iterate(cmd_args->second, show_debug, [&](const auto& cmdopt_name, const auto& cmdopt_info) {
+                            usage += HelpMessageSubOpt(cmdopt_name, cmdopt_info.m_help_param, cmdopt_info.m_help_text);
+                        });
+                    }
                 }
-                usage += HelpMessageOpt(name, arg.second.m_help_text);
             }
         }
     }
@@ -700,10 +765,11 @@ std::string HelpMessageGroup(const std::string &message) {
     return std::string(message) + std::string("\n\n");
 }
 
-std::string HelpMessageOpt(const std::string &option, const std::string &message) {
+std::string HelpMessageOpt(const std::string& option, const std::string& help_param, const std::string &message, int indent)
-    return std::string(optIndent,' ') + std::string(option) +
+{
-           std::string("\n") + std::string(msgIndent,' ') +
+    return std::string(optIndent+indent,' ') + option + help_param +
-           FormatParagraph(message, screenWidth - msgIndent, msgIndent) +
+           std::string("\n") + std::string(msgIndent+indent,' ') +
+           FormatParagraph(message, screenWidth - msgIndent - indent, msgIndent + indent) +
            std::string("\n\n");
 }
 
@@ -720,6 +786,12 @@ bool HasTestOption(const ArgsManager& args, const std::string& test_option)
     });
 }
 
+std::string HelpMessageSubOpt(const std::string& option, const std::string& help_param, const std::string &message)
+{
+
+    return HelpMessageOpt(option, help_param, message, msgIndent - optIndent);
+}
+
 fs::path GetDefaultDataDir()
 {
     // Windows:

src/common/args.h

@@ -66,6 +66,8 @@ enum class OptionsCategory {
     CLI_COMMANDS,
     IPC,
 
+    COMMAND_OPTIONS, // Specific to one or more commands
+
     HIDDEN // Always the last option to avoid printing these in the help
 };
 
@@ -137,6 +139,7 @@ protected:
     std::string m_network GUARDED_BY(cs_args);
     std::set<std::string> m_network_only_args GUARDED_BY(cs_args);
     std::map<OptionsCategory, std::map<std::string, Arg>> m_available_args GUARDED_BY(cs_args);
+    std::map<std::string, std::set<std::string>> m_command_args GUARDED_BY(cs_args);
     bool m_accept_any_command GUARDED_BY(cs_args){true};
     std::list<SectionInfo> m_config_sections GUARDED_BY(cs_args);
     std::optional<fs::path> m_config_path GUARDED_BY(cs_args);
@@ -212,6 +215,11 @@ protected:
      */
     std::optional<const Command> GetCommand() const;
 
+    /**
+     * Check for invalid command options
+     */
+    bool CheckCommandOptions(const std::string& command, std::vector<std::string>* errors = nullptr) const;
+
     /**
      * Get blocks directory path
      *
@@ -347,9 +355,9 @@ protected:
     void AddArg(const std::string& name, const std::string& help, unsigned int flags, const OptionsCategory& cat);
 
     /**
-     * Add subcommand
+     * Add command
      */
-    void AddCommand(const std::string& cmd, const std::string& help);
+    void AddCommand(const std::string& cmd, const std::string& help, std::set<std::string>&& options={});
 
     /**
      * Add many hidden arguments
@@ -472,11 +480,23 @@ std::string HelpMessageGroup(const std::string& message);
 /**
  * Format a string to be used as option description in help messages
  *
- * @param option Option message (e.g. "-rpcuser=<user>")
+ * @param option Option name (e.g. "-rpcuser")
+ * @param help_param Help parameter (e.g. "=<user>" or "")
+ * @param message Option description (e.g. "Username for JSON-RPC connections")
+ * @param indent Additional indentation
+ * @return the formatted string
+ */
+std::string HelpMessageOpt(const std::string& option, const std::string& help_param, const std::string& message, int indent=0);
+
+/**
+ * Same as HelpMessageOpt, but indents for command-specific options
+ *
+ * @param option Option name (e.g. "-rpcuser")
+ * @param help_param Help parameter (e.g. "=<user>" or "")
  * @param message Option description (e.g. "Username for JSON-RPC connections")
  * @return the formatted string
  */
-std::string HelpMessageOpt(const std::string& option, const std::string& message);
+std::string HelpMessageSubOpt(const std::string& option, const std::string& help_param, const std::string& message);
 
 namespace common {
 #ifdef WIN32

src/test/fuzz/string.cpp

@@ -68,7 +68,8 @@ FUZZ_TARGET(string)
     (void)HelpExampleCli(random_string_1, random_string_2);
     (void)HelpExampleRpc(random_string_1, random_string_2);
     (void)HelpMessageGroup(random_string_1);
-    (void)HelpMessageOpt(random_string_1, random_string_2);
+    (void)HelpMessageOpt(random_string_1, "", random_string_2);
+    (void)HelpMessageOpt(random_string_1, random_string_2, "");
     (void)IsDeprecatedRPCEnabled(random_string_1);
     (void)Join(random_string_vector, random_string_1);
     (void)JSONRPCError(fuzzed_data_provider.ConsumeIntegral<int>(), random_string_1);

src/wallet/wallettool.cpp

@@ -112,21 +112,12 @@ static void WalletShowInfo(CWallet* wallet_instance)
 
 bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command)
 {
-    if (args.IsArgSet("-format") && command != "createfromdump") {
+    {
-        tfm::format(std::cerr, "The -format option can only be used with the \"createfromdump\" command.\n");
+        std::vector<std::string> details;
+        if (!args.CheckCommandOptions(command, &details)) {
+            tfm::format(std::cerr, "Error: Invalid arguments provided:\n%s\n", util::MakeUnorderedList(details));
             return false;
         }
-    if (args.IsArgSet("-dumpfile") && command != "dump" && command != "createfromdump") {
-        tfm::format(std::cerr, "The -dumpfile option can only be used with the \"dump\" and \"createfromdump\" commands.\n");
-        return false;
-    }
-    if (args.IsArgSet("-descriptors") && command != "create") {
-        tfm::format(std::cerr, "The -descriptors option can only be used with the 'create' command.\n");
-        return false;
-    }
-    if (args.IsArgSet("-legacy") && command != "create") {
-        tfm::format(std::cerr, "The -legacy option can only be used with the 'create' command.\n");
-        return false;
     }
     if (command == "create" && !args.IsArgSet("-wallet")) {
         tfm::format(std::cerr, "Wallet name must be provided when creating a new wallet.\n");

test/functional/tool_wallet.py

@@ -361,7 +361,7 @@ class ToolWalletTest(BitcoinTestFramework):
         self.assert_raises_tool_error('Dump file {} does not exist.'.format(non_exist_dump), '-wallet=todump', '-dumpfile={}'.format(non_exist_dump), 'createfromdump')
         wallet_path = self.nodes[0].wallets_path / "todump2"
         self.assert_raises_tool_error('Failed to create database path \'{}\'. Database already exists.'.format(wallet_path), '-wallet=todump2', '-dumpfile={}'.format(wallet_dump), 'createfromdump')
-        self.assert_raises_tool_error("The -descriptors option can only be used with the 'create' command.", '-descriptors', '-wallet=todump2', '-dumpfile={}'.format(wallet_dump), 'createfromdump')
+        self.assert_raises_tool_error("Error: Invalid arguments provided:\n- The -descriptors option cannot be used with the 'createfromdump' command.", '-descriptors', '-wallet=todump2', '-dumpfile={}'.format(wallet_dump), 'createfromdump')
 
         self.log.info('Checking createfromdump')
         self.do_tool_createfromdump("load", "wallet.dump")