mirrors/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-03-13 11:35:20 +01:00
Code Issues Projects Releases Wiki Activity

Merge 78cb0df4d5 into a50af6e4c4

Browse source
This commit is contained in:
Ava Chow 2025-03-13 02:07:18 +01:00 • committed by GitHub
commit 10af33bc28
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 134 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

79
src/wallet/walletdb.cpp
View file

@ -542,7 +542,7 @@ static LoadResult LoadRecords(CWallet* pwallet, DatabaseBatch& batch, const std:
return LoadRecords(pwallet, batch, key, prefix, load_func);
}
static DBErrors LoadLegacyWalletRecords(CWallet* pwallet, DatabaseBatch& batch, int last_client) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)
static DBErrors LoadLegacyWalletRecords(CWallet* pwallet, WalletBatch& batch, int last_client) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)
{
AssertLockHeld(pwallet->cs_wallet);
DBErrors result = DBErrors::LOAD_OK;
@ -555,7 +555,7 @@ static DBErrors LoadLegacyWalletRecords(CWallet* pwallet, DatabaseBatch& batch,
DataStream prefix;
prefix << type;
std::unique_ptr<DatabaseCursor> cursor = batch.GetNewPrefixCursor(prefix);
std::unique_ptr<DatabaseCursor> cursor = batch.m_batch->GetNewPrefixCursor(prefix);
if (!cursor) {
pwallet->WalletLogPrintf("Error getting database cursor for '%s' records\n", type);
return DBErrors::CORRUPT;
@ -573,28 +573,28 @@ static DBErrors LoadLegacyWalletRecords(CWallet* pwallet, DatabaseBatch& batch,
// Load HD Chain
// Note: There should only be one HDCHAIN record with no data following the type
LoadResult hd_chain_res = LoadRecords(pwallet, batch, DBKeys::HDCHAIN,
LoadResult hd_chain_res = LoadRecords(pwallet, *batch.m_batch, DBKeys::HDCHAIN,
[] (CWallet* pwallet, DataStream& key, DataStream& value, std::string& err) {
return LoadHDChain(pwallet, value, err) ? DBErrors:: LOAD_OK : DBErrors::CORRUPT;
});
result = std::max(result, hd_chain_res.m_result);
// Load unencrypted keys
LoadResult key_res = LoadRecords(pwallet, batch, DBKeys::KEY,
LoadResult key_res = LoadRecords(pwallet, *batch.m_batch, DBKeys::KEY,
[] (CWallet* pwallet, DataStream& key, DataStream& value, std::string& err) {
return LoadKey(pwallet, key, value, err) ? DBErrors::LOAD_OK : DBErrors::CORRUPT;
});
result = std::max(result, key_res.m_result);
// Load encrypted keys
LoadResult ckey_res = LoadRecords(pwallet, batch, DBKeys::CRYPTED_KEY,
LoadResult ckey_res = LoadRecords(pwallet, *batch.m_batch, DBKeys::CRYPTED_KEY,
[] (CWallet* pwallet, DataStream& key, DataStream& value, std::string& err) {
return LoadCryptedKey(pwallet, key, value, err) ? DBErrors::LOAD_OK : DBErrors::CORRUPT;
});
result = std::max(result, ckey_res.m_result);
// Load scripts
LoadResult script_res = LoadRecords(pwallet, batch, DBKeys::CSCRIPT,
LoadResult script_res = LoadRecords(pwallet, *batch.m_batch, DBKeys::CSCRIPT,
[] (CWallet* pwallet, DataStream& key, DataStream& value, std::string& strErr) {
uint160 hash;
key >> hash;
@ -617,13 +617,13 @@ static DBErrors LoadLegacyWalletRecords(CWallet* pwallet, DatabaseBatch& batch,
// Load keymeta
std::map<uint160, CHDChain> hd_chains;
LoadResult keymeta_res = LoadRecords(pwallet, batch, DBKeys::KEYMETA,
[&hd_chains] (CWallet* pwallet, DataStream& key, DataStream& value, std::string& strErr) {
std::vector<CPubKey> updated_metas;
LoadResult keymeta_res = LoadRecords(pwallet, *batch.m_batch, DBKeys::KEYMETA,
[&hd_chains, &updated_metas] (CWallet* pwallet, DataStream& key, DataStream& value, std::string& strErr) {
CPubKey vchPubKey;
key >> vchPubKey;
CKeyMetadata keyMeta;
value >> keyMeta;
pwallet->GetOrCreateLegacyDataSPKM()->LoadKeyMetadata(vchPubKey.GetID(), keyMeta);
// Extract some CHDChain info from this metadata if it has any
if (keyMeta.nVersion >= CKeyMetadata::VERSION_WITH_HDDATA && !keyMeta.hd_seed_id.IsNull() && keyMeta.hdKeypath.size() > 0) {
@ -634,13 +634,32 @@ static DBErrors LoadLegacyWalletRecords(CWallet* pwallet, DatabaseBatch& batch,
uint32_t index = 0;
if (keyMeta.hdKeypath != "s" && keyMeta.hdKeypath != "m") {
std::vector<uint32_t> path;
if (keyMeta.has_key_origin) {
// We have a key origin, so pull it from its path vector
path = keyMeta.key_origin.path;
} else {
// No key origin, have to parse the string
if (!ParseHDKeypath(keyMeta.hdKeypath, path)) {
strErr = "Error reading wallet database: keymeta with invalid HD keypath";
// Always parse the path string
if (!ParseHDKeypath(keyMeta.hdKeypath, path)) {
strErr = "Error reading wallet database: keymeta with invalid HD keypath";
return DBErrors::NONCRITICAL_ERROR;
}
// If there is a key origin, make sure that path matches
if (keyMeta.has_key_origin && path != keyMeta.key_origin.path) {
// Detect if this metadata has a bad derivation path from a bug in inactive hd key derivation that was present in 0.21 and 22.x
// See https://github.com/bitcoin/bitcoin/issues/29109
// Markers for bad derivation:
// - 6 indexes
// - path[5] == path[2] + 1
// - path[0:2] == path[3:5]
// - path[3:6] matches hdKeypath
if (keyMeta.key_origin.path.size() == 6
&& keyMeta.key_origin.path[5] == keyMeta.key_origin.path[2] + 1
&& keyMeta.key_origin.path[0] == keyMeta.key_origin.path[3]
&& keyMeta.key_origin.path[1] == keyMeta.key_origin.path[4]
&& std::equal(keyMeta.key_origin.path.begin() + 3, keyMeta.key_origin.path.end(), path.begin(), path.end())
) {
// Matches the pattern, just reset it to the parsed path
pwallet->WalletLogPrintf("Repairing derivation path for %s\n", HexStr(vchPubKey));
keyMeta.key_origin.path = path;
updated_metas.push_back(vchPubKey);
} else {
strErr = "keymeta with mismatched hdkeypath string and key origin derivation path";
return DBErrors::NONCRITICAL_ERROR;
}
}
@ -684,10 +703,26 @@ static DBErrors LoadLegacyWalletRecords(CWallet* pwallet, DatabaseBatch& batch,
chain.nExternalChainCounter = std::max(chain.nExternalChainCounter, index + 1);
}
}
pwallet->GetOrCreateLegacyDataSPKM()->LoadKeyMetadata(vchPubKey.GetID(), keyMeta);
return DBErrors::LOAD_OK;
});
result = std::max(result, keymeta_res.m_result);
// Update any changed metadata
if (!updated_metas.empty()) {
LegacyDataSPKM* spkm = pwallet->GetLegacyDataSPKM();
Assert(spkm);
LOCK(spkm->cs_KeyStore);
for (const auto& pubkey : updated_metas) {
if (!batch.WriteKeyMetadata(spkm->mapKeyMetadata.at(pubkey.GetID()), pubkey, true)) {
pwallet->WalletLogPrintf("Unable to write repaired keymeta for %s\n", HexStr(pubkey));
result = std::max(result, DBErrors::NONCRITICAL_ERROR);
}
}
}
// Set inactive chains
if (!hd_chains.empty()) {
LegacyDataSPKM* legacy_spkm = pwallet->GetLegacyDataSPKM();
@ -704,7 +739,7 @@ static DBErrors LoadLegacyWalletRecords(CWallet* pwallet, DatabaseBatch& batch,
}
// Load watchonly scripts
LoadResult watch_script_res = LoadRecords(pwallet, batch, DBKeys::WATCHS,
LoadResult watch_script_res = LoadRecords(pwallet, *batch.m_batch, DBKeys::WATCHS,
[] (CWallet* pwallet, DataStream& key, DataStream& value, std::string& err) {
CScript script;
key >> script;
@ -718,7 +753,7 @@ static DBErrors LoadLegacyWalletRecords(CWallet* pwallet, DatabaseBatch& batch,
result = std::max(result, watch_script_res.m_result);
// Load watchonly meta
LoadResult watch_meta_res = LoadRecords(pwallet, batch, DBKeys::WATCHMETA,
LoadResult watch_meta_res = LoadRecords(pwallet, *batch.m_batch, DBKeys::WATCHMETA,
[] (CWallet* pwallet, DataStream& key, DataStream& value, std::string& err) {
CScript script;
key >> script;
@ -730,7 +765,7 @@ static DBErrors LoadLegacyWalletRecords(CWallet* pwallet, DatabaseBatch& batch,
result = std::max(result, watch_meta_res.m_result);
// Load keypool
LoadResult pool_res = LoadRecords(pwallet, batch, DBKeys::POOL,
LoadResult pool_res = LoadRecords(pwallet, *batch.m_batch, DBKeys::POOL,
[] (CWallet* pwallet, DataStream& key, DataStream& value, std::string& err) {
int64_t nIndex;
key >> nIndex;
@ -747,7 +782,7 @@ static DBErrors LoadLegacyWalletRecords(CWallet* pwallet, DatabaseBatch& batch,
// We don't want or need the default key, but if there is one set,
// we want to make sure that it is valid so that we can detect corruption
// Note: There should only be one DEFAULTKEY with nothing trailing the type
LoadResult default_key_res = LoadRecords(pwallet, batch, DBKeys::DEFAULTKEY,
LoadResult default_key_res = LoadRecords(pwallet, *batch.m_batch, DBKeys::DEFAULTKEY,
[] (CWallet* pwallet, DataStream& key, DataStream& value, std::string& err) {
CPubKey default_pubkey;
try {
@ -765,7 +800,7 @@ static DBErrors LoadLegacyWalletRecords(CWallet* pwallet, DatabaseBatch& batch,
result = std::max(result, default_key_res.m_result);
// "wkey" records are unsupported, if we see any, throw an error
LoadResult wkey_res = LoadRecords(pwallet, batch, DBKeys::OLD_KEY,
LoadResult wkey_res = LoadRecords(pwallet, *batch.m_batch, DBKeys::OLD_KEY,
[] (CWallet* pwallet, DataStream& key, DataStream& value, std::string& err) {
err = "Found unsupported 'wkey' record, try loading with version 0.18";
return DBErrors::LOAD_FAIL;
@ -1198,7 +1233,7 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
#endif
// Load legacy wallet keys
result = std::max(LoadLegacyWalletRecords(pwallet, *m_batch, last_client), result);
result = std::max(LoadLegacyWalletRecords(pwallet, *this, last_client), result);
// Load descriptors
result = std::max(LoadDescriptorWalletRecords(pwallet, *m_batch, last_client), result);

2
src/wallet/walletdb.h
View file

@ -303,8 +303,8 @@ public:
//! Registers db txn callback functions
void RegisterTxnListener(const DbTxnListener& l);
private:
std::unique_ptr<DatabaseBatch> m_batch;
private:
WalletDatabase& m_database;
// External functions listening to the current db txn outcome.

77
test/functional/wallet_backwards_compatibility.py
View file

@ -41,7 +41,7 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # v25.0
["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # v24.0.1
["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # v23.0
["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # v22.0
["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=noban@127.0.0.1", "-keypool=10"], # v22.0
["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # v0.21.0
["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # v0.20.1
["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # v0.19.1
@ -128,6 +128,79 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
wallet = node_v19.get_wallet_rpc("w1_v19")
assert wallet.getaddressinfo(address_18075)["solvable"]
def test_v22_inactivehdchain_path(self):
LAST_KEYPOOL_INDEX = 9 # Index of the last derived address with the keypool size of 10
self.log.info("Testing inactive hd chain bad derivation path cleanup")
# 0.21.x and 22.x would both produce bad derivation paths when topping up an inactive hd chain
# Make sure that this is being detected and automatically cleaned up
node_master = self.nodes[1]
node_v22 = self.nodes[self.num_nodes - 6]
wallet_name = "bad_deriv_path"
node_v22.createwallet(wallet_name=wallet_name, descriptors=False)
wallet = node_v22.get_wallet_rpc(wallet_name)
# Make a dump of the wallet to get an unused address
dump_path = node_v22.wallets_path / f"{wallet_name}.dump"
wallet.dumpwallet(dump_path)
addr = None
seed = None
with open(dump_path, encoding="utf8") as f:
for line in f:
if f"hdkeypath=m/0'/0'/{LAST_KEYPOOL_INDEX}'" in line:
addr = line.split(" ")[4].split("=")[1]
elif " hdseed=1 " in line:
seed = line.split(" ")[0]
assert addr is not None
assert seed is not None
# Rotate seed and unload
wallet.sethdseed()
wallet.unloadwallet()
# Receive at addr to trigger inactive chain topup on next load
self.nodes[0].sendtoaddress(addr, 1)
self.generate(self.nodes[0], 1, sync_fun=self.no_op)
self.sync_all(nodes=[self.nodes[0], node_master, node_v22])
node_v22.loadwallet(wallet_name)
# Dump again to find bad hd keypath
bad_deriv_path = f"m/0'/0'/{LAST_KEYPOOL_INDEX}'/0'/0'/{LAST_KEYPOOL_INDEX + 1}'"
good_deriv_path = f"m/0h/0h/{LAST_KEYPOOL_INDEX + 1}h"
os.unlink(dump_path)
wallet.dumpwallet(dump_path)
bad_path_addr = None
with open(dump_path, encoding="utf8") as f:
for line in f:
if f"hdkeypath={bad_deriv_path}" in line:
bad_path_addr = line.split(" ")[4].split("=")[1]
assert bad_path_addr is not None
assert_equal(wallet.getaddressinfo(bad_path_addr)["hdkeypath"], bad_deriv_path)
# Verify that this addr is actually at m/0'/0'/10' by making a new wallet with the same seed
node_v22.createwallet(wallet_name="path_verify", descriptors=False, blank=True)
verify_wallet = node_v22.get_wallet_rpc("path_verify")
verify_wallet.sethdseed(True, seed)
# Bad addr is after keypool, so need to generate it by refilling
verify_wallet.keypoolrefill(LAST_KEYPOOL_INDEX + 2)
assert_equal(verify_wallet.getaddressinfo(bad_path_addr)["hdkeypath"], good_deriv_path.replace("h", "'"))
# Load into master, the path should now be fixed
backup_path = node_v22.wallets_path / f"{wallet_name}.bak"
wallet.backupwallet(backup_path)
wallet_dir_master = os.path.join(node_master.wallets_path, wallet_name)
os.makedirs(wallet_dir_master, exist_ok=True)
shutil.copy(backup_path, os.path.join(wallet_dir_master, "wallet.dat"))
with node_master.assert_debug_log(expected_msgs=["Repairing derivation path for"]):
node_master.migratewallet(wallet_name)
wallet_master = node_master.get_wallet_rpc(wallet_name)
assert_equal(wallet_master.getaddressinfo(bad_path_addr)["hdkeypath"], good_deriv_path)
# Check persistence
wallet_master.unloadwallet()
with node_master.assert_debug_log(expected_msgs=[], unexpected_msgs=["Repairing derivation path for"]):
node_master.loadwallet(wallet_name)
assert_equal(wallet_master.getaddressinfo(bad_path_addr)["hdkeypath"], good_deriv_path)
def run_test(self):
node_miner = self.nodes[0]
node_master = self.nodes[1]
@ -385,5 +458,7 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
info = wallet_res.getaddressinfo(address)
assert_equal(info, addr_info)
self.test_v22_inactivehdchain_path()
if __name__ == '__main__':
BackwardsCompatibilityTest(__file__).main()