From c46c18b788cb0862aafbb116fd37936cbed6a431 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Mon, 16 Nov 2020 18:55:32 +0100 Subject: wallet: refactor GetClosestWalletFeature() --- src/wallet/walletutil.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) (limited to 'src/wallet') diff --git a/src/wallet/walletutil.cpp b/src/wallet/walletutil.cpp index 702293e6c7..88a52cdf63 100644 --- a/src/wallet/walletutil.cpp +++ b/src/wallet/walletutil.cpp @@ -87,13 +87,9 @@ bool IsFeatureSupported(int wallet_version, int feature_version) WalletFeature GetClosestWalletFeature(int version) { - if (version >= FEATURE_LATEST) return FEATURE_LATEST; - if (version >= FEATURE_PRE_SPLIT_KEYPOOL) return FEATURE_PRE_SPLIT_KEYPOOL; - if (version >= FEATURE_NO_DEFAULT_KEY) return FEATURE_NO_DEFAULT_KEY; - if (version >= FEATURE_HD_SPLIT) return FEATURE_HD_SPLIT; - if (version >= FEATURE_HD) return FEATURE_HD; - if (version >= FEATURE_COMPRPUBKEY) return FEATURE_COMPRPUBKEY; - if (version >= FEATURE_WALLETCRYPT) return FEATURE_WALLETCRYPT; - if (version >= FEATURE_BASE) return FEATURE_BASE; + const std::array wallet_features{{FEATURE_LATEST, FEATURE_PRE_SPLIT_KEYPOOL, FEATURE_NO_DEFAULT_KEY, FEATURE_HD_SPLIT, FEATURE_HD, FEATURE_COMPRPUBKEY, FEATURE_WALLETCRYPT, FEATURE_BASE}}; + for (const WalletFeature& wf : wallet_features) { + if (version >= wf) return wf; + } return static_cast(0); } -- cgit v1.2.3 From 2498b04ce88696a3216fc38b7d393906b733e8b1 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 18 Nov 2020 13:14:19 -0500 Subject: Don't upgrade to HD split if it is already supported It is unnecessary to upgrade to FEATURE_HD_SPLIT if this feature is already supported by the wallet. Because upgrading to FEATURE_HD_SPLIT actually requires upgrading to FEATURE_PRE_SPLIT_KEYPOOL, users would accidentally be upgraded to FEATURE_PRE_SPLIT_KEYPOOL instead of nothing being done. Fixes the issue described at https://github.com/bitcoin/bitcoin/pull/20403#discussion_r526063920 --- src/wallet/scriptpubkeyman.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/wallet') diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index d2e1be6402..7dbbf17302 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -453,7 +453,7 @@ bool LegacyScriptPubKeyMan::Upgrade(int prev_version, int new_version, bilingual hd_upgrade = true; } // Upgrade to HD chain split if necessary - if (IsFeatureSupported(new_version, FEATURE_HD_SPLIT)) { + if (!IsFeatureSupported(prev_version, FEATURE_HD_SPLIT) && IsFeatureSupported(new_version, FEATURE_HD_SPLIT)) { WalletLogPrintf("Upgrading wallet to use HD chain split\n"); m_storage.SetMinVersion(FEATURE_PRE_SPLIT_KEYPOOL); split_upgrade = FEATURE_HD_SPLIT > prev_version; -- cgit v1.2.3 From 99d56e357159c7154f69f28cb5587c5ca20d6594 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Mon, 16 Nov 2020 18:23:10 +0100 Subject: wallet: fix and improve upgradewallet result responses --- src/wallet/rpcwallet.cpp | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) (limited to 'src/wallet') diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 8b08d7a5e6..ef8281bfe2 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4214,7 +4214,7 @@ static RPCHelpMan sethdseed() // Do not do anything to non-HD wallets if (!pwallet->CanSupportFeature(FEATURE_HD)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Cannot set a HD seed on a non-HD wallet. Use the upgradewallet RPC in order to upgrade a non-HD wallet to HD"); + throw JSONRPCError(RPC_WALLET_ERROR, "Cannot set an HD seed on a non-HD wallet. Use the upgradewallet RPC in order to upgrade a non-HD wallet to HD"); } EnsureWalletIsUnlocked(pwallet); @@ -4445,14 +4445,18 @@ static RPCHelpMan walletcreatefundedpsbt() static RPCHelpMan upgradewallet() { return RPCHelpMan{"upgradewallet", - "\nUpgrade the wallet. Upgrades to the latest version if no version number is specified\n" + "\nUpgrade the wallet. Upgrades to the latest version if no version number is specified.\n" "New keys may be generated and a new wallet backup will need to be made.", { - {"version", RPCArg::Type::NUM, /* default */ strprintf("%d", FEATURE_LATEST), "The version number to upgrade to. Default is the latest wallet version"} + {"version", RPCArg::Type::NUM, /* default */ strprintf("%d", FEATURE_LATEST), "The version number to upgrade to. Default is the latest wallet version."} }, RPCResult{ RPCResult::Type::OBJ, "", "", { + {RPCResult::Type::STR, "wallet_name", "Name of wallet this operation was performed on"}, + {RPCResult::Type::NUM, "previous_version", "Version of wallet before this operation"}, + {RPCResult::Type::NUM, "current_version", "Version of wallet after this operation"}, + {RPCResult::Type::STR, "result", /* optional */ true, "Description of result, if no error"}, {RPCResult::Type::STR, "error", /* optional */ true, "Error message (if there is one)"} }, }, @@ -4475,10 +4479,26 @@ static RPCHelpMan upgradewallet() version = request.params[0].get_int(); } bilingual_str error; - if (!pwallet->UpgradeWallet(version, error)) { + const int previous_version{pwallet->GetVersion()}; + const bool wallet_upgraded{pwallet->UpgradeWallet(version, error)}; + const int current_version{pwallet->GetVersion()}; + std::string result; + + if (!wallet_upgraded) { throw JSONRPCError(RPC_WALLET_ERROR, error.original); + } else if (previous_version == current_version) { + result = "Already at latest version. Wallet version unchanged."; + } else { + result = strprintf("Wallet upgraded successfully from version %i to version %i.", previous_version, current_version); } + UniValue obj(UniValue::VOBJ); + obj.pushKV("wallet_name", pwallet->GetName()); + obj.pushKV("previous_version", previous_version); + obj.pushKV("current_version", current_version); + if (!result.empty()) { + obj.pushKV("result", result); + } if (!error.empty()) { obj.pushKV("error", error.original); } -- cgit v1.2.3 From ca8cd893bb56bf5d455154b0498b1f58f77d20ed Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Tue, 17 Nov 2020 15:57:14 +0100 Subject: wallet: fix and improve upgradewallet error responses --- src/wallet/rpcwallet.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'src/wallet') diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index ef8281bfe2..2e3b34cbe3 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4484,12 +4484,12 @@ static RPCHelpMan upgradewallet() const int current_version{pwallet->GetVersion()}; std::string result; - if (!wallet_upgraded) { - throw JSONRPCError(RPC_WALLET_ERROR, error.original); - } else if (previous_version == current_version) { - result = "Already at latest version. Wallet version unchanged."; - } else { - result = strprintf("Wallet upgraded successfully from version %i to version %i.", previous_version, current_version); + if (wallet_upgraded) { + if (previous_version == current_version) { + result = "Already at latest version. Wallet version unchanged."; + } else { + result = strprintf("Wallet upgraded successfully from version %i to version %i.", previous_version, current_version); + } } UniValue obj(UniValue::VOBJ); @@ -4498,8 +4498,8 @@ static RPCHelpMan upgradewallet() obj.pushKV("current_version", current_version); if (!result.empty()) { obj.pushKV("result", result); - } - if (!error.empty()) { + } else { + CHECK_NONFATAL(!error.empty()); obj.pushKV("error", error.original); } return obj; -- cgit v1.2.3 From 3eb6f8b2e61c24a22ea9396d86672307845f35eb Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Tue, 17 Nov 2020 19:18:12 +0100 Subject: wallet (not for backport): improve upgradewallet error messages --- src/wallet/wallet.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'src/wallet') diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3ce8272fb9..12a725b5a2 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4111,9 +4111,8 @@ bool CWallet::UpgradeWallet(int version, bilingual_str& error) } else { WalletLogPrintf("Allowing wallet upgrade up to %i\n", version); } - if (version < prev_version) - { - error = _("Cannot downgrade wallet"); + if (version < prev_version) { + error = strprintf(_("Cannot downgrade wallet from version %i to version %i. Wallet version unchanged."), prev_version, version); return false; } @@ -4121,7 +4120,7 @@ bool CWallet::UpgradeWallet(int version, bilingual_str& error) // Do not upgrade versions to any version between HD_SPLIT and FEATURE_PRE_SPLIT_KEYPOOL unless already supporting HD_SPLIT if (!CanSupportFeature(FEATURE_HD_SPLIT) && version >= FEATURE_HD_SPLIT && version < FEATURE_PRE_SPLIT_KEYPOOL) { - error = _("Cannot upgrade a non HD split wallet without upgrading to support pre split keypool. Please use version 169900 or no version specified."); + error = strprintf(_("Cannot upgrade a non HD split wallet from version %i to version %i without upgrading to support pre-split keypool. Please use version %i or no version specified."), prev_version, version, FEATURE_PRE_SPLIT_KEYPOOL); return false; } -- cgit v1.2.3