wallet: fix and improve upgradewallet result responses

This commit is contained in:
Jon Atack 2020-11-16 18:23:10 +01:00
parent 2498b04ce8
commit 99d56e3571
No known key found for this signature in database
GPG key ID: 4F5721B3D0E3921D
2 changed files with 51 additions and 32 deletions

View file

@ -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);
}

View file

@ -22,7 +22,6 @@ from test_framework.messages import deser_compact_size, deser_string
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_equal,
assert_greater_than,
assert_is_hex_string,
assert_raises_rpc_error,
sha256sum_file,
@ -92,6 +91,20 @@ class UpgradeWalletTest(BitcoinTestFramework):
v16_3_node.submitblock(b)
assert_equal(v16_3_node.getblockcount(), to_height)
def test_upgradewallet(self, wallet, previous_version, requested_version=None, expected_version=None):
unchanged = expected_version == previous_version
new_version = previous_version if unchanged else expected_version if expected_version else requested_version
assert_equal(wallet.getwalletinfo()["walletversion"], previous_version)
assert_equal(wallet.upgradewallet(requested_version),
{
"wallet_name": "",
"previous_version": previous_version,
"current_version": new_version,
"result": "Already at latest version. Wallet version unchanged." if unchanged else "Wallet upgraded successfully from version {} to version {}.".format(previous_version, new_version),
}
)
assert_equal(wallet.getwalletinfo()["walletversion"], new_version)
def run_test(self):
self.nodes[0].generatetoaddress(101, self.nodes[0].getnewaddress())
self.dumb_sync_blocks()
@ -158,14 +171,8 @@ class UpgradeWalletTest(BitcoinTestFramework):
self.restart_node(0)
copy_v16()
wallet = node_master.get_wallet_rpc(self.default_wallet_name)
old_version = wallet.getwalletinfo()["walletversion"]
# calling upgradewallet without version arguments
# should return nothing if successful
assert_equal(wallet.upgradewallet(), {})
new_version = wallet.getwalletinfo()["walletversion"]
# upgraded wallet version should be greater than older one
assert_greater_than(new_version, old_version)
self.log.info("Test upgradewallet without a version argument")
self.test_upgradewallet(wallet, previous_version=159900, expected_version=169900)
# wallet should still contain the same balance
assert_equal(wallet.getbalance(), v16_3_balance)
@ -173,25 +180,20 @@ class UpgradeWalletTest(BitcoinTestFramework):
wallet = node_master.get_wallet_rpc(self.default_wallet_name)
# should have no master key hash before conversion
assert_equal('hdseedid' in wallet.getwalletinfo(), False)
# calling upgradewallet with explicit version number
# should return nothing if successful
assert_equal(wallet.upgradewallet(169900), {})
new_version = wallet.getwalletinfo()["walletversion"]
# upgraded wallet should have version 169900
assert_equal(new_version, 169900)
self.log.info("Test upgradewallet with explicit version number")
self.test_upgradewallet(wallet, previous_version=60000, requested_version=169900)
# after conversion master key hash should be present
assert_is_hex_string(wallet.getwalletinfo()['hdseedid'])
self.log.info('Intermediary versions don\'t effect anything')
self.log.info("Intermediary versions don't effect anything")
copy_non_hd()
# Wallet starts with 60000
assert_equal(60000, wallet.getwalletinfo()['walletversion'])
wallet.unloadwallet()
before_checksum = sha256sum_file(node_master_wallet)
node_master.loadwallet('')
# Can "upgrade" to 129999 which should have no effect on the wallet
wallet.upgradewallet(129999)
assert_equal(60000, wallet.getwalletinfo()['walletversion'])
# Test an "upgrade" from 60000 to 129999 has no effect, as the next version is 130000
self.test_upgradewallet(wallet, previous_version=60000, requested_version=129999, expected_version=60000)
wallet.unloadwallet()
assert_equal(before_checksum, sha256sum_file(node_master_wallet))
node_master.loadwallet('')
@ -208,8 +210,7 @@ class UpgradeWalletTest(BitcoinTestFramework):
orig_kvs = dump_bdb_kv(node_master_wallet)
assert b'\x07hdchain' not in orig_kvs
# Upgrade to HD, no split
wallet.upgradewallet(130000)
assert_equal(130000, wallet.getwalletinfo()['walletversion'])
self.test_upgradewallet(wallet, previous_version=60000, requested_version=130000)
# Check that there is now a hd chain and it is version 1, no internal chain counter
new_kvs = dump_bdb_kv(node_master_wallet)
assert b'\x07hdchain' in new_kvs
@ -244,8 +245,7 @@ class UpgradeWalletTest(BitcoinTestFramework):
assert_equal(130000, wallet.getwalletinfo()['walletversion'])
self.log.info('Upgrade HD to HD chain split')
wallet.upgradewallet(169900)
assert_equal(169900, wallet.getwalletinfo()['walletversion'])
self.test_upgradewallet(wallet, previous_version=130000, requested_version=169900)
# Check that the hdchain updated correctly
new_kvs = dump_bdb_kv(node_master_wallet)
hd_chain = new_kvs[b'\x07hdchain']
@ -271,8 +271,7 @@ class UpgradeWalletTest(BitcoinTestFramework):
self.log.info('Upgrade non-HD to HD chain split')
copy_non_hd()
wallet.upgradewallet(169900)
assert_equal(169900, wallet.getwalletinfo()['walletversion'])
self.test_upgradewallet(wallet, previous_version=60000, requested_version=169900)
# Check that the hdchain updated correctly
new_kvs = dump_bdb_kv(node_master_wallet)
hd_chain = new_kvs[b'\x07hdchain']
@ -333,15 +332,15 @@ class UpgradeWalletTest(BitcoinTestFramework):
# Check the wallet has a default key initially
old_kvs = dump_bdb_kv(node_master_wallet)
defaultkey = old_kvs[b'\x0adefaultkey']
# Upgrade the wallet. Should still have the same default key
wallet.upgradewallet(159900)
self.log.info("Upgrade the wallet. Should still have the same default key.")
self.test_upgradewallet(wallet, previous_version=139900, requested_version=159900)
new_kvs = dump_bdb_kv(node_master_wallet)
up_defaultkey = new_kvs[b'\x0adefaultkey']
assert_equal(defaultkey, up_defaultkey)
assert_equal(wallet.getwalletinfo()["walletversion"], 159900)
# 0.16.3 doesn't have a default key
v16_3_kvs = dump_bdb_kv(v16_3_wallet)
assert b'\x0adefaultkey' not in v16_3_kvs
if __name__ == '__main__':
UpgradeWalletTest().main()