Merge bitcoin/bitcoin#23002: Make descriptor wallets by default

9c1052a521 wallet: Default new wallets to descriptor wallets (Andrew Chow)
f19ad40463 rpc, wallet: Descriptor wallets are no longer experimental (Andrew Chow)

Pull request description:

  Changes the default wallet type from legacy to descriptors. Descriptor wallets will now by the default type. Additionally, descriptor wallets will no longer be marked as experimental.

  This follows the timeline proposed in #20160

ACKs for top commit:
  lsilva01:
    Tested ACK 9c1052a521 on Ubuntu 20.04
  prayank23:
    tACK 9c1052a521
  meshcollider:
    Code review ACK 9c1052a521

Tree-SHA512: 834e6fec88e0c18673af7ebe135bd5333694d1be502164eb93a90e3e76c27974165aa4e59426945100c88e4eca07356e16886ef5b05cf789683ecb23fc71a12a

src/bitcoin-wallet.cpp

@@ -30,6 +30,7 @@ static void SetupWalletToolArgs(ArgsManager& argsman)
     argsman.AddArg("-dumpfile=<file name>", "When used with 'dump', writes out the records to this file. When used with 'createfromdump', loads the records into a new wallet.", ArgsManager::ALLOW_STRING, OptionsCategory::OPTIONS);
     argsman.AddArg("-debug=<category>", "Output debugging information (default: 0).", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
     argsman.AddArg("-descriptors", "Create descriptors wallet. Only for 'create'", ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS);
+    argsman.AddArg("-legacy", "Create legacy wallet. Only for 'create'", ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS);
     argsman.AddArg("-format=<format>", "The format of the wallet file to create. Either \"bdb\" or \"sqlite\". Only used with 'createfromdump'", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-printtoconsole", "Send trace/debug info to console (default: 1 when no -debug is true, 0 otherwise).", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
 

src/qt/forms/createwalletdialog.ui

@@ -107,6 +107,9 @@
         <property name="text">
          <string>Descriptor Wallet</string>
         </property>
+        <property name="checked">
+         <bool>true</bool>
+        </property>
        </widget>
       </item>
       <item>

src/wallet/rpcwallet.cpp

@@ -2770,7 +2770,7 @@ 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 sethdseed."},
             {"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{false}, "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"},
             {"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."},
         },
@@ -2812,12 +2812,11 @@ static RPCHelpMan createwallet()
     if (!request.params[4].isNull() && request.params[4].get_bool()) {
         flags |= WALLET_FLAG_AVOID_REUSE;
     }
-    if (!request.params[5].isNull() && request.params[5].get_bool()) {
+    if (request.params[5].isNull() || request.params[5].get_bool()) {
 #ifndef USE_SQLITE
         throw JSONRPCError(RPC_WALLET_ERROR, "Compiled without sqlite support (required for descriptor wallets)");
 #endif
         flags |= WALLET_FLAG_DESCRIPTORS;
-        warnings.emplace_back(Untranslated("Wallet is an experimental descriptor wallet"));
     }
     if (!request.params[7].isNull() && request.params[7].get_bool()) {
 #ifdef ENABLE_EXTERNAL_SIGNER

src/wallet/wallettool.cpp

@@ -120,6 +120,10 @@ bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command)
         tfm::format(std::cerr, "The -descriptors option can only be used with the 'create' command.\n");
         return false;
     }
+    if (args.IsArgSet("-legacy") && command != "create") {
+        tfm::format(std::cerr, "The -legacy option can only be used with the 'create' command.\n");
+        return false;
+    }
     if (command == "create" && !args.IsArgSet("-wallet")) {
         tfm::format(std::cerr, "Wallet name must be provided when creating a new wallet.\n");
         return false;
@@ -130,7 +134,19 @@ bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command)
     if (command == "create") {
         DatabaseOptions options;
         options.require_create = true;
-        if (args.GetBoolArg("-descriptors", false)) {
+        // If -legacy is set, use it. Otherwise default to false.
+        bool make_legacy = args.GetBoolArg("-legacy", false);
+        // If neither -legacy nor -descriptors is set, default to true. If -descriptors is set, use its value.
+        bool make_descriptors = (!args.IsArgSet("-descriptors") && !args.IsArgSet("-legacy")) || (args.IsArgSet("-descriptors") && args.GetBoolArg("-descriptors", true));
+        if (make_legacy && make_descriptors) {
+            tfm::format(std::cerr, "Only one of -legacy or -descriptors can be set to true, not both\n");
+            return false;
+        }
+        if (!make_legacy && !make_descriptors) {
+            tfm::format(std::cerr, "One of -legacy or -descriptors must be set to true (or omitted)\n");
+            return false;
+        }
+        if (make_descriptors) {
             options.create_flags |= WALLET_FLAG_DESCRIPTORS;
             options.require_format = DatabaseFormat::SQLITE;
         }

test/functional/tool_wallet.py

@@ -31,8 +31,8 @@ class ToolWalletTest(BitcoinTestFramework):
     def bitcoin_wallet_process(self, *args):
         binary = self.config["environment"]["BUILDDIR"] + '/src/bitcoin-wallet' + self.config["environment"]["EXEEXT"]
         default_args = ['-datadir={}'.format(self.nodes[0].datadir), '-chain=%s' % self.chain]
-        if self.options.descriptors and 'create' in args:
-            default_args.append('-descriptors')
+        if not self.options.descriptors and 'create' in args:
+            default_args.append('-legacy')
 
         return subprocess.Popen([binary] + default_args + list(args), stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)