From 6b247279b72df17b1510241d75c970bc0514cbe2 Mon Sep 17 00:00:00 2001
From: Ava Chow <github@achow101.com>
Date: Fri, 18 Apr 2025 14:45:50 -0700
Subject: [PATCH] wallet: Disallow legacy wallet creation from the wallet tool

---
 src/wallet/wallettool.cpp      | 42 ++++++++++++++++------------------
 test/functional/tool_wallet.py |  9 ++++++++
 2 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp
index 2e40f4feb3f..e2185a72fd7 100644
--- a/src/wallet/wallettool.cpp
+++ b/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) {
diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py
index 552f31f68a4..5bda1edbb9d 100755
--- a/test/functional/tool_wallet.py
+++ b/test/functional/tool_wallet.py
@@ -452,6 +452,14 @@ class ToolWalletTest(BitcoinTestFramework):
         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 @@ class ToolWalletTest(BitcoinTestFramework):
         self.test_dump_createfromdump()
         self.test_chainless_conflicts()
         self.test_dump_very_large_records()
+        self.test_no_create_legacy()
 
 
 if __name__ == '__main__':