From 51052e8bb752c395e1ddb2d6c9ae5006b7d4bae5 Mon Sep 17 00:00:00 2001
From: fanquake <fanquake@gmail.com>
Date: Thu, 19 Oct 2023 13:18:31 +0100
Subject: [PATCH 1/8] Merge bitcoin/bitcoin#22764: build: Include qt sources
 for parsing with extract_strings.py
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

b59b31ae0b04054c5cf225dad87046d3771707fc build: Drop redundant qt/bitcoin.cpp (Hennadii Stepanov)
d90ad5a42ec6f48d0e504edc16d41c8ef266cc1d build: Include qt sources for parsing with extract_strings.py (Hennadii Stepanov)

Pull request description:

  On master (4fc15d15667d9d9c4fb5515ce73c05b4596298ec) some strings are still untranslated.

  This PR fixes this issue.

  To verify:
  1) `./autogen.sh && ./configure && make -C src translate` _before_ applying this change
  2) apply this change
  3) `./autogen.sh && ./configure && make -C src translate` _after_ applying this change

  The result of `git diff src/qt/bitcoinstrings.cpp`:
  ```diff
  --- a/src/qt/bitcoinstrings.cpp
  +++ b/src/qt/bitcoinstrings.cpp
  @@ -126,6 +126,7 @@ QT_TRANSLATE_NOOP("bitcoin-core", ""
   "You need to rebuild the database using -reindex to go back to unpruned "
   "mode.  This will redownload the entire blockchain"),
   QT_TRANSLATE_NOOP("bitcoin-core", "%s is set very high!"),
  +QT_TRANSLATE_NOOP("bitcoin-core", "(press q to shutdown and continue later)"),
   QT_TRANSLATE_NOOP("bitcoin-core", "-maxmempool must be at least %d MB"),
   QT_TRANSLATE_NOOP("bitcoin-core", "A fatal internal error occurred, see debug.log for details"),
   QT_TRANSLATE_NOOP("bitcoin-core", "Cannot resolve -%s address: '%s'"),
  @@ -204,6 +205,8 @@ QT_TRANSLATE_NOOP("bitcoin-core", "SQLiteDatabase: Failed to prepare statement t
   QT_TRANSLATE_NOOP("bitcoin-core", "SQLiteDatabase: Failed to read database verification error: %s"),
   QT_TRANSLATE_NOOP("bitcoin-core", "SQLiteDatabase: Unexpected application id. Expected %u, got %u"),
   QT_TRANSLATE_NOOP("bitcoin-core", "Section [%s] is not recognized."),
  +QT_TRANSLATE_NOOP("bitcoin-core", "Settings file could not be read"),
  +QT_TRANSLATE_NOOP("bitcoin-core", "Settings file could not be written"),
   QT_TRANSLATE_NOOP("bitcoin-core", "Signing transaction failed"),
   QT_TRANSLATE_NOOP("bitcoin-core", "Specified -walletdir \"%s\" does not exist"),
   QT_TRANSLATE_NOOP("bitcoin-core", "Specified -walletdir \"%s\" is a relative path"),
  @@ -242,4 +245,5 @@ QT_TRANSLATE_NOOP("bitcoin-core", "User Agent comment (%s) contains unsafe chara
   QT_TRANSLATE_NOOP("bitcoin-core", "Verifying blocks…"),
   QT_TRANSLATE_NOOP("bitcoin-core", "Verifying wallet(s)…"),
   QT_TRANSLATE_NOOP("bitcoin-core", "Wallet needed to be rewritten: restart %s to complete"),
  +QT_TRANSLATE_NOOP("bitcoin-core", "press q to shutdown"),
   };
  ```

ACKs for top commit:
  ryanofsky:
    Code review ACK b59b31ae0b04054c5cf225dad87046d3771707fc. Being able to use `_()` macro in qt would allow simplifying some code, for example replacing repetitive:
  TheCharlatan:
    ACK b59b31ae0b04054c5cf225dad87046d3771707fc

Tree-SHA512: 13d9d86b487a1b6e718ae96c198a0a927c881bf33df318412793ec9efba3a7e59cfa836204f73f5b53ff4c99edce778c11bffaa88138b80e37b71e36df6b816f
---
 src/Makefile.qt.include | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/Makefile.qt.include b/src/Makefile.qt.include
index 04652688dbef..06de1e2237dc 100644
--- a/src/Makefile.qt.include
+++ b/src/Makefile.qt.include
@@ -489,11 +489,14 @@ SECONDARY: $(QT_QM)
 
 $(srcdir)/qt/dashstrings.cpp: FORCE
 	@test -n $(XGETTEXT) || echo "xgettext is required for updating translations"
-	$(AM_V_GEN) cd $(srcdir); XGETTEXT=$(XGETTEXT) COPYRIGHT_HOLDERS="$(COPYRIGHT_HOLDERS)" $(PYTHON) ../share/qt/extract_strings_qt.py $(libbitcoin_node_a_SOURCES) $(libbitcoin_wallet_a_SOURCES) $(libbitcoin_common_a_SOURCES) $(libbitcoin_zmq_a_SOURCES) $(libbitcoin_consensus_a_SOURCES) $(libbitcoin_util_a_SOURCES)
+	$(AM_V_GEN) cd $(srcdir); XGETTEXT=$(XGETTEXT) COPYRIGHT_HOLDERS="$(COPYRIGHT_HOLDERS)" $(PYTHON) ../share/qt/extract_strings_qt.py \
+    $(libbitcoin_node_a_SOURCES) $(libbitcoin_wallet_a_SOURCES) $(libbitcoin_common_a_SOURCES) \
+    $(libbitcoin_zmq_a_SOURCES) $(libbitcoin_consensus_a_SOURCES) $(libbitcoin_util_a_SOURCES) \
+    $(BITCOIN_QT_BASE_CPP) $(BITCOIN_QT_WINDOWS_CPP) $(BITCOIN_QT_WALLET_CPP) $(BITCOIN_QT_H) $(BITCOIN_MM)
 
 # The resulted dash_en.xlf source file should follow Transifex requirements.
 # See: https://docs.transifex.com/formats/xliff#how-to-distinguish-between-a-source-file-and-a-translation-file
-translate: $(srcdir)/qt/dashstrings.cpp $(QT_FORMS_UI) $(QT_FORMS_UI) $(BITCOIN_QT_BASE_CPP) qt/bitcoin.cpp $(BITCOIN_QT_WINDOWS_CPP) $(BITCOIN_QT_WALLET_CPP) $(BITCOIN_QT_H) $(BITCOIN_MM)
+translate: $(srcdir)/qt/dashstrings.cpp $(QT_FORMS_UI) $(QT_FORMS_UI) $(BITCOIN_QT_BASE_CPP) $(BITCOIN_QT_WINDOWS_CPP) $(BITCOIN_QT_WALLET_CPP) $(BITCOIN_QT_H) $(BITCOIN_MM)
 	@test -n $(LUPDATE) || echo "lupdate is required for updating translations"
 	$(AM_V_GEN) QT_SELECT=$(QT_SELECT) $(LUPDATE) -no-obsolete -I $(srcdir) -locations relative $^ -ts $(srcdir)/qt/locale/dash_en.ts
 	@test -n $(LCONVERT) || echo "lconvert is required for updating translations"

From 6eccb6078c1864449d34a5dcd36bcd7a175cf1b1 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Mon, 14 Mar 2022 08:37:43 +0100
Subject: [PATCH 2/8] Merge bitcoin/bitcoin#24505: wallet: Add a deprecation
 warning for newly created legacy wallets

61152183ab18960c8b42cf22ff7168762946678e wallet: Add a deprecation warning for newly created legacy wallets (Andrew Chow)

Pull request description:

  As we slowly deprecate legacy wallets, we need to warn users that are making new legacy wallets that their wallet type is going to be unsupported in the future.

ACKs for top commit:
  jonatack:
    ACK 61152183ab18960c8b42cf22ff7168762946678e
  S3RK:
    reACK 61152183ab18960c8b42cf22ff7168762946678e
  theStack:
    ACK 61152183ab18960c8b42cf22ff7168762946678e

Tree-SHA512: e89bfb8168869542498958f0c9a2ab302dfd43287f8a49e7d9e09f60438a567bb8b7219a4e569797ee819b30b624f532fcc0b70c6aa0edcb392a301b8ce8b541
---
 src/wallet/rpc/wallet.cpp              | 4 +++-
 src/wallet/wallet.cpp                  | 5 +++++
 test/functional/wallet_createwallet.py | 5 +++++
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp
index c48a0d41b0f5..3b9d28d20f0a 100644
--- a/src/wallet/rpc/wallet.cpp
+++ b/src/wallet/rpc/wallet.cpp
@@ -634,7 +634,9 @@ static RPCHelpMan createwallet()
             {"blank", RPCArg::Type::BOOL, RPCArg::Default{false}, "Create a blank wallet. A blank wallet has no keys or HD seed. One can be set using upgradetohd (by mnemonic) or sethdseed (WIF private key)."},
             {"passphrase", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Encrypt the wallet with this passphrase."},
             {"avoid_reuse", RPCArg::Type::BOOL, RPCArg::Default{false}, "Keep track of coin reuse, and treat dirty and clean coins differently with privacy considerations in mind."},
-            {"descriptors", RPCArg::Type::BOOL, RPCArg::Default{true}, "Create a native descriptor wallet. The wallet will use descriptors internally to handle address creation."},
+            {"descriptors", RPCArg::Type::BOOL, RPCArg::Default{true}, "Create a native descriptor wallet. The wallet will use descriptors internally to handle address creation."
+                                                                       " Setting to \"false\" will create a legacy wallet; however, the legacy wallet type is being deprecated and"
+                                                                       " support for creating and opening legacy wallets will be removed in the future."},
             {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED_NAMED_ARG, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."},
             {"external_signer", RPCArg::Type::BOOL, RPCArg::Default{false}, "Use an external signer such as a hardware wallet. Requires -signer to be configured. Wallet creation will fail if keys cannot be fetched. Requires disable_private_keys and descriptors set to true."},
         },
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index d7be196ed58b..d6cdcf824e6f 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -484,6 +484,11 @@ std::shared_ptr<CWallet> CreateWallet(WalletContext& context, const std::string&
     // Write the wallet settings
     UpdateWalletSetting(*context.chain, name, load_on_start, warnings);
 
+    // Legacy wallets are being deprecated, warn if a newly created wallet is legacy
+    if (!(wallet_creation_flags & WALLET_FLAG_DESCRIPTORS)) {
+        warnings.push_back(_("Wallet created successfully. The legacy wallet type is being deprecated and support for creating and opening legacy wallets will be removed in the future."));
+    }
+
     status = DatabaseStatus::SUCCESS;
     return wallet;
 }
diff --git a/test/functional/wallet_createwallet.py b/test/functional/wallet_createwallet.py
index f8964ecf428d..2052ec5b7b29 100755
--- a/test/functional/wallet_createwallet.py
+++ b/test/functional/wallet_createwallet.py
@@ -173,5 +173,10 @@ def run_test(self):
         self.log.info('Using a passphrase with private keys disabled returns error')
         assert_raises_rpc_error(-4, 'Passphrase provided but private keys are disabled. A passphrase is only used to encrypt private keys, so cannot be used for wallets with private keys disabled.', self.nodes[0].createwallet, wallet_name='w9', disable_private_keys=True, passphrase='thisisapassphrase')
 
+        if self.is_bdb_compiled():
+            self.log.info("Test legacy wallet deprecation")
+            res = self.nodes[0].createwallet(wallet_name="legacy_w0", descriptors=False, load_on_startup=True)
+            assert_equal(res["warning"], "Wallet created successfully. The legacy wallet type is being deprecated and support for creating and opening legacy wallets will be removed in the future.")
+
 if __name__ == '__main__':
     CreateWalletTest().main()

From 8a4df322b35d38262042db5b87adef79839c9c7e Mon Sep 17 00:00:00 2001
From: MacroFake <falke.marco@gmail.com>
Date: Tue, 13 Sep 2022 13:38:37 +0200
Subject: [PATCH 3/8] rpc: Set RPCArg options with designated initializers

---
 src/rpc/blockchain.cpp    |  8 ++++----
 src/rpc/mining.cpp        |  8 +++-----
 src/rpc/server.cpp        |  2 +-
 src/rpc/util.cpp          | 14 +++++++-------
 src/rpc/util.h            | 29 +++++++++++++----------------
 src/wallet/rpc/backup.cpp | 30 +++++++++++++++---------------
 src/wallet/rpc/coins.cpp  |  2 +-
 src/wallet/rpc/spend.cpp  | 10 +++++-----
 8 files changed, 49 insertions(+), 54 deletions(-)

diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 1803294f7b3c..96cb775cd086 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -1146,7 +1146,7 @@ static RPCHelpMan gettxoutsetinfo()
                 "Note this call may take some time if you are not using coinstatsindex.\n",
                 {
                     {"hash_type", RPCArg::Type::STR, RPCArg::Default{"hash_serialized_2"}, "Which UTXO set hash should be calculated. Options: 'hash_serialized_2' (the legacy algorithm), 'muhash', 'none'."},
-                    {"hash_or_height", RPCArg::Type::NUM, RPCArg::DefaultHint{"the current best block"}, "The block hash or height of the target height (only available with coinstatsindex).", "", {"", "string or numeric"}},
+                    {"hash_or_height", RPCArg::Type::NUM, RPCArg::DefaultHint{"the current best block"}, "The block hash or height of the target height (only available with coinstatsindex).", RPCArgOptions{.type_str={"", "string or numeric"}}},
                     {"use_index", RPCArg::Type::BOOL, RPCArg::Default{true}, "Use coinstatsindex, if available."},
                 },
                 RPCResult{
@@ -2019,13 +2019,13 @@ static RPCHelpMan getblockstats()
                 "\nCompute per block statistics for a given window. All amounts are in duffs.\n"
                 "It won't work for some heights with pruning.\n",
                 {
-                    {"hash_or_height", RPCArg::Type::NUM, RPCArg::Optional::NO, "The block hash or height of the target block", "", {"", "string or numeric"}},
+                    {"hash_or_height", RPCArg::Type::NUM, RPCArg::Optional::NO, "The block hash or height of the target block", RPCArgOptions{.type_str={"", "string or numeric"}}},
                     {"stats", RPCArg::Type::ARR, RPCArg::DefaultHint{"all values"}, "Values to plot (see result below)",
                         {
                             {"height", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Selected statistic"},
                             {"time", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Selected statistic"},
                         },
-                        "stats"},
+                        RPCArgOptions{.oneline_description="stats"}},
                 },
                 RPCResult{
             RPCResult::Type::OBJ, "", "",
@@ -2457,7 +2457,7 @@ static RPCHelpMan scantxoutset()
                         },
                     },
                 },
-                "[scanobjects,...]"},
+                RPCArgOptions{.oneline_description="[scanobjects,...]"}},
         },
         {
             RPCResult{"when action=='start'; only returns after scan completes", RPCResult::Type::OBJ, "", "", {
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index 385240ee1e93..33c85856dec3 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -552,15 +552,13 @@ static RPCHelpMan getblocktemplate()
                     {"capabilities", RPCArg::Type::ARR, /* treat as named arg */ RPCArg::Optional::OMITTED_NAMED_ARG, "A list of strings",
                         {
                             {"str", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "client side supported feature, 'longpoll', 'coinbasevalue', 'proposal', 'serverlist', 'workid'"},
-                        },
-                        },
+                        }},
                     {"rules", RPCArg::Type::ARR, RPCArg::Optional::NO, "A list of strings",
                         {
                             {"str", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "client side supported softfork deployment"},
-                        },
-                        },
+                        }},
                 },
-                "\"template_request\""},
+                RPCArgOptions{.oneline_description="\"template_request\""}},
         },
         {
             RPCResult{"If the proposal was accepted with mode=='proposal'", RPCResult::Type::NONE, "", ""},
diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp
index 3889e09aa9e2..d864e4f04d3f 100644
--- a/src/rpc/server.cpp
+++ b/src/rpc/server.cpp
@@ -183,7 +183,7 @@ static RPCHelpMan stop()
     // to the client (intended for testing)
                 "\nRequest a graceful shutdown of " PACKAGE_NAME ".",
                 {
-                    {"wait", RPCArg::Type::NUM, RPCArg::Optional::OMITTED_NAMED_ARG, "how long to wait in ms", "", {}, /*hidden=*/true},
+                    {"wait", RPCArg::Type::NUM, RPCArg::Optional::OMITTED_NAMED_ARG, "how long to wait in ms", RPCArgOptions{.hidden=true}},
                 },
                 RPCResult{RPCResult::Type::STR, "", "A string with the content '" + RESULT + "'"},
                 RPCExamples{""},
diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp
index 738a5dea162e..9e36db172c86 100644
--- a/src/rpc/util.cpp
+++ b/src/rpc/util.cpp
@@ -361,8 +361,8 @@ struct Sections {
         case RPCArg::Type::BOOL: {
             if (is_top_level_arg) return; // Nothing more to do for non-recursive types on first recursion
             auto left = indent;
-            if (arg.m_type_str.size() != 0 && push_name) {
-                left += "\"" + arg.GetName() + "\": " + arg.m_type_str.at(0);
+            if (arg.m_opts.type_str.size() != 0 && push_name) {
+                left += "\"" + arg.GetName() + "\": " + arg.m_opts.type_str.at(0);
             } else {
                 left += push_name ? arg.ToStringObj(/*oneline=*/false) : arg.ToString(/*oneline=*/false);
             }
@@ -561,7 +561,7 @@ std::string RPCHelpMan::ToString() const
     ret += m_name;
     bool was_optional{false};
     for (const auto& arg : m_args) {
-        if (arg.m_hidden) break; // Any arg that follows is also hidden
+        if (arg.m_opts.hidden) break; // Any arg that follows is also hidden
         const bool optional = arg.IsOptional();
         ret += " ";
         if (optional) {
@@ -582,7 +582,7 @@ std::string RPCHelpMan::ToString() const
     Sections sections;
     for (size_t i{0}; i < m_args.size(); ++i) {
         const auto& arg = m_args.at(i);
-        if (arg.m_hidden) break; // Any arg that follows is also hidden
+        if (arg.m_opts.hidden) break; // Any arg that follows is also hidden
 
         if (i == 0) ret += "\nArguments:\n";
 
@@ -647,8 +647,8 @@ std::string RPCArg::ToDescriptionString(bool is_named_arg) const
 {
     std::string ret;
     ret += "(";
-    if (m_type_str.size() != 0) {
-        ret += m_type_str.at(1);
+    if (m_opts.type_str.size() != 0) {
+        ret += m_opts.type_str.at(1);
     } else {
         switch (m_type) {
         case Type::STR_HEX:
@@ -890,7 +890,7 @@ std::string RPCArg::ToStringObj(const bool oneline) const
 
 std::string RPCArg::ToString(const bool oneline) const
 {
-    if (oneline && !m_oneline_description.empty()) return m_oneline_description;
+    if (oneline && !m_opts.oneline_description.empty()) return m_opts.oneline_description;
 
     switch (m_type) {
     case Type::STR_HEX:
diff --git a/src/rpc/util.h b/src/rpc/util.h
index cbbd8831b283..fabd9666e9cc 100644
--- a/src/rpc/util.h
+++ b/src/rpc/util.h
@@ -133,6 +133,12 @@ enum class OuterType {
 /** Evaluate a descriptor given as a string, or as a {"desc":...,"range":...} object, with default range of 1000. */
 std::vector<CScript> EvalDescriptorStringOrObject(const UniValue& scanobject, FlatSigningProvider& provider);
 
+struct RPCArgOptions {
+    std::string oneline_description{};   //!< Should be empty unless it is supposed to override the auto-generated summary line
+    std::vector<std::string> type_str{}; //!< Should be empty unless it is supposed to override the auto-generated type strings. Vector length is either 0 or 2, m_opts.type_str.at(0) will override the type of the value in a key-value pair, m_opts.type_str.at(1) will override the type in the argument description.
+    bool hidden{false};                  //!< For testing only
+};
+
 struct RPCArg {
     enum class Type {
         OBJ,
@@ -167,32 +173,26 @@ struct RPCArg {
     using DefaultHint = std::string;
     /** Default constant value */
     using Default = UniValue;
-    using Fallback = std::variant<Optional, DefaultHint, Default>;
+    using Fallback = std::variant<Optional, /* hint for default value */ DefaultHint, /* default constant value */ Default>;
 
     const std::string m_names; //!< The name of the arg (can be empty for inner args, can contain multiple aliases separated by | for named request arguments)
     const Type m_type;
-    const bool m_hidden;
     const std::vector<RPCArg> m_inner; //!< Only used for arrays or dicts
     const Fallback m_fallback;
     const std::string m_description;
-    const std::string m_oneline_description; //!< Should be empty unless it is supposed to override the auto-generated summary line
-    const std::vector<std::string> m_type_str; //!< Should be empty unless it is supposed to override the auto-generated type strings. Vector length is either 0 or 2, m_type_str.at(0) will override the type of the value in a key-value pair, m_type_str.at(1) will override the type in the argument description.
+    const RPCArgOptions m_opts;
 
     RPCArg(
         std::string name,
         Type type,
         Fallback fallback,
         std::string description,
-        std::string oneline_description = "",
-        std::vector<std::string> type_str = {},
-        bool hidden = false)
+        RPCArgOptions opts = {})
         : m_names{std::move(name)},
           m_type{std::move(type)},
-          m_hidden{hidden},
           m_fallback{std::move(fallback)},
           m_description{std::move(description)},
-          m_oneline_description{std::move(oneline_description)},
-          m_type_str{std::move(type_str)}
+          m_opts{std::move(opts)}
     {
         CHECK_NONFATAL(type != Type::ARR && type != Type::OBJ && type != Type::OBJ_USER_KEYS);
     }
@@ -203,16 +203,13 @@ struct RPCArg {
         Fallback fallback,
         std::string description,
         std::vector<RPCArg> inner,
-        std::string oneline_description = "",
-        std::vector<std::string> type_str = {})
+        RPCArgOptions opts = {})
         : m_names{std::move(name)},
           m_type{std::move(type)},
-          m_hidden{false},
           m_inner{std::move(inner)},
           m_fallback{std::move(fallback)},
           m_description{std::move(description)},
-          m_oneline_description{std::move(oneline_description)},
-          m_type_str{std::move(type_str)}
+          m_opts{std::move(opts)}
     {
         CHECK_NONFATAL(type == Type::ARR || type == Type::OBJ || type == Type::OBJ_USER_KEYS);
     }
@@ -227,7 +224,7 @@ struct RPCArg {
 
     /**
      * Return the type string of the argument.
-     * Set oneline to allow it to be overridden by a custom oneline type string (m_oneline_description).
+     * Set oneline to allow it to be overridden by a custom oneline type string (m_opts.oneline_description).
      */
     std::string ToString(bool oneline) const;
     /**
diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp
index 666516516923..dab1919ea4c3 100644
--- a/src/wallet/rpc/backup.cpp
+++ b/src/wallet/rpc/backup.cpp
@@ -1493,15 +1493,15 @@ RPCHelpMan importmulti()
                                 {
                                     {"desc", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Descriptor to import. If using descriptor, do not also provide address/scriptPubKey, scripts, or pubkeys"},
                                     {"scriptPubKey", RPCArg::Type::STR, RPCArg::Optional::NO, "Type of scriptPubKey (string for script, json for address). Should not be provided if using a descriptor",
-                                        /*oneline_description=*/"", {"\"<script>\" | { \"address\":\"<address>\" }", "string / json"}
+                                        RPCArgOptions{.type_str={"\"<script>\" | { \"address\":\"<address>\" }", "string / json"}}
                                     },
                                     {"timestamp", RPCArg::Type::NUM, RPCArg::Optional::NO, "Creation time of the key expressed in " + UNIX_EPOCH_TIME + ",\n"
-        "                                                              or the string \"now\" to substitute the current synced blockchain time. The timestamp of the oldest\n"
-        "                                                              key will determine how far back blockchain rescans need to begin for missing wallet transactions.\n"
-        "                                                              \"now\" can be specified to bypass scanning, for keys which are known to never have been used, and\n"
-        "                                                              0 can be specified to scan the entire blockchain. Blocks up to 2 hours before the earliest key\n"
-        "                                                              creation time of all keys being imported by the importmulti call will be scanned.",
-                                        /*oneline_description=*/"", {"timestamp | \"now\"", "integer / string"}
+                                        "or the string \"now\" to substitute the current synced blockchain time. The timestamp of the oldest\n"
+                                        "key will determine how far back blockchain rescans need to begin for missing wallet transactions.\n"
+                                        "\"now\" can be specified to bypass scanning, for keys which are known to never have been used, and\n"
+                                        "0 can be specified to scan the entire blockchain. Blocks up to 2 hours before the earliest key\n"
+                                        "creation time of all keys being imported by the importmulti call will be scanned.",
+                                        RPCArgOptions{.type_str={"timestamp | \"now\"", "integer / string"}}
                                     },
                                     {"redeemscript", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Allowed only if the scriptPubKey is a P2SH address or  a P2SH scriptPubKey"},
                                     {"pubkeys", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "Array of strings giving pubkeys to import. They must occur in P2PKH or P2WPKH scripts. They are not required when the private key is also provided (see the \"keys\" argument).",
@@ -1522,12 +1522,12 @@ RPCHelpMan importmulti()
                                 },
                             },
                         },
-                        "\"requests\""},
+                        RPCArgOptions{.oneline_description="\"requests\""}},
                     {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "",
                         {
                             {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Scan the chain and mempool for wallet transactions after all imports."},
                         },
-                        "\"options\""},
+                        RPCArgOptions{.oneline_description="\"options\""}},
                 },
                 RPCResult{
                     RPCResult::Type::ARR, "", "Response is an array with the same size as the input that has the execution result",
@@ -1835,18 +1835,18 @@ RPCHelpMan importdescriptors() {
                                     {"range", RPCArg::Type::RANGE, RPCArg::Optional::OMITTED, "If a ranged descriptor is used, this specifies the end or the range (in the form [begin,end]) to import"},
                                     {"next_index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "If a ranged descriptor is set to active, this specifies the next index to generate addresses from"},
                                     {"timestamp", RPCArg::Type::NUM, RPCArg::Optional::NO, "Time from which to start rescanning the blockchain for this descriptor, in " + UNIX_EPOCH_TIME + "\n"
-        "                                                              Use the string \"now\" to substitute the current synced blockchain time.\n"
-        "                                                              \"now\" can be specified to bypass scanning, for outputs which are known to never have been used, and\n"
-        "                                                              0 can be specified to scan the entire blockchain. Blocks up to 2 hours before the earliest timestamp\n"
-        "                                                              of all descriptors being imported will be scanned as well as the mempool.",
-                                        /*oneline_description=*/"", {"timestamp | \"now\"", "integer / string"}
+                                        "Use the string \"now\" to substitute the current synced blockchain time.\n"
+                                        "\"now\" can be specified to bypass scanning, for outputs which are known to never have been used, and\n"
+                                        "0 can be specified to scan the entire blockchain. Blocks up to 2 hours before the earliest timestamp\n"
+                                        "of all descriptors being imported will be scanned as well as the mempool.",
+                                        RPCArgOptions{.type_str={"timestamp | \"now\"", "integer / string"}}
                                     },
                                     {"internal", RPCArg::Type::BOOL, RPCArg::Default{false}, "Whether matching outputs should be treated as not incoming payments (e.g. change)"},
                                     {"label", RPCArg::Type::STR, RPCArg::Default{""}, "Label to assign to the address, only allowed with internal=false. Disabled for ranged descriptors"},
                                 },
                             },
                         },
-                        "\"requests\""},
+                        RPCArgOptions{.oneline_description="\"requests\""}},
                 },
                 RPCResult{
                     RPCResult::Type::ARR, "", "Response is an array with the same size as the input that has the execution result",
diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp
index 51fd86088526..282ecd9143b3 100644
--- a/src/wallet/rpc/coins.cpp
+++ b/src/wallet/rpc/coins.cpp
@@ -529,7 +529,7 @@ RPCHelpMan listunspent()
                                 "0=ALL_COINS, 1=ONLY_FULLY_MIXED, 2=ONLY_READY_TO_MIX, 3=ONLY_NONDENOMINATED,\n"
                                 "4=ONLY_MASTERNODE_COLLATERAL, 5=ONLY_COINJOIN_COLLATERAL" },
                         },
-                        "query_options"},
+                        RPCArgOptions{.oneline_description="query_options"}},
                 },
                 RPCResult{
                     RPCResult::Type::ARR, "", "",
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
index de9385e82b5d..9edcf79d8cd0 100644
--- a/src/wallet/rpc/spend.cpp
+++ b/src/wallet/rpc/spend.cpp
@@ -315,7 +315,7 @@ RPCHelpMan sendmany()
                 "\nSend multiple times. Amounts are double-precision floating point numbers." +
         HELP_REQUIRING_PASSPHRASE,
                 {
-                    {"dummy", RPCArg::Type::STR, RPCArg::Optional::NO, "Must be set to \"\" for backwards compatibility.", "\"\""},
+                    {"dummy", RPCArg::Type::STR, RPCArg::Optional::NO, "Must be set to \"\" for backwards compatibility.", RPCArgOptions{.oneline_description="\"\""}},
                     {"amounts", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::NO, "The addresses and amounts",
                         {
                             {"address", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "The Dash address is the key, the numeric amount (can be string) in " + CURRENCY_UNIT + " is the value"},
@@ -755,7 +755,7 @@ RPCHelpMan fundrawtransaction()
                              },
                         },
                         FundTxDoc()),
-                        "options"},
+                        RPCArgOptions{.oneline_description="options"}},
                 },
                 RPCResult{
                     RPCResult::Type::OBJ, "", "",
@@ -961,7 +961,7 @@ RPCHelpMan send()
                     },
                 },
                 FundTxDoc()),
-                "options"},
+                RPCArgOptions{.oneline_description="options"}},
         },
         RPCResult{
             RPCResult::Type::OBJ, "", "",
@@ -1068,7 +1068,7 @@ RPCHelpMan sendall()
                     },
                     FundTxDoc()
                 ),
-                "options"
+                RPCArgOptions{.oneline_description = "options"}
             },
         },
         RPCResult{
@@ -1397,7 +1397,7 @@ RPCHelpMan walletcreatefundedpsbt()
                             },
                         },
                         FundTxDoc()),
-                        "options"},
+                        RPCArgOptions{.oneline_description="options"}},
                     {"bip32derivs", RPCArg::Type::BOOL, RPCArg::Default{true}, "Include BIP 32 derivation paths for public keys if we know them"},
                 },
                 RPCResult{

From 9e4bfc1646681e5ac92b9d1ba1c936bda789fa4b Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Mon, 12 Dec 2022 14:08:21 +0100
Subject: [PATCH 4/8] test: Fix wrong types passed to RPCs

---
 src/rpc/evo.cpp                               |  3 ++-
 test/functional/feature_addressindex.py       | 20 ++++++++--------
 test/functional/feature_asset_locks.py        | 12 +++++-----
 .../feature_dip3_deterministicmns.py          |  2 +-
 test/functional/feature_dip3_v19.py           |  8 +++----
 test/functional/feature_llmq_chainlocks.py    |  6 ++---
 test/functional/feature_mnehf.py              |  2 +-
 test/functional/feature_spentindex.py         |  8 +++----
 test/functional/feature_txindex.py            |  2 +-
 test/functional/p2p_disconnect_ban.py         |  2 +-
 test/functional/p2p_instantsend.py            |  4 ++--
 test/functional/rpc_rawtransaction.py         | 23 ++++++++++---------
 .../test_framework/test_framework.py          | 10 ++++----
 test/functional/test_framework/util.py        |  2 +-
 test/functional/wallet_avoidreuse.py          |  4 ++--
 test/functional/wallet_descriptor.py          |  2 +-
 test/functional/wallet_listsinceblock.py      |  2 +-
 test/functional/wallet_txn_clone.py           |  2 +-
 18 files changed, 58 insertions(+), 56 deletions(-)

diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp
index 735f524e808b..c6bcb7c58531 100644
--- a/src/rpc/evo.cpp
+++ b/src/rpc/evo.cpp
@@ -1408,7 +1408,8 @@ static RPCHelpMan protx_list()
                 "                 This will also include ProTx which failed PoSe verification.\n"
 #endif
             },
-            {"detailed", RPCArg::Type::BOOL, RPCArg::Default{false}, "If not specified, only the hashes of the ProTx will be returned."},
+            {"detailed", RPCArg::Type::BOOL, RPCArg::Default{false}, "If not specified, only the hashes of the ProTx will be returned.",
+             RPCArgOptions{.skip_type_check = true}},
             {"height", RPCArg::Type::NUM, RPCArg::DefaultHint{"current chain-tip"}, ""},
         },
         RPCResult{
diff --git a/test/functional/feature_addressindex.py b/test/functional/feature_addressindex.py
index f30fd81fa391..e9a6372501fb 100755
--- a/test/functional/feature_addressindex.py
+++ b/test/functional/feature_addressindex.py
@@ -70,8 +70,8 @@ def run_test(self):
         assert_equal(self.nodes[2].getbalance(), 0)
 
         # Check that balances are correct
-        balance0 = self.nodes[1].getaddressbalance("93bVhahvUKmQu8gu9g3QnPPa2cxFK98pMB")
-        balance_mining = self.nodes[1].getaddressbalance(mining_address)
+        balance0 = self.nodes[1].getaddressbalance({"addresses": ["93bVhahvUKmQu8gu9g3QnPPa2cxFK98pMB"]})
+        balance_mining = self.nodes[1].getaddressbalance({"addresses": [mining_address]})
         assert_equal(balance0["balance"], 0)
         assert_equal(balance_mining["balance"], 105 * 500 * COIN)
         assert_equal(balance_mining["balance_immature"], 100 * 500 * COIN)
@@ -98,13 +98,13 @@ def run_test(self):
         txidb2 = self.nodes[0].sendtoaddress("93bVhahvUKmQu8gu9g3QnPPa2cxFK98pMB", 20)
         self.generate(self.nodes[0], 1)
 
-        txids = self.nodes[1].getaddresstxids("yMNJePdcKvXtWWQnFYHNeJ5u8TF2v1dfK4")
+        txids = self.nodes[1].getaddresstxids({"addresses": ["yMNJePdcKvXtWWQnFYHNeJ5u8TF2v1dfK4"]})
         assert_equal(len(txids), 3)
         assert_equal(txids[0], txid0)
         assert_equal(txids[1], txid1)
         assert_equal(txids[2], txid2)
 
-        txidsb = self.nodes[1].getaddresstxids("93bVhahvUKmQu8gu9g3QnPPa2cxFK98pMB")
+        txidsb = self.nodes[1].getaddresstxids({"addresses": ["93bVhahvUKmQu8gu9g3QnPPa2cxFK98pMB"]})
         assert_equal(len(txidsb), 3)
         assert_equal(txidsb[0], txidb0)
         assert_equal(txidsb[1], txidb1)
@@ -132,7 +132,7 @@ def run_test(self):
         assert_equal(multitxids[5], txidb2)
 
         # Check that balances are correct
-        balance0 = self.nodes[1].getaddressbalance("93bVhahvUKmQu8gu9g3QnPPa2cxFK98pMB")
+        balance0 = self.nodes[1].getaddressbalance({"addresses": ["93bVhahvUKmQu8gu9g3QnPPa2cxFK98pMB"]})
         assert_equal(balance0["balance"], 45 * 100000000)
 
         # Check that outputs with the same address will only return one txid
@@ -150,13 +150,13 @@ def run_test(self):
 
         self.generate(self.nodes[0], 1)
 
-        txidsmany = self.nodes[1].getaddresstxids("93bVhahvUKmQu8gu9g3QnPPa2cxFK98pMB")
+        txidsmany = self.nodes[1].getaddresstxids({"addresses": ["93bVhahvUKmQu8gu9g3QnPPa2cxFK98pMB"]})
         assert_equal(len(txidsmany), 4)
         assert_equal(txidsmany[3], sent_txid)
 
         # Check that balances are correct
         self.log.info("Testing balances...")
-        balance0 = self.nodes[1].getaddressbalance("93bVhahvUKmQu8gu9g3QnPPa2cxFK98pMB")
+        balance0 = self.nodes[1].getaddressbalance({"addresses": ["93bVhahvUKmQu8gu9g3QnPPa2cxFK98pMB"]})
         assert_equal(balance0["balance"], (45 + 21) * 100000000)
 
         # Check that balances are correct after spending
@@ -177,7 +177,7 @@ def run_test(self):
         signed_tx = self.nodes[0].signrawtransactionwithwallet(tx.serialize().hex())
         spending_txid = self.nodes[0].sendrawtransaction(signed_tx["hex"], 0)
         self.generate(self.nodes[0], 1)
-        balance1 = self.nodes[1].getaddressbalance(address2)
+        balance1 = self.nodes[1].getaddressbalance({"addresses": [address2]})
         assert_equal(balance1["balance"], amount)
 
         tx = CTransaction()
@@ -191,7 +191,7 @@ def run_test(self):
         sent_txid = self.nodes[0].sendrawtransaction(signed_tx["hex"], 0)
         self.generate(self.nodes[0], 1)
 
-        balance2 = self.nodes[1].getaddressbalance(address2)
+        balance2 = self.nodes[1].getaddressbalance({"addresses": [address2]})
         assert_equal(balance2["balance"], change_amount)
 
         # Check that deltas are returned correctly
@@ -229,7 +229,7 @@ def run_test(self):
         self.bump_mocktime(2)
         self.sync_all()
 
-        balance4 = self.nodes[1].getaddressbalance(address2)
+        balance4 = self.nodes[1].getaddressbalance({"addresses": [address2]})
         assert_equal(balance4, balance1)
 
         utxos2 = self.nodes[1].getaddressutxos({"addresses": [address2]})
diff --git a/test/functional/feature_asset_locks.py b/test/functional/feature_asset_locks.py
index 709cc386e7b5..f9ccf4f5eab1 100755
--- a/test/functional/feature_asset_locks.py
+++ b/test/functional/feature_asset_locks.py
@@ -285,7 +285,7 @@ def test_asset_locks(self, node_wallet, node, pubkey):
         self.check_mempool_result(tx=asset_lock_tx, result_expected={'allowed': True, 'fees': {'base': Decimal(str(tiny_amount / COIN))}})
         self.validate_credit_pool_balance(0)
         txid_in_block = self.send_tx(asset_lock_tx)
-        rpc_tx = node.getrawtransaction(txid_in_block, 1)
+        rpc_tx = node.getrawtransaction(txid_in_block, True)
         assert_equal(len(rpc_tx["assetLockTx"]["creditOutputs"]), 2)
         assert_equal(rpc_tx["assetLockTx"]["creditOutputs"][0]["valueSat"] + rpc_tx["assetLockTx"]["creditOutputs"][1]["valueSat"], locked_1)
         assert_equal(rpc_tx["assetLockTx"]["creditOutputs"][0]["scriptPubKey"]["hex"], key_to_p2pkh_script(pubkey).hex())
@@ -393,8 +393,8 @@ def test_asset_unlocks(self, node_wallet, node, pubkey):
         is_id = node_wallet.sendtoaddress(node_wallet.getnewaddress(), 1)
         self.wait_for_instantlock(is_id)
 
-        rawtx = node.getrawtransaction(txid, 1)
-        rawtx_is = node.getrawtransaction(is_id, 1)
+        rawtx = node.getrawtransaction(txid, True)
+        rawtx_is = node.getrawtransaction(is_id, True)
         assert_equal(rawtx["instantlock"], False)
         assert_equal(rawtx_is["instantlock"], True)
         assert_equal(rawtx["chainlock"], False)
@@ -404,7 +404,7 @@ def test_asset_unlocks(self, node_wallet, node, pubkey):
         self.log.info("Reset IS spork")
         self.set_sporks()
 
-        assert "assetUnlockTx" in node.getrawtransaction(txid, 1)
+        assert "assetUnlockTx" in node.getrawtransaction(txid, True)
 
         self.mempool_size += 2
         self.check_mempool_size()
@@ -412,12 +412,12 @@ def test_asset_unlocks(self, node_wallet, node, pubkey):
         self.generate(node, 1)
         assert_equal(rawtx["instantlock"], False)
         assert_equal(rawtx["chainlock"], False)
-        rawtx = node.getrawtransaction(txid, 1)
+        rawtx = node.getrawtransaction(txid, True)
         assert_equal(rawtx["confirmations"], 1)
         self.validate_credit_pool_balance(locked - COIN)
         self.mempool_size -= 2
         self.check_mempool_size()
-        block_asset_unlock = node.getrawtransaction(asset_unlock_tx.rehash(), 1)['blockhash']
+        block_asset_unlock = node.getrawtransaction(asset_unlock_tx.rehash(), True)['blockhash']
         self.log.info("Checking rpc `getblock` and `getblockstats` succeeds as they use own fee calculation mechanism")
         assert_equal(node.getblockstats(node.getblockcount())['maxfee'], tiny_amount)
         node.getblock(block_asset_unlock, 2)
diff --git a/test/functional/feature_dip3_deterministicmns.py b/test/functional/feature_dip3_deterministicmns.py
index 74096321588b..9be518f27784 100755
--- a/test/functional/feature_dip3_deterministicmns.py
+++ b/test/functional/feature_dip3_deterministicmns.py
@@ -182,7 +182,7 @@ def run_test(self):
             self.generate(self.nodes[0], 1)
             if expected_payee == multisig:
                 block = self.nodes[0].getblock(self.nodes[0].getbestblockhash())
-                cbtx = self.nodes[0].getrawtransaction(block['tx'][0], 1)
+                cbtx = self.nodes[0].getrawtransaction(block['tx'][0], True)
                 for out in cbtx['vout']:
                     if 'address' in out['scriptPubKey']:
                         if expected_payee in out['scriptPubKey']['address'] and out['valueSat'] == expected_amount:
diff --git a/test/functional/feature_dip3_v19.py b/test/functional/feature_dip3_v19.py
index f1010dcad0ac..1077ac2703f2 100755
--- a/test/functional/feature_dip3_v19.py
+++ b/test/functional/feature_dip3_v19.py
@@ -135,12 +135,12 @@ def test_revoke_protx(self, node_idx, revoke_mn: MasternodeInfo):
         fund_txid = self.nodes[0].sendtoaddress(funds_address, 1)
         self.bump_mocktime(10 * 60 + 1) # to make tx safe to include in block
         tip = self.generate(self.nodes[0], 1)[0]
-        assert_equal(self.nodes[0].getrawtransaction(fund_txid, 1, tip)['confirmations'], 1)
+        assert_equal(self.nodes[0].getrawtransaction(fund_txid, True, tip)['confirmations'], 1)
 
         protx_result = revoke_mn.revoke(self.nodes[0], submit=True, reason=1, fundsAddr=funds_address)
         self.bump_mocktime(10 * 60 + 1) # to make tx safe to include in block
         tip = self.generate(self.nodes[0], 1, sync_fun=self.no_op)[0]
-        assert_equal(self.nodes[0].getrawtransaction(protx_result, 1, tip)['confirmations'], 1)
+        assert_equal(self.nodes[0].getrawtransaction(protx_result, True, tip)['confirmations'], 1)
         # Revoking a MN results in disconnects. Wait for disconnects to actually happen
         # and then reconnect the corresponding node back to let sync_blocks finish correctly.
         self.wait_until(lambda: self.nodes[node_idx].getconnectioncount() == 0)
@@ -156,12 +156,12 @@ def test_update_service_protx(self, mn: MasternodeInfo):
         fund_txid = self.nodes[0].sendtoaddress(mn.fundsAddr, 1)
         self.bump_mocktime(10 * 60 + 1) # to make tx safe to include in block
         tip = self.generate(self.nodes[0], 1)[0]
-        assert_equal(self.nodes[0].getrawtransaction(fund_txid, 1, tip)['confirmations'], 1)
+        assert_equal(self.nodes[0].getrawtransaction(fund_txid, True, tip)['confirmations'], 1)
 
         protx_result = mn.update_service(self.nodes[0], submit=True, addrs_core_p2p=[f'127.0.0.2:{mn.nodePort}'])
         self.bump_mocktime(10 * 60 + 1) # to make tx safe to include in block
         tip = self.generate(self.nodes[0], 1)[0]
-        assert_equal(self.nodes[0].getrawtransaction(protx_result, 1, tip)['confirmations'], 1)
+        assert_equal(self.nodes[0].getrawtransaction(protx_result, True, tip)['confirmations'], 1)
 
         for node in self.nodes:
             protx_info = node.protx('info', mn.proTxHash)
diff --git a/test/functional/feature_llmq_chainlocks.py b/test/functional/feature_llmq_chainlocks.py
index 44861b622dff..8676da0bb8b2 100755
--- a/test/functional/feature_llmq_chainlocks.py
+++ b/test/functional/feature_llmq_chainlocks.py
@@ -208,7 +208,7 @@ def run_test(self):
         self.log.info("Assert that after block generation these TXs are NOT included (as they are \"unsafe\")")
         node0_tip = self.generate(self.nodes[0], 1, sync_fun=self.no_op)[-1]
         for txid in txs:
-            tx = self.nodes[0].getrawtransaction(txid, 1)
+            tx = self.nodes[0].getrawtransaction(txid, True)
             assert "confirmations" not in tx
 
         def test_cb(self):
@@ -226,7 +226,7 @@ def test_cb(self):
         self.nodes[0].sporkupdate("SPORK_2_INSTANTSEND_ENABLED", 0)
         self.log.info("Assert that TXs got included now")
         for txid in txs:
-            tx = self.nodes[0].getrawtransaction(txid, 1)
+            tx = self.nodes[0].getrawtransaction(txid, True)
             assert "confirmations" in tx and tx["confirmations"] > 0
         # Enable network on first node again, which will cause the blocks to propagate and IS locks to happen retroactively
         # for the mined TXs, which will then allow the network to create a CLSIG
@@ -256,7 +256,7 @@ def test_cb(self):
 
     def create_chained_txs(self, node, amount):
         txid = node.sendtoaddress(node.getnewaddress(), amount)
-        tx = node.getrawtransaction(txid, 1)
+        tx = node.getrawtransaction(txid, True)
         inputs = []
         valueIn = 0
         for txout in tx["vout"]:
diff --git a/test/functional/feature_mnehf.py b/test/functional/feature_mnehf.py
index 40b9631f07de..6ac0815db8f5 100755
--- a/test/functional/feature_mnehf.py
+++ b/test/functional/feature_mnehf.py
@@ -98,7 +98,7 @@ def check_fork(self, expected):
 
     def ensure_tx_is_not_mined(self, tx_id):
         try:
-            assert_equal(self.nodes[0].getrawtransaction(tx_id, 1)['height'], -1)
+            assert_equal(self.nodes[0].getrawtransaction(tx_id, True)['height'], -1)
             raise AssertionError("Transaction should not be mined")
         except KeyError:
             # KeyError is expected
diff --git a/test/functional/feature_spentindex.py b/test/functional/feature_spentindex.py
index 71fd9bb968ad..6303289f7063 100755
--- a/test/functional/feature_spentindex.py
+++ b/test/functional/feature_spentindex.py
@@ -97,13 +97,13 @@ def run_test(self):
         self.log.info("Testing getrawtransaction method...")
 
         # Check that verbose raw transaction includes spent info
-        txVerbose = self.nodes[3].getrawtransaction(unspent[0]["txid"], 1)
+        txVerbose = self.nodes[3].getrawtransaction(unspent[0]["txid"], True)
         assert_equal(txVerbose["vout"][unspent[0]["vout"]]["spentTxId"], txid)
         assert_equal(txVerbose["vout"][unspent[0]["vout"]]["spentIndex"], 0)
         assert_equal(txVerbose["vout"][unspent[0]["vout"]]["spentHeight"], 106)
 
         # Check that verbose raw transaction includes input values
-        txVerbose2 = self.nodes[3].getrawtransaction(txid, 1)
+        txVerbose2 = self.nodes[3].getrawtransaction(txid, True)
         assert_equal(txVerbose2["vin"][0]["value"], Decimal(unspent[0]["amount"]))
         assert_equal(txVerbose2["vin"][0]["valueSat"] - tx_fee_sat, amount)
 
@@ -121,7 +121,7 @@ def run_test(self):
 
         # Check the mempool index
         self.sync_all()
-        txVerbose3 = self.nodes[1].getrawtransaction(txid2, 1)
+        txVerbose3 = self.nodes[1].getrawtransaction(txid2, True)
         assert_equal(txVerbose3["vin"][0]["address"], address2)
         assert_equal(txVerbose3["vin"][0]["value"], Decimal(unspent[0]["amount"]) - tx_fee)
         assert_equal(txVerbose3["vin"][0]["valueSat"], amount)
@@ -129,7 +129,7 @@ def run_test(self):
         # Check the database index
         self.generate(self.nodes[0], 1)
 
-        txVerbose4 = self.nodes[3].getrawtransaction(txid2, 1)
+        txVerbose4 = self.nodes[3].getrawtransaction(txid2, True)
         assert_equal(txVerbose4["vin"][0]["address"], address2)
         assert_equal(txVerbose4["vin"][0]["value"], Decimal(unspent[0]["amount"]) - tx_fee)
         assert_equal(txVerbose4["vin"][0]["valueSat"], amount)
diff --git a/test/functional/feature_txindex.py b/test/functional/feature_txindex.py
index 183d08de93b2..984c2384700c 100755
--- a/test/functional/feature_txindex.py
+++ b/test/functional/feature_txindex.py
@@ -69,7 +69,7 @@ def run_test(self):
         self.generate(self.nodes[0], 1)
 
         # Check verbose raw transaction results
-        verbose = self.nodes[3].getrawtransaction(txid, 1)
+        verbose = self.nodes[3].getrawtransaction(txid, True)
         assert_equal(verbose["vout"][0]["valueSat"], 50000000000 - tx_fee_sat)
         assert_equal(verbose["vout"][0]["value"] * 100000000, 50000000000 - tx_fee_sat)
 
diff --git a/test/functional/p2p_disconnect_ban.py b/test/functional/p2p_disconnect_ban.py
index 181b69e6583c..cb512910c3f7 100755
--- a/test/functional/p2p_disconnect_ban.py
+++ b/test/functional/p2p_disconnect_ban.py
@@ -115,7 +115,7 @@ def run_test(self):
 
         self.log.info("disconnectnode: fail to disconnect when calling with address and nodeid")
         address1 = self.nodes[0].getpeerinfo()[0]['addr']
-        node1 = self.nodes[0].getpeerinfo()[0]['addr']
+        node1 = self.nodes[0].getpeerinfo()[0]["id"]
         assert_raises_rpc_error(-32602, "Only one of address and nodeid should be provided.", self.nodes[0].disconnectnode, address=address1, nodeid=node1)
 
         self.log.info("disconnectnode: fail to disconnect when calling with junk address")
diff --git a/test/functional/p2p_instantsend.py b/test/functional/p2p_instantsend.py
index 8dd14eb35cd7..b5226b78590d 100755
--- a/test/functional/p2p_instantsend.py
+++ b/test/functional/p2p_instantsend.py
@@ -65,11 +65,11 @@ def test_block_doublespend(self):
         # generate block on isolated node with doublespend transaction
         self.bump_mocktime(599)
         wrong_early_block = self.generate(isolated, 1, sync_fun=self.no_op)[0]
-        assert not "confirmation" in isolated.getrawtransaction(dblspnd_txid, 1)
+        assert not "confirmation" in isolated.getrawtransaction(dblspnd_txid, True)
         isolated.invalidateblock(wrong_early_block)
         self.bump_mocktime(1)
         wrong_block = self.generate(isolated, 1, sync_fun=self.no_op)[0]
-        assert_equal(isolated.getrawtransaction(dblspnd_txid, 1)["confirmations"], 1)
+        assert_equal(isolated.getrawtransaction(dblspnd_txid, True)["confirmations"], 1)
         # connect isolated block to network
         self.reconnect_isolated_node(self.isolated_idx, 0)
         # check doublespend block is rejected by other nodes
diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py
index ad0c55ba82e3..8d19ff72dfac 100755
--- a/test/functional/rpc_rawtransaction.py
+++ b/test/functional/rpc_rawtransaction.py
@@ -105,25 +105,21 @@ def getrawtransaction_tests(self):
                 assert_equal(self.nodes[n].getrawtransaction(txId), tx['hex'])
                 assert_equal(self.nodes[n].getrawtransactionmulti({"0":[txId]})[txId], tx['hex'])
 
-                # 2. valid parameters - supply txid and 0 for non-verbose
-                assert_equal(self.nodes[n].getrawtransaction(txId, 0), tx['hex'])
-                assert_equal(self.nodes[n].getrawtransactionmulti({"0":[txId]}, 0)[txId], tx['hex'])
-
-                # 3. valid parameters - supply txid and False for non-verbose
+                # 2. valid parameters - supply txid and False for non-verbose
                 assert_equal(self.nodes[n].getrawtransaction(txId, False), tx['hex'])
                 assert_equal(self.nodes[n].getrawtransactionmulti({"0":[txId]}, False)[txId], tx['hex'])
 
-                # 4. valid parameters - supply txid and 1 for verbose.
+                # 3. valid parameters - supply txid and True for verbose.
                 # We only check the "hex" field of the output so we don't need to update this test every time the output format changes.
-                assert_equal(self.nodes[n].getrawtransaction(txId, 1)["hex"], tx['hex'])
-                assert_equal(self.nodes[n].getrawtransactionmulti({"0":[txId]}, 1)[txId]['hex'], tx['hex'])
-
-                # 5. valid parameters - supply txid and True for non-verbose
                 assert_equal(self.nodes[n].getrawtransaction(txId, True)["hex"], tx['hex'])
+                assert_equal(self.nodes[n].getrawtransactionmulti({"0":[txId]}, True)[txId]['hex'], tx['hex'])
+
+                # 4. valid parameters - supply txid and True for verbose using named args
+                assert_equal(self.nodes[n].getrawtransaction(txid=txId, verbose=True)["hex"], tx['hex'])
                 assert_equal(self.nodes[n].getrawtransactionmulti(verbose=True, transactions={"0":[txId]})[txId]['hex'], tx['hex'])
             else:
                 # Without -txindex, expect to raise.
-                for verbose in [None, 0, False, 1, True]:
+                for verbose in [None, False, True]:
                     assert_raises_rpc_error(-5, err_msg, self.nodes[n].getrawtransaction, txId, verbose)
                     # getrawtransactionmulti does not need -txindex
                     assert_equal(self.nodes[n].getrawtransactionmulti({"0":[txId]}, verbose)[txId], "None")
@@ -139,6 +135,11 @@ def getrawtransaction_tests(self):
             # 8. invalid parameters - supply txid and empty dict
             assert_raises_rpc_error(-3, "not of expected type bool", self.nodes[n].getrawtransaction, txId, {})
 
+            # 9. invalid parameters - supply txid and integer values for verbose (type checking rejects non-bool)
+            for value in [0, 1]:
+                assert_raises_rpc_error(-3, "not of expected type bool", self.nodes[n].getrawtransaction, txid=txId, verbose=value)
+                assert_raises_rpc_error(-3, "not of expected type bool", self.nodes[n].getrawtransactionmulti, transactions={"0":[txId]}, verbose=value)
+
         # Make a tx by sending, then generate 2 blocks; block1 has the tx in it
         tx = self.wallet.send_self_transfer(from_node=self.nodes[2])['txid']
         block1, block2 = self.generate(self.nodes[2], 2)
diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index f63d486717e4..87123b726b9f 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -1183,7 +1183,7 @@ class MasternodeInfo:
 
     def bury_tx(self, test: BitcoinTestFramework, genIdx: int, txid: str, depth: int):
         chain_tip = test.generate(test.nodes[genIdx], depth)[0]
-        assert_equal(test.nodes[genIdx].getrawtransaction(txid, 1, chain_tip)['confirmations'], depth)
+        assert_equal(test.nodes[genIdx].getrawtransaction(txid, True, chain_tip)['confirmations'], depth)
 
     def generate_addresses(self, node: TestNode, force_all: bool = False):
         if not self.collateral_address or force_all:
@@ -1205,7 +1205,7 @@ def get_collateral_value(self) -> int:
         return EVONODE_COLLATERAL if self.evo else MASTERNODE_COLLATERAL
 
     def get_collateral_vout(self, node: TestNode, txid: str, should_throw: bool = True) -> int:
-        for txout in node.getrawtransaction(txid, 1)['vout']:
+        for txout in node.getrawtransaction(txid, True)['vout']:
             if txout['value'] == self.get_collateral_value():
                 return txout['n']
         if should_throw:
@@ -1268,12 +1268,12 @@ def register(self, node: TestNode, submit: bool, collateral_txid: Optional[str]
         # Common arguments shared between regular masternodes and EvoNodes
         args = [
             collateral_txid or self.collateral_txid,
-            collateral_vout or self.collateral_vout,
+            collateral_vout if collateral_vout is not None else self.collateral_vout,
             [f'127.0.0.1:{self.nodePort}'] if addrs_core_p2p is None else addrs_core_p2p,
             ownerAddr or self.ownerAddr,
             pubKeyOperator or self.pubKeyOperator,
             votingAddr or self.votingAddr,
-            operator_reward or self.operator_reward,
+            str(operator_reward if operator_reward is not None else self.operator_reward),
             rewards_address or self.rewards_address,
         ]
         address_funds = fundsAddr or self.fundsAddr
@@ -1327,7 +1327,7 @@ def register_fund(self, node: TestNode, submit: bool, collateral_address: Option
             ownerAddr or self.ownerAddr,
             pubKeyOperator or self.pubKeyOperator,
             votingAddr or self.votingAddr,
-            operator_reward or self.operator_reward,
+            str(operator_reward if operator_reward is not None else self.operator_reward),
             rewards_address or self.rewards_address,
         ]
         address_funds = fundsAddr or self.fundsAddr
diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py
index 7cdcbc8983d8..168920de092d 100644
--- a/test/functional/test_framework/util.py
+++ b/test/functional/test_framework/util.py
@@ -584,7 +584,7 @@ def find_output(node, txid, amount, *, blockhash=None):
     Return index to output of txid with value amount
     Raises exception if there is none.
     """
-    txdata = node.getrawtransaction(txid, 1, blockhash)
+    txdata = node.getrawtransaction(txid, True, blockhash)
     for i in range(len(txdata["vout"])):
         if txdata["vout"][i]["value"] == amount:
             return i
diff --git a/test/functional/wallet_avoidreuse.py b/test/functional/wallet_avoidreuse.py
index 0d25df114ca0..802e3444409d 100755
--- a/test/functional/wallet_avoidreuse.py
+++ b/test/functional/wallet_avoidreuse.py
@@ -331,7 +331,7 @@ def test_full_destination_group_is_preferred(self):
         # Sending a transaction that is smaller than each one of the
         # available outputs
         txid = self.nodes[1].sendtoaddress(address=ret_addr, amount=0.5)
-        inputs = self.nodes[1].getrawtransaction(txid, 1)["vin"]
+        inputs = self.nodes[1].getrawtransaction(txid, True)["vin"]
 
         # The transaction should use 100 inputs exactly
         assert_equal(len(inputs), 100)
@@ -359,7 +359,7 @@ def test_all_destination_groups_are_used(self):
         # Sending a transaction that needs to use the full groups
         # of 100 inputs but also the incomplete group of 2 inputs.
         txid = self.nodes[1].sendtoaddress(address=ret_addr, amount=200.5)
-        inputs = self.nodes[1].getrawtransaction(txid, 1)["vin"]
+        inputs = self.nodes[1].getrawtransaction(txid, True)["vin"]
 
         # The transaction should use 202 inputs exactly
         assert_equal(len(inputs), 202)
diff --git a/test/functional/wallet_descriptor.py b/test/functional/wallet_descriptor.py
index e12337c02887..774d9a8efb0e 100755
--- a/test/functional/wallet_descriptor.py
+++ b/test/functional/wallet_descriptor.py
@@ -76,7 +76,7 @@ def run_test(self):
         # Make sure things are disabled
         self.log.info("Test disabled RPCs")
         assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.importprivkey, "cVpF924EspNh8KjYsfhgY96mmxvT6DgdWiTYMtMjuM74hJaU5psW")
-        assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.importpubkey, send_wrpc.getaddressinfo(send_wrpc.getnewaddress()))
+        assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.importpubkey, send_wrpc.getaddressinfo(send_wrpc.getnewaddress())["pubkey"])
         assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.importaddress, recv_wrpc.getnewaddress())
         assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.importmulti, [])
         assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.addmultisigaddress, 1, [recv_wrpc.getnewaddress()])
diff --git a/test/functional/wallet_listsinceblock.py b/test/functional/wallet_listsinceblock.py
index 6cea156b1e58..1c08c9bc8294 100755
--- a/test/functional/wallet_listsinceblock.py
+++ b/test/functional/wallet_listsinceblock.py
@@ -369,7 +369,7 @@ def double_spends_filtered(self):
 
         double_signedtx = spending_node.signrawtransactionwithwallet(double_rawtx)
         dbl_tx_id = double_spending_node.sendrawtransaction(double_signedtx["hex"])
-        double_tx = double_spending_node.getrawtransaction(dbl_tx_id, 1)
+        double_tx = double_spending_node.getrawtransaction(dbl_tx_id, True)
         lastblockhash = self.generate(double_spending_node, 1, sync_fun=self.no_op)[0]
 
         self.reconnect_isolated_node(3, 2)
diff --git a/test/functional/wallet_txn_clone.py b/test/functional/wallet_txn_clone.py
index cc2a2488dfe4..485e70ad8887 100755
--- a/test/functional/wallet_txn_clone.py
+++ b/test/functional/wallet_txn_clone.py
@@ -68,7 +68,7 @@ def run_test(self):
         txid2 = self.spend_txid(node0_txid2, find_vout_for_address(self.nodes[0], node0_txid2, node0_address2), {node1_address: 200})
 
         # Construct a clone of tx1, to be malleated
-        rawtx1 = self.nodes[0].getrawtransaction(txid1, 1)
+        rawtx1 = self.nodes[0].getrawtransaction(txid1, True)
         clone_inputs = [{"txid": rawtx1["vin"][0]["txid"], "vout": rawtx1["vin"][0]["vout"], "sequence": rawtx1["vin"][0]["sequence"]}]
         clone_outputs = {rawtx1["vout"][0]["scriptPubKey"]["address"]: rawtx1["vout"][0]["value"],
                          rawtx1["vout"][1]["scriptPubKey"]["address"]: rawtx1["vout"][1]["value"]}

From 1f94e125543ed8edf8d0d7267d34e1f5f979050a Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Mon, 12 Dec 2022 14:30:14 +0100
Subject: [PATCH 5/8] rpc: Run type check against RPCArgs

---
 src/rpc/blockchain.cpp     | 22 ++++++++-------
 src/rpc/evo.cpp            | 57 +++++++++++++++++++++----------------
 src/rpc/fees.cpp           |  4 ---
 src/rpc/mempool.cpp        | 14 ---------
 src/rpc/net.cpp            |  1 -
 src/rpc/node.cpp           | 33 ++++++++++------------
 src/rpc/output_script.cpp  |  5 ----
 src/rpc/rawtransaction.cpp | 38 +------------------------
 src/rpc/util.cpp           | 58 +++++++++++++++++++++++++++-----------
 src/rpc/util.h             | 11 +++-----
 src/wallet/rpc/backup.cpp  |  3 --
 src/wallet/rpc/spend.cpp   | 47 ++++++++----------------------
 src/wallet/rpc/wallet.cpp  |  4 ---
 13 files changed, 117 insertions(+), 180 deletions(-)

diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 96cb775cd086..c685daa32f6c 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -500,11 +500,6 @@ static RPCHelpMan getblockfrompeer()
         },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    RPCTypeCheck(request.params, {
-        UniValue::VSTR, // blockhash
-        UniValue::VNUM, // peer_id
-    });
-
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     ChainstateManager& chainman = EnsureChainman(node);
     PeerManager& peerman = EnsurePeerman(node);
@@ -929,7 +924,8 @@ static RPCHelpMan getblock()
                 "If verbosity is 3, returns an Object with information about block <hash> and information about each transaction, including prevout information for inputs (only for unpruned blocks in the current best chain).\n",
                 {
                     {"blockhash", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The block hash"},
-                    {"verbosity|verbose", RPCArg::Type::NUM, RPCArg::Default{1}, "0 for hex-encoded data, 1 for a JSON object, 2 for JSON object with transaction data, and 3 for JSON object with transaction data including prevout information for inputs"},
+                    {"verbosity|verbose", RPCArg::Type::NUM, RPCArg::Default{1}, "0 for hex-encoded data, 1 for a JSON object, 2 for JSON object with transaction data, and 3 for JSON object with transaction data including prevout information for inputs",
+                     RPCArgOptions{.skip_type_check = true}},
                 },
                 {
                     RPCResult{"for verbosity = 0",
@@ -1146,7 +1142,11 @@ static RPCHelpMan gettxoutsetinfo()
                 "Note this call may take some time if you are not using coinstatsindex.\n",
                 {
                     {"hash_type", RPCArg::Type::STR, RPCArg::Default{"hash_serialized_2"}, "Which UTXO set hash should be calculated. Options: 'hash_serialized_2' (the legacy algorithm), 'muhash', 'none'."},
-                    {"hash_or_height", RPCArg::Type::NUM, RPCArg::DefaultHint{"the current best block"}, "The block hash or height of the target height (only available with coinstatsindex).", RPCArgOptions{.type_str={"", "string or numeric"}}},
+                    {"hash_or_height", RPCArg::Type::NUM, RPCArg::DefaultHint{"the current best block"}, "The block hash or height of the target height (only available with coinstatsindex).",
+                     RPCArgOptions{
+                         .skip_type_check = true,
+                         .type_str = {"", "string or numeric"},
+                     }},
                     {"use_index", RPCArg::Type::BOOL, RPCArg::Default{true}, "Use coinstatsindex, if available."},
                 },
                 RPCResult{
@@ -2019,7 +2019,11 @@ static RPCHelpMan getblockstats()
                 "\nCompute per block statistics for a given window. All amounts are in duffs.\n"
                 "It won't work for some heights with pruning.\n",
                 {
-                    {"hash_or_height", RPCArg::Type::NUM, RPCArg::Optional::NO, "The block hash or height of the target block", RPCArgOptions{.type_str={"", "string or numeric"}}},
+                    {"hash_or_height", RPCArg::Type::NUM, RPCArg::Optional::NO, "The block hash or height of the target block",
+                     RPCArgOptions{
+                         .skip_type_check = true,
+                         .type_str = {"", "string or numeric"},
+                     }},
                     {"stats", RPCArg::Type::ARR, RPCArg::DefaultHint{"all values"}, "Values to plot (see result below)",
                         {
                             {"height", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Selected statistic"},
@@ -2497,8 +2501,6 @@ static RPCHelpMan scantxoutset()
         },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VARR});
-
     UniValue result(UniValue::VOBJ);
     if (request.params[0].get_str() == "status") {
         CoinsViewScanReserver reserver;
diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp
index c6bcb7c58531..c2e7f9d8a1f4 100644
--- a/src/rpc/evo.cpp
+++ b/src/rpc/evo.cpp
@@ -91,14 +91,16 @@ static RPCArg GetRpcArg(const std::string& strParamName)
                 "Can be set to an empty string, which will require a ProUpServTx afterwards.",
                 {
                     {"address", RPCArg::Type::STR, RPCArg::Optional::NO, ""},
-                }}
+                },
+                RPCArgOptions{.skip_type_check = true}}
         },
         {"coreP2PAddrs_update",
             {"coreP2PAddrs", RPCArg::Type::ARR, RPCArg::Optional::NO,
                 "Array of addresses in the form \"ADDR:PORT\". Must be unique on the network.",
                 {
                     {"address", RPCArg::Type::STR, RPCArg::Optional::NO, ""},
-                }}
+                },
+                RPCArgOptions{.skip_type_check = true}}
         },
         {"operatorKey",
             {"operatorKey", RPCArg::Type::STR, RPCArg::Optional::NO,
@@ -187,7 +189,8 @@ static RPCArg GetRpcArg(const std::string& strParamName)
                 "Must be unique on the network. Can be set to an empty string, which will require a ProUpServTx afterwards.",
                 {
                     {"address", RPCArg::Type::STR, RPCArg::Optional::NO, ""},
-                }}
+                },
+                RPCArgOptions{.skip_type_check = true}}
         },
         {"platformP2PAddrs_update",
             {"platformP2PAddrs", RPCArg::Type::ARR, RPCArg::Optional::NO,
@@ -195,7 +198,8 @@ static RPCArg GetRpcArg(const std::string& strParamName)
                 "Must be unique on the network.",
                 {
                     {"address", RPCArg::Type::STR, RPCArg::Optional::NO, ""},
-                }}
+                },
+                RPCArgOptions{.skip_type_check = true}}
         },
         {"platformHTTPSAddrs",
             {"platformHTTPSAddrs", RPCArg::Type::ARR, RPCArg::Optional::NO,
@@ -203,7 +207,8 @@ static RPCArg GetRpcArg(const std::string& strParamName)
                 "Must be unique on the network. Can be set to an empty string, which will require a ProUpServTx afterwards.",
                 {
                     {"address", RPCArg::Type::STR, RPCArg::Optional::NO, ""},
-                }}
+                },
+                RPCArgOptions{.skip_type_check = true}}
         },
         {"platformHTTPSAddrs_update",
             {"platformHTTPSAddrs", RPCArg::Type::ARR, RPCArg::Optional::NO,
@@ -211,7 +216,8 @@ static RPCArg GetRpcArg(const std::string& strParamName)
                 "Must be unique on the network.",
                 {
                     {"address", RPCArg::Type::STR, RPCArg::Optional::NO, ""},
-                }}
+                },
+                RPCArgOptions{.skip_type_check = true}}
         },
     };
 
@@ -1605,17 +1611,12 @@ static const CBlockIndex* ParseBlockIndex(const UniValue& v, const ChainstateMan
 {
     AssertLockHeld(::cs_main);
 
-    try {
-        const auto hash{ParseBlock(v, chainman, strName)};
-        const CBlockIndex* pindex = chainman.m_blockman.LookupBlockIndex(hash);
-        if (!pindex) {
-            throw std::runtime_error(strprintf("Block %s with hash %s not found", strName, v.getValStr()));
-        }
-        return pindex;
-    } catch (...) {
-        // Same phrasing as ParseBlock() as it can parse heights
-        throw std::runtime_error(strprintf("%s must be a block hash or chain height and not %s", strName, v.getValStr()));
+    const auto hash{ParseBlock(v, chainman, strName)};
+    const CBlockIndex* pindex = chainman.m_blockman.LookupBlockIndex(hash);
+    if (!pindex) {
+        throw std::runtime_error(strprintf("Block %s with hash %s not found", strName, v.getValStr()));
     }
+    return pindex;
 }
 
 static RPCHelpMan protx_diff()
@@ -1623,8 +1624,10 @@ static RPCHelpMan protx_diff()
     return RPCHelpMan{"protx diff",
         "\nCalculates a diff between two deterministic masternode lists. The result also contains proof data.\n",
         {
-            {"baseBlock", RPCArg::Type::STR, RPCArg::Optional::NO, "The starting block hash or height."},
-            {"block", RPCArg::Type::STR, RPCArg::Optional::NO, "The ending block hash or height."},
+            {"baseBlock", RPCArg::Type::STR, RPCArg::Optional::NO, "The starting block hash or height.",
+             RPCArgOptions{.skip_type_check = true}},
+            {"block", RPCArg::Type::STR, RPCArg::Optional::NO, "The ending block hash or height.",
+             RPCArgOptions{.skip_type_check = true}},
             {"extended", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED, "Show additional fields."},
         },
         CSimplifiedMNListDiff::GetJsonHelp(/*key=*/"", /*optional=*/false),
@@ -1664,8 +1667,10 @@ static RPCHelpMan protx_listdiff()
     return RPCHelpMan{"protx listdiff",
                "\nCalculate a full MN list diff between two masternode lists.\n",
                {
-                       {"baseBlock", RPCArg::Type::STR, RPCArg::Optional::NO, "The starting block hash or height."},
-                       {"block", RPCArg::Type::STR, RPCArg::Optional::NO, "The ending block hash or height."},
+                       {"baseBlock", RPCArg::Type::STR, RPCArg::Optional::NO, "The starting block hash or height.",
+                        RPCArgOptions{.skip_type_check = true}},
+                       {"block", RPCArg::Type::STR, RPCArg::Optional::NO, "The ending block hash or height.",
+                        RPCArgOptions{.skip_type_check = true}},
                },
                 RPCResult {
                     RPCResult::Type::OBJ, "", "",
@@ -1840,8 +1845,10 @@ static RPCHelpMan evodb_verify()
         "This is a read-only operation that does not modify the database.\n"
         "If no heights are specified, defaults to the full range from DIP0003 activation to chain tip.\n",
         {
-            {"startBlock", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The starting block hash or height (defaults to DIP0003 activation height)."},
-            {"stopBlock", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The ending block hash or height (defaults to current chain tip)."},
+            {"startBlock", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The starting block hash or height (defaults to DIP0003 activation height).",
+             RPCArgOptions{.skip_type_check = true}},
+            {"stopBlock", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The ending block hash or height (defaults to current chain tip).",
+             RPCArgOptions{.skip_type_check = true}},
         },
         RPCResult{
             RPCResult::Type::OBJ, "", "",
@@ -1878,8 +1885,10 @@ static RPCHelpMan evodb_repair()
         "If verification fails, recalculates diffs from blockchain data and replaces corrupted records.\n"
         "If no heights are specified, defaults to the full range from DIP0003 activation to chain tip.\n",
         {
-            {"startBlock", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The starting block hash or height (defaults to DIP0003 activation height)."},
-            {"stopBlock", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The ending block hash or height (defaults to current chain tip)."},
+            {"startBlock", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The starting block hash or height (defaults to DIP0003 activation height).",
+             RPCArgOptions{.skip_type_check = true}},
+            {"stopBlock", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The ending block hash or height (defaults to current chain tip).",
+             RPCArgOptions{.skip_type_check = true}},
         },
         RPCResult{
             RPCResult::Type::OBJ, "", "",
diff --git a/src/rpc/fees.cpp b/src/rpc/fees.cpp
index 0835759b3e75..a4b15f0dc4aa 100644
--- a/src/rpc/fees.cpp
+++ b/src/rpc/fees.cpp
@@ -64,8 +64,6 @@ static RPCHelpMan estimatesmartfee()
         },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
         {
-            RPCTypeCheck(request.params, {UniValue::VNUM, UniValue::VSTR});
-
             CBlockPolicyEstimator& fee_estimator = EnsureAnyFeeEstimator(request.context);
             const NodeContext& node = EnsureAnyNodeContext(request.context);
             const CTxMemPool& mempool = EnsureMemPool(node);
@@ -155,8 +153,6 @@ static RPCHelpMan estimaterawfee()
         },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
         {
-            RPCTypeCheck(request.params, {UniValue::VNUM, UniValue::VNUM}, true);
-
             CBlockPolicyEstimator& fee_estimator = EnsureAnyFeeEstimator(request.context);
 
             unsigned int max_target = fee_estimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE);
diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp
index 8315c6a96db3..4c5f97938496 100644
--- a/src/rpc/mempool.cpp
+++ b/src/rpc/mempool.cpp
@@ -60,13 +60,6 @@ RPCHelpMan sendrawtransaction()
                 },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
         {
-            RPCTypeCheck(request.params, {
-                UniValue::VSTR,
-                UniValueType(), // VNUM or VSTR, checked inside AmountFromValue()
-                UniValue::VBOOL,
-                UniValue::VBOOL,
-            });
-
             CMutableTransaction mtx;
             if (!DecodeHexTx(mtx, request.params[0].get_str())) {
                 throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed. Make sure the tx has at least one input.");
@@ -145,10 +138,6 @@ static RPCHelpMan testmempoolaccept()
                 },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
         {
-            RPCTypeCheck(request.params, {
-                UniValue::VARR,
-                UniValueType(), // VNUM or VSTR, checked inside AmountFromValue()
-            });
             const UniValue raw_transactions = request.params[0].get_array();
             if (raw_transactions.size() < 1 || raw_transactions.size() > MAX_PACKAGE_COUNT) {
                 throw JSONRPCError(RPC_INVALID_PARAMETER,
@@ -809,9 +798,6 @@ static RPCHelpMan submitpackage()
             if (!Params().IsMockableChain()) {
                 throw std::runtime_error("submitpackage is for regression testing (-regtest mode) only");
             }
-            RPCTypeCheck(request.params, {
-                UniValue::VARR,
-            });
             const UniValue raw_transactions = request.params[0].get_array();
             if (raw_transactions.size() < 1 || raw_transactions.size() > MAX_PACKAGE_COUNT) {
                 throw JSONRPCError(RPC_INVALID_PARAMETER,
diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp
index 2e75a19f3bcc..4d722640bf6c 100644
--- a/src/rpc/net.cpp
+++ b/src/rpc/net.cpp
@@ -401,7 +401,6 @@ static RPCHelpMan addconnection()
         throw std::runtime_error("addconnection is for regression testing (-regtest mode) only.");
     }
 
-    RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VSTR});
     const std::string address = request.params[0].get_str();
     const std::string conn_type_in{TrimString(request.params[1].get_str())};
     ConnectionType conn_type{};
diff --git a/src/rpc/node.cpp b/src/rpc/node.cpp
index a095d2f42a54..bf962589f158 100644
--- a/src/rpc/node.cpp
+++ b/src/rpc/node.cpp
@@ -262,7 +262,6 @@ static RPCHelpMan setmocktime()
     // ensure all call sites of GetTime() are accessing this safely.
     LOCK(cs_main);
 
-    RPCTypeCheck(request.params, {UniValue::VNUM});
     const int64_t time{request.params[0].getInt<int64_t>()};
     if (time < 0) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Mocktime cannot be negative: %s.", time));
@@ -388,7 +387,7 @@ static RPCHelpMan getaddressmempool()
                 {
                     {"address", RPCArg::Type::STR, RPCArg::Default{""}, "The base58check encoded address"},
                 },
-            },
+                RPCArgOptions{.skip_type_check = true}},
         },
         RPCResult{
             RPCResult::Type::ARR, "", "",
@@ -461,7 +460,7 @@ static RPCHelpMan getaddressutxos()
                 {
                     {"address", RPCArg::Type::STR, RPCArg::Default{""}, "The base58check encoded address"},
                 },
-            },
+                RPCArgOptions{.skip_type_check = true}},
         },
         RPCResult{
             RPCResult::Type::ARR, "", "",
@@ -532,7 +531,7 @@ static RPCHelpMan getaddressdeltas()
                 {
                     {"address", RPCArg::Type::STR, RPCArg::Default{""}, "The base58check encoded address"},
                 },
-            },
+                RPCArgOptions{.skip_type_check = true}},
         },
         RPCResult{
             RPCResult::Type::ARR, "", "",
@@ -619,7 +618,7 @@ static RPCHelpMan getaddressbalance()
                 {
                     {"address", RPCArg::Type::STR, RPCArg::Default{""}, "The base58check encoded address"},
                 },
-            },
+                RPCArgOptions{.skip_type_check = true}},
         },
         RPCResult{
             RPCResult::Type::OBJ, "", "",
@@ -696,7 +695,7 @@ static RPCHelpMan getaddresstxids()
                 {
                     {"address", RPCArg::Type::STR, RPCArg::Default{""}, "The base58check encoded address"},
                 },
-            },
+                RPCArgOptions{.skip_type_check = true}},
         },
         RPCResult{
             RPCResult::Type::ARR, "", "",
@@ -835,8 +834,6 @@ static RPCHelpMan mockscheduler()
         throw std::runtime_error("mockscheduler is for regression testing (-regtest mode) only");
     }
 
-    // check params are valid values
-    RPCTypeCheck(request.params, {UniValue::VNUM});
     int64_t delta_seconds = request.params[0].getInt<int64_t>();
     if (delta_seconds <= 0 || delta_seconds > 3600) {
         throw std::runtime_error("delta_time must be between 1 and 3600 seconds (1 hr)");
@@ -1030,16 +1027,16 @@ static RPCHelpMan echo(const std::string& name)
                 "\nThe difference between echo and echojson is that echojson has argument conversion enabled in the client-side table in "
                 "dash-cli and the GUI. There is no server-side difference.",
         {
-            {"arg0", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""},
-            {"arg1", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""},
-            {"arg2", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""},
-            {"arg3", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""},
-            {"arg4", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""},
-            {"arg5", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""},
-            {"arg6", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""},
-            {"arg7", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""},
-            {"arg8", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""},
-            {"arg9", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""},
+            {"arg0", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}},
+            {"arg1", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}},
+            {"arg2", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}},
+            {"arg3", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}},
+            {"arg4", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}},
+            {"arg5", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}},
+            {"arg6", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}},
+            {"arg7", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}},
+            {"arg8", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}},
+            {"arg9", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}},
         },
                 RPCResult{RPCResult::Type::ANY, "", "Returns whatever was passed in"},
                 RPCExamples{""},
diff --git a/src/rpc/output_script.cpp b/src/rpc/output_script.cpp
index 2ad98f709d38..97209a5d0fee 100644
--- a/src/rpc/output_script.cpp
+++ b/src/rpc/output_script.cpp
@@ -158,9 +158,6 @@ static RPCHelpMan getdescriptorinfo()
         },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
         {
-
-            RPCTypeCheck(request.params, {UniValue::VSTR});
-
             FlatSigningProvider provider;
             std::string error;
             auto desc = Parse(request.params[0].get_str(), provider, error);
@@ -209,8 +206,6 @@ static RPCHelpMan deriveaddresses()
         },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
         {
-
-            RPCTypeCheck(request.params, {UniValue::VSTR, UniValueType()}); // Range argument is checked later
             const std::string desc_str = request.params[0].get_str();
 
             int64_t range_begin = 0;
diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
index 7a0dd1f71e05..8b6aa0fff410 100644
--- a/src/rpc/rawtransaction.cpp
+++ b/src/rpc/rawtransaction.cpp
@@ -216,7 +216,7 @@ static std::vector<RPCArg> CreateTxDoc()
                     },
                 },
             },
-        },
+         RPCArgOptions{.skip_type_check = true}},
         {"locktime", RPCArg::Type::NUM, RPCArg::Default{0}, "Raw locktime. Non-0 value also locktime-activates inputs"},
     };
 }
@@ -726,7 +726,6 @@ static RPCHelpMan getassetunlockstatuses()
     std::optional<CCreditPool> poolCL{std::nullopt};
     std::optional<CCreditPool> poolOnTip{std::nullopt};
     std::optional<int> nSpecificCoreHeight{std::nullopt};
-
     if (!request.params[1].isNull()) {
         nSpecificCoreHeight = request.params[1].getInt<int>();
         if (nSpecificCoreHeight.value() < 0 || nSpecificCoreHeight.value() > chainman.ActiveChain().Height()) {
@@ -820,13 +819,6 @@ static RPCHelpMan createrawtransaction()
                 },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    RPCTypeCheck(request.params, {
-        UniValue::VARR,
-        UniValueType(), // ARR or OBJ, checked later
-        UniValue::VNUM,
-        }, true
-    );
-
     CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2]);
 
     return EncodeHexTx(CTransaction(rawTx));
@@ -851,8 +843,6 @@ static RPCHelpMan decoderawtransaction()
                 },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    RPCTypeCheck(request.params, {UniValue::VSTR});
-
     CMutableTransaction mtx;
 
     if (!DecodeHexTx(mtx, request.params[0].get_str()))
@@ -889,8 +879,6 @@ static RPCHelpMan decodescript()
         },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    RPCTypeCheck(request.params, {UniValue::VSTR});
-
     UniValue r(UniValue::VOBJ);
     CScript script;
     if (request.params[0].get_str().size() > 0){
@@ -1062,8 +1050,6 @@ static RPCHelpMan signrawtransactionwithkey()
                 },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VARR, UniValue::VARR, UniValue::VSTR}, true);
-
     CMutableTransaction mtx;
     if (!DecodeHexTx(mtx, request.params[0].get_str())) {
         throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed. Make sure the tx has at least one input.");
@@ -1253,8 +1239,6 @@ static RPCHelpMan decodepsbt()
                 },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    RPCTypeCheck(request.params, {UniValue::VSTR});
-
     // Unserialize the transactions
     PartiallySignedTransaction psbtx;
     std::string error;
@@ -1533,8 +1517,6 @@ static RPCHelpMan combinepsbt()
         },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    RPCTypeCheck(request.params, {UniValue::VARR}, true);
-
     // Unserialize the transactions
     std::vector<PartiallySignedTransaction> psbtxs;
     UniValue txs = request.params[0].get_array();
@@ -1588,8 +1570,6 @@ static RPCHelpMan finalizepsbt()
                 },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VBOOL}, true);
-
     // Unserialize the transactions
     PartiallySignedTransaction psbtx;
     std::string error;
@@ -1637,13 +1617,6 @@ static RPCHelpMan createpsbt()
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
 
-    RPCTypeCheck(request.params, {
-        UniValue::VARR,
-        UniValueType(), // ARR or OBJ, checked later
-        UniValue::VNUM,
-        }, true
-    );
-
     CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2]);
 
     // Make a blank psbt
@@ -1686,8 +1659,6 @@ static RPCHelpMan converttopsbt()
                 },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VBOOL, UniValue::VBOOL}, true);
-
     // parse hex string from parameter
     CMutableTransaction tx;
     bool permitsigdata = request.params[1].isNull() ? false : request.params[1].get_bool();
@@ -1744,9 +1715,6 @@ static RPCHelpMan utxoupdatepsbt()
             },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VARR}, true);
-
-
     // Parse descriptors, if any.
     FlatSigningProvider provider;
     if (!request.params[1].isNull()) {
@@ -1788,8 +1756,6 @@ static RPCHelpMan joinpsbts()
             },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    RPCTypeCheck(request.params, {UniValue::VARR}, true);
-
     // Unserialize the transactions
     std::vector<PartiallySignedTransaction> psbtxs;
     UniValue txs = request.params[0].get_array();
@@ -1893,8 +1859,6 @@ static RPCHelpMan analyzepsbt()
             },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    RPCTypeCheck(request.params, {UniValue::VSTR});
-
     // Unserialize the transaction
     PartiallySignedTransaction psbtx;
     std::string error;
diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp
index 9e36db172c86..69999feef00b 100644
--- a/src/rpc/util.cpp
+++ b/src/rpc/util.cpp
@@ -31,23 +31,6 @@ std::string GetAllOutputTypes()
     return Join(ret, ", ");
 }
 
-void RPCTypeCheck(const UniValue& params,
-                  const std::list<UniValueType>& typesExpected,
-                  bool fAllowNull)
-{
-    unsigned int i = 0;
-    for (const UniValueType& t : typesExpected) {
-        if (params.size() <= i)
-            break;
-
-        const UniValue& v = params[i];
-        if (!(fAllowNull && v.isNull())) {
-            RPCTypeCheckArgument(v, t);
-        }
-        i++;
-    }
-}
-
 void RPCTypeCheckArgument(const UniValue& value, const UniValueType& typeExpected)
 {
     if (!typeExpected.typeAny && value.type() != typeExpected.type) {
@@ -525,6 +508,9 @@ UniValue RPCHelpMan::HandleRequest(const JSONRPCRequest& request) const
     if (request.mode == JSONRPCRequest::GET_HELP || !IsValidNumArgs(request.params.size())) {
         throw std::runtime_error(ToString());
     }
+    for (size_t i{0}; i < m_args.size(); ++i) {
+        m_args.at(i).MatchesType(request.params[i]);
+    }
     UniValue ret = m_fun(*this, request);
     if (gArgs.GetBoolArg("-rpcdoccheck", DEFAULT_RPC_DOC_CHECK)) {
         CHECK_NONFATAL(std::any_of(m_results.m_results.begin(), m_results.m_results.end(), [&ret](const RPCResult& res) { return res.MatchesType(ret); }));
@@ -623,6 +609,44 @@ UniValue RPCHelpMan::GetArgMap() const
     return arr;
 }
 
+void RPCArg::MatchesType(const UniValue& request) const
+{
+    if (m_opts.skip_type_check) return;
+    if (IsOptional() && request.isNull()) return;
+    switch (m_type) {
+    case Type::STR_HEX:
+    case Type::STR: {
+        RPCTypeCheckArgument(request, UniValue::VSTR);
+        return;
+    }
+    case Type::NUM: {
+        RPCTypeCheckArgument(request, UniValue::VNUM);
+        return;
+    }
+    case Type::AMOUNT: {
+        // VNUM or VSTR, checked inside AmountFromValue()
+        return;
+    }
+    case Type::RANGE: {
+        // VNUM or VARR, checked inside ParseRange()
+        return;
+    }
+    case Type::BOOL: {
+        RPCTypeCheckArgument(request, UniValue::VBOOL);
+        return;
+    }
+    case Type::OBJ:
+    case Type::OBJ_USER_KEYS: {
+        RPCTypeCheckArgument(request, UniValue::VOBJ);
+        return;
+    }
+    case Type::ARR: {
+        RPCTypeCheckArgument(request, UniValue::VARR);
+        return;
+    }
+    } // no default case, so the compiler can warn about missing cases
+}
+
 std::string RPCArg::GetFirstName() const
 {
     return m_names.substr(0, m_names.find("|"));
diff --git a/src/rpc/util.h b/src/rpc/util.h
index fabd9666e9cc..19befd1d6b73 100644
--- a/src/rpc/util.h
+++ b/src/rpc/util.h
@@ -62,13 +62,6 @@ struct UniValueType {
     UniValue::VType type;
 };
 
-/**
- * Type-check arguments; throws JSONRPCError if wrong type given. Does not check that
- * the right number of arguments are passed, just that any passed are the correct type.
- */
-void RPCTypeCheck(const UniValue& params,
-                  const std::list<UniValueType>& typesExpected, bool fAllowNull=false);
-
 /**
  * Type-check one argument; throws JSONRPCError if wrong type given.
  */
@@ -134,6 +127,7 @@ enum class OuterType {
 std::vector<CScript> EvalDescriptorStringOrObject(const UniValue& scanobject, FlatSigningProvider& provider);
 
 struct RPCArgOptions {
+    bool skip_type_check{false};
     std::string oneline_description{};   //!< Should be empty unless it is supposed to override the auto-generated summary line
     std::vector<std::string> type_str{}; //!< Should be empty unless it is supposed to override the auto-generated type strings. Vector length is either 0 or 2, m_opts.type_str.at(0) will override the type of the value in a key-value pair, m_opts.type_str.at(1) will override the type in the argument description.
     bool hidden{false};                  //!< For testing only
@@ -216,6 +210,9 @@ struct RPCArg {
 
     bool IsOptional() const;
 
+    /** Check whether the request JSON type matches. */
+    void MatchesType(const UniValue& request) const;
+
     /** Return the first of all aliases */
     std::string GetFirstName() const;
 
diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp
index dab1919ea4c3..916779b71645 100644
--- a/src/wallet/rpc/backup.cpp
+++ b/src/wallet/rpc/backup.cpp
@@ -1553,7 +1553,6 @@ RPCHelpMan importmulti()
                 },
         [&](const RPCHelpMan& self, const JSONRPCRequest& mainRequest) -> UniValue
 {
-    RPCTypeCheck(mainRequest.params, {UniValue::VARR, UniValue::VOBJ});
 
     const UniValue& requests = mainRequest.params[0];
 
@@ -1886,8 +1885,6 @@ RPCHelpMan importdescriptors() {
         throw JSONRPCError(RPC_WALLET_ERROR, "importdescriptors is not available for non-descriptor wallets");
     }
 
-    RPCTypeCheck(main_request.params, {UniValue::VARR, UniValue::VOBJ});
-
     WalletRescanReserver reserver(*pwallet);
     if (!reserver.reserve()) {
         throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait.");
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
index 9edcf79d8cd0..b15557d20dee 100644
--- a/src/wallet/rpc/spend.cpp
+++ b/src/wallet/rpc/spend.cpp
@@ -315,7 +315,11 @@ RPCHelpMan sendmany()
                 "\nSend multiple times. Amounts are double-precision floating point numbers." +
         HELP_REQUIRING_PASSPHRASE,
                 {
-                    {"dummy", RPCArg::Type::STR, RPCArg::Optional::NO, "Must be set to \"\" for backwards compatibility.", RPCArgOptions{.oneline_description="\"\""}},
+                    {"dummy", RPCArg::Type::STR, RPCArg::Optional::NO, "Must be set to \"\" for backwards compatibility.",
+                     RPCArgOptions{
+                         .skip_type_check = true,
+                         .oneline_description = "\"\"",
+                     }},
                     {"amounts", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::NO, "The addresses and amounts",
                         {
                             {"address", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "The Dash address is the key, the numeric amount (can be string) in " + CURRENCY_UNIT + " is the value"},
@@ -755,7 +759,10 @@ RPCHelpMan fundrawtransaction()
                              },
                         },
                         FundTxDoc()),
-                        RPCArgOptions{.oneline_description="options"}},
+                        RPCArgOptions{
+                            .skip_type_check = true,
+                            .oneline_description = "options",
+                        }},
                 },
                 RPCResult{
                     RPCResult::Type::OBJ, "", "",
@@ -777,8 +784,6 @@ RPCHelpMan fundrawtransaction()
                                 },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    RPCTypeCheck(request.params, {UniValue::VSTR, UniValueType(), UniValue::VBOOL});
-
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
     if (!pwallet) return UniValue::VNULL;
 
@@ -859,8 +864,6 @@ RPCHelpMan signrawtransactionwithwallet()
                 },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VARR, UniValue::VSTR}, true);
-
     const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
     if (!pwallet) return UniValue::VNULL;
 
@@ -918,7 +921,7 @@ RPCHelpMan send()
                         },
                     },
                 },
-            },
+                RPCArgOptions{.skip_type_check = true}},
             {"conf_target", RPCArg::Type::NUM, RPCArg::DefaultHint{"wallet -txconfirmtarget"}, "Confirmation target in blocks"},
             {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"unset"}, std::string() + "The fee estimate mode, must be one of (case insensitive):\n"
                         "       \"" + FeeModes("\"\n\"") + "\""},
@@ -986,15 +989,6 @@ RPCHelpMan send()
         },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
         {
-            RPCTypeCheck(request.params, {
-                UniValueType(), // outputs (ARR or OBJ, checked later)
-                UniValue::VNUM, // conf_target
-                UniValue::VSTR, // estimate_mode
-                UniValueType(), // fee_rate, will be checked by AmountFromValue() in SetFeeEstimateMode()
-                UniValue::VOBJ, // options
-                }, true
-            );
-
             std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
             if (!pwallet) return UniValue::VNULL;
 
@@ -1094,15 +1088,6 @@ RPCHelpMan sendall()
         },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
         {
-            RPCTypeCheck(request.params, {
-                UniValue::VARR, // recipients
-                UniValue::VNUM, // conf_target
-                UniValue::VSTR, // estimate_mode
-                UniValueType(), // fee_rate, will be checked by AmountFromValue() in SetFeeEstimateMode()
-                UniValue::VOBJ, // options
-                }, true
-            );
-
             std::shared_ptr<CWallet> const pwallet{GetWalletForJSONRPCRequest(request)};
             if (!pwallet) return UniValue::VNULL;
             // Make sure the results are valid at least up to the most recent block
@@ -1285,8 +1270,6 @@ RPCHelpMan walletprocesspsbt()
                 },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    RPCTypeCheck(request.params, {UniValue::VSTR});
-
     const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
     if (!pwallet) return UniValue::VNULL;
 
@@ -1372,7 +1355,7 @@ RPCHelpMan walletcreatefundedpsbt()
                                 },
                             },
                         },
-                    },
+                     RPCArgOptions{.skip_type_check = true}},
                     {"locktime", RPCArg::Type::NUM, RPCArg::Default{0}, "Raw locktime. Non-0 value also locktime-activates inputs"},
                     {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "",
                         Cat<std::vector<RPCArg>>(
@@ -1414,14 +1397,6 @@ RPCHelpMan walletcreatefundedpsbt()
                                 },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    RPCTypeCheck(request.params, {
-        UniValue::VARR,
-        UniValueType(), // ARR or OBJ, checked later
-        UniValue::VNUM,
-        UniValue::VOBJ,
-        UniValue::VBOOL,
-        }, true
-    );
 
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
     if (!pwallet) return UniValue::VNULL;
diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp
index 3b9d28d20f0a..04b9d35ed6e4 100644
--- a/src/wallet/rpc/wallet.cpp
+++ b/src/wallet/rpc/wallet.cpp
@@ -957,8 +957,6 @@ static RPCHelpMan upgradewallet()
     std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
     if (!pwallet) return UniValue::VNULL;
 
-    RPCTypeCheck(request.params, {UniValue::VNUM}, true);
-
     EnsureWalletIsUnlocked(*pwallet);
 
     int version = 0;
@@ -1026,8 +1024,6 @@ RPCHelpMan simulaterawtransaction()
     if (!rpc_wallet) return UniValue::VNULL;
     const CWallet& wallet = *rpc_wallet;
 
-    RPCTypeCheck(request.params, {UniValue::VARR, UniValue::VOBJ}, true);
-
     LOCK(wallet.cs_wallet);
 
     UniValue include_watchonly(UniValue::VNULL);

From 821760e0d383d38d854821e6da25897e32e8fb44 Mon Sep 17 00:00:00 2001
From: fanquake <fanquake@gmail.com>
Date: Thu, 24 Aug 2023 11:34:42 +0100
Subject: [PATCH 6/8] Merge bitcoin/bitcoin#28230: rpc: Add MaybeArg() and
 Arg() default helper

c00000df1605788acadceb90c22ae9f00db8a9dc rpc: Add MaybeArg() and Arg() default helper (MarcoFalke)

Pull request description:

  Currently the RPC method implementations have many issues:

  * Default RPC argument values (and their optionality state) are duplicated in the documentation and the C++ code, with no checks to prevent them from going out of sync.
  * Getting an optional RPC argument is verbose, using a ternary operator, or worse, a multi-line `if`.

  Fix all issues by adding default helper that can be called via `self.Arg<int>(0)`. The helper needs a few lines of code in the `src/rpc/util.h` header file. Everything else will be implemented in the cpp file once and if an RPC method needs it.

  There is also an `self.MaybeArg<int>(0)` helper that works on any arg to return the argument, the default, or a falsy value.

ACKs for top commit:
  ajtowns:
    reACK c00000df1605788acadceb90c22ae9f00db8a9dc
  stickies-v:
    re-ACK c00000df1605788acadceb90c22ae9f00db8a9dc
  TheCharlatan:
    re-ACK c00000df1605788acadceb90c22ae9f00db8a9dc

Tree-SHA512: e7ddcab3faa319bc53edbdf3f89ce83389d2c4e571d5db42401620ff105e522a4a0669dad08e08cde5fd05c790aec3b806f63261a9100c2778865a489e57381e
---
 src/rpc/mining.cpp        | 10 +++----
 src/rpc/util.cpp          | 56 ++++++++++++++++++++++++++++++++++++-
 src/rpc/util.h            | 58 +++++++++++++++++++++++++++++++++++++++
 src/wallet/rpc/coins.cpp  |  4 +--
 src/wallet/rpc/wallet.cpp |  2 +-
 5 files changed, 121 insertions(+), 9 deletions(-)

diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index 33c85856dec3..97196b7968e1 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -116,7 +116,7 @@ static RPCHelpMan getnetworkhashps()
 
     ChainstateManager& chainman = EnsureAnyChainman(request.context);
     LOCK(cs_main);
-    return GetNetworkHashPS(!request.params[0].isNull() ? request.params[0].getInt<int>() : 120, !request.params[1].isNull() ? request.params[1].getInt<int>() : -1, chainman.ActiveChain());
+    return GetNetworkHashPS(self.Arg<int>(0), self.Arg<int>(1), chainman.ActiveChain());
 },
     };
 }
@@ -228,12 +228,12 @@ static RPCHelpMan generatetodescriptor()
             "\nGenerate 11 blocks to mydesc\n" + HelpExampleCli("generatetodescriptor", "11 \"mydesc\"")},
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    const int num_blocks{request.params[0].getInt<int>()};
-    const uint64_t max_tries{request.params[2].isNull() ? DEFAULT_MAX_TRIES : request.params[2].getInt<int>()};
+    const auto num_blocks{self.Arg<int>(0)};
+    const auto max_tries{self.Arg<uint64_t>(2)};
 
     CScript coinbase_script;
     std::string error;
-    if (!getScriptFromDescriptor(request.params[1].get_str(), coinbase_script, error)) {
+    if (!getScriptFromDescriptor(self.Arg<std::string>(1), coinbase_script, error)) {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, error);
     }
 
@@ -498,7 +498,7 @@ static RPCHelpMan prioritisetransaction()
 {
     LOCK(cs_main);
 
-    uint256 hash(ParseHashV(request.params[0].get_str(), "txid"));
+    uint256 hash(ParseHashV(request.params[0], "txid"));
     CAmount nAmount = request.params[1].getInt<int64_t>();
 
     EnsureAnyMemPool(request.context).PrioritiseTransaction(hash, nAmount);
diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp
index 69999feef00b..098bbf6862b1 100644
--- a/src/rpc/util.cpp
+++ b/src/rpc/util.cpp
@@ -508,16 +508,70 @@ UniValue RPCHelpMan::HandleRequest(const JSONRPCRequest& request) const
     if (request.mode == JSONRPCRequest::GET_HELP || !IsValidNumArgs(request.params.size())) {
         throw std::runtime_error(ToString());
     }
+    UniValue arg_mismatch{UniValue::VOBJ};
     for (size_t i{0}; i < m_args.size(); ++i) {
-        m_args.at(i).MatchesType(request.params[i]);
+        const auto& arg{m_args.at(i)};
+        UniValue match{arg.MatchesType(request.params[i])};
+        if (!match.isTrue()) {
+            arg_mismatch.pushKV(strprintf("Position %s (%s)", i + 1, arg.m_names), std::move(match));
+        }
+    }
+    if (!arg_mismatch.empty()) {
+        throw JSONRPCError(RPC_TYPE_ERROR, strprintf("Wrong type passed:\n%s", arg_mismatch.write(4)));
     }
+    CHECK_NONFATAL(m_req == nullptr);
+    m_req = &request;
     UniValue ret = m_fun(*this, request);
+    m_req = nullptr;
     if (gArgs.GetBoolArg("-rpcdoccheck", DEFAULT_RPC_DOC_CHECK)) {
         CHECK_NONFATAL(std::any_of(m_results.m_results.begin(), m_results.m_results.end(), [&ret](const RPCResult& res) { return res.MatchesType(ret); }));
     }
     return ret;
 }
 
+using CheckFn = void(const RPCArg&);
+static const UniValue* DetailMaybeArg(CheckFn* check, const std::vector<RPCArg>& params, const JSONRPCRequest* req, size_t i)
+{
+    CHECK_NONFATAL(i < params.size());
+    const UniValue& arg{CHECK_NONFATAL(req)->params[i]};
+    const RPCArg& param{params.at(i)};
+    if (check) check(param);
+
+    if (!arg.isNull()) return &arg;
+    if (!std::holds_alternative<RPCArg::Default>(param.m_fallback)) return nullptr;
+    return &std::get<RPCArg::Default>(param.m_fallback);
+}
+
+static void CheckRequiredOrDefault(const RPCArg& param)
+{
+    // Must use `Arg<Type>(i)` to get the argument or its default value.
+    const bool required{
+        std::holds_alternative<RPCArg::Optional>(param.m_fallback) && RPCArg::Optional::NO == std::get<RPCArg::Optional>(param.m_fallback),
+    };
+    CHECK_NONFATAL(required || std::holds_alternative<RPCArg::Default>(param.m_fallback));
+}
+
+#define TMPL_INST(check_param, ret_type, return_code)       \
+    template <>                                             \
+    ret_type RPCHelpMan::ArgValue<ret_type>(size_t i) const \
+    {                                                       \
+        const UniValue* maybe_arg{                          \
+            DetailMaybeArg(check_param, m_args, m_req, i),  \
+        };                                                  \
+        return return_code                                  \
+    }                                                       \
+    void force_semicolon(ret_type)
+
+// Optional arg (without default). Can also be called on required args, if needed.
+TMPL_INST(nullptr, std::optional<double>, maybe_arg ? std::optional{maybe_arg->get_real()} : std::nullopt;);
+TMPL_INST(nullptr, std::optional<bool>, maybe_arg ? std::optional{maybe_arg->get_bool()} : std::nullopt;);
+TMPL_INST(nullptr, const std::string*, maybe_arg ? &maybe_arg->get_str() : nullptr;);
+
+// Required arg or optional arg with default value.
+TMPL_INST(CheckRequiredOrDefault, int, CHECK_NONFATAL(maybe_arg)->getInt<int>(););
+TMPL_INST(CheckRequiredOrDefault, uint64_t, CHECK_NONFATAL(maybe_arg)->getInt<uint64_t>(););
+TMPL_INST(CheckRequiredOrDefault, const std::string&, CHECK_NONFATAL(maybe_arg)->get_str(););
+
 bool RPCHelpMan::IsValidNumArgs(size_t num_args) const
 {
     size_t num_required_args = 0;
diff --git a/src/rpc/util.h b/src/rpc/util.h
index 19befd1d6b73..0514ab73377d 100644
--- a/src/rpc/util.h
+++ b/src/rpc/util.h
@@ -5,6 +5,7 @@
 #ifndef BITCOIN_RPC_UTIL_H
 #define BITCOIN_RPC_UTIL_H
 
+#include <consensus/amount.h>
 #include <node/transaction.h>
 #include <pubkey.h>
 #include <rpc/protocol.h>
@@ -13,14 +14,30 @@
 #include <script/sign.h>
 #include <script/signingprovider.h>
 #include <script/standard.h>
+#include <uint256.h>
 #include <univalue.h>
 #include <util/check.h>
 #include <util/strencodings.h>
 
+#include <cstddef>
+#include <cstdint>
+#include <functional>
+#include <initializer_list>
+#include <map>
+#include <optional>
 #include <string>
+#include <type_traits>
+#include <utility>
 #include <variant>
 #include <vector>
 
+class JSONRPCRequest;
+enum ServiceFlags : uint64_t;
+enum class OutputType;
+enum class TransactionError;
+struct FlatSigningProvider;
+struct bilingual_str;
+
 static constexpr bool DEFAULT_RPC_DOC_CHECK{
 #ifdef RPC_DOC_CHECK
     true
@@ -359,6 +376,44 @@ class RPCHelpMan
     RPCHelpMan(std::string name, std::string description, std::vector<RPCArg> args, RPCResults results, RPCExamples examples, RPCMethodImpl fun);
 
     UniValue HandleRequest(const JSONRPCRequest& request) const;
+    /**
+     * Helper to get a request argument.
+     * This function only works during m_fun(), i.e. it should only be used in
+     * RPC method implementations. The helper internally checks whether the
+     * user-passed argument isNull() and parses (from JSON) and returns the
+     * user-passed argument, or the default value derived from the RPCArg
+     * documention, or a falsy value if no default was given.
+     *
+     * Use Arg<Type>(i) to get the argument or its default value. Otherwise,
+     * use MaybeArg<Type>(i) to get the optional argument or a falsy value.
+     *
+     * The Type passed to this helper must match the corresponding
+     * RPCArg::Type.
+     */
+    template <typename R>
+    auto Arg(size_t i) const
+    {
+        // Return argument (required or with default value).
+        if constexpr (std::is_integral_v<R> || std::is_floating_point_v<R>) {
+            // Return numbers by value.
+            return ArgValue<R>(i);
+        } else {
+            // Return everything else by reference.
+            return ArgValue<const R&>(i);
+        }
+    }
+    template <typename R>
+    auto MaybeArg(size_t i) const
+    {
+        // Return optional argument (without default).
+        if constexpr (std::is_integral_v<R> || std::is_floating_point_v<R>) {
+            // Return numbers by value, wrapped in optional.
+            return ArgValue<std::optional<R>>(i);
+        } else {
+            // Return other types by pointer.
+            return ArgValue<const R*>(i);
+        }
+    }
     std::string ToString() const;
     /** Return the named args that need to be converted from string to another JSON type */
     UniValue GetArgMap() const;
@@ -375,6 +430,9 @@ class RPCHelpMan
     const std::vector<RPCArg> m_args;
     const RPCResults m_results;
     const RPCExamples m_examples;
+    mutable const JSONRPCRequest* m_req{nullptr}; // A pointer to the request for the duration of m_fun()
+    template <typename R>
+    R ArgValue(size_t i) const;
 };
 
 RPCErrorCode RPCErrorFromTransactionError(TransactionError terr);
diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp
index 282ecd9143b3..502b48f8a306 100644
--- a/src/wallet/rpc/coins.cpp
+++ b/src/wallet/rpc/coins.cpp
@@ -197,8 +197,8 @@ RPCHelpMan getbalance()
 
     LOCK(pwallet->cs_wallet);
 
-    const UniValue& dummy_value = request.params[0];
-    if (!dummy_value.isNull() && dummy_value.get_str() != "*") {
+    const auto dummy_value{self.MaybeArg<std::string>(0)};
+    if (dummy_value && *dummy_value != "*") {
         throw JSONRPCError(RPC_METHOD_DEPRECATED, "dummy first argument must be excluded or set to \"*\".");
     }
 
diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp
index 04b9d35ed6e4..dbda6d30a8d9 100644
--- a/src/wallet/rpc/wallet.cpp
+++ b/src/wallet/rpc/wallet.cpp
@@ -767,7 +767,7 @@ static RPCHelpMan unloadwallet()
         // Release the "main" shared pointer and prevent further notifications.
         // Note that any attempt to load the same wallet would fail until the wallet
         // is destroyed (see CheckUniqueFileid).
-        std::optional<bool> load_on_start = request.params[1].isNull() ? std::nullopt : std::optional<bool>(request.params[1].get_bool());
+        std::optional<bool> load_on_start{self.MaybeArg<bool>(1)};
         if (!RemoveWallet(context, wallet, load_on_start, warnings)) {
             throw JSONRPCError(RPC_MISC_ERROR, "Requested wallet already unloaded");
         }

From 27fd25f375e87ddc8c3cae9d6fdad7becd3e9665 Mon Sep 17 00:00:00 2001
From: Ryan Ofsky <ryan@ofsky.org>
Date: Mon, 29 Apr 2024 10:14:38 -0400
Subject: [PATCH 7/8] Merge bitcoin/bitcoin#29277: RPC: access RPC arguments by
 name
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

30a6c999351041d6a1e8712a9659be1296a1b46a rpc: access some args by name (stickies-v)
bbb31269bfa449e82d3b6a20c2c3481fb3dcc316 rpc: add named arg helper (stickies-v)
13525e0c248eab9b199583cde76430c6da2426e2 rpc: add arg helper unit test (stickies-v)

Pull request description:

  Adds string overloads for the `RPCHelpMan::Arg` and `RPCHelpMan::MaybeArg` helpers to be able to access RPC arguments by name instead of index number. Especially in RPCs with a large number of parameters, this can be quite helpful.

  Example usage:
  ```cpp
  const auto action{self.Arg<std::string>("action")};
  ```

  Most of the LoC is adding test coverage and documentation updates. No behaviour change.

  An alternative approach to #27788 with significantly less overhaul.

ACKs for top commit:
  fjahr:
    Code review ACK 30a6c999351041d6a1e8712a9659be1296a1b46a
  maflcko:
    ACK 30a6c999351041d6a1e8712a9659be1296a1b46a 🥑
  ryanofsky:
    Code review ACK 30a6c999351041d6a1e8712a9659be1296a1b46a. Nice change! Implementation is surprisingly simple and additional unit test coverage is welcome, too.

Tree-SHA512: 4904f5f914fe1d421d32f60edb7c5a028c8ea0f140a2f207a106b4752d441164e073066a6bf2e17693f859fe847815a96609d3cf521e0ac4178d8cd09362ea3d
---
 src/rpc/blockchain.cpp              |  9 ++--
 src/rpc/mining.cpp                  |  8 ++--
 src/rpc/net.cpp                     |  6 +--
 src/rpc/signmessage.cpp             |  6 +--
 src/rpc/util.cpp                    | 15 +++++++
 src/rpc/util.h                      | 62 +++++++++++++++++++++-----
 src/test/rpc_tests.cpp              | 68 +++++++++++++++++++++++++++++
 src/wallet/rpc/coins.cpp            |  7 +--
 test/functional/rpc_scantxoutset.py |  2 +-
 9 files changed, 153 insertions(+), 30 deletions(-)

diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index c685daa32f6c..443b23dbbac2 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -2502,7 +2502,8 @@ static RPCHelpMan scantxoutset()
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
     UniValue result(UniValue::VOBJ);
-    if (request.params[0].get_str() == "status") {
+    const auto action{self.Arg<std::string>("action")};
+    if (action == "status") {
         CoinsViewScanReserver reserver;
         if (reserver.reserve()) {
             // no scan in progress
@@ -2510,7 +2511,7 @@ static RPCHelpMan scantxoutset()
         }
         result.pushKV("progress", g_scan_progress.load());
         return result;
-    } else if (request.params[0].get_str() == "abort") {
+    } else if (action == "abort") {
         CoinsViewScanReserver reserver;
         if (reserver.reserve()) {
             // reserve was possible which means no scan was running
@@ -2519,7 +2520,7 @@ static RPCHelpMan scantxoutset()
         // set the abort flag
         g_should_abort_scan = true;
         return true;
-    } else if (request.params[0].get_str() == "start") {
+    } else if (action == "start") {
         CoinsViewScanReserver reserver;
         if (!reserver.reserve()) {
             throw JSONRPCError(RPC_INVALID_PARAMETER, "Scan already in progress, use action \"abort\" or \"status\"");
@@ -2588,7 +2589,7 @@ static RPCHelpMan scantxoutset()
         result.pushKV("unspents", unspents);
         result.pushKV("total_amount", ValueFromAmount(total_in));
     } else {
-        throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid command");
+        throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid action '%s'", action));
     }
     return result;
 },
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index 97196b7968e1..fb224e3701f1 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -116,7 +116,7 @@ static RPCHelpMan getnetworkhashps()
 
     ChainstateManager& chainman = EnsureAnyChainman(request.context);
     LOCK(cs_main);
-    return GetNetworkHashPS(self.Arg<int>(0), self.Arg<int>(1), chainman.ActiveChain());
+    return GetNetworkHashPS(self.Arg<int>("nblocks"), self.Arg<int>("height"), chainman.ActiveChain());
 },
     };
 }
@@ -228,12 +228,12 @@ static RPCHelpMan generatetodescriptor()
             "\nGenerate 11 blocks to mydesc\n" + HelpExampleCli("generatetodescriptor", "11 \"mydesc\"")},
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    const auto num_blocks{self.Arg<int>(0)};
-    const auto max_tries{self.Arg<uint64_t>(2)};
+    const auto num_blocks{self.Arg<int>("num_blocks")};
+    const auto max_tries{self.Arg<uint64_t>("maxtries")};
 
     CScript coinbase_script;
     std::string error;
-    if (!getScriptFromDescriptor(self.Arg<std::string>(1), coinbase_script, error)) {
+    if (!getScriptFromDescriptor(self.Arg<std::string>("descriptor"), coinbase_script, error)) {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, error);
     }
 
diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp
index 4d722640bf6c..e8f3f61acc0a 100644
--- a/src/rpc/net.cpp
+++ b/src/rpc/net.cpp
@@ -334,7 +334,7 @@ static RPCHelpMan addnode()
                 },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    const std::string command{request.params[1].get_str()};
+    const auto command{self.Arg<std::string>("command")};
     if (command != "onetry" && command != "add" && command != "remove") {
         throw std::runtime_error(
             self.ToString());
@@ -343,9 +343,9 @@ static RPCHelpMan addnode()
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     CConnman& connman = EnsureConnman(node);
 
-    const std::string node_arg = request.params[0].get_str();
+    const auto node_arg{self.Arg<std::string>("node")};
     bool node_v2transport = connman.GetLocalServices() & NODE_P2P_V2;
-    bool use_v2transport = request.params[2].isNull() ? node_v2transport : request.params[2].get_bool();
+    bool use_v2transport = self.MaybeArg<bool>("v2transport").value_or(node_v2transport);
 
     if (use_v2transport && !node_v2transport) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "Error: v2transport requested but not enabled (see -v2transport)");
diff --git a/src/rpc/signmessage.cpp b/src/rpc/signmessage.cpp
index 58c682a65c9b..13d0b67e7d12 100644
--- a/src/rpc/signmessage.cpp
+++ b/src/rpc/signmessage.cpp
@@ -39,9 +39,9 @@ static RPCHelpMan verifymessage()
         },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
         {
-            std::string strAddress = request.params[0].get_str();
-            std::string strSign = request.params[1].get_str();
-            std::string strMessage = request.params[2].get_str();
+            std::string strAddress = self.Arg<std::string>("address");
+            std::string strSign = self.Arg<std::string>("signature");
+            std::string strMessage = self.Arg<std::string>("message");
 
             switch (MessageVerify(strAddress, strSign, strMessage)) {
             case MessageVerificationResult::ERR_INVALID_ADDRESS:
diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp
index 098bbf6862b1..0f69c0d140c6 100644
--- a/src/rpc/util.cpp
+++ b/src/rpc/util.cpp
@@ -18,6 +18,11 @@
 #include <util/system.h>
 #include <util/translation.h>
 
+#include <algorithm>
+#include <iterator>
+#include <string_view>
+#include <tuple>
+
 const std::string UNIX_EPOCH_TIME = "UNIX epoch time";
 const std::string EXAMPLE_ADDRESS[2] = {"XunLY9Tf7Zsef8gMGL2fhWA9ZmMjt4KPw0", "XwQQkwA4FYkq2XERzMY2CiAZhJTEDAbtc0"};
 
@@ -593,6 +598,16 @@ std::vector<std::string> RPCHelpMan::GetArgNames() const
     return ret;
 }
 
+size_t RPCHelpMan::GetParamIndex(std::string_view key) const
+{
+    auto it{std::find_if(
+        m_args.begin(), m_args.end(), [&key](const auto& arg) { return arg.GetName() == key;}
+    )};
+
+    CHECK_NONFATAL(it != m_args.end());  // TODO: ideally this is checked at compile time
+    return std::distance(m_args.begin(), it);
+}
+
 std::string RPCHelpMan::ToString() const
 {
     std::string ret;
diff --git a/src/rpc/util.h b/src/rpc/util.h
index 0514ab73377d..33255a065810 100644
--- a/src/rpc/util.h
+++ b/src/rpc/util.h
@@ -377,18 +377,25 @@ class RPCHelpMan
 
     UniValue HandleRequest(const JSONRPCRequest& request) const;
     /**
-     * Helper to get a request argument.
-     * This function only works during m_fun(), i.e. it should only be used in
-     * RPC method implementations. The helper internally checks whether the
-     * user-passed argument isNull() and parses (from JSON) and returns the
-     * user-passed argument, or the default value derived from the RPCArg
-     * documention, or a falsy value if no default was given.
+     * @brief Helper to get a required or default-valued request argument.
      *
-     * Use Arg<Type>(i) to get the argument or its default value. Otherwise,
-     * use MaybeArg<Type>(i) to get the optional argument or a falsy value.
+     * Use this function when the argument is required or when it has a default value. If the
+     * argument is optional and may not be provided, use MaybeArg instead.
      *
-     * The Type passed to this helper must match the corresponding
-     * RPCArg::Type.
+     * This function only works during m_fun(), i.e., it should only be used in
+     * RPC method implementations. It internally checks whether the user-passed
+     * argument isNull() and parses (from JSON) and returns the user-passed argument,
+     * or the default value derived from the RPCArg documentation.
+     *
+     * There are two overloads of this function:
+     * - Use Arg<Type>(size_t i) to get the argument (or the default value) by index.
+     * - Use Arg<Type>(const std::string& key) to get the argument (or the default value) by key.
+     *
+     * The Type passed to this helper must match the corresponding RPCArg::Type.
+     *
+     * @return The value of the RPC argument (or the default value) cast to type Type.
+     *
+     * @see MaybeArg for handling optional arguments without default values.
      */
     template <typename R>
     auto Arg(size_t i) const
@@ -402,6 +409,34 @@ class RPCHelpMan
             return ArgValue<const R&>(i);
         }
     }
+    template<typename R>
+    auto Arg(std::string_view key) const
+    {
+        return Arg<R>(GetParamIndex(key));
+    }
+    /**
+     * @brief Helper to get an optional request argument.
+     *
+     * Use this function when the argument is optional and does not have a default value. If the
+     * argument is required or has a default value, use Arg instead.
+     *
+     * This function only works during m_fun(), i.e., it should only be used in
+     * RPC method implementations. It internally checks whether the user-passed
+     * argument isNull() and parses (from JSON) and returns the user-passed argument,
+     * or a falsy value if no argument was passed.
+     *
+     * There are two overloads of this function:
+     * - Use MaybeArg<Type>(size_t i) to get the optional argument by index.
+     * - Use MaybeArg<Type>(const std::string& key) to get the optional argument by key.
+     *
+     * The Type passed to this helper must match the corresponding RPCArg::Type.
+     *
+     * @return For integral and floating-point types, a std::optional<Type> is returned.
+    *          For other types, a Type* pointer to the argument is returned. If the
+    *          argument is not provided, std::nullopt or a null pointer is returned.
+    *
+     * @see Arg for handling arguments that are required or have a default value.
+     */
     template <typename R>
     auto MaybeArg(size_t i) const
     {
@@ -414,6 +449,11 @@ class RPCHelpMan
             return ArgValue<const R*>(i);
         }
     }
+    template<typename R>
+    auto MaybeArg(std::string_view key) const
+    {
+        return MaybeArg<R>(GetParamIndex(key));
+    }
     std::string ToString() const;
     /** Return the named args that need to be converted from string to another JSON type */
     UniValue GetArgMap() const;
@@ -433,6 +473,8 @@ class RPCHelpMan
     mutable const JSONRPCRequest* m_req{nullptr}; // A pointer to the request for the duration of m_fun()
     template <typename R>
     R ArgValue(size_t i) const;
+    //! Return positional index of a parameter using its name as key.
+    size_t GetParamIndex(std::string_view key) const;
 };
 
 RPCErrorCode RPCErrorFromTransactionError(TransactionError terr);
diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp
index 7638ab309fe3..36bfdd74dbc8 100644
--- a/src/test/rpc_tests.cpp
+++ b/src/test/rpc_tests.cpp
@@ -702,4 +702,72 @@ BOOST_AUTO_TEST_CASE(rpc_convert_composite_commands)
     BOOST_CHECK_EQUAL(result[1][1].get_str(), "avgfeerate");
 }
 
+static void CheckRpc(const std::vector<RPCArg>& params, const UniValue& args, RPCHelpMan::RPCMethodImpl test_impl)
+{
+    auto null_result{RPCResult{RPCResult::Type::NONE, "", "None"}};
+    const RPCHelpMan rpc{"dummy", "dummy description", params, null_result, RPCExamples{""}, test_impl};
+    JSONRPCRequest req;
+    req.params = args;
+
+    rpc.HandleRequest(req);
+}
+
+BOOST_AUTO_TEST_CASE(rpc_arg_helper)
+{
+    constexpr bool DEFAULT_BOOL = true;
+    constexpr auto DEFAULT_STRING = "default";
+    constexpr uint64_t DEFAULT_UINT64_T = 3;
+
+    //! Parameters with which the RPCHelpMan is instantiated
+    const std::vector<RPCArg> params{
+        // Required arg
+        {"req_int", RPCArg::Type::NUM, RPCArg::Optional::NO, ""},
+        {"req_str", RPCArg::Type::STR, RPCArg::Optional::NO, ""},
+        // Default arg
+        {"def_uint64_t", RPCArg::Type::NUM, RPCArg::Default{DEFAULT_UINT64_T}, ""},
+        {"def_string", RPCArg::Type::STR, RPCArg::Default{DEFAULT_STRING}, ""},
+        {"def_bool", RPCArg::Type::BOOL, RPCArg::Default{DEFAULT_BOOL}, ""},
+        // Optional arg without default
+        {"opt_double", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, ""},
+        {"opt_string", RPCArg::Type::STR, RPCArg::Optional::OMITTED, ""}
+    };
+
+    //! Check that `self.Arg` returns the same value as the `request.params` accessors
+    RPCHelpMan::RPCMethodImpl check_positional = [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue {
+            BOOST_CHECK_EQUAL(self.Arg<int>(0), request.params[0].getInt<int>());
+            BOOST_CHECK_EQUAL(self.Arg<std::string>(1), request.params[1].get_str());
+            BOOST_CHECK_EQUAL(self.Arg<uint64_t>(2), request.params[2].isNull() ? DEFAULT_UINT64_T : request.params[2].getInt<uint64_t>());
+            BOOST_CHECK_EQUAL(self.Arg<std::string>(3), request.params[3].isNull() ? DEFAULT_STRING : request.params[3].get_str());
+            BOOST_CHECK_EQUAL(self.Arg<bool>(4), request.params[4].isNull() ? DEFAULT_BOOL : request.params[4].get_bool());
+            if (!request.params[5].isNull()) {
+                BOOST_CHECK_EQUAL(self.MaybeArg<double>(5).value(), request.params[5].get_real());
+            } else {
+                BOOST_CHECK(!self.MaybeArg<double>(5));
+            }
+            if (!request.params[6].isNull()) {
+                BOOST_CHECK(self.MaybeArg<std::string>(6));
+                BOOST_CHECK_EQUAL(*self.MaybeArg<std::string>(6), request.params[6].get_str());
+            } else {
+                BOOST_CHECK(!self.MaybeArg<std::string>(6));
+            }
+            return UniValue{};
+        };
+    CheckRpc(params, UniValue{JSON(R"([5, "hello", null, null, null, null, null])")}, check_positional);
+    CheckRpc(params, UniValue{JSON(R"([5, "hello", 4, "test", true, 1.23, "world"])")}, check_positional);
+
+    //! Check that `self.Arg` returns the same value when using index and key
+    RPCHelpMan::RPCMethodImpl check_named = [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue {
+            BOOST_CHECK_EQUAL(self.Arg<int>(0), self.Arg<int>("req_int"));
+            BOOST_CHECK_EQUAL(self.Arg<std::string>(1), self.Arg<std::string>("req_str"));
+            BOOST_CHECK_EQUAL(self.Arg<uint64_t>(2), self.Arg<uint64_t>("def_uint64_t"));
+            BOOST_CHECK_EQUAL(self.Arg<std::string>(3), self.Arg<std::string>("def_string"));
+            BOOST_CHECK_EQUAL(self.Arg<bool>(4), self.Arg<bool>("def_bool"));
+            BOOST_CHECK(self.MaybeArg<double>(5) == self.MaybeArg<double>("opt_double"));
+            BOOST_CHECK(self.MaybeArg<std::string>(6) == self.MaybeArg<std::string>("opt_string"));
+            return UniValue{};
+        };
+    CheckRpc(params, UniValue{JSON(R"([5, "hello", null, null, null, null, null])")}, check_named);
+    CheckRpc(params, UniValue{JSON(R"([5, "hello", 4, "test", true, 1.23, "world"])")}, check_named);
+}
+
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp
index 502b48f8a306..ffa28b908258 100644
--- a/src/wallet/rpc/coins.cpp
+++ b/src/wallet/rpc/coins.cpp
@@ -197,15 +197,12 @@ RPCHelpMan getbalance()
 
     LOCK(pwallet->cs_wallet);
 
-    const auto dummy_value{self.MaybeArg<std::string>(0)};
+    const auto dummy_value{self.MaybeArg<std::string>("dummy")};
     if (dummy_value && *dummy_value != "*") {
         throw JSONRPCError(RPC_METHOD_DEPRECATED, "dummy first argument must be excluded or set to \"*\".");
     }
 
-    int min_depth = 0;
-    if (!request.params[1].isNull()) {
-        min_depth = request.params[1].getInt<int>();
-    }
+    const auto min_depth{self.Arg<int>("minconf")};
 
     const UniValue& addlocked = request.params[2];
     bool fAddLocked = false;
diff --git a/test/functional/rpc_scantxoutset.py b/test/functional/rpc_scantxoutset.py
index 326922ca0ba9..e9247e0eac7f 100755
--- a/test/functional/rpc_scantxoutset.py
+++ b/test/functional/rpc_scantxoutset.py
@@ -129,7 +129,7 @@ def run_test(self):
         assert_raises_rpc_error(-1, "scanobjects argument is required for the start action", self.nodes[0].scantxoutset, "start")
 
         # Check that invalid command give error
-        assert_raises_rpc_error(-8, "Invalid command", self.nodes[0].scantxoutset, "invalid_command")
+        assert_raises_rpc_error(-8, "Invalid action 'invalid_command'", self.nodes[0].scantxoutset, "invalid_command")
 
 
 if __name__ == "__main__":

From 24daadcf6dd495c76b5ec2ab6c1e6c9dfec979b8 Mon Sep 17 00:00:00 2001
From: Andrew Chow <github@achow101.com>
Date: Thu, 5 Oct 2023 15:29:24 -0400
Subject: [PATCH 8/8] Merge bitcoin/bitcoin#28597: wallet: No BDB creation,
 unless -deprecatedrpc=create_bdb

fa071aeb61dcc42cd122d3fb1abe4b9c238f8010 wallet: No BDB creation, unless -deprecatedrpc=create_bdb (MarcoFalke)

Pull request description:

  With BDB being removed soon, it seems confusing and harmful to allow users to create fresh BDB wallets going forward, as it would load them with an additional burden of having to migrate them soon after.

  Also, it would be good to allow for one release for test (and external) scripts to adapt.

  Fix all issues by introducing the `-deprecatedrpc=create_bdb` setting.

ACKs for top commit:
  Sjors:
    tACK fa071aeb61dcc42cd122d3fb1abe4b9c238f8010
  achow101:
    ACK fa071aeb61dcc42cd122d3fb1abe4b9c238f8010
  furszy:
    utACK fa071aeb

Tree-SHA512: 37a4c3e4ba659e0ebe2382e71d9c80e42a895d9ad743f5dda7c110fbbb7d2a36f46769982552a9ac0c3a57203379ef164be97aa8033eb7674d6b4da030ba8f9b
---
 src/rpc/util.cpp                       | 41 ++++++++++++++++----------
 src/rpc/util.h                         |  7 +++--
 src/wallet/rpc/wallet.cpp              |  9 ++++--
 test/functional/test_framework/util.py |  1 +
 test/functional/wallet_createwallet.py |  2 +-
 5 files changed, 39 insertions(+), 21 deletions(-)

diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp
index 0f69c0d140c6..0a9334a8eba7 100644
--- a/src/rpc/util.cpp
+++ b/src/rpc/util.cpp
@@ -573,6 +573,7 @@ TMPL_INST(nullptr, std::optional<bool>, maybe_arg ? std::optional{maybe_arg->get
 TMPL_INST(nullptr, const std::string*, maybe_arg ? &maybe_arg->get_str() : nullptr;);
 
 // Required arg or optional arg with default value.
+TMPL_INST(CheckRequiredOrDefault, bool, CHECK_NONFATAL(maybe_arg)->get_bool(););
 TMPL_INST(CheckRequiredOrDefault, int, CHECK_NONFATAL(maybe_arg)->getInt<int>(););
 TMPL_INST(CheckRequiredOrDefault, uint64_t, CHECK_NONFATAL(maybe_arg)->getInt<uint64_t>(););
 TMPL_INST(CheckRequiredOrDefault, const std::string&, CHECK_NONFATAL(maybe_arg)->get_str(););
@@ -678,42 +679,50 @@ UniValue RPCHelpMan::GetArgMap() const
     return arr;
 }
 
-void RPCArg::MatchesType(const UniValue& request) const
+static std::optional<UniValue::VType> ExpectedType(RPCArg::Type type)
 {
-    if (m_opts.skip_type_check) return;
-    if (IsOptional() && request.isNull()) return;
-    switch (m_type) {
+    using Type = RPCArg::Type;
+    switch (type) {
     case Type::STR_HEX:
     case Type::STR: {
-        RPCTypeCheckArgument(request, UniValue::VSTR);
-        return;
+        return UniValue::VSTR;
     }
     case Type::NUM: {
-        RPCTypeCheckArgument(request, UniValue::VNUM);
-        return;
+        return UniValue::VNUM;
     }
     case Type::AMOUNT: {
         // VNUM or VSTR, checked inside AmountFromValue()
-        return;
+        return std::nullopt;
     }
     case Type::RANGE: {
         // VNUM or VARR, checked inside ParseRange()
-        return;
+        return std::nullopt;
     }
     case Type::BOOL: {
-        RPCTypeCheckArgument(request, UniValue::VBOOL);
-        return;
+        return UniValue::VBOOL;
     }
     case Type::OBJ:
     case Type::OBJ_USER_KEYS: {
-        RPCTypeCheckArgument(request, UniValue::VOBJ);
-        return;
+        return UniValue::VOBJ;
     }
     case Type::ARR: {
-        RPCTypeCheckArgument(request, UniValue::VARR);
-        return;
+        return UniValue::VARR;
     }
     } // no default case, so the compiler can warn about missing cases
+    NONFATAL_UNREACHABLE();
+}
+
+UniValue RPCArg::MatchesType(const UniValue& request) const
+{
+    if (m_opts.skip_type_check) return true;
+    if (IsOptional() && request.isNull()) return true;
+    const auto exp_type{ExpectedType(m_type)};
+    if (!exp_type) return true; // nothing to check
+
+    if (*exp_type != request.getType()) {
+        return strprintf("JSON value of type %s is not of expected type %s", uvTypeName(request.getType()), uvTypeName(*exp_type));
+    }
+    return true;
 }
 
 std::string RPCArg::GetFirstName() const
diff --git a/src/rpc/util.h b/src/rpc/util.h
index 33255a065810..c9f384f77b94 100644
--- a/src/rpc/util.h
+++ b/src/rpc/util.h
@@ -227,8 +227,11 @@ struct RPCArg {
 
     bool IsOptional() const;
 
-    /** Check whether the request JSON type matches. */
-    void MatchesType(const UniValue& request) const;
+    /**
+     * Check whether the request JSON type matches.
+     * Returns true if type matches, or object describing error(s) if not.
+     */
+    UniValue MatchesType(const UniValue& request) const;
 
     /** Return the first of all aliases */
     std::string GetFirstName() const;
diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp
index dbda6d30a8d9..92d62604d596 100644
--- a/src/wallet/rpc/wallet.cpp
+++ b/src/wallet/rpc/wallet.cpp
@@ -635,7 +635,7 @@ static RPCHelpMan createwallet()
             {"passphrase", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Encrypt the wallet with this passphrase."},
             {"avoid_reuse", RPCArg::Type::BOOL, RPCArg::Default{false}, "Keep track of coin reuse, and treat dirty and clean coins differently with privacy considerations in mind."},
             {"descriptors", RPCArg::Type::BOOL, RPCArg::Default{true}, "Create a native descriptor wallet. The wallet will use descriptors internally to handle address creation."
-                                                                       " Setting to \"false\" will create a legacy wallet; however, the legacy wallet type is being deprecated and"
+                                                                       " Setting to \"false\" will create a legacy wallet; This is only possible with the -deprecatedrpc=create_bdb setting because, the legacy wallet type is being deprecated and"
                                                                        " support for creating and opening legacy wallets will be removed in the future."},
             {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED_NAMED_ARG, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."},
             {"external_signer", RPCArg::Type::BOOL, RPCArg::Default{false}, "Use an external signer such as a hardware wallet. Requires -signer to be configured. Wallet creation will fail if keys cannot be fetched. Requires disable_private_keys and descriptors set to true."},
@@ -681,11 +681,16 @@ static RPCHelpMan createwallet()
     if (!request.params[5].isNull() && request.params[6].isNull()) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "The createwallet RPC requires specifying the 'load_on_startup' flag when param 'descriptors' is specified. Dash Core v21 introduced this requirement due to breaking changes in the createwallet RPC.");
     }
-    if (request.params[5].isNull() || request.params[5].get_bool()) {
+    if (self.Arg<bool>("descriptors")) {
 #ifndef USE_SQLITE
         throw JSONRPCError(RPC_WALLET_ERROR, "Compiled without sqlite support (required for descriptor wallets)");
 #endif
         flags |= WALLET_FLAG_DESCRIPTORS;
+    } else {
+        if (!context.chain->rpcEnableDeprecated("create_bdb")) {
+            throw JSONRPCError(RPC_WALLET_ERROR, "BDB wallet creation is deprecated and will be removed in a future release."
+                                                 " In this release it can be re-enabled temporarily with the -deprecatedrpc=create_bdb setting.");
+        }
     }
     if (!request.params[7].isNull() && request.params[7].get_bool()) {
 #ifdef ENABLE_EXTERNAL_SIGNER
diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py
index 168920de092d..3fc9f5bd927c 100644
--- a/test/functional/test_framework/util.py
+++ b/test/functional/test_framework/util.py
@@ -421,6 +421,7 @@ def write_config(config_path, *, n, chain, extra_config="", disable_autoconnect=
         f.write("shrinkdebugfile=0\n")
         # To reduce IO and consumed disk storage use tiny size for allocated blk and rev files
         f.write("tinyblk=1\n")
+        f.write("deprecatedrpc=create_bdb\n")  # Required to run the tests
         # To improve SQLite wallet performance so that the tests don't timeout, use -unsafesqlitesync
         f.write("unsafesqlitesync=1\n")
         if disable_autoconnect:
diff --git a/test/functional/wallet_createwallet.py b/test/functional/wallet_createwallet.py
index 2052ec5b7b29..bf3d27eda8d9 100755
--- a/test/functional/wallet_createwallet.py
+++ b/test/functional/wallet_createwallet.py
@@ -175,7 +175,7 @@ def run_test(self):
 
         if self.is_bdb_compiled():
             self.log.info("Test legacy wallet deprecation")
-            res = self.nodes[0].createwallet(wallet_name="legacy_w0", descriptors=False, load_on_startup=True)
+            res = self.nodes[0].createwallet(wallet_name="legacy_w0", passphrase=None, descriptors=False, load_on_startup=True)
             assert_equal(res["warning"], "Wallet created successfully. The legacy wallet type is being deprecated and support for creating and opening legacy wallets will be removed in the future.")
 
 if __name__ == '__main__':