Merge bitcoin#27757: rpc: remove deprecated "warning" field from {create,load,restore,unload}wallet

5524fa0 doc: add release note about removal of `deprecatedrpc=walletwarningfield` flag (Sebastian Falbesoner)
5c77db7 Restorewallet/createwallet help documentation fixups/improvements (Jon Atack)
a00ae31 rpc: remove deprecated "warning" field from {create,load,restore,unload}wallet (Sebastian Falbesoner)

Pull request description:

  The "warning" string field for wallet creating/loading RPCs (`createwallet`, `loadwallet`, `unloadwallet` and `restorewallet`) has been deprecated with the configuration option `-deprecatedrpc=walletwarningfield` in PR bitcoin#27279 (released in v25.0). For the next release v26.0, the field and the configuration option can be removed.

ACKs for top commit:
  achow101:
    ACK 5524fa0
  jonatack:
    ACK 5524fa0

Tree-SHA512: 8212f72067d08095304018b8a95d2ebef630004b65123483fbbfb078cc5709c2d825bbc35b16ea5f6b28ae7377347382d7e9afaf7bdbf0575d2c229d970784de

Author: achow101

doc/release-notes-27757.md

@@ -0,0 +1,8 @@
+Wallet
+------
+
+- The `deprecatedrpc=walletwarningfield` configuration option has been removed.
+  The `createwallet`, `loadwallet`, `restorewallet` and `unloadwallet` RPCs no
+  longer return the "warning" string field. The same information is provided
+  through the "warnings" field added in v25.0, which returns a JSON array of
+  strings. The "warning" string field was deprecated also in v25.0. (#27757)

src/wallet/rpc/backup.cpp

@@ -2059,15 +2059,15 @@ RPCHelpMan listdescriptors()
 RPCHelpMan backupwallet()
 {
     return RPCHelpMan{"backupwallet",
-        "\nSafely copies current wallet file to destination, which can be a directory or a path with filename.\n",
-        {
-            {"destination", RPCArg::Type::STR, RPCArg::Optional::NO, "The destination directory or file"},
-        },
-        RPCResult{RPCResult::Type::NONE, "", ""},
-        RPCExamples{
-            HelpExampleCli("backupwallet", "\"backup.dat\"")
-    + HelpExampleRpc("backupwallet", "\"backup.dat\"")
-        },
+                "\nSafely copies the current wallet file to the specified destination, which can either be a directory or a path with a filename.\n",
+                {
+                    {"destination", RPCArg::Type::STR, RPCArg::Optional::NO, "The destination directory or file"},
+                },
+                RPCResult{RPCResult::Type::NONE, "", ""},
+                RPCExamples{
+                    HelpExampleCli("backupwallet", "\"backup.dat\"")
+            + HelpExampleRpc("backupwallet", "\"backup.dat\"")
+                },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
     const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request);
@@ -2093,7 +2093,9 @@ RPCHelpMan restorewallet()
 {
     return RPCHelpMan{
         "restorewallet",
-        "\nRestore and loads a wallet from backup.\n",
+        "\nRestores and loads a wallet from backup.\n"
+        "\nThe rescan is significantly faster if a descriptor wallet is restored"
+        "\nand block filters are available (using startup option \"-blockfilterindex=1\").\n",
         {
             {"wallet_name", RPCArg::Type::STR, RPCArg::Optional::NO, "The name that will be applied to the restored wallet"},
             {"backup_file", RPCArg::Type::STR, RPCArg::Optional::NO, "The backup file that will be used to restore the wallet."},
@@ -2103,7 +2105,10 @@ RPCHelpMan restorewallet()
             RPCResult::Type::OBJ, "", "",
             {
                 {RPCResult::Type::STR, "name", "The wallet name if restored successfully."},
-                {RPCResult::Type::STR, "warning", "Warning message if wallet was not loaded cleanly."},
+                {RPCResult::Type::ARR, "warnings", /*optional=*/true, "Warning messages, if any, related to restoring and loading the wallet.",
+                {
+                    {RPCResult::Type::STR, "", ""},
+                }},
             }
         },
         RPCExamples{
@@ -2133,7 +2138,7 @@ RPCHelpMan restorewallet()
 
     UniValue obj(UniValue::VOBJ);
     obj.pushKV("name", wallet->GetName());
-    obj.pushKV("warning", Join(warnings, Untranslated("\n")).original);
+    PushWarnings(warnings, obj);
 
     return obj;
 

src/wallet/rpc/wallet.cpp

@@ -444,24 +444,28 @@ static RPCHelpMan upgradetohd()
 static RPCHelpMan loadwallet()
 {
     return RPCHelpMan{"loadwallet",
-        "\nLoads a wallet from a wallet file or directory."
-        "\nNote that all wallet command-line options used when starting dashd will be"
-        "\napplied to the new wallet (eg, rescan, etc).\n",
-        {
-            {"filename", RPCArg::Type::STR, RPCArg::Optional::NO, "The wallet directory or .dat file."},
-            {"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."},
-        },
-        RPCResult{
-            RPCResult::Type::OBJ, "", "",
-            {
-                {RPCResult::Type::STR, "name", "The wallet name if loaded successfully."},
-                {RPCResult::Type::STR, "warning", "Warning message if wallet was not loaded cleanly."},
-            }
-        },
-        RPCExamples{
-            HelpExampleCli("loadwallet", "\"test.dat\"")
-    + HelpExampleRpc("loadwallet", "\"test.dat\"")
-        },
+                "\nLoads a wallet from a wallet file or directory."
+                "\nNote that all wallet command-line options used when starting dashd will be"
+                "\napplied to the new wallet.\n",
+                {
+                    {"filename", RPCArg::Type::STR, RPCArg::Optional::NO, "The wallet directory or .dat file."},
+                    {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."},
+                },
+                RPCResult{
+                    RPCResult::Type::OBJ, "", "",
+                    {
+                        {RPCResult::Type::STR, "name", "The wallet name if loaded successfully."},
+                        {RPCResult::Type::ARR, "warnings", /*optional=*/true, "Warning messages, if any, related to loading the wallet.",
+                        {
+                            {RPCResult::Type::STR, "", ""},
+                        }},
+                    }
+                },
+                RPCExamples{
+                    HelpExampleCli("loadwallet", "\"test.dat\"")
+            + HelpExampleRpc("loadwallet", "\"test.dat\"")
+                },
+>>>>>>> f0758d8a66 (Merge bitcoin/bitcoin#27757: rpc: remove deprecated "warning" field from {create,load,restore,unload}wallet)
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
     WalletContext& context = EnsureWalletContext(request.context);
@@ -480,7 +484,7 @@ static RPCHelpMan loadwallet()
 
     UniValue obj(UniValue::VOBJ);
     obj.pushKV("name", wallet->GetName());
-    obj.pushKV("warning", Join(warnings, Untranslated("\n")).original);
+    PushWarnings(warnings, obj);
 
     return obj;
 },
@@ -573,7 +577,10 @@ static RPCHelpMan createwallet()
             RPCResult::Type::OBJ, "", "",
             {
                 {RPCResult::Type::STR, "name", "The wallet name if created successfully. If the wallet was created using a full path, the wallet_name will be the full path."},
-                {RPCResult::Type::STR, "warning", "Warning message if wallet was not loaded cleanly."},
+                {RPCResult::Type::ARR, "warnings", /*optional=*/true, "Warning messages, if any, related to creating and loading the wallet.",
+                {
+                    {RPCResult::Type::STR, "", ""},
+                }},
             }
         },
         RPCExamples{
@@ -640,7 +647,7 @@ static RPCHelpMan createwallet()
 
     UniValue obj(UniValue::VOBJ);
     obj.pushKV("name", wallet->GetName());
-    obj.pushKV("warning", Join(warnings, Untranslated("\n")).original);
+    PushWarnings(warnings, obj);
 
     return obj;
 },
@@ -650,19 +657,22 @@ static RPCHelpMan createwallet()
 static RPCHelpMan unloadwallet()
 {
     return RPCHelpMan{"unloadwallet",
-        "Unloads the wallet referenced by the request endpoint otherwise unloads the wallet specified in the argument.\n"
-        "Specifying the wallet name on a wallet endpoint is invalid.",
-        {
-            {"wallet_name", RPCArg::Type::STR, RPCArg::DefaultHint{"the wallet name from the RPC endpoint"}, "The name of the wallet to unload. If provided both here and in the RPC endpoint, the two must be identical."},
-            {"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."},
-        },
-        RPCResult{RPCResult::Type::OBJ, "", "", {
-            {RPCResult::Type::STR, "warning", "Warning message if wallet was not unloaded cleanly."},
-        }},
-        RPCExamples{
-            HelpExampleCli("unloadwallet", "wallet_name")
-    + HelpExampleRpc("unloadwallet", "wallet_name")
-        },
+                "Unloads the wallet referenced by the request endpoint, otherwise unloads the wallet specified in the argument.\n"
+                "Specifying the wallet name on a wallet endpoint is invalid.",
+                {
+                    {"wallet_name", RPCArg::Type::STR, RPCArg::DefaultHint{"the wallet name from the RPC endpoint"}, "The name of the wallet to unload. If provided both here and in the RPC endpoint, the two must be identical."},
+                    {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."},
+                },
+                RPCResult{RPCResult::Type::OBJ, "", "", {
+                    {RPCResult::Type::ARR, "warnings", /*optional=*/true, "Warning messages, if any, related to unloading the wallet.",
+                    {
+                        {RPCResult::Type::STR, "", ""},
+                    }},
+                }},
+                RPCExamples{
+                    HelpExampleCli("unloadwallet", "wallet_name")
+            + HelpExampleRpc("unloadwallet", "wallet_name")
+                },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
     std::string wallet_name;
@@ -699,7 +709,8 @@ static RPCHelpMan unloadwallet()
     UnloadWallet(std::move(wallet));
 
     UniValue result(UniValue::VOBJ);
-    result.pushKV("warning", Join(warnings, Untranslated("\n")).original);
+    PushWarnings(warnings, result);
+
     return result;
 },
     };

test/functional/wallet_createwallet.py

@@ -15,6 +15,9 @@
 )
 from test_framework.wallet_util import bytes_to_wif, generate_wif_key
 
+EMPTY_PASSPHRASE_MSG = "Empty string given as passphrase, wallet will not be encrypted."
+LEGACY_WALLET_MSG = "Wallet created successfully. The legacy wallet type is being deprecated and support for creating and opening legacy wallets will be removed in the future."
+
 class CreateWalletTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 1
@@ -157,7 +160,7 @@ def run_test(self):
         assert_equal(walletinfo['keypoolsize_hd_internal'], keys)
         # Allow empty passphrase, but there should be a warning
         resp = self.nodes[0].createwallet(wallet_name='w7', disable_private_keys=False, blank=False, passphrase='')
-        assert 'Empty string given as passphrase, wallet will not be encrypted.' in resp['warning']
+        assert_equal(resp["warnings"], [EMPTY_PASSPHRASE_MSG] if self.options.descriptors else [EMPTY_PASSPHRASE_MSG, LEGACY_WALLET_MSG])
         w7 = node.get_wallet_rpc('w7')
         assert_raises_rpc_error(-15, 'Error: running with an unencrypted wallet, but walletpassphrase was called.', w7.walletpassphrase, '', 60)
 
@@ -170,5 +173,19 @@ 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")
+            result = self.nodes[0].createwallet(wallet_name="legacy_w0", descriptors=False, passphrase=None)
+            assert_equal(result, {
+                "name": "legacy_w0",
+                "warnings": [LEGACY_WALLET_MSG],
+            })
+            result = self.nodes[0].createwallet(wallet_name="legacy_w1", descriptors=False, passphrase="")
+            assert_equal(result, {
+                "name": "legacy_w1",
+                "warnings": [EMPTY_PASSPHRASE_MSG, LEGACY_WALLET_MSG],
+            })
+
+
 if __name__ == '__main__':
     CreateWalletTest().main()