Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#22461: wallet: Change ScriptPubKeyMan::Upgrade …
Browse files Browse the repository at this point in the history
…default to True

5012a79 Test that descriptor wallet upgrade does nothing (Andrew Chow)
48bd7d3 Change ScriptPubKeyMan::Upgrade to default to return true (Andrew Chow)

Pull request description:

  When adding a new ScriptPubKeyMan, it's likely that there will be nothing for `Upgrade` to do. If it is called (via `upgradewallet`), then it should do nothing, successfully. This PR changes the default `ScriptPubKeyMan::Upgrade` function so that it returns a success instead of failure when doing nothing.

  Fixes #22460

ACKs for top commit:
  jonatack:
    ACK 5012a79
  meshcollider:
    utACK 5012a79

Tree-SHA512: 578c6521e997f7bb5cc44be2cfe9e0a760b6bd4aa301026a6b8b3282e8757473e4cb9f68b2e79dacdc2b42dddae718450072e0a38817df205dfea177a74d7f3d
  • Loading branch information
meshcollider committed Jul 18, 2021
2 parents 4371e63 + 5012a79 commit 5341c3b
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/wallet/scriptpubkeyman.h
Expand Up @@ -207,7 +207,7 @@ class ScriptPubKeyMan
virtual bool CanGetAddresses(bool internal = false) const { return false; }

/** Upgrades the wallet to the specified version */
virtual bool Upgrade(int prev_version, int new_version, bilingual_str& error) { return false; }
virtual bool Upgrade(int prev_version, int new_version, bilingual_str& error) { return true; }

virtual bool HavePrivateKeys() const { return false; }

Expand Down
10 changes: 8 additions & 2 deletions test/functional/wallet_upgradewallet.py
Expand Up @@ -94,10 +94,11 @@ def dumb_sync_blocks(self):
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)
old_wallet_info = wallet.getwalletinfo()
assert_equal(old_wallet_info["walletversion"], previous_version)
assert_equal(wallet.upgradewallet(requested_version),
{
"wallet_name": "",
"wallet_name": old_wallet_info["walletname"],
"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),
Expand Down Expand Up @@ -352,6 +353,11 @@ def copy_split_hd():
v16_3_kvs = dump_bdb_kv(v16_3_wallet)
assert b'\x0adefaultkey' not in v16_3_kvs

if self.is_sqlite_compiled():
self.log.info("Checking that descriptor wallets do nothing, successfully")
self.nodes[0].createwallet(wallet_name="desc_upgrade", descriptors=True)
desc_wallet = self.nodes[0].get_wallet_rpc("desc_upgrade")
self.test_upgradewallet(desc_wallet, previous_version=169900, expected_version=169900)

if __name__ == '__main__':
UpgradeWalletTest().main()

0 comments on commit 5341c3b

Please sign in to comment.