wallet: Disallow legacy wallet creation from the wallet tool

Author: achow101

src/wallet/wallettool.cpp

@@ -115,13 +115,25 @@ bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command)
         tfm::format(std::cerr, "The -dumpfile option can only be used with the \"dump\" and \"createfromdump\" commands.\n");
         return false;
     }
-    if (args.IsArgSet("-descriptors") && command != "create") {
-        tfm::format(std::cerr, "The -descriptors option can only be used with the 'create' command.\n");
-        return false;
+    if (args.IsArgSet("-descriptors")) {
+        if (command != "create") {
+            tfm::format(std::cerr, "The -descriptors option can only be used with the 'create' command.\n");
+            return false;
+        }
+        if (!args.GetBoolArg("-descriptors", true)) {
+            tfm::format(std::cerr, "The -descriptors option must be set to \"true\"\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 (args.IsArgSet("-legacy")) {
+        if (command != "create") {
+            tfm::format(std::cerr, "The -legacy option can only be used with the 'create' command.\n");
+            return false;
+        }
+        if (args.GetBoolArg("-legacy", true)) {
+            tfm::format(std::cerr, "The -legacy option must be set to \"false\"");
+            return false;
+        }
     }
     if (command == "create" && !args.IsArgSet("-wallet")) {
         tfm::format(std::cerr, "Wallet name must be provided when creating a new wallet.\n");
@@ -134,22 +146,8 @@ bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command)
         DatabaseOptions options;
         ReadDatabaseArgs(args, options);
         options.require_create = true;
-        // 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;
-        }
+        options.create_flags |= WALLET_FLAG_DESCRIPTORS;
+        options.require_format = DatabaseFormat::SQLITE;
 
         const std::shared_ptr<CWallet> wallet_instance = MakeWallet(name, path, options);
         if (wallet_instance) {

test/functional/tool_wallet.py

@@ -452,6 +452,14 @@ def test_dump_very_large_records(self):
         else:
             assert False, "Big transaction was not found in wallet dump"
 
+    def test_no_create_legacy(self):
+        self.log.info("Test that legacy wallets cannot be created")
+
+        self.assert_raises_tool_error("The -legacy option must be set to \"false\"", "-wallet=legacy", "-legacy", "create")
+        assert not (self.nodes[0].wallets_path / "legacy").exists()
+        self.assert_raises_tool_error("The -descriptors option must be set to \"true\"", "-wallet=legacy", "-descriptors=false", "create")
+        assert not (self.nodes[0].wallets_path / "legacy").exists()
+
     def run_test(self):
         self.wallet_path = self.nodes[0].wallets_path / self.default_wallet_name / self.wallet_data_filename
         self.test_invalid_tool_commands_and_args()
@@ -463,6 +471,7 @@ def run_test(self):
         self.test_dump_createfromdump()
         self.test_chainless_conflicts()
         self.test_dump_very_large_records()
+        self.test_no_create_legacy()
 
 
 if __name__ == '__main__':