From b8c069b7a952e326d2d974cc671889d1a3b38aa4 Mon Sep 17 00:00:00 2001
From: Russell Yanofsky <russ@yanofsky.org>
Date: Sat, 21 Aug 2021 14:06:49 -0400
Subject: [PATCH] refactor: Add explicit DISALLOW_NEGATION ArgsManager flag to
 clarify flag usage

Currently, ALLOW_{INT|BOOL|STRING} flags don't do any real validation,
so current uses of these flags are misleading and will also break
backwards compatibility whenever these flags are implemented in a future
PR (draft PR is #16545).

An additional complication is that while these flags don't do any real
settings validation, they do affect whether setting negation syntax is
allowed.

Fix this mess by disabling ALLOW_{INT|BOOL|STRING} flags until they are
implemented, and adding an unambiguous DISALLOW_NEGATION flag. This is
done in two commits, with this commit adding the DISALLOW_NEGATION flag,
and the next commit disabling the ALLOW_{INT|BOOL|STRING} flags.
---
 src/util/system.cpp |  3 ++-
 src/util/system.h   | 16 +++++++++++-----
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/src/util/system.cpp b/src/util/system.cpp
index 2bc6b3bdf85..47c2cc5af82 100644
--- a/src/util/system.cpp
+++ b/src/util/system.cpp
@@ -230,7 +230,7 @@ static std::optional<util::SettingsValue> InterpretValue(const KeyInfo& key, con
 {
     // Return negated settings as false values.
     if (key.negated) {
-        if (!(flags & ArgsManager::ALLOW_BOOL)) {
+        if (flags & ArgsManager::DISALLOW_NEGATION) {
             error = strprintf("Negating of -%s is meaningless and therefore forbidden", key.name);
             return std::nullopt;
         }
@@ -652,6 +652,7 @@ void ArgsManager::AddArg(const std::string& name, const std::string& help, unsig
 
     LOCK(cs_args);
     std::map<std::string, Arg>& arg_map = m_available_args[cat];
+    if ((flags & (ALLOW_ANY | ALLOW_BOOL)) == 0) flags |= DISALLOW_NEGATION; // Temporary, removed in next scripted-diff
     auto ret = arg_map.emplace(arg_name, Arg{name.substr(eq_index, name.size() - eq_index), help, flags});
     assert(ret.second); // Make sure an insertion actually happened
 
diff --git a/src/util/system.h b/src/util/system.h
index 2e217f6f90c..a4323e0f9ec 100644
--- a/src/util/system.h
+++ b/src/util/system.h
@@ -158,12 +158,18 @@ struct SectionInfo
 class ArgsManager
 {
 public:
+    /**
+     * Flags controlling how config and command line arguments are validated and
+     * interpreted.
+     */
     enum Flags : uint32_t {
-        // Boolean options can accept negation syntax -noOPTION or -noOPTION=1
-        ALLOW_BOOL = 0x01,
-        ALLOW_INT = 0x02,
-        ALLOW_STRING = 0x04,
-        ALLOW_ANY = ALLOW_BOOL | ALLOW_INT | ALLOW_STRING,
+        ALLOW_ANY = 0x01,         //!< disable validation
+        ALLOW_BOOL = 0x02,        //!< unimplemented, draft implementation in #16545
+        ALLOW_INT = 0x04,         //!< unimplemented, draft implementation in #16545
+        ALLOW_STRING = 0x08,      //!< unimplemented, draft implementation in #16545
+        ALLOW_LIST = 0x10,        //!< unimplemented, draft implementation in #16545
+        DISALLOW_NEGATION = 0x20, //!< disallow -nofoo syntax
+
         DEBUG_ONLY = 0x100,
         /* Some options would cause cross-contamination if values for
          * mainnet were used while running on regtest/testnet (or vice-versa).