From d5e680ee63086d9408da3c80fb5621e819cdefcd Mon Sep 17 00:00:00 2001
From: Anthony Towns <aj@erisian.com.au>
Date: Mon, 6 Nov 2023 18:58:01 +1000
Subject: [PATCH] ArgsManager: automate checking for correct command options

---
 src/common/args.cpp            | 23 +++++++++++++++++++++++
 src/common/args.h              |  5 +++++
 src/wallet/wallettool.cpp      | 21 ++++++---------------
 test/functional/tool_wallet.py |  2 +-
 4 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/src/common/args.cpp b/src/common/args.cpp
index acafe878539fb7..08d6df8112a61f 100644
--- a/src/common/args.cpp
+++ b/src/common/args.cpp
@@ -358,6 +358,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;
diff --git a/src/common/args.h b/src/common/args.h
index b6497a02fd77e9..f9417f0325c885 100644
--- a/src/common/args.h
+++ b/src/common/args.h
@@ -212,6 +212,11 @@ class ArgsManager
      */
     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
      *
diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp
index 2f3f8ef77de2f6..d4ee54dada2522 100644
--- a/src/wallet/wallettool.cpp
+++ b/src/wallet/wallettool.cpp
@@ -115,21 +115,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");
-        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;
+    {
+        std::vector<std::string> details;
+        if (!args.CheckCommandOptions(command, &details)) {
+            tfm::format(std::cerr, "Error: Invalid arguments provided:\n%s\n", MakeUnorderedList(details));
+            return false;
+        }
     }
     if (command == "create" && !args.IsArgSet("-wallet")) {
         tfm::format(std::cerr, "Wallet name must be provided when creating a new wallet.\n");
diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py
index fc042bca666988..2b8466b4c1a7a6 100755
--- a/test/functional/tool_wallet.py
+++ b/test/functional/tool_wallet.py
@@ -344,7 +344,7 @@ def test_dump_createfromdump(self):
         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")