diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 3351cb3a9f..cfbd995174 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -37,7 +37,7 @@ public: virtual bool IsWalletFlagSet(uint64_t) const = 0; virtual void UnsetBlankWalletFlag(WalletBatch&) = 0; virtual bool CanSupportFeature(enum WalletFeature) const = 0; - virtual void SetMinVersion(enum WalletFeature, WalletBatch* = nullptr, bool = false) = 0; + virtual void SetMinVersion(enum WalletFeature, WalletBatch* = nullptr) = 0; virtual const CKeyingMaterial& GetEncryptionKey() const = 0; virtual bool HasEncryptionKeys() const = 0; virtual bool IsLocked(bool fForMixing = false) const = 0; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 0e38d6e2a8..248d5b5c3d 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -442,21 +442,13 @@ void CWallet::chainStateFlushed(const CBlockLocator& loc) batch.WriteBestBlock(loc); } -void CWallet::SetMinVersion(enum WalletFeature nVersion, WalletBatch* batch_in, bool fExplicit) +void CWallet::SetMinVersion(enum WalletFeature nVersion, WalletBatch* batch_in) { LOCK(cs_wallet); if (nWalletVersion >= nVersion) return; - - // when doing an explicit upgrade, if we pass the max version permitted, upgrade all the way - if (fExplicit && nVersion > nWalletMaxVersion) - nVersion = FEATURE_LATEST; - nWalletVersion = nVersion; - if (nVersion > nWalletMaxVersion) - nWalletMaxVersion = nVersion; - { WalletBatch* batch = batch_in ? batch_in : new WalletBatch(GetDatabase()); if (nWalletVersion > 40000) @@ -466,18 +458,6 @@ void CWallet::SetMinVersion(enum WalletFeature nVersion, WalletBatch* batch_in, } } -bool CWallet::SetMaxVersion(int nVersion) -{ - LOCK(cs_wallet); - // cannot downgrade below current version - if (nWalletVersion > nVersion) - return false; - - nWalletMaxVersion = nVersion; - - return true; -} - std::set CWallet::GetConflicts(const uint256& txid) const { std::set result; @@ -665,7 +645,7 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) // Encryption was introduced in version 0.4.0 - SetMinVersion(FEATURE_WALLETCRYPT, encrypted_batch, true); + SetMinVersion(FEATURE_WALLETCRYPT, encrypted_batch); if (!encrypted_batch->TxnCommit()) { delete encrypted_batch; @@ -4583,7 +4563,7 @@ std::shared_ptr CWallet::Create(interfaces::Chain& chain, interfaces::C if (fFirstRun) { - walletInstance->SetMaxVersion(FEATURE_LATEST); + walletInstance->SetMinVersion(FEATURE_LATEST); walletInstance->AddWalletFlags(wallet_creation_flags); @@ -4918,7 +4898,7 @@ bool CWallet::UpgradeWallet(int version, bilingual_str& error) return false; } - SetMaxVersion(nMaxVersion); + SetMinVersion(GetClosestWalletFeature(version)); return true; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 8889cf364a..7cf53b9ccb 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -706,9 +706,6 @@ private: //! the current wallet version: clients below this version are not able to load the wallet int nWalletVersion GUARDED_BY(cs_wallet){FEATURE_BASE}; - //! the maximum wallet format version: memory-only variable that specifies to what version this wallet may be upgraded - int nWalletMaxVersion GUARDED_BY(cs_wallet) = FEATURE_BASE; - int64_t nNextResend = 0; bool fBroadcastTransactions = false; // Local time that the tip block was received. Used to schedule wallet rebroadcasts. @@ -911,8 +908,8 @@ public: const CWalletTx* GetWalletTx(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool IsTrusted(const CWalletTx& wtx, std::set& trusted_parents) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - //! check whether we are allowed to upgrade (or already support) to the named feature - bool CanSupportFeature(enum WalletFeature wf) const override EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { AssertLockHeld(cs_wallet); return nWalletMaxVersion >= wf; } + //! check whether we support the named feature + bool CanSupportFeature(enum WalletFeature wf) const override EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { AssertLockHeld(cs_wallet); return IsFeatureSupported(nWalletVersion, wf); } /** * populate vCoins with vector of available COutputs. @@ -981,7 +978,7 @@ public: //! Upgrade stored CKeyMetadata objects to store key origin info as KeyOriginInfo void UpgradeKeyMetadata() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - bool LoadMinVersion(int nVersion) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { AssertLockHeld(cs_wallet); nWalletVersion = nVersion; nWalletMaxVersion = std::max(nWalletMaxVersion, nVersion); return true; } + bool LoadMinVersion(int nVersion) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { AssertLockHeld(cs_wallet); nWalletVersion = nVersion; return true; } //! Adds a destination data tuple to the store, without saving it to disk void LoadDestData(const CTxDestination& dest, const std::string& key, const std::string& value) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); @@ -1198,11 +1195,8 @@ public: unsigned int GetKeyPoolSize() const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - //! signify that a particular wallet feature is now used. this may change nWalletVersion and nWalletMaxVersion if those are lower - void SetMinVersion(enum WalletFeature, WalletBatch* batch_in = nullptr, bool fExplicit = false) override; - - //! change which version we're allowed to upgrade to (note that this does not immediately imply upgrading to that format) - bool SetMaxVersion(int nVersion); + //! signify that a particular wallet feature is now used. + void SetMinVersion(enum WalletFeature, WalletBatch* batch_in = nullptr) override; //! get the current wallet format (the oldest client version guaranteed to understand this wallet) int GetVersion() const { LOCK(cs_wallet); return nWalletVersion; } diff --git a/src/wallet/walletutil.cpp b/src/wallet/walletutil.cpp index 2e98940e3b..ba529e9d06 100644 --- a/src/wallet/walletutil.cpp +++ b/src/wallet/walletutil.cpp @@ -28,3 +28,18 @@ fs::path GetWalletDir() return path; } + +bool IsFeatureSupported(int wallet_version, int feature_version) +{ + return wallet_version >= feature_version; +} + +WalletFeature GetClosestWalletFeature(int version) +{ + if (version >= FEATURE_LATEST) return FEATURE_LATEST; + 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; + return static_cast(0); +} diff --git a/src/wallet/walletutil.h b/src/wallet/walletutil.h index 93547ce0d9..212714cc20 100644 --- a/src/wallet/walletutil.h +++ b/src/wallet/walletutil.h @@ -23,6 +23,8 @@ enum WalletFeature FEATURE_LATEST = FEATURE_HD }; +bool IsFeatureSupported(int wallet_version, int feature_version); +WalletFeature GetClosestWalletFeature(int version); enum WalletFlags : uint64_t { // wallet flags in the upper section (> 1 << 31) will lead to not opening the wallet if flag is unknown diff --git a/test/functional/data/wallets/high_minversion/.walletlock b/test/functional/data/wallets/high_minversion/.walletlock deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/test/functional/data/wallets/high_minversion/GENERATE.md b/test/functional/data/wallets/high_minversion/GENERATE.md deleted file mode 100644 index e55c4557ca..0000000000 --- a/test/functional/data/wallets/high_minversion/GENERATE.md +++ /dev/null @@ -1,8 +0,0 @@ -The wallet has been created by starting Bitcoin Core with the options -`-regtest -datadir=/tmp -nowallet -walletdir=$(pwd)/test/functional/data/wallets/`. - -In the source code, `WalletFeature::FEATURE_LATEST` has been modified to be large, so that the minversion is too high -for a current build of the wallet. - -The wallet has then been created with the RPC `createwallet high_minversion true true`, so that a blank wallet with -private keys disabled is created. diff --git a/test/functional/data/wallets/high_minversion/db.log b/test/functional/data/wallets/high_minversion/db.log deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/test/functional/data/wallets/high_minversion/wallet.dat b/test/functional/data/wallets/high_minversion/wallet.dat deleted file mode 100644 index 99ab809263..0000000000 Binary files a/test/functional/data/wallets/high_minversion/wallet.dat and /dev/null differ diff --git a/test/functional/test_framework/bdb.py b/test/functional/test_framework/bdb.py new file mode 100644 index 0000000000..9de358aa0a --- /dev/null +++ b/test/functional/test_framework/bdb.py @@ -0,0 +1,152 @@ +#!/usr/bin/env python3 +# Copyright (c) 2020 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +""" +Utilities for working directly with the wallet's BDB database file + +This is specific to the configuration of BDB used in this project: + - pagesize: 4096 bytes + - Outer database contains single subdatabase named 'main' + - btree + - btree leaf pages + +Each key-value pair is two entries in a btree leaf. The first is the key, the one that follows +is the value. And so on. Note that the entry data is itself not in the correct order. Instead +entry offsets are stored in the correct order and those offsets are needed to then retrieve +the data itself. + +Page format can be found in BDB source code dbinc/db_page.h +This only implements the deserialization of btree metadata pages and normal btree pages. Overflow +pages are not implemented but may be needed in the future if dealing with wallets with large +transactions. + +`db_dump -da wallet.dat` is useful to see the data in a wallet.dat BDB file +""" + +import binascii +import struct + +# Important constants +PAGESIZE = 4096 +OUTER_META_PAGE = 0 +INNER_META_PAGE = 2 + +# Page type values +BTREE_INTERNAL = 3 +BTREE_LEAF = 5 +BTREE_META = 9 + +# Some magic numbers for sanity checking +BTREE_MAGIC = 0x053162 +DB_VERSION = 9 + +# Deserializes a leaf page into a dict. +# Btree internal pages have the same header, for those, return None. +# For the btree leaf pages, deserialize them and put all the data into a dict +def dump_leaf_page(data): + page_info = {} + page_header = data[0:26] + _, pgno, prev_pgno, next_pgno, entries, hf_offset, level, pg_type = struct.unpack('QIIIHHBB', page_header) + page_info['pgno'] = pgno + page_info['prev_pgno'] = prev_pgno + page_info['next_pgno'] = next_pgno + page_info['entries'] = entries + page_info['hf_offset'] = hf_offset + page_info['level'] = level + page_info['pg_type'] = pg_type + page_info['entry_offsets'] = struct.unpack('{}H'.format(entries), data[26:26 + entries * 2]) + page_info['entries'] = [] + + if pg_type == BTREE_INTERNAL: + # Skip internal pages. These are the internal nodes of the btree and don't contain anything relevant to us + return None + + assert pg_type == BTREE_LEAF, 'A non-btree leaf page has been encountered while dumping leaves' + + for i in range(0, entries): + offset = page_info['entry_offsets'][i] + entry = {'offset': offset} + page_data_header = data[offset:offset + 3] + e_len, pg_type = struct.unpack('HB', page_data_header) + entry['len'] = e_len + entry['pg_type'] = pg_type + entry['data'] = data[offset + 3:offset + 3 + e_len] + page_info['entries'].append(entry) + + return page_info + +# Deserializes a btree metadata page into a dict. +# Does a simple sanity check on the magic value, type, and version +def dump_meta_page(page): + # metadata page + # general metadata + metadata = {} + meta_page = page[0:72] + _, pgno, magic, version, pagesize, encrypt_alg, pg_type, metaflags, _, free, last_pgno, nparts, key_count, record_count, flags, uid = struct.unpack('QIIIIBBBBIIIIII20s', meta_page) + metadata['pgno'] = pgno + metadata['magic'] = magic + metadata['version'] = version + metadata['pagesize'] = pagesize + metadata['encrypt_alg'] = encrypt_alg + metadata['pg_type'] = pg_type + metadata['metaflags'] = metaflags + metadata['free'] = free + metadata['last_pgno'] = last_pgno + metadata['nparts'] = nparts + metadata['key_count'] = key_count + metadata['record_count'] = record_count + metadata['flags'] = flags + metadata['uid'] = binascii.hexlify(uid) + + assert magic == BTREE_MAGIC, 'bdb magic does not match bdb btree magic' + assert pg_type == BTREE_META, 'Metadata page is not a btree metadata page' + assert version == DB_VERSION, 'Database too new' + + # btree metadata + btree_meta_page = page[72:512] + _, minkey, re_len, re_pad, root, _, crypto_magic, _, iv, chksum = struct.unpack('IIIII368sI12s16s20s', btree_meta_page) + metadata['minkey'] = minkey + metadata['re_len'] = re_len + metadata['re_pad'] = re_pad + metadata['root'] = root + metadata['crypto_magic'] = crypto_magic + metadata['iv'] = binascii.hexlify(iv) + metadata['chksum'] = binascii.hexlify(chksum) + return metadata + +# Given the dict from dump_leaf_page, get the key-value pairs and put them into a dict +def extract_kv_pairs(page_data): + out = {} + last_key = None + for i, entry in enumerate(page_data['entries']): + # By virtue of these all being pairs, even number entries are keys, and odd are values + if i % 2 == 0: + out[entry['data']] = b'' + last_key = entry['data'] + else: + out[last_key] = entry['data'] + return out + +# Extract the key-value pairs of the BDB file given in filename +def dump_bdb_kv(filename): + # Read in the BDB file and start deserializing it + pages = [] + with open(filename, 'rb') as f: + data = f.read(PAGESIZE) + while len(data) > 0: + pages.append(data) + data = f.read(PAGESIZE) + + # Sanity check the meta pages + dump_meta_page(pages[OUTER_META_PAGE]) + dump_meta_page(pages[INNER_META_PAGE]) + + # Fetch the kv pairs from the leaf pages + kv = {} + for i in range(3, len(pages)): + info = dump_leaf_page(pages[i]) + if info is not None: + info_kv = extract_kv_pairs(info) + kv = {**kv, **info_kv} + return kv diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index fd636ec136..0736669811 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -9,6 +9,7 @@ from base64 import b64encode from binascii import unhexlify from decimal import Decimal, ROUND_DOWN from subprocess import CalledProcessError +import hashlib import inspect import json import logging @@ -268,6 +269,14 @@ def wait_until_helper(predicate, *, attempts=float('inf'), timeout=float('inf'), else: return False +def sha256sum_file(filename): + h = hashlib.sha256() + with open(filename, 'rb') as f: + d = f.read(4096) + while len(d) > 0: + h.update(d) + d = f.read(4096) + return h.digest() # RPC/P2P connection constants and functions ############################################ diff --git a/test/functional/wallet_upgradewallet.py b/test/functional/wallet_upgradewallet.py index 4d81201c01..3f33d27110 100755 --- a/test/functional/wallet_upgradewallet.py +++ b/test/functional/wallet_upgradewallet.py @@ -10,7 +10,9 @@ Only v0.15.2 and v0.16.3 are required by this test. The others are used in featu import os import shutil +import struct +from test_framework.bdb import dump_bdb_kv from test_framework.blocktools import COINBASE_MATURITY from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( @@ -18,17 +20,21 @@ from test_framework.util import ( assert_greater_than, assert_greater_than_or_equal, assert_is_hex_string, + assert_raises_rpc_error, + sha256sum_file, ) +UPGRADED_KEYMETA_VERSION = 12 + class UpgradeWalletTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 3 self.extra_args = [ - [], # current wallet version - ["-usehd=1"], # v18.2.2 wallet - ["-usehd=0"] # v0.16.1.1 wallet + ["-keypool=2"], # current wallet version + ["-usehd=1", "-keypool=2"], # v18.2.2 wallet + ["-usehd=0", "-keypool=2"], # v0.16.1.1 wallet ] self.wallet_names = [self.default_wallet_name, None, None] @@ -84,12 +90,12 @@ class UpgradeWalletTest(BitcoinTestFramework): self.log.info("Test upgradewallet RPC...") # Prepare for copying of the older wallet - node_master_wallet_dir = os.path.join(node_master.datadir, "regtest/wallets") + node_master_wallet_dir = os.path.join(node_master.datadir, "regtest/wallets", self.default_wallet_name) + node_master_wallet = os.path.join(node_master_wallet_dir, self.default_wallet_name, self.wallet_data_filename) v18_2_wallet = os.path.join(v18_2_node.datadir, "regtest/wallets/wallet.dat") v16_1_wallet = os.path.join(v16_1_node.datadir, "regtest/wallets/wallet.dat") self.stop_nodes() - # Copy the 0.16.3 wallet to the last Dash Core version and open it: shutil.rmtree(node_master_wallet_dir) os.mkdir(node_master_wallet_dir) shutil.copy( @@ -99,7 +105,32 @@ class UpgradeWalletTest(BitcoinTestFramework): self.restart_node(0, ['-nowallet']) node_master.loadwallet('') - wallet = node_master.get_wallet_rpc('') + def copy_v16(): + node_master.get_wallet_rpc(self.default_wallet_name).unloadwallet() + # Copy the 0.16.3 wallet to the last Dash Core version and open it: + shutil.rmtree(node_master_wallet_dir) + os.mkdir(node_master_wallet_dir) + shutil.copy( + v18_2_wallet, + node_master_wallet_dir + ) + node_master.loadwallet(self.default_wallet_name) + + def copy_non_hd(): + node_master.get_wallet_rpc(self.default_wallet_name).unloadwallet() + # Copy the 19.3.0 wallet to the last Dash Core version and open it: + shutil.rmtree(node_master_wallet_dir) + os.mkdir(node_master_wallet_dir) + shutil.copy( + v16_1_wallet, + node_master_wallet_dir + ) + node_master.loadwallet(self.default_wallet_name) + + + 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 @@ -111,27 +142,17 @@ class UpgradeWalletTest(BitcoinTestFramework): # wallet should still contain the same balance assert_equal(wallet.getbalance(), v18_2_balance) - self.stop_node(0) - # Copy the 19.3.0 wallet to the last Dash Core version and open it: - shutil.rmtree(node_master_wallet_dir) - os.mkdir(node_master_wallet_dir) - shutil.copy( - v16_1_wallet, - node_master_wallet_dir - ) - self.restart_node(0, ['-nowallet']) - node_master.loadwallet('') - - wallet = node_master.get_wallet_rpc('') + copy_non_hd() + 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) + assert_equal('hdchainid' 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 would not have 120200 version until HD seed actually appeared - assert_greater_than(120200, new_version) + # upgraded wallet would have 120200 but no HD seed actually appeared + assert_equal(120200, new_version) # after conversion master key hash should not be present yet assert 'hdchainid' not in wallet.getwalletinfo() assert_equal(wallet.upgradetohd(), True) @@ -139,5 +160,66 @@ class UpgradeWalletTest(BitcoinTestFramework): assert_equal(new_version, 120200) assert_is_hex_string(wallet.getwalletinfo()['hdchainid']) + self.log.info('Intermediary versions don\'t effect anything') + copy_non_hd() + # Wallet starts with 61000 (legacy "latest") + assert_equal(61000, wallet.getwalletinfo()['walletversion']) + wallet.unloadwallet() + before_checksum = sha256sum_file(node_master_wallet) + node_master.loadwallet('') + # Can "upgrade" to 120199 which should have no effect on the wallet + wallet.upgradewallet(120199) + assert_equal(61000, wallet.getwalletinfo()['walletversion']) + 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) + wallet.unloadwallet() + assert_equal(before_checksum, sha256sum_file(node_master_wallet)) + node_master.loadwallet('') + + self.log.info('Can upgrade to HD') + # Inspect the old wallet and make sure there is no hdchain + orig_kvs = dump_bdb_kv(node_master_wallet) + assert b'\x07hdchain' not in orig_kvs + # Upgrade to HD + wallet.upgradewallet(120200) + assert_equal(120200, wallet.getwalletinfo()['walletversion']) + # 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) + wallet.upgradetohd() + new_kvs = dump_bdb_kv(node_master_wallet) + assert b'\x07hdchain' in new_kvs + hd_chain = new_kvs[b'\x07hdchain'] + # hd_chain: + # obj.nVersion int + # obj.id uint256 + # obj.fCrypted bool + # obj.vchSeed SecureVector + # obj.vchMnemonic SecureVector + # obj.vchMnemonicPassphrase SecureVector + # obj.mapAccounts map<> accounts + assert_greater_than(220, len(hd_chain)) + assert_greater_than(len(hd_chain), 180) + hd_chain_version, seed_id, is_crypted = struct.unpack('