From f73782a9032a462a71569e9424db9bf9eeababf3 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Sun, 19 Mar 2023 10:23:45 -0700 Subject: doc: fix/improve warning helps in {create,load,unload,restore}wallet - clarify that there can be multiple warning messages - specify the correct wallet action - describe the use of newlines as delimiters --- src/wallet/rpc/backup.cpp | 2 +- src/wallet/rpc/wallet.cpp | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 9b6b3d629c..9154632520 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -1903,7 +1903,7 @@ 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::STR, "warning", "Warning messages, if any, related to restoring the wallet. Multiple messages will be delimited by newlines."}, } }, RPCExamples{ diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index 16595267b4..20cd697646 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -207,7 +207,7 @@ static RPCHelpMan loadwallet() 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."}, + {RPCResult::Type::STR, "warning", "Warning messages, if any, related to loading the wallet. Multiple messages will be delimited by newlines."}, } }, RPCExamples{ @@ -335,7 +335,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", "Warning message if wallet was not loaded cleanly."}, + {RPCResult::Type::STR, "warning", "Warning messages, if any, related to creating the wallet. Multiple messages will be delimited by newlines."}, } }, RPCExamples{ @@ -422,7 +422,7 @@ static RPCHelpMan unloadwallet() {"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", "Warning message if wallet was not unloaded cleanly."}, + {RPCResult::Type::STR, "warning", "Warning messages, if any, related to unloading the wallet. Multiple messages will be delimited by newlines."}, }}, RPCExamples{ HelpExampleCli("unloadwallet", "wallet_name") -- cgit v1.2.3 From 079d8cdda8eeebe199fb6592fca2630c37662731 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Sun, 19 Mar 2023 10:16:09 -0700 Subject: rpc: extract wallet "warnings" fields to a util helper --- src/rpc/output_script.cpp | 2 +- src/rpc/util.cpp | 6 ++++++ src/rpc/util.h | 8 ++++++++ src/wallet/rpc/addresses.cpp | 2 +- src/wallet/rpc/backup.cpp | 4 ++-- 5 files changed, 18 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/rpc/output_script.cpp b/src/rpc/output_script.cpp index bb04f58424..990ec3ab0c 100644 --- a/src/rpc/output_script.cpp +++ b/src/rpc/output_script.cpp @@ -162,7 +162,7 @@ static RPCHelpMan createmultisig() // Only warns if the user has explicitly chosen an address type we cannot generate warnings.push_back("Unable to make chosen address type, please ensure no uncompressed public keys are present."); } - if (!warnings.empty()) result.pushKV("warnings", warnings); + PushWarnings(warnings, result); return result; }, diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index ee9e3544a4..f01944cd0b 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -1174,3 +1174,9 @@ UniValue GetServicesNames(ServiceFlags services) return servicesNames; } + +void PushWarnings(const UniValue& warnings, UniValue& obj) +{ + if (warnings.empty()) return; + obj.pushKV("warnings", warnings); +} diff --git a/src/rpc/util.h b/src/rpc/util.h index e3783c8f76..15c73068b9 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -381,4 +381,12 @@ private: const RPCExamples m_examples; }; +/** + * Push warning messages to an RPC "warnings" field as a JSON array of strings. + * + * @param[in] warnings Warning messages to push. + * @param[out] obj UniValue object to push the warnings array object to. + */ +void PushWarnings(const UniValue& warnings, UniValue& obj); + #endif // BITCOIN_RPC_UTIL_H diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index da63d45d11..4c27b08373 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -300,7 +300,7 @@ RPCHelpMan addmultisigaddress() // Only warns if the user has explicitly chosen an address type we cannot generate warnings.push_back("Unable to make chosen address type, please ensure no uncompressed public keys are present."); } - if (!warnings.empty()) result.pushKV("warnings", warnings); + PushWarnings(warnings, result); return result; }, diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 9154632520..026acf939b 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -1225,7 +1225,7 @@ static UniValue ProcessImport(CWallet& wallet, const UniValue& data, const int64 result.pushKV("error", JSONRPCError(RPC_MISC_ERROR, "Missing required fields")); } - if (warnings.size()) result.pushKV("warnings", warnings); + PushWarnings(warnings, result); return result; } @@ -1579,7 +1579,7 @@ static UniValue ProcessDescriptorImport(CWallet& wallet, const UniValue& data, c result.pushKV("success", UniValue(false)); result.pushKV("error", e); } - if (warnings.size()) result.pushKV("warnings", warnings); + PushWarnings(warnings, result); return result; } -- cgit v1.2.3 From 4a1e479ca612056761e6247dd5b715dcd6824413 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Sun, 19 Mar 2023 10:19:06 -0700 Subject: rpc: add "warnings" field to RPCs {create,load,unload,restore}wallet This new "warnings" field is a JSON array of strings intended to replace the "warning" string field in these four RPCs, to better handle returning multiple warning messages and for consistency with other wallet RPCs. --- src/rpc/util.cpp | 17 +++++++++++++++++ src/rpc/util.h | 1 + src/wallet/rpc/backup.cpp | 5 +++++ src/wallet/rpc/wallet.cpp | 16 ++++++++++++++++ 4 files changed, 39 insertions(+) (limited to 'src') diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index f01944cd0b..f95ac4cb4b 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -1175,8 +1175,25 @@ UniValue GetServicesNames(ServiceFlags services) return servicesNames; } +/** Convert a vector of bilingual strings to a UniValue::VARR containing their original untranslated values. */ +[[nodiscard]] static UniValue BilingualStringsToUniValue(const std::vector& bilingual_strings) +{ + CHECK_NONFATAL(!bilingual_strings.empty()); + UniValue result{UniValue::VARR}; + for (const auto& s : bilingual_strings) { + result.push_back(s.original); + } + return result; +} + void PushWarnings(const UniValue& warnings, UniValue& obj) { if (warnings.empty()) return; obj.pushKV("warnings", warnings); } + +void PushWarnings(const std::vector& warnings, UniValue& obj) +{ + if (warnings.empty()) return; + obj.pushKV("warnings", BilingualStringsToUniValue(warnings)); +} diff --git a/src/rpc/util.h b/src/rpc/util.h index 15c73068b9..bb5c30a2f4 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -388,5 +388,6 @@ private: * @param[out] obj UniValue object to push the warnings array object to. */ void PushWarnings(const UniValue& warnings, UniValue& obj); +void PushWarnings(const std::vector& warnings, UniValue& obj); #endif // BITCOIN_RPC_UTIL_H diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 026acf939b..76a736d14d 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -1904,6 +1904,10 @@ RPCHelpMan restorewallet() { {RPCResult::Type::STR, "name", "The wallet name if restored successfully."}, {RPCResult::Type::STR, "warning", "Warning messages, if any, related to restoring the wallet. Multiple messages will be delimited by newlines."}, + {RPCResult::Type::ARR, "warnings", /*optional=*/true, "Warning messages, if any, related to restoring the wallet.", + { + {RPCResult::Type::STR, "", ""}, + }}, } }, RPCExamples{ @@ -1934,6 +1938,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; diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index 20cd697646..a28ddfb01b 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -208,6 +208,10 @@ static RPCHelpMan loadwallet() { {RPCResult::Type::STR, "name", "The wallet name if loaded successfully."}, {RPCResult::Type::STR, "warning", "Warning messages, if any, related to loading the wallet. Multiple messages will be delimited by newlines."}, + {RPCResult::Type::ARR, "warnings", /*optional=*/true, "Warning messages, if any, related to loading the wallet.", + { + {RPCResult::Type::STR, "", ""}, + }}, } }, RPCExamples{ @@ -241,6 +245,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; }, @@ -336,6 +341,10 @@ static RPCHelpMan createwallet() { {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 messages, if any, related to creating the wallet. Multiple messages will be delimited by newlines."}, + {RPCResult::Type::ARR, "warnings", /*optional=*/true, "Warning messages, if any, related to creating the wallet.", + { + {RPCResult::Type::STR, "", ""}, + }}, } }, RPCExamples{ @@ -406,6 +415,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; }, @@ -423,6 +433,10 @@ static RPCHelpMan unloadwallet() }, RPCResult{RPCResult::Type::OBJ, "", "", { {RPCResult::Type::STR, "warning", "Warning messages, if any, related to unloading the wallet. Multiple messages will be delimited by newlines."}, + {RPCResult::Type::ARR, "warnings", /*optional=*/true, "Warning messages, if any, related to unloading the wallet.", + { + {RPCResult::Type::STR, "", ""}, + }}, }}, RPCExamples{ HelpExampleCli("unloadwallet", "wallet_name") @@ -465,6 +479,8 @@ static RPCHelpMan unloadwallet() UniValue result(UniValue::VOBJ); result.pushKV("warning", Join(warnings, Untranslated("\n")).original); + PushWarnings(warnings, result); + return result; }, }; -- cgit v1.2.3 From 645d7f75ac1b40e4ea88119b3711f89943d35d6c Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Sun, 19 Mar 2023 19:26:14 -0700 Subject: rpc: deprecate "warning" field in {create,load,unload,restore}wallet This string field has been replaced in these four RPCs by a "warnings" field returning a JSON array of strings. --- src/wallet/rpc/backup.cpp | 6 ++++-- src/wallet/rpc/wallet.cpp | 22 +++++++++++++--------- 2 files changed, 17 insertions(+), 11 deletions(-) (limited to 'src') diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 76a736d14d..fb175fa253 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -1903,7 +1903,7 @@ RPCHelpMan restorewallet() RPCResult::Type::OBJ, "", "", { {RPCResult::Type::STR, "name", "The wallet name if restored successfully."}, - {RPCResult::Type::STR, "warning", "Warning messages, if any, related to restoring the wallet. Multiple messages will be delimited by newlines."}, + {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::STR, "", ""}, @@ -1937,7 +1937,9 @@ RPCHelpMan restorewallet() UniValue obj(UniValue::VOBJ); obj.pushKV("name", wallet->GetName()); - obj.pushKV("warning", Join(warnings, Untranslated("\n")).original); + if (wallet->chain().rpcEnableDeprecated("walletwarningfield")) { + obj.pushKV("warning", Join(warnings, Untranslated("\n")).original); + } PushWarnings(warnings, obj); return obj; diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index a28ddfb01b..d4aff4dfd6 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -207,7 +207,7 @@ static RPCHelpMan loadwallet() RPCResult::Type::OBJ, "", "", { {RPCResult::Type::STR, "name", "The wallet name if loaded successfully."}, - {RPCResult::Type::STR, "warning", "Warning messages, if any, related to loading the wallet. Multiple messages will be delimited by newlines."}, + {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, "", ""}, @@ -244,7 +244,9 @@ static RPCHelpMan loadwallet() UniValue obj(UniValue::VOBJ); obj.pushKV("name", wallet->GetName()); - obj.pushKV("warning", Join(warnings, Untranslated("\n")).original); + if (wallet->chain().rpcEnableDeprecated("walletwarningfield")) { + obj.pushKV("warning", Join(warnings, Untranslated("\n")).original); + } PushWarnings(warnings, obj); return obj; @@ -340,7 +342,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", "Warning messages, if any, related to creating the wallet. Multiple messages will be delimited by newlines."}, + {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::STR, "", ""}, @@ -414,7 +416,9 @@ static RPCHelpMan createwallet() UniValue obj(UniValue::VOBJ); obj.pushKV("name", wallet->GetName()); - obj.pushKV("warning", Join(warnings, Untranslated("\n")).original); + if (wallet->chain().rpcEnableDeprecated("walletwarningfield")) { + obj.pushKV("warning", Join(warnings, Untranslated("\n")).original); + } PushWarnings(warnings, obj); return obj; @@ -432,7 +436,7 @@ static RPCHelpMan unloadwallet() {"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", "Warning messages, if any, related to unloading the wallet. Multiple messages will be delimited by newlines."}, + {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, "", ""}, @@ -474,13 +478,13 @@ static RPCHelpMan unloadwallet() throw JSONRPCError(RPC_MISC_ERROR, "Requested wallet already unloaded"); } } - - UnloadWallet(std::move(wallet)); - UniValue result(UniValue::VOBJ); - result.pushKV("warning", Join(warnings, Untranslated("\n")).original); + if (wallet->chain().rpcEnableDeprecated("walletwarningfield")) { + result.pushKV("warning", Join(warnings, Untranslated("\n")).original); + } PushWarnings(warnings, result); + UnloadWallet(std::move(wallet)); return result; }, }; -- cgit v1.2.3 From 19d888ce407f44d90785c456a1a3e2a6870e9245 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Wed, 29 Mar 2023 09:31:14 -0700 Subject: rpc: move WALLET_FLAG_CAVEATS to the compilation unit of its caller and add the walletutil.h include header for WALLET_FLAG_AVOID_REUSE that was already missing before this change. WALLET_FLAG_CAVEATS is only used in one RPC, so no need to encumber wallet.h and wallet.cpp with it, along with all of the files that include wallet.h during their compilation. Also apply clang-format per: git diff -U0 HEAD~1.. | ./contrib/devtools/clang-format-diff.py -p1 -i -v --- src/wallet/rpc/wallet.cpp | 9 +++++++++ src/wallet/wallet.cpp | 7 ------- src/wallet/wallet.h | 2 -- 3 files changed, 9 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index d4aff4dfd6..ea3507bc75 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include @@ -20,6 +21,14 @@ namespace wallet { + +static const std::map WALLET_FLAG_CAVEATS{ + {WALLET_FLAG_AVOID_REUSE, + "You need to rescan the blockchain in order to correctly mark used " + "destinations in the past. Until this is done, some destinations may " + "be considered unused, even if the opposite is the case."}, +}; + /** Checks if a CKey is in the given CWallet compressed or otherwise*/ bool HaveKey(const SigningProvider& wallet, const CKey& key) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ef8fb29e64..2b8aeacb2e 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -51,13 +51,6 @@ using interfaces::FoundBlock; namespace wallet { -const std::map WALLET_FLAG_CAVEATS{ - {WALLET_FLAG_AVOID_REUSE, - "You need to rescan the blockchain in order to correctly mark used " - "destinations in the past. Until this is done, some destinations may " - "be considered unused, even if the opposite is the case." - }, -}; bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index e8c18dbb67..346b63fd86 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -145,8 +145,6 @@ static const std::map WALLET_FLAG_MAP{ {"external_signer", WALLET_FLAG_EXTERNAL_SIGNER} }; -extern const std::map WALLET_FLAG_CAVEATS; - /** A wrapper to reserve an address from a wallet * * ReserveDestination is used to reserve an address. -- cgit v1.2.3