Merge #13112: Throw an error for unknown args

903055730b Test gArgs erroring on unknown args (Andrew Chow)
4f8704d57f Give an error and exit if there are unknown parameters (Andrew Chow)
174f7c8080 Use a struct for arguments and nested map for categories (Andrew Chow)

Pull request description:

  Following #13190, gArgs is aware of all of the command line arguments. This PR has gArgs check whether the arguments provided are actually valid arguments. When an unknown argument is encountered, an error is printed to stderr and the program exist.

  Since gArgs is used for everything that has command line arguments, `bitcoind`, `bitcoin-cli`, `bitcoin-qt`, `bitcoin-tx`, and `bench_bitcoin` are all effected by this change and all now have the same argument checking behavior.

  Closes #1044

Tree-SHA512: 388201319a7d6493204bb5433da47e8e6c8266882e809f6df45f86d925f1f320f2fd13edb3e57ffc6a37415dfdfc689f83929452bca224229783accb367032e7

2 files changed, 59 insertions, 24 deletions

diff --git a/src/test/getarg_tests.cpp b/src/test/getarg_tests.cpp
index cd603b7f58..c065e25676 100644
--- a/src/test/getarg_tests.cpp
+++ b/src/test/getarg_tests.cpp
@@ -27,11 +27,21 @@ static void ResetArgs(const std::string& strArg)
     for (std::string& s : vecArg)
         vecChar.push_back(s.c_str());
 
-    gArgs.ParseParameters(vecChar.size(), vecChar.data());
+    std::string error;
+    gArgs.ParseParameters(vecChar.size(), vecChar.data(), error);
+}
+
+static void SetupArgs(const std::vector<std::string>& args)
+{
+    gArgs.ClearArgs();
+    for (const std::string& arg : args) {
+        gArgs.AddArg(arg, "", false, OptionsCategory::OPTIONS);
+    }
 }
 
 BOOST_AUTO_TEST_CASE(boolarg)
 {
+    SetupArgs({"-foo"});
     ResetArgs("-foo");
     BOOST_CHECK(gArgs.GetBoolArg("-foo", false));
     BOOST_CHECK(gArgs.GetBoolArg("-foo", true));
@@ -84,6 +94,7 @@ BOOST_AUTO_TEST_CASE(boolarg)
 
 BOOST_AUTO_TEST_CASE(stringarg)
 {
+    SetupArgs({"-foo", "-bar"});
     ResetArgs("");
     BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", ""), "");
     BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", "eleven"), "eleven");
@@ -108,6 +119,7 @@ BOOST_AUTO_TEST_CASE(stringarg)
 
 BOOST_AUTO_TEST_CASE(intarg)
 {
+    SetupArgs({"-foo", "-bar"});
     ResetArgs("");
     BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", 11), 11);
     BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", 0), 0);
@@ -127,6 +139,7 @@ BOOST_AUTO_TEST_CASE(intarg)
 
 BOOST_AUTO_TEST_CASE(doubledash)
 {
+    SetupArgs({"-foo", "-bar"});
     ResetArgs("--foo");
     BOOST_CHECK_EQUAL(gArgs.GetBoolArg("-foo", false), true);
 
@@ -137,6 +150,7 @@ BOOST_AUTO_TEST_CASE(doubledash)
 
 BOOST_AUTO_TEST_CASE(boolargno)
 {
+    SetupArgs({"-foo", "-bar"});
     ResetArgs("-nofoo");
     BOOST_CHECK(!gArgs.GetBoolArg("-foo", true));
     BOOST_CHECK(!gArgs.GetBoolArg("-foo", false));
diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp
index 2af6d9e0f3..611ccc9b77 100644
--- a/src/test/util_tests.cpp
+++ b/src/test/util_tests.cpp
@@ -186,27 +186,37 @@ struct TestArgsManager : public ArgsManager
             LOCK(cs_args);
             m_config_args.clear();
         }
-        ReadConfigStream(streamConfig);
+        std::string error;
+        ReadConfigStream(streamConfig, error);
     }
     void SetNetworkOnlyArg(const std::string arg)
     {
         LOCK(cs_args);
         m_network_only_args.insert(arg);
     }
+    void SetupArgs(int argv, const char* args[])
+    {
+        for (int i = 0; i < argv; ++i) {
+            AddArg(args[i], "", false, OptionsCategory::OPTIONS);
+        }
+    }
 };
 
 BOOST_AUTO_TEST_CASE(util_ParseParameters)
 {
     TestArgsManager testArgs;
+    const char* avail_args[] = {"-a", "-b", "-ccc", "-d"};
     const char *argv_test[] = {"-ignored", "-a", "-b", "-ccc=argument", "-ccc=multiple", "f", "-d=e"};
 
-    testArgs.ParseParameters(0, (char**)argv_test);
+    std::string error;
+    testArgs.SetupArgs(4, avail_args);
+    testArgs.ParseParameters(0, (char**)argv_test, error);
     BOOST_CHECK(testArgs.GetOverrideArgs().empty() && testArgs.GetConfigArgs().empty());
 
-    testArgs.ParseParameters(1, (char**)argv_test);
+    testArgs.ParseParameters(1, (char**)argv_test, error);
     BOOST_CHECK(testArgs.GetOverrideArgs().empty() && testArgs.GetConfigArgs().empty());
 
-    testArgs.ParseParameters(7, (char**)argv_test);
+    testArgs.ParseParameters(7, (char**)argv_test, error);
     // expectation: -ignored is ignored (program name argument),
     // -a, -b and -ccc end up in map, -d ignored because it is after
     // a non-option argument (non-GNU option parsing)
@@ -227,9 +237,12 @@ BOOST_AUTO_TEST_CASE(util_ParseParameters)
 BOOST_AUTO_TEST_CASE(util_GetBoolArg)
 {
     TestArgsManager testArgs;
+    const char* avail_args[] = {"-a", "-b", "-c", "-d", "-e", "-f"};
     const char *argv_test[] = {
         "ignored", "-a", "-nob", "-c=0", "-d=1", "-e=false", "-f=true"};
-    testArgs.ParseParameters(7, (char**)argv_test);
+    std::string error;
+    testArgs.SetupArgs(6, avail_args);
+    testArgs.ParseParameters(7, (char**)argv_test, error);
 
     // Each letter should be set.
     for (char opt : "abcdef")
@@ -261,8 +274,11 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArgEdgeCases)
     TestArgsManager testArgs;
 
     // Params test
+    const char* avail_args[] = {"-foo", "-bar"};
     const char *argv_test[] = {"ignored", "-nofoo", "-foo", "-nobar=0"};
-    testArgs.ParseParameters(4, (char**)argv_test);
+    testArgs.SetupArgs(2, avail_args);
+    std::string error;
+    testArgs.ParseParameters(4, (char**)argv_test, error);
 
     // This was passed twice, second one overrides the negative setting.
     BOOST_CHECK(!testArgs.IsArgNegated("-foo"));
@@ -274,7 +290,7 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArgEdgeCases)
 
     // Config test
     const char *conf_test = "nofoo=1\nfoo=1\nnobar=0\n";
-    testArgs.ParseParameters(1, (char**)argv_test);
+    testArgs.ParseParameters(1, (char**)argv_test, error);
     testArgs.ReadConfigString(conf_test);
 
     // This was passed twice, second one overrides the negative setting,
@@ -289,7 +305,7 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArgEdgeCases)
     // Combined test
     const char *combo_test_args[] = {"ignored", "-nofoo", "-bar"};
     const char *combo_test_conf = "foo=1\nnobar=1\n";
-    testArgs.ParseParameters(3, (char**)combo_test_args);
+    testArgs.ParseParameters(3, (char**)combo_test_args, error);
     testArgs.ReadConfigString(combo_test_conf);
 
     // Command line overrides, but doesn't erase old setting
@@ -329,6 +345,8 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream)
        "iii=2\n";
 
     TestArgsManager test_args;
+    const char* avail_args[] = {"-a", "-b", "-ccc", "-d", "-e", "-fff", "-ggg", "-h", "-i", "-iii"};
+    test_args.SetupArgs(10, avail_args);
 
     test_args.ReadConfigString(str_config);
     // expectation: a, b, ccc, d, fff, ggg, h, i end up in map
@@ -526,6 +544,8 @@ BOOST_AUTO_TEST_CASE(util_GetArg)
 BOOST_AUTO_TEST_CASE(util_GetChainName)
 {
     TestArgsManager test_args;
+    const char* avail_args[] = {"-testnet", "-regtest"};
+    test_args.SetupArgs(2, avail_args);
 
     const char* argv_testnet[] = {"cmd", "-testnet"};
     const char* argv_regtest[] = {"cmd", "-regtest"};
@@ -535,39 +555,40 @@ BOOST_AUTO_TEST_CASE(util_GetChainName)
     // equivalent to "-testnet"
     // regtest in testnet section is ignored
     const char* testnetconf = "testnet=1\nregtest=0\n[test]\nregtest=1";
+    std::string error;
 
-    test_args.ParseParameters(0, (char**)argv_testnet);
+    test_args.ParseParameters(0, (char**)argv_testnet, error);
     BOOST_CHECK_EQUAL(test_args.GetChainName(), "main");
 
-    test_args.ParseParameters(2, (char**)argv_testnet);
+    test_args.ParseParameters(2, (char**)argv_testnet, error);
     BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
 
-    test_args.ParseParameters(2, (char**)argv_regtest);
+    test_args.ParseParameters(2, (char**)argv_regtest, error);
     BOOST_CHECK_EQUAL(test_args.GetChainName(), "regtest");
 
-    test_args.ParseParameters(3, (char**)argv_test_no_reg);
+    test_args.ParseParameters(3, (char**)argv_test_no_reg, error);
     BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
 
-    test_args.ParseParameters(3, (char**)argv_both);
+    test_args.ParseParameters(3, (char**)argv_both, error);
     BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error);
 
-    test_args.ParseParameters(0, (char**)argv_testnet);
+    test_args.ParseParameters(0, (char**)argv_testnet, error);
     test_args.ReadConfigString(testnetconf);
     BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
 
-    test_args.ParseParameters(2, (char**)argv_testnet);
+    test_args.ParseParameters(2, (char**)argv_testnet, error);
     test_args.ReadConfigString(testnetconf);
     BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
 
-    test_args.ParseParameters(2, (char**)argv_regtest);
+    test_args.ParseParameters(2, (char**)argv_regtest, error);
     test_args.ReadConfigString(testnetconf);
     BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error);
 
-    test_args.ParseParameters(3, (char**)argv_test_no_reg);
+    test_args.ParseParameters(3, (char**)argv_test_no_reg, error);
     test_args.ReadConfigString(testnetconf);
     BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
 
-    test_args.ParseParameters(3, (char**)argv_both);
+    test_args.ParseParameters(3, (char**)argv_both, error);
     test_args.ReadConfigString(testnetconf);
     BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error);
 
@@ -575,23 +596,23 @@ BOOST_AUTO_TEST_CASE(util_GetChainName)
     // [test] regtest=1 potentially relevant) doesn't break things
     test_args.SelectConfigNetwork("test");
 
-    test_args.ParseParameters(0, (char**)argv_testnet);
+    test_args.ParseParameters(0, (char**)argv_testnet, error);
     test_args.ReadConfigString(testnetconf);
     BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
 
-    test_args.ParseParameters(2, (char**)argv_testnet);
+    test_args.ParseParameters(2, (char**)argv_testnet, error);
     test_args.ReadConfigString(testnetconf);
     BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
 
-    test_args.ParseParameters(2, (char**)argv_regtest);
+    test_args.ParseParameters(2, (char**)argv_regtest, error);
     test_args.ReadConfigString(testnetconf);
     BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error);
 
-    test_args.ParseParameters(2, (char**)argv_test_no_reg);
+    test_args.ParseParameters(2, (char**)argv_test_no_reg, error);
     test_args.ReadConfigString(testnetconf);
     BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
 
-    test_args.ParseParameters(3, (char**)argv_both);
+    test_args.ParseParameters(3, (char**)argv_both, error);
     test_args.ReadConfigString(testnetconf);
     BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error);
 }