Merge bitcoin#28597: wallet: No BDB creation, unless -deprecatedrpc=create_bdb

fa071ae 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 fa071ae
  achow101:
    ACK fa071ae
  furszy:
    utACK fa071ae

Tree-SHA512: 37a4c3e4ba659e0ebe2382e71d9c80e42a895d9ad743f5dda7c110fbbb7d2a36f46769982552a9ac0c3a57203379ef164be97aa8033eb7674d6b4da030ba8f9b

Author: achow101

src/rpc/util.cpp

@@ -579,6 +579,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(););

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

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:

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__':