Merge #20490: [backport] wallet: upgradewallet fixes, improvements, test coverage
ca8cd893bb
wallet: fix and improve upgradewallet error responses (Jon Atack)99d56e3571
wallet: fix and improve upgradewallet result responses (Jon Atack)2498b04ce8
Don't upgrade to HD split if it is already supported (Andrew Chow)c46c18b788
wallet: refactor GetClosestWalletFeature() (Jon Atack) Pull request description: Github-Pull: #20403 Rebased-From:c46c18b788
Github-Pull: #20403 Rebased-From:2498b04ce8
Github-Pull: #20403 Rebased-From:99d56e3571
Github-Pull: #20403 Rebased-From:ca8cd893bb
Top commit has no ACKs. Tree-SHA512: b18a1d015c963298740c585385eaa056988464112c88a519fe619be22dc78a8f6a102365cf799c50b781a77a09bec82b58ce411ab007b48f8b5de876e9c75060
This commit is contained in:
commit
9facca3ce0
4 changed files with 74 additions and 50 deletions
|
@ -4239,7 +4239,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);
|
||||
|
@ -4470,14 +4470,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)"}
|
||||
},
|
||||
},
|
||||
|
@ -4500,11 +4504,27 @@ static RPCHelpMan upgradewallet()
|
|||
version = request.params[0].get_int();
|
||||
}
|
||||
bilingual_str error;
|
||||
if (!pwallet->UpgradeWallet(version, error)) {
|
||||
throw JSONRPCError(RPC_WALLET_ERROR, error.original);
|
||||
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) {
|
||||
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);
|
||||
if (!error.empty()) {
|
||||
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);
|
||||
} else {
|
||||
CHECK_NONFATAL(!error.empty());
|
||||
obj.pushKV("error", error.original);
|
||||
}
|
||||
return obj;
|
||||
|
|
|
@ -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;
|
||||
|
|
|
@ -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<WalletFeature, 8> 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<WalletFeature>(0);
|
||||
}
|
||||
|
|
|
@ -22,9 +22,7 @@ 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 +90,32 @@ 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 test_upgradewallet_error(self, wallet, previous_version, requested_version, msg):
|
||||
assert_equal(wallet.getwalletinfo()["walletversion"], previous_version)
|
||||
assert_equal(wallet.upgradewallet(requested_version),
|
||||
{
|
||||
"wallet_name": "",
|
||||
"previous_version": previous_version,
|
||||
"current_version": previous_version,
|
||||
"error": msg,
|
||||
}
|
||||
)
|
||||
assert_equal(wallet.getwalletinfo()["walletversion"], previous_version)
|
||||
|
||||
def run_test(self):
|
||||
self.nodes[0].generatetoaddress(101, self.nodes[0].getnewaddress())
|
||||
self.dumb_sync_blocks()
|
||||
|
@ -158,14 +182,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,32 +191,27 @@ 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('')
|
||||
|
||||
self.log.info('Wallets cannot be downgraded')
|
||||
copy_non_hd()
|
||||
assert_raises_rpc_error(-4, 'Cannot downgrade wallet', wallet.upgradewallet, 40000)
|
||||
self.test_upgradewallet_error(wallet, previous_version=60000, requested_version=40000, msg="Cannot downgrade wallet")
|
||||
wallet.unloadwallet()
|
||||
assert_equal(before_checksum, sha256sum_file(node_master_wallet))
|
||||
node_master.loadwallet('')
|
||||
|
@ -208,8 +221,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
|
||||
|
@ -236,16 +248,12 @@ class UpgradeWalletTest(BitcoinTestFramework):
|
|||
assert_equal('m/0\'/0\'/1\'', info['hdkeypath'])
|
||||
|
||||
self.log.info('Cannot upgrade to HD Split, needs Pre Split Keypool')
|
||||
assert_raises_rpc_error(-4, 'Cannot upgrade a non HD split wallet without upgrading to support pre split keypool', wallet.upgradewallet, 139900)
|
||||
assert_equal(130000, wallet.getwalletinfo()['walletversion'])
|
||||
assert_raises_rpc_error(-4, 'Cannot upgrade a non HD split wallet without upgrading to support pre split keypool', wallet.upgradewallet, 159900)
|
||||
assert_equal(130000, wallet.getwalletinfo()['walletversion'])
|
||||
assert_raises_rpc_error(-4, 'Cannot upgrade a non HD split wallet without upgrading to support pre split keypool', wallet.upgradewallet, 169899)
|
||||
assert_equal(130000, wallet.getwalletinfo()['walletversion'])
|
||||
for version in [139900, 159900, 169899]:
|
||||
self.test_upgradewallet_error(wallet, previous_version=130000, requested_version=version,
|
||||
msg="Cannot upgrade a non HD split wallet without upgrading to support pre split keypool. Please use version 169900 or no version specified.")
|
||||
|
||||
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 +279,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,8 +340,8 @@ 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)
|
||||
|
@ -342,5 +349,6 @@ class UpgradeWalletTest(BitcoinTestFramework):
|
|||
v16_3_kvs = dump_bdb_kv(v16_3_wallet)
|
||||
assert b'\x0adefaultkey' not in v16_3_kvs
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
UpgradeWalletTest().main()
|
||||
|
|
Loading…
Reference in a new issue