Merge bitcoin/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 #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

@@ -1862,7 +1862,7 @@ 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",
+                "\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"},
                 },
@@ -1897,7 +1897,7 @@ 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",
         {
@@ -1909,8 +1909,7 @@ RPCHelpMan restorewallet()
             RPCResult::Type::OBJ, "", "",
             {
                 {RPCResult::Type::STR, "name", "The wallet name if restored successfully."},
-                {RPCResult::Type::STR, "warning", /*optional=*/true, "Warning messages, if any, related to restoring the wallet. Multiple messages will be delimited by newlines. (DEPRECATED, returned only if config option -deprecatedrpc=walletwarningfield is passed.)"},
-                {RPCResult::Type::ARR, "warnings", /*optional=*/true, "Warning messages, if any, related to restoring the wallet.",
+                {RPCResult::Type::ARR, "warnings", /*optional=*/true, "Warning messages, if any, related to restoring and loading the wallet.",
                 {
                     {RPCResult::Type::STR, "", ""},
                 }},
@@ -1943,9 +1942,6 @@ RPCHelpMan restorewallet()
 
     UniValue obj(UniValue::VOBJ);
     obj.pushKV("name", wallet->GetName());
-    if (wallet->chain().rpcEnableDeprecated("walletwarningfield")) {
-        obj.pushKV("warning", Join(warnings, Untranslated("\n")).original);
-    }
     PushWarnings(warnings, obj);
 
     return obj;

src/wallet/rpc/wallet.cpp

@@ -221,7 +221,6 @@ static RPCHelpMan loadwallet()
                     RPCResult::Type::OBJ, "", "",
                     {
                         {RPCResult::Type::STR, "name", "The wallet name if loaded successfully."},
-                        {RPCResult::Type::STR, "warning", /*optional=*/true, "Warning messages, if any, related to loading the wallet. Multiple messages will be delimited by newlines. (DEPRECATED, returned only if config option -deprecatedrpc=walletwarningfield is passed.)"},
                         {RPCResult::Type::ARR, "warnings", /*optional=*/true, "Warning messages, if any, related to loading the wallet.",
                         {
                             {RPCResult::Type::STR, "", ""},
@@ -258,9 +257,6 @@ static RPCHelpMan loadwallet()
 
     UniValue obj(UniValue::VOBJ);
     obj.pushKV("name", wallet->GetName());
-    if (wallet->chain().rpcEnableDeprecated("walletwarningfield")) {
-        obj.pushKV("warning", Join(warnings, Untranslated("\n")).original);
-    }
     PushWarnings(warnings, obj);
 
     return obj;
@@ -356,8 +352,7 @@ 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", /*optional=*/true, "Warning messages, if any, related to creating the wallet. Multiple messages will be delimited by newlines. (DEPRECATED, returned only if config option -deprecatedrpc=walletwarningfield is passed.)"},
-                {RPCResult::Type::ARR, "warnings", /*optional=*/true, "Warning messages, if any, related to creating the wallet.",
+                {RPCResult::Type::ARR, "warnings", /*optional=*/true, "Warning messages, if any, related to creating and loading the wallet.",
                 {
                     {RPCResult::Type::STR, "", ""},
                 }},
@@ -430,9 +425,6 @@ static RPCHelpMan createwallet()
 
     UniValue obj(UniValue::VOBJ);
     obj.pushKV("name", wallet->GetName());
-    if (wallet->chain().rpcEnableDeprecated("walletwarningfield")) {
-        obj.pushKV("warning", Join(warnings, Untranslated("\n")).original);
-    }
     PushWarnings(warnings, obj);
 
     return obj;
@@ -443,14 +435,13 @@ 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"
+                "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::STR, "warning", /*optional=*/true, "Warning messages, if any, related to unloading the wallet. Multiple messages will be delimited by newlines. (DEPRECATED, returned only if config option -deprecatedrpc=walletwarningfield is passed.)"},
                     {RPCResult::Type::ARR, "warnings", /*optional=*/true, "Warning messages, if any, related to unloading the wallet.",
                     {
                         {RPCResult::Type::STR, "", ""},
@@ -492,13 +483,12 @@ static RPCHelpMan unloadwallet()
             throw JSONRPCError(RPC_MISC_ERROR, "Requested wallet already unloaded");
         }
     }
+
+    UnloadWallet(std::move(wallet));
+
     UniValue result(UniValue::VOBJ);
-    if (wallet->chain().rpcEnableDeprecated("walletwarningfield")) {
-        result.pushKV("warning", Join(warnings, Untranslated("\n")).original);
-    }
     PushWarnings(warnings, result);
 
-    UnloadWallet(std::move(wallet));
     return result;
 },
     };

test/functional/wallet_createwallet.py

@@ -25,7 +25,6 @@ def add_options(self, parser):
 
     def set_test_params(self):
         self.num_nodes = 1
-        self.extra_args = [["-deprecatedrpc=walletwarningfield"]]
 
     def skip_test_if_missing_module(self):
         self.skip_if_no_wallet()
@@ -164,7 +163,6 @@ 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_equal(resp["warning"], EMPTY_PASSPHRASE_MSG if self.options.descriptors else f"{EMPTY_PASSPHRASE_MSG}\n{LEGACY_WALLET_MSG}")
         assert_equal(resp["warnings"], [EMPTY_PASSPHRASE_MSG] if self.options.descriptors else [EMPTY_PASSPHRASE_MSG, LEGACY_WALLET_MSG])
 
         w7 = node.get_wallet_rpc('w7')
@@ -184,21 +182,14 @@ def run_test(self):
             result = self.nodes[0].createwallet(wallet_name="legacy_w0", descriptors=False, passphrase=None)
             assert_equal(result, {
                 "name": "legacy_w0",
-                "warning": LEGACY_WALLET_MSG,
                 "warnings": [LEGACY_WALLET_MSG],
             })
             result = self.nodes[0].createwallet(wallet_name="legacy_w1", descriptors=False, passphrase="")
             assert_equal(result, {
                 "name": "legacy_w1",
-                "warning": f"{EMPTY_PASSPHRASE_MSG}\n{LEGACY_WALLET_MSG}",
                 "warnings": [EMPTY_PASSPHRASE_MSG, LEGACY_WALLET_MSG],
             })
 
-        self.log.info('Test "warning" field deprecation, i.e. not returned without -deprecatedrpc=walletwarningfield')
-        self.restart_node(0, extra_args=[])
-        result = self.nodes[0].createwallet(wallet_name="w7_again", disable_private_keys=False, blank=False, passphrase="")
-        assert "warning" not in result
-
 
 if __name__ == '__main__':
     CreateWalletTest().main()