laantungir
/
bitcoin
mirror of https://github.com/bitcoin/bitcoin.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#33031: wallet: Set descriptor cache upgraded flag for migrated wallets 

88b0647f02 wallet: Always write last hardened cache flag in migrated wallets (Ava Chow)
8a08eef645 tests: Check that the last hardened cache upgrade occurs (Ava Chow)

Pull request description:

  #32597 set the descriptor cache upgraded flag for newly created wallets, but migrated wallets still did not have the flag set when they are migrated. For consistency, and to avoid an unnecessary upgrade, we should be setting this flag for migrated wallets.

  The flag would end up being set anyways at the end of migration when the wallet is reloaded as it would perform the automatic upgrade at that time. However, this is unnecessary and we should just set it from the get go.

  This PR also adds a couple tests to verify that the flag is being set, and that the upgrade is being performed.

ACKs for top commit:
  cedwies:
    re-ACK 88b0647
  rkrux:
    lgtm ACK 88b0647f02
  pablomartin4btc:
    ACK 88b0647f02

Tree-SHA512: 7d0850db0ae38eedd1e6a3bfaa548c6c612182291059fb1a47279a4c4984ee7914ecd02d8c7e427ef67bf9f5e67cbc57a7ae4412fad539e1bf3e05c512a60d69
This commit is contained in:
merge-script 2025-09-23 15:27:17 -04:00
parent 350692e561 88b0647f02
commit dd61f08fd5
No known key found for this signature in database
GPG Key ID: BA03F4DBE0C63FB4
4 changed files with 67 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/wallet/wallet.cpp
View File

@ -3884,7 +3884,7 @@ util::Result<void> CWallet::ApplyMigrationData(WalletBatch& local_wallet_batch,
m_internal_spk_managers.clear();
// Setup new descriptors (only if we are migrating any key material)
SetWalletFlagWithDB(local_wallet_batch, WALLET_FLAG_DESCRIPTORS);
SetWalletFlagWithDB(local_wallet_batch, WALLET_FLAG_DESCRIPTORS | WALLET_FLAG_LAST_HARDENED_XPUB_CACHED);
if (has_spendable_material && !IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
// Use the existing master key if we have it
if (data.master_key.key.IsValid()) {

12
test/functional/test_framework/test_framework.py
View File

@ -1110,3 +1110,15 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
if self.options.usecli:
return json.dumps(text)
return text
def inspect_sqlite_db(self, path, fn, *args, **kwargs):
try:
import sqlite3 # type: ignore[import]
conn = sqlite3.connect(path)
with conn:
result = fn(conn, *args, **kwargs)
conn.close()
return result
except ImportError:
self.log.warning("sqlite3 module not available, skipping tests that inspect the database")

43
test/functional/wallet_backwards_compatibility.py
View File

@ -21,9 +21,11 @@ import shutil
from test_framework.blocktools import COINBASE_MATURITY
from test_framework.test_framework import BitcoinTestFramework
from test_framework.descriptors import descsum_create
from test_framework.messages import ser_string
from test_framework.util import (
assert_equal,
assert_greater_than,
assert_raises_rpc_error,
)
@ -149,18 +151,13 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
assert_equal(bad_deriv_wallet_master.getaddressinfo(bad_path_addr)["hdkeypath"], good_deriv_path)
bad_deriv_wallet_master.unloadwallet()
# If we have sqlite3, verify that there are no keymeta records
try:
import sqlite3
wallet_db = node_master.wallets_path / wallet_name / "wallet.dat"
conn = sqlite3.connect(wallet_db)
with conn:
# Retrieve all records that have the "keymeta" prefix. The remaining key data varies for each record.
keymeta_rec = conn.execute("SELECT value FROM main where key >= x'076b65796d657461' AND key < x'076b65796d657462'").fetchone()
assert_equal(keymeta_rec, None)
conn.close()
except ImportError:
self.log.warning("sqlite3 module not available, skipping lack of keymeta records check")
def check_keymeta(conn):
# Retrieve all records that have the "keymeta" prefix. The remaining key data varies for each record.
keymeta_rec = conn.execute(f"SELECT value FROM main where key >= x'{ser_string(b'keymeta').hex()}' AND key < x'{ser_string(b'keymetb').hex()}'").fetchone()
assert_equal(keymeta_rec, None)
wallet_db = node_master.wallets_path / wallet_name / "wallet.dat"
self.inspect_sqlite_db(wallet_db, check_keymeta)
def test_ignore_legacy_during_startup(self, legacy_nodes, node_master):
self.log.info("Test that legacy wallets are ignored during startup on v29+")
@ -342,6 +339,13 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
# Remove the wallet from old node
wallet_prev.unloadwallet()
# Open backup with sqlite and get flags
def get_flags(conn):
flags_rec = conn.execute(f"SELECT value FROM main WHERE key = x'{ser_string(b'flags').hex()}'").fetchone()
return int.from_bytes(flags_rec[0], byteorder="little")
old_flags = self.inspect_sqlite_db(backup_path, get_flags)
# Restore the wallet to master
load_res = node_master.restorewallet(wallet_name, backup_path)
@ -378,6 +382,21 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
wallet.unloadwallet()
# Open the wallet with sqlite and inspect the flags and records
def check_upgraded_records(conn, old_flags):
flags_rec = conn.execute(f"SELECT value FROM main WHERE key = x'{ser_string(b'flags').hex()}'").fetchone()
new_flags = int.from_bytes(flags_rec[0], byteorder="little")
diff_flags = new_flags & ~old_flags
# Check for last hardened xpubs if the flag is newly set
if diff_flags & (1 << 2):
self.log.debug("Checking descriptor cache was upgraded")
# Fetch all records with the walletdescriptorlhcache prefix
lh_cache_recs = conn.execute(f"SELECT value FROM main where key >= x'{ser_string(b'walletdescriptorlhcache').hex()}' AND key < x'{ser_string(b'walletdescriptorlhcachf').hex()}'").fetchall()
assert_greater_than(len(lh_cache_recs), 0)
self.inspect_sqlite_db(down_backup_path, check_upgraded_records, old_flags)
# Check that no automatic upgrade broke downgrading the wallet
target_dir = node.wallets_path / down_wallet_name
os.makedirs(target_dir, exist_ok=True)

25
test/functional/wallet_migration.py
View File

@ -18,11 +18,12 @@ from test_framework.address import (
from test_framework.descriptors import descsum_create
from test_framework.key import ECPubKey
from test_framework.test_framework import BitcoinTestFramework
from test_framework.messages import COIN, CTransaction, CTxOut
from test_framework.messages import COIN, CTransaction, CTxOut, ser_string
from test_framework.script import hash160
from test_framework.script_util import key_to_p2pkh_script, key_to_p2pk_script, script_to_p2sh_script, script_to_p2wsh_script
from test_framework.util import (
assert_equal,
assert_greater_than,
assert_raises_rpc_error,
find_vout_for_address,
sha256sum_file,
@ -140,7 +141,8 @@ class WalletMigrationTest(BitcoinTestFramework):
# (in which case the wallet name would be suffixed by the 'watchonly' term)
migrated_wallet_name = migrate_info['wallet_name']
wallet = self.master_node.get_wallet_rpc(migrated_wallet_name)
assert_equal(wallet.getwalletinfo()["descriptors"], True)
wallet_info = wallet.getwalletinfo()
assert_equal(wallet_info["descriptors"], True)
self.assert_is_sqlite(migrated_wallet_name)
# Always verify the backup path exist after migration
assert os.path.exists(migrate_info['backup_path'])
@ -154,6 +156,25 @@ class WalletMigrationTest(BitcoinTestFramework):
assert_equal(str(expected_backup_path), migrate_info['backup_path'])
assert {"name": backup_filename} not in self.master_node.listwalletdir()["wallets"]
# Open the wallet with sqlite and verify that the wallet has the last hardened cache flag
# set and the last hardened cache entries
def check_last_hardened(conn):
flags_rec = conn.execute(f"SELECT value FROM main WHERE key = x'{ser_string(b'flags').hex()}'").fetchone()
flags = int.from_bytes(flags_rec[0], byteorder="little")
# All wallets should have the upgrade flag set
assert_equal(bool(flags & (1 << 2)), True)
# Fetch all records with the walletdescriptorlhcache prefix
# if the wallet has private keys and is not blank
if wallet_info["private_keys_enabled"] and not wallet_info["blank"]:
lh_cache_recs = conn.execute(f"SELECT value FROM main where key >= x'{ser_string(b'walletdescriptorlhcache').hex()}' AND key < x'{ser_string(b'walletdescriptorlhcachf').hex()}'").fetchall()
assert_greater_than(len(lh_cache_recs), 0)
inspect_path = os.path.join(self.options.tmpdir, os.path.basename(f"{migrated_wallet_name}_inspect.dat"))
wallet.backupwallet(inspect_path)
self.inspect_sqlite_db(inspect_path, check_last_hardened)
return migrate_info, wallet
def test_basic(self):