From 01b35b55a119dc7ac915fc621ecebcd5c50ccb55 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 12 Apr 2022 11:35:22 -0400 Subject: [PATCH 01/16] walletdb: Refactor minversion loading Move minversion loading to its own function in WalletBatch --- src/wallet/walletdb.cpp | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 34fe8ab17f..96ded3d876 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -791,6 +791,18 @@ bool WalletBatch::IsKeyType(const std::string& strType) strType == DBKeys::MASTER_KEY || strType == DBKeys::CRYPTED_KEY); } +static DBErrors LoadMinVersion(CWallet* pwallet, DatabaseBatch& batch) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) +{ + AssertLockHeld(pwallet->cs_wallet); + int nMinVersion = 0; + if (batch.Read(DBKeys::MINVERSION, nMinVersion)) { + if (nMinVersion > FEATURE_LATEST) + return DBErrors::TOO_NEW; + pwallet->LoadMinVersion(nMinVersion); + } + return DBErrors::LOAD_OK; +} + DBErrors WalletBatch::LoadWallet(CWallet* pwallet) { CWalletScanState wss; @@ -806,12 +818,7 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) pwallet->WalletLogPrintf("Wallet file version = %d, last client version = %d\n", pwallet->GetVersion(), last_client); try { - int nMinVersion = 0; - if (m_batch->Read(DBKeys::MINVERSION, nMinVersion)) { - if (nMinVersion > FEATURE_LATEST) - return DBErrors::TOO_NEW; - pwallet->LoadMinVersion(nMinVersion); - } + if ((result = LoadMinVersion(pwallet, *m_batch)) != DBErrors::LOAD_OK) return result; // Load wallet flags, so they are known when processing other records. // The FLAGS key is absent during wallet creation. From 52932c5adb29bb9ec5f0bcde9a31b74113a20651 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 12 Apr 2022 11:39:46 -0400 Subject: [PATCH 02/16] walletdb: Refactor wallet flags loading Move wallet flags loading to its own function in WalletBatch The return value is changed to be TOO_NEW rather than CORRUPT when unknown flags are found. --- src/wallet/walletdb.cpp | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 96ded3d876..05de63fd3e 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -803,6 +803,19 @@ static DBErrors LoadMinVersion(CWallet* pwallet, DatabaseBatch& batch) EXCLUSIVE return DBErrors::LOAD_OK; } +static DBErrors LoadWalletFlags(CWallet* pwallet, DatabaseBatch& batch) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) +{ + AssertLockHeld(pwallet->cs_wallet); + uint64_t flags; + if (batch.Read(DBKeys::FLAGS, flags)) { + if (!pwallet->LoadWalletFlags(flags)) { + pwallet->WalletLogPrintf("Error reading wallet database: Unknown non-tolerable wallet flags found\n"); + return DBErrors::TOO_NEW; + } + } + return DBErrors::LOAD_OK; +} + DBErrors WalletBatch::LoadWallet(CWallet* pwallet) { CWalletScanState wss; @@ -822,13 +835,7 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) // Load wallet flags, so they are known when processing other records. // The FLAGS key is absent during wallet creation. - uint64_t flags; - if (m_batch->Read(DBKeys::FLAGS, flags)) { - if (!pwallet->LoadWalletFlags(flags)) { - pwallet->WalletLogPrintf("Error reading wallet database: Unknown non-tolerable wallet flags found\n"); - return DBErrors::CORRUPT; - } - } + if ((result = LoadWalletFlags(pwallet, *m_batch)) != DBErrors::LOAD_OK) return result; #ifndef ENABLE_EXTERNAL_SIGNER if (pwallet->IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER)) { @@ -873,9 +880,6 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) // we assume the user can live with: if (IsKeyType(strType) || strType == DBKeys::DEFAULTKEY) { result = DBErrors::CORRUPT; - } else if (strType == DBKeys::FLAGS) { - // reading the wallet flags can only fail if unknown flags are present - result = DBErrors::TOO_NEW; } else if (wss.tx_corrupt) { pwallet->WalletLogPrintf("Error: Corrupt transaction found. This can be fixed by removing transactions from wallet and rescanning.\n"); // Set tx_corrupt back to false so that the error is only printed once (per corrupt tx) From 7be10adff36c0dc49ae56ac571bb033cba7a565b Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 13 Apr 2022 14:42:00 -0400 Subject: [PATCH 03/16] walletdb: Refactor key reading and loading to its own function --- src/wallet/walletdb.cpp | 120 ++++++++++++++++++++++------------------ src/wallet/walletdb.h | 2 + 2 files changed, 69 insertions(+), 53 deletions(-) diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 05de63fd3e..3958211da8 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -320,6 +320,72 @@ public: CWalletScanState() = default; }; +bool LoadKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, std::string& strErr) +{ + LOCK(pwallet->cs_wallet); + try { + CPubKey vchPubKey; + ssKey >> vchPubKey; + if (!vchPubKey.IsValid()) + { + strErr = "Error reading wallet database: CPubKey corrupt"; + return false; + } + CKey key; + CPrivKey pkey; + uint256 hash; + + ssValue >> pkey; + + // Old wallets store keys as DBKeys::KEY [pubkey] => [privkey] + // ... which was slow for wallets with lots of keys, because the public key is re-derived from the private key + // using EC operations as a checksum. + // Newer wallets store keys as DBKeys::KEY [pubkey] => [privkey][hash(pubkey,privkey)], which is much faster while + // remaining backwards-compatible. + try + { + ssValue >> hash; + } + catch (const std::ios_base::failure&) {} + + bool fSkipCheck = false; + + if (!hash.IsNull()) + { + // hash pubkey/privkey to accelerate wallet load + std::vector vchKey; + vchKey.reserve(vchPubKey.size() + pkey.size()); + vchKey.insert(vchKey.end(), vchPubKey.begin(), vchPubKey.end()); + vchKey.insert(vchKey.end(), pkey.begin(), pkey.end()); + + if (Hash(vchKey) != hash) + { + strErr = "Error reading wallet database: CPubKey/CPrivKey corrupt"; + return false; + } + + fSkipCheck = true; + } + + if (!key.Load(pkey, vchPubKey, fSkipCheck)) + { + strErr = "Error reading wallet database: CPrivKey corrupt"; + return false; + } + if (!pwallet->GetOrCreateLegacyScriptPubKeyMan()->LoadKey(key, vchPubKey)) + { + strErr = "Error reading wallet database: LegacyScriptPubKeyMan::LoadKey failed"; + return false; + } + } catch (const std::exception& e) { + if (strErr.empty()) { + strErr = e.what(); + } + return false; + } + return true; +} + static bool ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, CWalletScanState &wss, std::string& strType, std::string& strErr, const KeyFilterFn& filter_fn = nullptr) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) @@ -410,60 +476,8 @@ ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, pwallet->GetOrCreateLegacyScriptPubKeyMan()->LoadWatchOnly(script); } } else if (strType == DBKeys::KEY) { - CPubKey vchPubKey; - ssKey >> vchPubKey; - if (!vchPubKey.IsValid()) - { - strErr = "Error reading wallet database: CPubKey corrupt"; - return false; - } - CKey key; - CPrivKey pkey; - uint256 hash; - wss.nKeys++; - ssValue >> pkey; - - // Old wallets store keys as DBKeys::KEY [pubkey] => [privkey] - // ... which was slow for wallets with lots of keys, because the public key is re-derived from the private key - // using EC operations as a checksum. - // Newer wallets store keys as DBKeys::KEY [pubkey] => [privkey][hash(pubkey,privkey)], which is much faster while - // remaining backwards-compatible. - try - { - ssValue >> hash; - } - catch (const std::ios_base::failure&) {} - - bool fSkipCheck = false; - - if (!hash.IsNull()) - { - // hash pubkey/privkey to accelerate wallet load - std::vector vchKey; - vchKey.reserve(vchPubKey.size() + pkey.size()); - vchKey.insert(vchKey.end(), vchPubKey.begin(), vchPubKey.end()); - vchKey.insert(vchKey.end(), pkey.begin(), pkey.end()); - - if (Hash(vchKey) != hash) - { - strErr = "Error reading wallet database: CPubKey/CPrivKey corrupt"; - return false; - } - - fSkipCheck = true; - } - - if (!key.Load(pkey, vchPubKey, fSkipCheck)) - { - strErr = "Error reading wallet database: CPrivKey corrupt"; - return false; - } - if (!pwallet->GetOrCreateLegacyScriptPubKeyMan()->LoadKey(key, vchPubKey)) - { - strErr = "Error reading wallet database: LegacyScriptPubKeyMan::LoadKey failed"; - return false; - } + if (!LoadKey(pwallet, ssKey, ssValue, strErr)) return false; } else if (strType == DBKeys::MASTER_KEY) { // Master encryption key is loaded into only the wallet and not any of the ScriptPubKeyMans. unsigned int nID; diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index f84a89b23f..4228d428fa 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -305,6 +305,8 @@ using KeyFilterFn = std::function; //! Unserialize a given Key-Value pair and load it into the wallet bool ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, std::string& strType, std::string& strErr, const KeyFilterFn& filter_fn = nullptr); + +bool LoadKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, std::string& strErr); } // namespace wallet #endif // BITCOIN_WALLET_WALLETDB_H From 3ccde4599b5150577400c4fa9029f4146617f751 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 13 Apr 2022 15:34:23 -0400 Subject: [PATCH 04/16] walletdb: Refactor crypted key loading to its own function --- src/wallet/walletdb.cpp | 67 ++++++++++++++++++++++++----------------- src/wallet/walletdb.h | 1 + 2 files changed, 41 insertions(+), 27 deletions(-) diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 3958211da8..95df773bca 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -386,6 +386,45 @@ bool LoadKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, std::stri return true; } +bool LoadCryptedKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, std::string& strErr) +{ + LOCK(pwallet->cs_wallet); + try { + CPubKey vchPubKey; + ssKey >> vchPubKey; + if (!vchPubKey.IsValid()) + { + strErr = "Error reading wallet database: CPubKey corrupt"; + return false; + } + std::vector vchPrivKey; + ssValue >> vchPrivKey; + + // Get the checksum and check it + bool checksum_valid = false; + if (!ssValue.eof()) { + uint256 checksum; + ssValue >> checksum; + if (!(checksum_valid = Hash(vchPrivKey) == checksum)) { + strErr = "Error reading wallet database: Encrypted key corrupt"; + return false; + } + } + + if (!pwallet->GetOrCreateLegacyScriptPubKeyMan()->LoadCryptedKey(vchPubKey, vchPrivKey, checksum_valid)) + { + strErr = "Error reading wallet database: LegacyScriptPubKeyMan::LoadCryptedKey failed"; + return false; + } + } catch (const std::exception& e) { + if (strErr.empty()) { + strErr = e.what(); + } + return false; + } + return true; +} + static bool ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, CWalletScanState &wss, std::string& strType, std::string& strErr, const KeyFilterFn& filter_fn = nullptr) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) @@ -493,34 +532,8 @@ ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, if (pwallet->nMasterKeyMaxID < nID) pwallet->nMasterKeyMaxID = nID; } else if (strType == DBKeys::CRYPTED_KEY) { - CPubKey vchPubKey; - ssKey >> vchPubKey; - if (!vchPubKey.IsValid()) - { - strErr = "Error reading wallet database: CPubKey corrupt"; - return false; - } - std::vector vchPrivKey; - ssValue >> vchPrivKey; - - // Get the checksum and check it - bool checksum_valid = false; - if (!ssValue.eof()) { - uint256 checksum; - ssValue >> checksum; - if (!(checksum_valid = Hash(vchPrivKey) == checksum)) { - strErr = "Error reading wallet database: Encrypted key corrupt"; - return false; - } - } - wss.nCKeys++; - - if (!pwallet->GetOrCreateLegacyScriptPubKeyMan()->LoadCryptedKey(vchPubKey, vchPrivKey, checksum_valid)) - { - strErr = "Error reading wallet database: LegacyScriptPubKeyMan::LoadCryptedKey failed"; - return false; - } + if (!LoadCryptedKey(pwallet, ssKey, ssValue, strErr)) return false; wss.fIsEncrypted = true; } else if (strType == DBKeys::KEYMETA) { CPubKey vchPubKey; diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 4228d428fa..86b92e5f35 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -307,6 +307,7 @@ using KeyFilterFn = std::function; bool ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, std::string& strType, std::string& strErr, const KeyFilterFn& filter_fn = nullptr); bool LoadKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, std::string& strErr); +bool LoadCryptedKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, std::string& strErr); } // namespace wallet #endif // BITCOIN_WALLET_WALLETDB_H From 72c2a54ebb99fa3d91d7d15bd8a38a8d16e0ea6c Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 13 Apr 2022 16:07:59 -0400 Subject: [PATCH 05/16] walletdb: Refactor encryption key loading to its own function --- src/wallet/walletdb.cpp | 41 ++++++++++++++++++++++++++++------------- src/wallet/walletdb.h | 1 + 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 95df773bca..6af7501924 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -425,6 +425,33 @@ bool LoadCryptedKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, st return true; } +bool LoadEncryptionKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, std::string& strErr) +{ + LOCK(pwallet->cs_wallet); + try { + // Master encryption key is loaded into only the wallet and not any of the ScriptPubKeyMans. + unsigned int nID; + ssKey >> nID; + CMasterKey kMasterKey; + ssValue >> kMasterKey; + if(pwallet->mapMasterKeys.count(nID) != 0) + { + strErr = strprintf("Error reading wallet database: duplicate CMasterKey id %u", nID); + return false; + } + pwallet->mapMasterKeys[nID] = kMasterKey; + if (pwallet->nMasterKeyMaxID < nID) + pwallet->nMasterKeyMaxID = nID; + + } catch (const std::exception& e) { + if (strErr.empty()) { + strErr = e.what(); + } + return false; + } + return true; +} + static bool ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, CWalletScanState &wss, std::string& strType, std::string& strErr, const KeyFilterFn& filter_fn = nullptr) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) @@ -518,19 +545,7 @@ ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, wss.nKeys++; if (!LoadKey(pwallet, ssKey, ssValue, strErr)) return false; } else if (strType == DBKeys::MASTER_KEY) { - // Master encryption key is loaded into only the wallet and not any of the ScriptPubKeyMans. - unsigned int nID; - ssKey >> nID; - CMasterKey kMasterKey; - ssValue >> kMasterKey; - if(pwallet->mapMasterKeys.count(nID) != 0) - { - strErr = strprintf("Error reading wallet database: duplicate CMasterKey id %u", nID); - return false; - } - pwallet->mapMasterKeys[nID] = kMasterKey; - if (pwallet->nMasterKeyMaxID < nID) - pwallet->nMasterKeyMaxID = nID; + if (!LoadEncryptionKey(pwallet, ssKey, ssValue, strErr)) return false; } else if (strType == DBKeys::CRYPTED_KEY) { wss.nCKeys++; if (!LoadCryptedKey(pwallet, ssKey, ssValue, strErr)) return false; diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 86b92e5f35..698f05304a 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -308,6 +308,7 @@ bool ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, std bool LoadKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, std::string& strErr); bool LoadCryptedKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, std::string& strErr); +bool LoadEncryptionKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, std::string& strErr); } // namespace wallet #endif // BITCOIN_WALLET_WALLETDB_H From ad779e9ece9829677c1735d8865f14b23459da80 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 13 Apr 2022 16:37:29 -0400 Subject: [PATCH 06/16] walletdb: Refactor hd chain loading to its own function --- src/wallet/walletdb.cpp | 20 +++++++++++++++++--- src/wallet/walletdb.h | 1 + 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 6af7501924..0099741258 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -452,6 +452,22 @@ bool LoadEncryptionKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, return true; } +bool LoadHDChain(CWallet* pwallet, DataStream& ssValue, std::string& strErr) +{ + LOCK(pwallet->cs_wallet); + try { + CHDChain chain; + ssValue >> chain; + pwallet->GetOrCreateLegacyScriptPubKeyMan()->LoadHDChain(chain); + } catch (const std::exception& e) { + if (strErr.empty()) { + strErr = e.what(); + } + return false; + } + return true; +} + static bool ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, CWalletScanState &wss, std::string& strType, std::string& strErr, const KeyFilterFn& filter_fn = nullptr) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) @@ -672,9 +688,7 @@ ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, pwallet->LoadAddressReceiveRequest(dest, strKey.substr(2), strValue); } } else if (strType == DBKeys::HDCHAIN) { - CHDChain chain; - ssValue >> chain; - pwallet->GetOrCreateLegacyScriptPubKeyMan()->LoadHDChain(chain); + if (!LoadHDChain(pwallet, ssValue, strErr)) return false; } else if (strType == DBKeys::OLD_KEY) { strErr = "Found unsupported 'wkey' record, try loading with version 0.18"; return false; diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 698f05304a..7d33baad25 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -309,6 +309,7 @@ bool ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, std bool LoadKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, std::string& strErr); bool LoadCryptedKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, std::string& strErr); bool LoadEncryptionKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, std::string& strErr); +bool LoadHDChain(CWallet* pwallet, DataStream& ssValue, std::string& strErr); } // namespace wallet #endif // BITCOIN_WALLET_WALLETDB_H From 9e077d9b422ac3c371fe0f63da40e5092171a25e Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 13 Apr 2022 16:38:44 -0400 Subject: [PATCH 07/16] salvage: Remove use of ReadKeyValue in salvage To prepare to remove ReadKeyValue, change salvage to not use it --- src/wallet/salvage.cpp | 28 +++++++++++++++------------- src/wallet/walletdb.cpp | 24 +++++------------------- src/wallet/walletdb.h | 8 -------- 3 files changed, 20 insertions(+), 40 deletions(-) diff --git a/src/wallet/salvage.cpp b/src/wallet/salvage.cpp index e303310273..da16435f04 100644 --- a/src/wallet/salvage.cpp +++ b/src/wallet/salvage.cpp @@ -18,11 +18,6 @@ static const char *HEADER_END = "HEADER=END"; static const char *DATA_END = "DATA=END"; typedef std::pair, std::vector > KeyValPair; -static bool KeyFilter(const std::string& type) -{ - return WalletBatch::IsKeyType(type) || type == DBKeys::HDCHAIN; -} - class DummyCursor : public DatabaseCursor { Status Next(DataStream& key, DataStream& value) override { return Status::FAIL; } @@ -186,17 +181,24 @@ bool RecoverDatabaseFile(const ArgsManager& args, const fs::path& file_path, bil { /* Filter for only private key type KV pairs to be added to the salvaged wallet */ DataStream ssKey{row.first}; - CDataStream ssValue(row.second, SER_DISK, CLIENT_VERSION); + DataStream ssValue(row.second); std::string strType, strErr; - bool fReadOK; - { - // Required in LoadKeyMetadata(): - LOCK(dummyWallet.cs_wallet); - fReadOK = ReadKeyValue(&dummyWallet, ssKey, ssValue, strType, strErr, KeyFilter); - } - if (!KeyFilter(strType)) { + + // We only care about KEY, MASTER_KEY, CRYPTED_KEY, and HDCHAIN types + ssKey >> strType; + bool fReadOK = false; + if (strType == DBKeys::KEY) { + fReadOK = LoadKey(&dummyWallet, ssKey, ssValue, strErr); + } else if (strType == DBKeys::CRYPTED_KEY) { + fReadOK = LoadCryptedKey(&dummyWallet, ssKey, ssValue, strErr); + } else if (strType == DBKeys::MASTER_KEY) { + fReadOK = LoadEncryptionKey(&dummyWallet, ssKey, ssValue, strErr); + } else if (strType == DBKeys::HDCHAIN) { + fReadOK = LoadHDChain(&dummyWallet, ssValue, strErr); + } else { continue; } + if (!fReadOK) { warnings.push_back(strprintf(Untranslated("WARNING: WalletBatch::Recover skipping %s: %s"), strType, strErr)); diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 0099741258..4cf3be436b 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -470,17 +470,13 @@ bool LoadHDChain(CWallet* pwallet, DataStream& ssValue, std::string& strErr) static bool ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, - CWalletScanState &wss, std::string& strType, std::string& strErr, const KeyFilterFn& filter_fn = nullptr) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) + CWalletScanState &wss, std::string& strType, std::string& strErr) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) { try { // Unserialize // Taking advantage of the fact that pair serialization // is just the two items serialized one after the other ssKey >> strType; - // If we have a filter, check if this matches the filter - if (filter_fn && !filter_fn(strType)) { - return true; - } // Legacy entries in descriptor wallets are not allowed, abort immediately if (pwallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS) && DBKeys::LEGACY_TYPES.count(strType) > 0) { wss.unexpected_legacy_entry = true; @@ -834,19 +830,6 @@ ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, return true; } -bool ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, std::string& strType, std::string& strErr, const KeyFilterFn& filter_fn) -{ - CWalletScanState dummy_wss; - LOCK(pwallet->cs_wallet); - return ReadKeyValue(pwallet, ssKey, ssValue, dummy_wss, strType, strErr, filter_fn); -} - -bool WalletBatch::IsKeyType(const std::string& strType) -{ - return (strType == DBKeys::KEY || - strType == DBKeys::MASTER_KEY || strType == DBKeys::CRYPTED_KEY); -} - static DBErrors LoadMinVersion(CWallet* pwallet, DatabaseBatch& batch) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) { AssertLockHeld(pwallet->cs_wallet); @@ -934,7 +917,10 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) } // losing keys is considered a catastrophic error, anything else // we assume the user can live with: - if (IsKeyType(strType) || strType == DBKeys::DEFAULTKEY) { + if (strType == DBKeys::KEY || + strType == DBKeys::MASTER_KEY || + strType == DBKeys::CRYPTED_KEY || + strType == DBKeys::DEFAULTKEY) { result = DBErrors::CORRUPT; } else if (wss.tx_corrupt) { pwallet->WalletLogPrintf("Error: Corrupt transaction found. This can be fixed by removing transactions from wallet and rescanning.\n"); diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 7d33baad25..9be1dcec48 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -276,8 +276,6 @@ public: DBErrors LoadWallet(CWallet* pwallet); DBErrors FindWalletTxHashes(std::vector& tx_hashes); DBErrors ZapSelectTx(std::vector& vHashIn, std::vector& vHashOut); - /* Function to determine if a certain KV/key-type is a key (cryptographical key) type */ - static bool IsKeyType(const std::string& strType); //! write the hdchain model (external chain child index counter) bool WriteHDChain(const CHDChain& chain); @@ -300,12 +298,6 @@ private: //! Compacts BDB state so that wallet.dat is self-contained (if there are changes) void MaybeCompactWalletDB(WalletContext& context); -//! Callback for filtering key types to deserialize in ReadKeyValue -using KeyFilterFn = std::function; - -//! Unserialize a given Key-Value pair and load it into the wallet -bool ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, std::string& strType, std::string& strErr, const KeyFilterFn& filter_fn = nullptr); - bool LoadKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, std::string& strErr); bool LoadCryptedKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, std::string& strErr); bool LoadEncryptionKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, std::string& strErr); From 30ab11c49793d5d55d66c4dedfa576ae8fd6129c Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 12 Apr 2022 14:27:39 -0400 Subject: [PATCH 08/16] walletdb: Refactor legacy wallet record loading into its own function Instead of loading legacy wallet records as we come across them when iterating the database, load them explicitly. Exception handling for these records changes to a per-record type basis, rather than globally. This results in some records now failing with a critical error rather than a non-critical one. --- src/wallet/walletdb.cpp | 410 ++++++++++++++++++++++++++-------------- src/wallet/walletdb.h | 26 +-- 2 files changed, 283 insertions(+), 153 deletions(-) diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 4cf3be436b..cf79025801 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -301,10 +301,8 @@ class CWalletScanState { public: unsigned int nKeys{0}; unsigned int nCKeys{0}; - unsigned int nWatchKeys{0}; unsigned int nKeyMeta{0}; unsigned int m_unknown_records{0}; - bool fIsEncrypted{false}; bool fAnyUnordered{false}; std::vector vWalletUpgrade; std::map m_active_external_spks; @@ -312,10 +310,8 @@ public: std::map m_descriptor_caches; std::map, CKey> m_descriptor_keys; std::map, std::pair>> m_descriptor_crypt_keys; - std::map m_hd_chains; bool tx_corrupt{false}; bool descriptor_unknown{false}; - bool unexpected_legacy_entry{false}; CWalletScanState() = default; }; @@ -477,10 +473,8 @@ ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, // Taking advantage of the fact that pair serialization // is just the two items serialized one after the other ssKey >> strType; - // Legacy entries in descriptor wallets are not allowed, abort immediately if (pwallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS) && DBKeys::LEGACY_TYPES.count(strType) > 0) { - wss.unexpected_legacy_entry = true; - return false; + return true; } if (strType == DBKeys::NAME) { std::string strAddress; @@ -545,97 +539,16 @@ ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, return false; } } else if (strType == DBKeys::WATCHS) { - wss.nWatchKeys++; - CScript script; - ssKey >> script; - uint8_t fYes; - ssValue >> fYes; - if (fYes == '1') { - pwallet->GetOrCreateLegacyScriptPubKeyMan()->LoadWatchOnly(script); - } } else if (strType == DBKeys::KEY) { wss.nKeys++; - if (!LoadKey(pwallet, ssKey, ssValue, strErr)) return false; } else if (strType == DBKeys::MASTER_KEY) { if (!LoadEncryptionKey(pwallet, ssKey, ssValue, strErr)) return false; } else if (strType == DBKeys::CRYPTED_KEY) { wss.nCKeys++; - if (!LoadCryptedKey(pwallet, ssKey, ssValue, strErr)) return false; - wss.fIsEncrypted = true; } else if (strType == DBKeys::KEYMETA) { - CPubKey vchPubKey; - ssKey >> vchPubKey; - CKeyMetadata keyMeta; - ssValue >> keyMeta; wss.nKeyMeta++; - pwallet->GetOrCreateLegacyScriptPubKeyMan()->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) { - // Get the path from the key origin or from the path string - // Not applicable when path is "s" or "m" as those indicate a seed - // See https://github.com/bitcoin/bitcoin/pull/12924 - bool internal = false; - uint32_t index = 0; - if (keyMeta.hdKeypath != "s" && keyMeta.hdKeypath != "m") { - std::vector 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"; - return false; - } - } - - // Extract the index and internal from the path - // Path string is m/0'/k'/i' - // Path vector is [0', k', i'] (but as ints OR'd with the hardened bit - // k == 0 for external, 1 for internal. i is the index - if (path.size() != 3) { - strErr = "Error reading wallet database: keymeta found with unexpected path"; - return false; - } - if (path[0] != 0x80000000) { - strErr = strprintf("Unexpected path index of 0x%08x (expected 0x80000000) for the element at index 0", path[0]); - return false; - } - if (path[1] != 0x80000000 && path[1] != (1 | 0x80000000)) { - strErr = strprintf("Unexpected path index of 0x%08x (expected 0x80000000 or 0x80000001) for the element at index 1", path[1]); - return false; - } - if ((path[2] & 0x80000000) == 0) { - strErr = strprintf("Unexpected path index of 0x%08x (expected to be greater than or equal to 0x80000000)", path[2]); - return false; - } - internal = path[1] == (1 | 0x80000000); - index = path[2] & ~0x80000000; - } - - // Insert a new CHDChain, or get the one that already exists - auto ins = wss.m_hd_chains.emplace(keyMeta.hd_seed_id, CHDChain()); - CHDChain& chain = ins.first->second; - if (ins.second) { - // For new chains, we want to default to VERSION_HD_BASE until we see an internal - chain.nVersion = CHDChain::VERSION_HD_BASE; - chain.seed_id = keyMeta.hd_seed_id; - } - if (internal) { - chain.nVersion = CHDChain::VERSION_HD_CHAIN_SPLIT; - chain.nInternalChainCounter = std::max(chain.nInternalChainCounter, index + 1); - } else { - chain.nExternalChainCounter = std::max(chain.nExternalChainCounter, index + 1); - } - } } else if (strType == DBKeys::WATCHMETA) { - CScript script; - ssKey >> script; - CKeyMetadata keyMeta; - ssValue >> keyMeta; wss.nKeyMeta++; - pwallet->GetOrCreateLegacyScriptPubKeyMan()->LoadScriptMetadata(CScriptID(script), keyMeta); } else if (strType == DBKeys::DEFAULTKEY) { // 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 @@ -646,22 +559,7 @@ ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, return false; } } else if (strType == DBKeys::POOL) { - int64_t nIndex; - ssKey >> nIndex; - CKeyPool keypool; - ssValue >> keypool; - - pwallet->GetOrCreateLegacyScriptPubKeyMan()->LoadKeyPool(nIndex, keypool); } else if (strType == DBKeys::CSCRIPT) { - uint160 hash; - ssKey >> hash; - CScript script; - ssValue >> script; - if (!pwallet->GetOrCreateLegacyScriptPubKeyMan()->LoadCScript(script)) - { - strErr = "Error reading wallet database: LegacyScriptPubKeyMan::LoadCScript failed"; - return false; - } } else if (strType == DBKeys::ORDERPOSNEXT) { ssValue >> pwallet->nOrderPosNext; } else if (strType == DBKeys::DESTDATA) { @@ -684,7 +582,6 @@ ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, pwallet->LoadAddressReceiveRequest(dest, strKey.substr(2), strValue); } } else if (strType == DBKeys::HDCHAIN) { - if (!LoadHDChain(pwallet, ssValue, strErr)) return false; } else if (strType == DBKeys::OLD_KEY) { strErr = "Found unsupported 'wkey' record, try loading with version 0.18"; return false; @@ -803,7 +700,6 @@ ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, wss.nCKeys++; wss.m_descriptor_crypt_keys.insert(std::make_pair(std::make_pair(desc_id, pubkey.GetID()), std::make_pair(pubkey, privkey))); - wss.fIsEncrypted = true; } else if (strType == DBKeys::LOCKED_UTXO) { uint256 hash; uint32_t n; @@ -855,6 +751,268 @@ static DBErrors LoadWalletFlags(CWallet* pwallet, DatabaseBatch& batch) EXCLUSIV return DBErrors::LOAD_OK; } +struct LoadResult +{ + DBErrors m_result{DBErrors::LOAD_OK}; + int m_records{0}; +}; + +using LoadFunc = std::function; +static LoadResult LoadRecords(CWallet* pwallet, DatabaseBatch& batch, const std::string& key, LoadFunc load_func) +{ + LoadResult result; + DataStream ssKey; + CDataStream ssValue(SER_DISK, CLIENT_VERSION); + + DataStream prefix; + prefix << key; + std::unique_ptr cursor = batch.GetNewPrefixCursor(prefix); + if (!cursor) { + pwallet->WalletLogPrintf("Error getting database cursor for '%s' records\n", key); + result.m_result = DBErrors::CORRUPT; + return result; + } + + while (true) { + DatabaseCursor::Status status = cursor->Next(ssKey, ssValue); + if (status == DatabaseCursor::Status::DONE) { + break; + } else if (status == DatabaseCursor::Status::FAIL) { + pwallet->WalletLogPrintf("Error reading next '%s' record for wallet database\n", key); + result.m_result = DBErrors::CORRUPT; + return result; + } + std::string type; + ssKey >> type; + assert(type == key); + std::string error; + DBErrors record_res = load_func(pwallet, ssKey, ssValue, error); + if (record_res != DBErrors::LOAD_OK) { + pwallet->WalletLogPrintf("%s\n", error); + } + result.m_result = std::max(result.m_result, record_res); + ++result.m_records; + } + return result; +} + +static DBErrors LoadLegacyWalletRecords(CWallet* pwallet, DatabaseBatch& batch, int last_client) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) +{ + AssertLockHeld(pwallet->cs_wallet); + DBErrors result = DBErrors::LOAD_OK; + + // Make sure descriptor wallets don't have any legacy records + if (pwallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) { + for (const auto& type : DBKeys::LEGACY_TYPES) { + DataStream key; + CDataStream value(SER_DISK, CLIENT_VERSION); + + DataStream prefix; + prefix << type; + std::unique_ptr cursor = batch.GetNewPrefixCursor(prefix); + if (!cursor) { + pwallet->WalletLogPrintf("Error getting database cursor for '%s' records\n", type); + return DBErrors::CORRUPT; + } + + DatabaseCursor::Status status = cursor->Next(key, value); + if (status != DatabaseCursor::Status::DONE) { + pwallet->WalletLogPrintf("Error: Unexpected legacy entry found in descriptor wallet %s. The wallet might have been tampered with or created with malicious intent.\n", pwallet->GetName()); + return DBErrors::UNEXPECTED_LEGACY_ENTRY; + } + } + + return DBErrors::LOAD_OK; + } + + // 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, + [] (CWallet* pwallet, DataStream& key, CDataStream& 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, + [] (CWallet* pwallet, DataStream& key, CDataStream& 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, + [] (CWallet* pwallet, DataStream& key, CDataStream& 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, + [] (CWallet* pwallet, DataStream& key, CDataStream& value, std::string& strErr) { + uint160 hash; + key >> hash; + CScript script; + value >> script; + if (!pwallet->GetOrCreateLegacyScriptPubKeyMan()->LoadCScript(script)) + { + strErr = "Error reading wallet database: LegacyScriptPubKeyMan::LoadCScript failed"; + return DBErrors::NONCRITICAL_ERROR; + } + return DBErrors::LOAD_OK; + }); + result = std::max(result, script_res.m_result); + + // Check whether rewrite is needed + if (ckey_res.m_records > 0) { + // Rewrite encrypted wallets of versions 0.4.0 and 0.5.0rc: + if (last_client == 40000 || last_client == 50000) result = std::max(result, DBErrors::NEED_REWRITE); + } + + // Load keymeta + std::map hd_chains; + LoadResult keymeta_res = LoadRecords(pwallet, batch, DBKeys::KEYMETA, + [&hd_chains] (CWallet* pwallet, DataStream& key, CDataStream& value, std::string& strErr) { + CPubKey vchPubKey; + key >> vchPubKey; + CKeyMetadata keyMeta; + value >> keyMeta; + pwallet->GetOrCreateLegacyScriptPubKeyMan()->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) { + // Get the path from the key origin or from the path string + // Not applicable when path is "s" or "m" as those indicate a seed + // See https://github.com/bitcoin/bitcoin/pull/12924 + bool internal = false; + uint32_t index = 0; + if (keyMeta.hdKeypath != "s" && keyMeta.hdKeypath != "m") { + std::vector 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"; + return DBErrors::NONCRITICAL_ERROR; + } + } + + // Extract the index and internal from the path + // Path string is m/0'/k'/i' + // Path vector is [0', k', i'] (but as ints OR'd with the hardened bit + // k == 0 for external, 1 for internal. i is the index + if (path.size() != 3) { + strErr = "Error reading wallet database: keymeta found with unexpected path"; + return DBErrors::NONCRITICAL_ERROR; + } + if (path[0] != 0x80000000) { + strErr = strprintf("Unexpected path index of 0x%08x (expected 0x80000000) for the element at index 0", path[0]); + return DBErrors::NONCRITICAL_ERROR; + } + if (path[1] != 0x80000000 && path[1] != (1 | 0x80000000)) { + strErr = strprintf("Unexpected path index of 0x%08x (expected 0x80000000 or 0x80000001) for the element at index 1", path[1]); + return DBErrors::NONCRITICAL_ERROR; + } + if ((path[2] & 0x80000000) == 0) { + strErr = strprintf("Unexpected path index of 0x%08x (expected to be greater than or equal to 0x80000000)", path[2]); + return DBErrors::NONCRITICAL_ERROR; + } + internal = path[1] == (1 | 0x80000000); + index = path[2] & ~0x80000000; + } + + // Insert a new CHDChain, or get the one that already exists + auto [ins, inserted] = hd_chains.emplace(keyMeta.hd_seed_id, CHDChain()); + CHDChain& chain = ins->second; + if (inserted) { + // For new chains, we want to default to VERSION_HD_BASE until we see an internal + chain.nVersion = CHDChain::VERSION_HD_BASE; + chain.seed_id = keyMeta.hd_seed_id; + } + if (internal) { + chain.nVersion = CHDChain::VERSION_HD_CHAIN_SPLIT; + chain.nInternalChainCounter = std::max(chain.nInternalChainCounter, index + 1); + } else { + chain.nExternalChainCounter = std::max(chain.nExternalChainCounter, index + 1); + } + } + return DBErrors::LOAD_OK; + }); + result = std::max(result, keymeta_res.m_result); + + // Set inactive chains + if (!hd_chains.empty()) { + LegacyScriptPubKeyMan* legacy_spkm = pwallet->GetLegacyScriptPubKeyMan(); + if (legacy_spkm) { + for (const auto& [hd_seed_id, chain] : hd_chains) { + if (hd_seed_id != legacy_spkm->GetHDChain().seed_id) { + legacy_spkm->AddInactiveHDChain(chain); + } + } + } else { + pwallet->WalletLogPrintf("Inactive HD Chains found but no Legacy ScriptPubKeyMan\n"); + result = DBErrors::CORRUPT; + } + } + + // Load watchonly scripts + LoadResult watch_script_res = LoadRecords(pwallet, batch, DBKeys::WATCHS, + [] (CWallet* pwallet, DataStream& key, CDataStream& value, std::string& err) { + CScript script; + key >> script; + uint8_t fYes; + value >> fYes; + if (fYes == '1') { + pwallet->GetOrCreateLegacyScriptPubKeyMan()->LoadWatchOnly(script); + } + return DBErrors::LOAD_OK; + }); + result = std::max(result, watch_script_res.m_result); + + // Load watchonly meta + LoadResult watch_meta_res = LoadRecords(pwallet, batch, DBKeys::WATCHMETA, + [] (CWallet* pwallet, DataStream& key, CDataStream& value, std::string& err) { + CScript script; + key >> script; + CKeyMetadata keyMeta; + value >> keyMeta; + pwallet->GetOrCreateLegacyScriptPubKeyMan()->LoadScriptMetadata(CScriptID(script), keyMeta); + return DBErrors::LOAD_OK; + }); + result = std::max(result, watch_meta_res.m_result); + + // Load keypool + LoadResult pool_res = LoadRecords(pwallet, batch, DBKeys::POOL, + [] (CWallet* pwallet, DataStream& key, CDataStream& value, std::string& err) { + int64_t nIndex; + key >> nIndex; + CKeyPool keypool; + value >> keypool; + pwallet->GetOrCreateLegacyScriptPubKeyMan()->LoadKeyPool(nIndex, keypool); + return DBErrors::LOAD_OK; + }); + result = std::max(result, pool_res.m_result); + + if (result <= DBErrors::NONCRITICAL_ERROR) { + // Only do logging and time first key update if there were no critical errors + pwallet->WalletLogPrintf("Legacy Wallet Keys: %u plaintext, %u encrypted, %u w/ metadata, %u total.\n", + key_res.m_records, ckey_res.m_records, keymeta_res.m_records, key_res.m_records + ckey_res.m_records); + + // nTimeFirstKey is only reliable if all keys have metadata + if (pwallet->IsLegacy() && (key_res.m_records + ckey_res.m_records + watch_script_res.m_records) != (keymeta_res.m_records + watch_meta_res.m_records)) { + auto spk_man = pwallet->GetOrCreateLegacyScriptPubKeyMan(); + if (spk_man) { + LOCK(spk_man->cs_KeyStore); + spk_man->UpdateTimeFirstKey(1); + } + } + } + + return result; +} + DBErrors WalletBatch::LoadWallet(CWallet* pwallet) { CWalletScanState wss; @@ -883,6 +1041,9 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) } #endif + // Load legacy wallet keys + result = std::max(LoadLegacyWalletRecords(pwallet, *m_batch, last_client), result); + // Get cursor std::unique_ptr cursor = m_batch->GetNewCursor(); if (!cursor) @@ -909,17 +1070,9 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) std::string strType, strErr; if (!ReadKeyValue(pwallet, ssKey, ssValue, wss, strType, strErr)) { - if (wss.unexpected_legacy_entry) { - strErr = strprintf("Error: Unexpected legacy entry found in descriptor wallet %s. ", pwallet->GetName()); - strErr += "The wallet might have been tampered with or created with malicious intent."; - pwallet->WalletLogPrintf("%s\n", strErr); - return DBErrors::UNEXPECTED_LEGACY_ENTRY; - } // losing keys is considered a catastrophic error, anything else // we assume the user can live with: - if (strType == DBKeys::KEY || - strType == DBKeys::MASTER_KEY || - strType == DBKeys::CRYPTED_KEY || + if (strType == DBKeys::MASTER_KEY || strType == DBKeys::DEFAULTKEY) { result = DBErrors::CORRUPT; } else if (wss.tx_corrupt) { @@ -946,6 +1099,8 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) pwallet->WalletLogPrintf("%s\n", strErr); } } catch (...) { + // Exceptions that can be ignored or treated as non-critical are handled by the individual loading functions. + // Any uncaught exceptions will be caught here and treated as critical. result = DBErrors::CORRUPT; } @@ -988,22 +1143,9 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) pwallet->WalletLogPrintf("Keys: %u plaintext, %u encrypted, %u w/ metadata, %u total. Unknown wallet records: %u\n", wss.nKeys, wss.nCKeys, wss.nKeyMeta, wss.nKeys + wss.nCKeys, wss.m_unknown_records); - // nTimeFirstKey is only reliable if all keys have metadata - if (pwallet->IsLegacy() && (wss.nKeys + wss.nCKeys + wss.nWatchKeys) != wss.nKeyMeta) { - auto spk_man = pwallet->GetOrCreateLegacyScriptPubKeyMan(); - if (spk_man) { - LOCK(spk_man->cs_KeyStore); - spk_man->UpdateTimeFirstKey(1); - } - } - for (const uint256& hash : wss.vWalletUpgrade) WriteTx(pwallet->mapWallet.at(hash)); - // Rewrite encrypted wallets of versions 0.4.0 and 0.5.0rc: - if (wss.fIsEncrypted && (last_client == 40000 || last_client == 50000)) - return DBErrors::NEED_REWRITE; - if (!has_last_client || last_client != CLIENT_VERSION) // Update m_batch->Write(DBKeys::VERSION, CLIENT_VERSION); @@ -1026,20 +1168,6 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) result = DBErrors::CORRUPT; } - // Set the inactive chain - if (wss.m_hd_chains.size() > 0) { - LegacyScriptPubKeyMan* legacy_spkm = pwallet->GetLegacyScriptPubKeyMan(); - if (!legacy_spkm) { - pwallet->WalletLogPrintf("Inactive HD Chains found but no Legacy ScriptPubKeyMan\n"); - return DBErrors::CORRUPT; - } - for (const auto& chain_pair : wss.m_hd_chains) { - if (chain_pair.first != pwallet->GetLegacyScriptPubKeyMan()->GetHDChain().seed_id) { - pwallet->GetLegacyScriptPubKeyMan()->AddInactiveHDChain(chain_pair.second); - } - } - } - return result; } diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 9be1dcec48..8f7c2f030c 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -42,19 +42,21 @@ struct WalletContext; static const bool DEFAULT_FLUSHWALLET = true; -/** Error statuses for the wallet database */ -enum class DBErrors +/** Error statuses for the wallet database. + * Values are in order of severity. When multiple errors occur, the most severe (highest value) will be returned. + */ +enum class DBErrors : int { - LOAD_OK, - CORRUPT, - NONCRITICAL_ERROR, - TOO_NEW, - EXTERNAL_SIGNER_SUPPORT_REQUIRED, - LOAD_FAIL, - NEED_REWRITE, - NEED_RESCAN, - UNKNOWN_DESCRIPTOR, - UNEXPECTED_LEGACY_ENTRY + LOAD_OK = 0, + NEED_RESCAN = 1, + NEED_REWRITE = 2, + EXTERNAL_SIGNER_SUPPORT_REQUIRED = 3, + NONCRITICAL_ERROR = 4, + TOO_NEW = 5, + UNKNOWN_DESCRIPTOR = 6, + LOAD_FAIL = 7, + UNEXPECTED_LEGACY_ENTRY = 8, + CORRUPT = 9, }; namespace DBKeys { From 405b4d914712b5de3b230a0e2960e89f6a0a2b2a Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 12 Apr 2022 16:16:49 -0400 Subject: [PATCH 09/16] walletdb: Refactor descriptor wallet records loading Instead of loading descriptor wallet records as we come across them when iterating the database, loading them explicitly. Exception handling for these records changes to a per-record type basis, rather than globally. This results in some records now failing with a critical error rather than a non-critical one. --- src/wallet/walletdb.cpp | 326 +++++++++++++++++++++++----------------- 1 file changed, 188 insertions(+), 138 deletions(-) diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index cf79025801..3b012cdba7 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -299,19 +300,12 @@ bool WalletBatch::EraseLockedUTXO(const COutPoint& output) class CWalletScanState { public: - unsigned int nKeys{0}; - unsigned int nCKeys{0}; - unsigned int nKeyMeta{0}; unsigned int m_unknown_records{0}; bool fAnyUnordered{false}; std::vector vWalletUpgrade; std::map m_active_external_spks; std::map m_active_internal_spks; - std::map m_descriptor_caches; - std::map, CKey> m_descriptor_keys; - std::map, std::pair>> m_descriptor_crypt_keys; bool tx_corrupt{false}; - bool descriptor_unknown{false}; CWalletScanState() = default; }; @@ -540,15 +534,11 @@ ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, } } else if (strType == DBKeys::WATCHS) { } else if (strType == DBKeys::KEY) { - wss.nKeys++; } else if (strType == DBKeys::MASTER_KEY) { if (!LoadEncryptionKey(pwallet, ssKey, ssValue, strErr)) return false; } else if (strType == DBKeys::CRYPTED_KEY) { - wss.nCKeys++; } else if (strType == DBKeys::KEYMETA) { - wss.nKeyMeta++; } else if (strType == DBKeys::WATCHMETA) { - wss.nKeyMeta++; } else if (strType == DBKeys::DEFAULTKEY) { // 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 @@ -599,107 +589,10 @@ ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, } spk_mans[static_cast(type)] = id; } else if (strType == DBKeys::WALLETDESCRIPTOR) { - uint256 id; - ssKey >> id; - WalletDescriptor desc; - try { - ssValue >> desc; - } catch (const std::ios_base::failure& e) { - strErr = e.what(); - wss.descriptor_unknown = true; - return false; - } - if (wss.m_descriptor_caches.count(id) == 0) { - wss.m_descriptor_caches[id] = DescriptorCache(); - } - pwallet->LoadDescriptorScriptPubKeyMan(id, desc); } else if (strType == DBKeys::WALLETDESCRIPTORCACHE) { - bool parent = true; - uint256 desc_id; - uint32_t key_exp_index; - uint32_t der_index; - ssKey >> desc_id; - ssKey >> key_exp_index; - - // if the der_index exists, it's a derived xpub - try - { - ssKey >> der_index; - parent = false; - } - catch (...) {} - - std::vector ser_xpub(BIP32_EXTKEY_SIZE); - ssValue >> ser_xpub; - CExtPubKey xpub; - xpub.Decode(ser_xpub.data()); - if (parent) { - wss.m_descriptor_caches[desc_id].CacheParentExtPubKey(key_exp_index, xpub); - } else { - wss.m_descriptor_caches[desc_id].CacheDerivedExtPubKey(key_exp_index, der_index, xpub); - } } else if (strType == DBKeys::WALLETDESCRIPTORLHCACHE) { - uint256 desc_id; - uint32_t key_exp_index; - ssKey >> desc_id; - ssKey >> key_exp_index; - - std::vector ser_xpub(BIP32_EXTKEY_SIZE); - ssValue >> ser_xpub; - CExtPubKey xpub; - xpub.Decode(ser_xpub.data()); - wss.m_descriptor_caches[desc_id].CacheLastHardenedExtPubKey(key_exp_index, xpub); } else if (strType == DBKeys::WALLETDESCRIPTORKEY) { - uint256 desc_id; - CPubKey pubkey; - ssKey >> desc_id; - ssKey >> pubkey; - if (!pubkey.IsValid()) - { - strErr = "Error reading wallet database: CPubKey corrupt"; - return false; - } - CKey key; - CPrivKey pkey; - uint256 hash; - - wss.nKeys++; - ssValue >> pkey; - ssValue >> hash; - - // hash pubkey/privkey to accelerate wallet load - std::vector to_hash; - to_hash.reserve(pubkey.size() + pkey.size()); - to_hash.insert(to_hash.end(), pubkey.begin(), pubkey.end()); - to_hash.insert(to_hash.end(), pkey.begin(), pkey.end()); - - if (Hash(to_hash) != hash) - { - strErr = "Error reading wallet database: CPubKey/CPrivKey corrupt"; - return false; - } - - if (!key.Load(pkey, pubkey, true)) - { - strErr = "Error reading wallet database: CPrivKey corrupt"; - return false; - } - wss.m_descriptor_keys.insert(std::make_pair(std::make_pair(desc_id, pubkey.GetID()), key)); } else if (strType == DBKeys::WALLETDESCRIPTORCKEY) { - uint256 desc_id; - CPubKey pubkey; - ssKey >> desc_id; - ssKey >> pubkey; - if (!pubkey.IsValid()) - { - strErr = "Error reading wallet database: CPubKey corrupt"; - return false; - } - std::vector privkey; - ssValue >> privkey; - wss.nCKeys++; - - wss.m_descriptor_crypt_keys.insert(std::make_pair(std::make_pair(desc_id, pubkey.GetID()), std::make_pair(pubkey, privkey))); } else if (strType == DBKeys::LOCKED_UTXO) { uint256 hash; uint32_t n; @@ -758,14 +651,13 @@ struct LoadResult }; using LoadFunc = std::function; -static LoadResult LoadRecords(CWallet* pwallet, DatabaseBatch& batch, const std::string& key, LoadFunc load_func) +static LoadResult LoadRecords(CWallet* pwallet, DatabaseBatch& batch, const std::string& key, DataStream& prefix, LoadFunc load_func) { LoadResult result; DataStream ssKey; CDataStream ssValue(SER_DISK, CLIENT_VERSION); - DataStream prefix; - prefix << key; + Assume(!prefix.empty()); std::unique_ptr cursor = batch.GetNewPrefixCursor(prefix); if (!cursor) { pwallet->WalletLogPrintf("Error getting database cursor for '%s' records\n", key); @@ -796,6 +688,13 @@ static LoadResult LoadRecords(CWallet* pwallet, DatabaseBatch& batch, const std: return result; } +static LoadResult LoadRecords(CWallet* pwallet, DatabaseBatch& batch, const std::string& key, LoadFunc load_func) +{ + DataStream prefix; + prefix << key; + return LoadRecords(pwallet, batch, key, prefix, load_func); +} + static DBErrors LoadLegacyWalletRecords(CWallet* pwallet, DatabaseBatch& batch, int last_client) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) { AssertLockHeld(pwallet->cs_wallet); @@ -1013,6 +912,177 @@ static DBErrors LoadLegacyWalletRecords(CWallet* pwallet, DatabaseBatch& batch, return result; } +template +static DataStream PrefixStream(const Args&... args) +{ + DataStream prefix; + SerializeMany(prefix, args...); + return prefix; +} + +static DBErrors LoadDescriptorWalletRecords(CWallet* pwallet, DatabaseBatch& batch, int last_client) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) +{ + AssertLockHeld(pwallet->cs_wallet); + + // Load descriptor record + int num_keys = 0; + int num_ckeys= 0; + LoadResult desc_res = LoadRecords(pwallet, batch, DBKeys::WALLETDESCRIPTOR, + [&batch, &num_keys, &num_ckeys, &last_client] (CWallet* pwallet, DataStream& key, CDataStream& value, std::string& strErr) { + DBErrors result = DBErrors::LOAD_OK; + + uint256 id; + key >> id; + WalletDescriptor desc; + try { + value >> desc; + } catch (const std::ios_base::failure&) { + strErr = strprintf("Error: Unrecognized descriptor found in wallet %s. ", pwallet->GetName()); + strErr += (last_client > CLIENT_VERSION) ? "The wallet might had been created on a newer version. " : + "The database might be corrupted or the software version is not compatible with one of your wallet descriptors. "; + strErr += "Please try running the latest software version"; + return DBErrors::UNKNOWN_DESCRIPTOR; + } + pwallet->LoadDescriptorScriptPubKeyMan(id, desc); + + DescriptorCache cache; + + // Get key cache for this descriptor + DataStream prefix = PrefixStream(DBKeys::WALLETDESCRIPTORCACHE, id); + LoadResult key_cache_res = LoadRecords(pwallet, batch, DBKeys::WALLETDESCRIPTORCACHE, prefix, + [&id, &cache] (CWallet* pwallet, DataStream& key, CDataStream& value, std::string& err) { + bool parent = true; + uint256 desc_id; + uint32_t key_exp_index; + uint32_t der_index; + key >> desc_id; + assert(desc_id == id); + key >> key_exp_index; + + // if the der_index exists, it's a derived xpub + try + { + key >> der_index; + parent = false; + } + catch (...) {} + + std::vector ser_xpub(BIP32_EXTKEY_SIZE); + value >> ser_xpub; + CExtPubKey xpub; + xpub.Decode(ser_xpub.data()); + if (parent) { + cache.CacheParentExtPubKey(key_exp_index, xpub); + } else { + cache.CacheDerivedExtPubKey(key_exp_index, der_index, xpub); + } + return DBErrors::LOAD_OK; + }); + result = std::max(result, key_cache_res.m_result); + + // Get last hardened cache for this descriptor + prefix = PrefixStream(DBKeys::WALLETDESCRIPTORLHCACHE, id); + LoadResult lh_cache_res = LoadRecords(pwallet, batch, DBKeys::WALLETDESCRIPTORLHCACHE, prefix, + [&id, &cache] (CWallet* pwallet, DataStream& key, CDataStream& value, std::string& err) { + uint256 desc_id; + uint32_t key_exp_index; + key >> desc_id; + assert(desc_id == id); + key >> key_exp_index; + + std::vector ser_xpub(BIP32_EXTKEY_SIZE); + value >> ser_xpub; + CExtPubKey xpub; + xpub.Decode(ser_xpub.data()); + cache.CacheLastHardenedExtPubKey(key_exp_index, xpub); + return DBErrors::LOAD_OK; + }); + result = std::max(result, lh_cache_res.m_result); + + // Set the cache for this descriptor + auto spk_man = (DescriptorScriptPubKeyMan*)pwallet->GetScriptPubKeyMan(id); + assert(spk_man); + spk_man->SetCache(cache); + + // Get unencrypted keys + prefix = PrefixStream(DBKeys::WALLETDESCRIPTORKEY, id); + LoadResult key_res = LoadRecords(pwallet, batch, DBKeys::WALLETDESCRIPTORKEY, prefix, + [&id, &spk_man] (CWallet* pwallet, DataStream& key, CDataStream& value, std::string& strErr) { + uint256 desc_id; + CPubKey pubkey; + key >> desc_id; + assert(desc_id == id); + key >> pubkey; + if (!pubkey.IsValid()) + { + strErr = "Error reading wallet database: descriptor unencrypted key CPubKey corrupt"; + return DBErrors::CORRUPT; + } + CKey privkey; + CPrivKey pkey; + uint256 hash; + + value >> pkey; + value >> hash; + + // hash pubkey/privkey to accelerate wallet load + std::vector to_hash; + to_hash.reserve(pubkey.size() + pkey.size()); + to_hash.insert(to_hash.end(), pubkey.begin(), pubkey.end()); + to_hash.insert(to_hash.end(), pkey.begin(), pkey.end()); + + if (Hash(to_hash) != hash) + { + strErr = "Error reading wallet database: descriptor unencrypted key CPubKey/CPrivKey corrupt"; + return DBErrors::CORRUPT; + } + + if (!privkey.Load(pkey, pubkey, true)) + { + strErr = "Error reading wallet database: descriptor unencrypted key CPrivKey corrupt"; + return DBErrors::CORRUPT; + } + spk_man->AddKey(pubkey.GetID(), privkey); + return DBErrors::LOAD_OK; + }); + result = std::max(result, key_res.m_result); + num_keys = key_res.m_records; + + // Get encrypted keys + prefix = PrefixStream(DBKeys::WALLETDESCRIPTORCKEY, id); + LoadResult ckey_res = LoadRecords(pwallet, batch, DBKeys::WALLETDESCRIPTORCKEY, prefix, + [&id, &spk_man] (CWallet* pwallet, DataStream& key, CDataStream& value, std::string& err) { + uint256 desc_id; + CPubKey pubkey; + key >> desc_id; + assert(desc_id == id); + key >> pubkey; + if (!pubkey.IsValid()) + { + err = "Error reading wallet database: descriptor encrypted key CPubKey corrupt"; + return DBErrors::CORRUPT; + } + std::vector privkey; + value >> privkey; + + spk_man->AddCryptedKey(pubkey.GetID(), pubkey, privkey); + return DBErrors::LOAD_OK; + }); + result = std::max(result, ckey_res.m_result); + num_ckeys = ckey_res.m_records; + + return result; + }); + + if (desc_res.m_result <= DBErrors::NONCRITICAL_ERROR) { + // Only log if there are no critical errors + pwallet->WalletLogPrintf("Descriptors: %u, Descriptor Keys: %u plaintext, %u encrypted, %u total.\n", + desc_res.m_records, num_keys, num_ckeys, num_keys + num_ckeys); + } + + return desc_res.m_result; +} + DBErrors WalletBatch::LoadWallet(CWallet* pwallet) { CWalletScanState wss; @@ -1044,6 +1114,13 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) // Load legacy wallet keys result = std::max(LoadLegacyWalletRecords(pwallet, *m_batch, last_client), result); + // Load descriptors + result = std::max(LoadDescriptorWalletRecords(pwallet, *m_batch, last_client), result); + // Early return if there are unknown descriptors. Later loading of ACTIVEINTERNALSPK and ACTIVEEXTERNALEXPK + // may reference the unknown descriptor's ID which can result in a misleading corruption error + // when in reality the wallet is simply too new. + if (result == DBErrors::UNKNOWN_DESCRIPTOR) return result; + // Get cursor std::unique_ptr cursor = m_batch->GetNewCursor(); if (!cursor) @@ -1080,13 +1157,6 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) // Set tx_corrupt back to false so that the error is only printed once (per corrupt tx) wss.tx_corrupt = false; result = DBErrors::CORRUPT; - } else if (wss.descriptor_unknown) { - strErr = strprintf("Error: Unrecognized descriptor found in wallet %s. ", pwallet->GetName()); - strErr += (last_client > CLIENT_VERSION) ? "The wallet might had been created on a newer version. " : - "The database might be corrupted or the software version is not compatible with one of your wallet descriptors. "; - strErr += "Please try running the latest software version"; - pwallet->WalletLogPrintf("%s\n", strErr); - return DBErrors::UNKNOWN_DESCRIPTOR; } else { // Leave other errors alone, if we try to fix them we might make things worse. fNoncriticalErrors = true; // ... but do warn the user there is something wrong. @@ -1112,23 +1182,6 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) pwallet->LoadActiveScriptPubKeyMan(spk_man_pair.second, spk_man_pair.first, /*internal=*/true); } - // Set the descriptor caches - for (const auto& desc_cache_pair : wss.m_descriptor_caches) { - auto spk_man = pwallet->GetScriptPubKeyMan(desc_cache_pair.first); - assert(spk_man); - ((DescriptorScriptPubKeyMan*)spk_man)->SetCache(desc_cache_pair.second); - } - - // Set the descriptor keys - for (const auto& desc_key_pair : wss.m_descriptor_keys) { - auto spk_man = pwallet->GetScriptPubKeyMan(desc_key_pair.first.first); - ((DescriptorScriptPubKeyMan*)spk_man)->AddKey(desc_key_pair.first.second, desc_key_pair.second); - } - for (const auto& desc_key_pair : wss.m_descriptor_crypt_keys) { - auto spk_man = pwallet->GetScriptPubKeyMan(desc_key_pair.first.first); - ((DescriptorScriptPubKeyMan*)spk_man)->AddCryptedKey(desc_key_pair.first.second, desc_key_pair.second.first, desc_key_pair.second.second); - } - if (rescan_required && result == DBErrors::LOAD_OK) { result = DBErrors::NEED_RESCAN; } else if (fNoncriticalErrors && result == DBErrors::LOAD_OK) { @@ -1140,9 +1193,6 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) if (result != DBErrors::LOAD_OK) return result; - pwallet->WalletLogPrintf("Keys: %u plaintext, %u encrypted, %u w/ metadata, %u total. Unknown wallet records: %u\n", - wss.nKeys, wss.nCKeys, wss.nKeyMeta, wss.nKeys + wss.nCKeys, wss.m_unknown_records); - for (const uint256& hash : wss.vWalletUpgrade) WriteTx(pwallet->mapWallet.at(hash)); From abcc13dd24889bc1c6af7b10da1da96d86aeafed Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 12 Apr 2022 18:25:10 -0400 Subject: [PATCH 10/16] walletdb: refactor address book loading Instead of loading address book records as we come across them when iterating the database, load them explicitly Due to exception handling changes, deserialization errors are now treated as critical. The error message for noncritical errors has also been updated to reflect that there's more data that could be missing than just address book entries and tx data. --- src/wallet/wallet.cpp | 2 +- src/wallet/walletdb.cpp | 96 +++++++++++++++++++++++++++-------------- 2 files changed, 65 insertions(+), 33 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index dce99b94bd..fe4f8e4442 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2929,7 +2929,7 @@ std::shared_ptr CWallet::Create(WalletContext& context, const std::stri else if (nLoadWalletRet == DBErrors::NONCRITICAL_ERROR) { warnings.push_back(strprintf(_("Error reading %s! All keys read correctly, but transaction data" - " or address book entries might be missing or incorrect."), + " or address metadata may be missing or incorrect."), walletFile)); } else if (nLoadWalletRet == DBErrors::TOO_NEW) { diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 3b012cdba7..c0a2cb6d60 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -471,21 +471,7 @@ ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, return true; } if (strType == DBKeys::NAME) { - std::string strAddress; - ssKey >> strAddress; - std::string label; - ssValue >> label; - pwallet->m_address_book[DecodeDestination(strAddress)].SetLabel(label); } else if (strType == DBKeys::PURPOSE) { - std::string strAddress; - ssKey >> strAddress; - std::string purpose_str; - ssValue >> purpose_str; - std::optional purpose{PurposeFromString(purpose_str)}; - if (!purpose) { - pwallet->WalletLogPrintf("Warning: nonstandard purpose string '%s' for address '%s'\n", purpose_str, strAddress); - } - pwallet->m_address_book[DecodeDestination(strAddress)].purpose = purpose; } else if (strType == DBKeys::TX) { uint256 hash; ssKey >> hash; @@ -553,24 +539,6 @@ ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, } else if (strType == DBKeys::ORDERPOSNEXT) { ssValue >> pwallet->nOrderPosNext; } else if (strType == DBKeys::DESTDATA) { - std::string strAddress, strKey, strValue; - ssKey >> strAddress; - ssKey >> strKey; - ssValue >> strValue; - const CTxDestination& dest{DecodeDestination(strAddress)}; - if (strKey.compare("used") == 0) { - // Load "used" key indicating if an IsMine address has - // previously been spent from with avoid_reuse option enabled. - // The strValue is not used for anything currently, but could - // hold more information in the future. Current values are just - // "1" or "p" for present (which was written prior to - // f5ba424cd44619d9b9be88b8593d69a7ba96db26). - pwallet->LoadAddressPreviouslySpent(dest); - } else if (strKey.compare(0, 2, "rr") == 0) { - // Load "rr##" keys where ## is a decimal number, and strValue - // is a serialized RecentRequestEntry object. - pwallet->LoadAddressReceiveRequest(dest, strKey.substr(2), strValue); - } } else if (strType == DBKeys::HDCHAIN) { } else if (strType == DBKeys::OLD_KEY) { strErr = "Found unsupported 'wkey' record, try loading with version 0.18"; @@ -1083,6 +1051,67 @@ static DBErrors LoadDescriptorWalletRecords(CWallet* pwallet, DatabaseBatch& bat return desc_res.m_result; } +static DBErrors LoadAddressBookRecords(CWallet* pwallet, DatabaseBatch& batch) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) +{ + AssertLockHeld(pwallet->cs_wallet); + DBErrors result = DBErrors::LOAD_OK; + + // Load name record + LoadResult name_res = LoadRecords(pwallet, batch, DBKeys::NAME, + [] (CWallet* pwallet, DataStream& key, CDataStream& value, std::string& err) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) { + std::string strAddress; + key >> strAddress; + std::string label; + value >> label; + pwallet->m_address_book[DecodeDestination(strAddress)].SetLabel(label); + return DBErrors::LOAD_OK; + }); + result = std::max(result, name_res.m_result); + + // Load purpose record + LoadResult purpose_res = LoadRecords(pwallet, batch, DBKeys::PURPOSE, + [] (CWallet* pwallet, DataStream& key, CDataStream& value, std::string& err) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) { + std::string strAddress; + key >> strAddress; + std::string purpose_str; + value >> purpose_str; + std::optional purpose{PurposeFromString(purpose_str)}; + if (!purpose) { + pwallet->WalletLogPrintf("Warning: nonstandard purpose string '%s' for address '%s'\n", purpose_str, strAddress); + } + pwallet->m_address_book[DecodeDestination(strAddress)].purpose = purpose; + return DBErrors::LOAD_OK; + }); + result = std::max(result, purpose_res.m_result); + + // Load destination data record + LoadResult dest_res = LoadRecords(pwallet, batch, DBKeys::DESTDATA, + [] (CWallet* pwallet, DataStream& key, CDataStream& value, std::string& err) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) { + std::string strAddress, strKey, strValue; + key >> strAddress; + key >> strKey; + value >> strValue; + const CTxDestination& dest{DecodeDestination(strAddress)}; + if (strKey.compare("used") == 0) { + // Load "used" key indicating if an IsMine address has + // previously been spent from with avoid_reuse option enabled. + // The strValue is not used for anything currently, but could + // hold more information in the future. Current values are just + // "1" or "p" for present (which was written prior to + // f5ba424cd44619d9b9be88b8593d69a7ba96db26). + pwallet->LoadAddressPreviouslySpent(dest); + } else if (strKey.compare(0, 2, "rr") == 0) { + // Load "rr##" keys where ## is a decimal number, and strValue + // is a serialized RecentRequestEntry object. + pwallet->LoadAddressReceiveRequest(dest, strKey.substr(2), strValue); + } + return DBErrors::LOAD_OK; + }); + result = std::max(result, dest_res.m_result); + + return result; +} + DBErrors WalletBatch::LoadWallet(CWallet* pwallet) { CWalletScanState wss; @@ -1121,6 +1150,9 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) // when in reality the wallet is simply too new. if (result == DBErrors::UNKNOWN_DESCRIPTOR) return result; + // Load address book + result = std::max(LoadAddressBookRecords(pwallet, *m_batch), result); + // Get cursor std::unique_ptr cursor = m_batch->GetNewCursor(); if (!cursor) From 6fabb7fc99e60584d5f3a2cb01d39f761769a25d Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 13 Apr 2022 12:18:11 -0400 Subject: [PATCH 11/16] walletdb: refactor tx loading Instead of loading tx records as we come across them when iterating the database, load them explicitly. --- src/wallet/walletdb.cpp | 164 +++++++++++++++++++++++----------------- 1 file changed, 96 insertions(+), 68 deletions(-) diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index c0a2cb6d60..f3a82734c1 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -301,11 +301,8 @@ bool WalletBatch::EraseLockedUTXO(const COutPoint& output) class CWalletScanState { public: unsigned int m_unknown_records{0}; - bool fAnyUnordered{false}; - std::vector vWalletUpgrade; std::map m_active_external_spks; std::map m_active_internal_spks; - bool tx_corrupt{false}; CWalletScanState() = default; }; @@ -473,51 +470,6 @@ ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, if (strType == DBKeys::NAME) { } else if (strType == DBKeys::PURPOSE) { } else if (strType == DBKeys::TX) { - uint256 hash; - ssKey >> hash; - // LoadToWallet call below creates a new CWalletTx that fill_wtx - // callback fills with transaction metadata. - auto fill_wtx = [&](CWalletTx& wtx, bool new_tx) { - if(!new_tx) { - // There's some corruption here since the tx we just tried to load was already in the wallet. - // We don't consider this type of corruption critical, and can fix it by removing tx data and - // rescanning. - wss.tx_corrupt = true; - return false; - } - ssValue >> wtx; - if (wtx.GetHash() != hash) - return false; - - // Undo serialize changes in 31600 - if (31404 <= wtx.fTimeReceivedIsTxTime && wtx.fTimeReceivedIsTxTime <= 31703) - { - if (!ssValue.empty()) - { - uint8_t fTmp; - uint8_t fUnused; - std::string unused_string; - ssValue >> fTmp >> fUnused >> unused_string; - strErr = strprintf("LoadWallet() upgrading tx ver=%d %d %s", - wtx.fTimeReceivedIsTxTime, fTmp, hash.ToString()); - wtx.fTimeReceivedIsTxTime = fTmp; - } - else - { - strErr = strprintf("LoadWallet() repairing tx ver=%d %s", wtx.fTimeReceivedIsTxTime, hash.ToString()); - wtx.fTimeReceivedIsTxTime = 0; - } - wss.vWalletUpgrade.push_back(hash); - } - - if (wtx.nOrderPos == -1) - wss.fAnyUnordered = true; - - return true; - }; - if (!pwallet->LoadToWallet(hash, fill_wtx)) { - return false; - } } else if (strType == DBKeys::WATCHS) { } else if (strType == DBKeys::KEY) { } else if (strType == DBKeys::MASTER_KEY) { @@ -537,7 +489,6 @@ ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, } else if (strType == DBKeys::POOL) { } else if (strType == DBKeys::CSCRIPT) { } else if (strType == DBKeys::ORDERPOSNEXT) { - ssValue >> pwallet->nOrderPosNext; } else if (strType == DBKeys::DESTDATA) { } else if (strType == DBKeys::HDCHAIN) { } else if (strType == DBKeys::OLD_KEY) { @@ -562,11 +513,6 @@ ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, } else if (strType == DBKeys::WALLETDESCRIPTORKEY) { } else if (strType == DBKeys::WALLETDESCRIPTORCKEY) { } else if (strType == DBKeys::LOCKED_UTXO) { - uint256 hash; - uint32_t n; - ssKey >> hash; - ssKey >> n; - pwallet->LockCoin(COutPoint(hash, n)); } else if (strType != DBKeys::BESTBLOCK && strType != DBKeys::BESTBLOCK_NOMERKLE && strType != DBKeys::MINVERSION && strType != DBKeys::ACENTRY && strType != DBKeys::VERSION && strType != DBKeys::SETTINGS && @@ -1112,12 +1058,101 @@ static DBErrors LoadAddressBookRecords(CWallet* pwallet, DatabaseBatch& batch) E return result; } +static DBErrors LoadTxRecords(CWallet* pwallet, DatabaseBatch& batch, std::vector& upgraded_txs, bool& any_unordered) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) +{ + AssertLockHeld(pwallet->cs_wallet); + DBErrors result = DBErrors::LOAD_OK; + + // Load tx record + any_unordered = false; + LoadResult tx_res = LoadRecords(pwallet, batch, DBKeys::TX, + [&any_unordered, &upgraded_txs] (CWallet* pwallet, DataStream& key, CDataStream& value, std::string& err) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) { + DBErrors result = DBErrors::LOAD_OK; + uint256 hash; + key >> hash; + // LoadToWallet call below creates a new CWalletTx that fill_wtx + // callback fills with transaction metadata. + auto fill_wtx = [&](CWalletTx& wtx, bool new_tx) { + if(!new_tx) { + // There's some corruption here since the tx we just tried to load was already in the wallet. + err = "Error: Corrupt transaction found. This can be fixed by removing transactions from wallet and rescanning."; + result = DBErrors::CORRUPT; + return false; + } + value >> wtx; + if (wtx.GetHash() != hash) + return false; + + // Undo serialize changes in 31600 + if (31404 <= wtx.fTimeReceivedIsTxTime && wtx.fTimeReceivedIsTxTime <= 31703) + { + if (!value.empty()) + { + uint8_t fTmp; + uint8_t fUnused; + std::string unused_string; + value >> fTmp >> fUnused >> unused_string; + pwallet->WalletLogPrintf("LoadWallet() upgrading tx ver=%d %d %s\n", + wtx.fTimeReceivedIsTxTime, fTmp, hash.ToString()); + wtx.fTimeReceivedIsTxTime = fTmp; + } + else + { + pwallet->WalletLogPrintf("LoadWallet() repairing tx ver=%d %s\n", wtx.fTimeReceivedIsTxTime, hash.ToString()); + wtx.fTimeReceivedIsTxTime = 0; + } + upgraded_txs.push_back(hash); + } + + if (wtx.nOrderPos == -1) + any_unordered = true; + + return true; + }; + if (!pwallet->LoadToWallet(hash, fill_wtx)) { + // Use std::max as fill_wtx may have already set result to CORRUPT + result = std::max(result, DBErrors::NEED_RESCAN); + } + return result; + }); + result = std::max(result, tx_res.m_result); + + // Load locked utxo record + LoadResult locked_utxo_res = LoadRecords(pwallet, batch, DBKeys::LOCKED_UTXO, + [] (CWallet* pwallet, DataStream& key, CDataStream& value, std::string& err) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) { + uint256 hash; + uint32_t n; + key >> hash; + key >> n; + pwallet->LockCoin(COutPoint(hash, n)); + return DBErrors::LOAD_OK; + }); + result = std::max(result, locked_utxo_res.m_result); + + // Load orderposnext record + // Note: There should only be one ORDERPOSNEXT record with nothing trailing the type + LoadResult order_pos_res = LoadRecords(pwallet, batch, DBKeys::ORDERPOSNEXT, + [] (CWallet* pwallet, DataStream& key, CDataStream& value, std::string& err) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) { + try { + value >> pwallet->nOrderPosNext; + } catch (const std::exception& e) { + err = e.what(); + return DBErrors::NONCRITICAL_ERROR; + } + return DBErrors::LOAD_OK; + }); + result = std::max(result, order_pos_res.m_result); + + return result; +} + DBErrors WalletBatch::LoadWallet(CWallet* pwallet) { CWalletScanState wss; bool fNoncriticalErrors = false; - bool rescan_required = false; DBErrors result = DBErrors::LOAD_OK; + bool any_unordered = false; + std::vector upgraded_txs; LOCK(pwallet->cs_wallet); @@ -1153,6 +1188,9 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) // Load address book result = std::max(LoadAddressBookRecords(pwallet, *m_batch), result); + // Load tx records + result = std::max(LoadTxRecords(pwallet, *m_batch, upgraded_txs, any_unordered), result); + // Get cursor std::unique_ptr cursor = m_batch->GetNewCursor(); if (!cursor) @@ -1184,17 +1222,9 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) if (strType == DBKeys::MASTER_KEY || strType == DBKeys::DEFAULTKEY) { result = DBErrors::CORRUPT; - } else if (wss.tx_corrupt) { - pwallet->WalletLogPrintf("Error: Corrupt transaction found. This can be fixed by removing transactions from wallet and rescanning.\n"); - // Set tx_corrupt back to false so that the error is only printed once (per corrupt tx) - wss.tx_corrupt = false; - result = DBErrors::CORRUPT; } else { // Leave other errors alone, if we try to fix them we might make things worse. fNoncriticalErrors = true; // ... but do warn the user there is something wrong. - if (strType == DBKeys::TX) - // Rescan if there is a bad transaction record: - rescan_required = true; } } if (!strErr.empty()) @@ -1214,9 +1244,7 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) pwallet->LoadActiveScriptPubKeyMan(spk_man_pair.second, spk_man_pair.first, /*internal=*/true); } - if (rescan_required && result == DBErrors::LOAD_OK) { - result = DBErrors::NEED_RESCAN; - } else if (fNoncriticalErrors && result == DBErrors::LOAD_OK) { + if (fNoncriticalErrors && result == DBErrors::LOAD_OK) { result = DBErrors::NONCRITICAL_ERROR; } @@ -1225,13 +1253,13 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) if (result != DBErrors::LOAD_OK) return result; - for (const uint256& hash : wss.vWalletUpgrade) + for (const uint256& hash : upgraded_txs) WriteTx(pwallet->mapWallet.at(hash)); if (!has_last_client || last_client != CLIENT_VERSION) // Update m_batch->Write(DBKeys::VERSION, CLIENT_VERSION); - if (wss.fAnyUnordered) + if (any_unordered) result = pwallet->ReorderTransactions(); // Upgrade all of the wallet keymetadata to have the hd master key id From c978c6d39cdeb78fc4720767b943d03d6a9a36d8 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Mon, 18 Apr 2022 14:26:26 -0400 Subject: [PATCH 12/16] walletdb: refactor active spkm loading Instead of loading active spkm records as we come across them when iterating the database, load them explicitly. Due to exception handling changes, deserialization errors are now treated as critical. --- src/wallet/walletdb.cpp | 54 ++++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 22 deletions(-) diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index f3a82734c1..20df4f589f 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -301,8 +301,6 @@ bool WalletBatch::EraseLockedUTXO(const COutPoint& output) class CWalletScanState { public: unsigned int m_unknown_records{0}; - std::map m_active_external_spks; - std::map m_active_internal_spks; CWalletScanState() = default; }; @@ -495,18 +493,6 @@ ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, strErr = "Found unsupported 'wkey' record, try loading with version 0.18"; return false; } else if (strType == DBKeys::ACTIVEEXTERNALSPK || strType == DBKeys::ACTIVEINTERNALSPK) { - uint8_t type; - ssKey >> type; - uint256 id; - ssValue >> id; - - bool internal = strType == DBKeys::ACTIVEINTERNALSPK; - auto& spk_mans = internal ? wss.m_active_internal_spks : wss.m_active_external_spks; - if (spk_mans.count(static_cast(type)) > 0) { - strErr = "Multiple ScriptPubKeyMans specified for a single type"; - return false; - } - spk_mans[static_cast(type)] = id; } else if (strType == DBKeys::WALLETDESCRIPTOR) { } else if (strType == DBKeys::WALLETDESCRIPTORCACHE) { } else if (strType == DBKeys::WALLETDESCRIPTORLHCACHE) { @@ -1146,6 +1132,35 @@ static DBErrors LoadTxRecords(CWallet* pwallet, DatabaseBatch& batch, std::vecto return result; } +static DBErrors LoadActiveSPKMs(CWallet* pwallet, DatabaseBatch& batch) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) +{ + AssertLockHeld(pwallet->cs_wallet); + DBErrors result = DBErrors::LOAD_OK; + + // Load spk records + std::set> seen_spks; + for (const auto& spk_key : {DBKeys::ACTIVEEXTERNALSPK, DBKeys::ACTIVEINTERNALSPK}) { + LoadResult spkm_res = LoadRecords(pwallet, batch, spk_key, + [&seen_spks, &spk_key] (CWallet* pwallet, DataStream& key, CDataStream& value, std::string& strErr) { + uint8_t output_type; + key >> output_type; + uint256 id; + value >> id; + + bool internal = spk_key == DBKeys::ACTIVEINTERNALSPK; + auto [it, insert] = seen_spks.emplace(static_cast(output_type), internal); + if (!insert) { + strErr = "Multiple ScriptpubKeyMans specified for a single type"; + return DBErrors::CORRUPT; + } + pwallet->LoadActiveScriptPubKeyMan(id, static_cast(output_type), /*internal=*/internal); + return DBErrors::LOAD_OK; + }); + result = std::max(result, spkm_res.m_result); + } + return result; +} + DBErrors WalletBatch::LoadWallet(CWallet* pwallet) { CWalletScanState wss; @@ -1191,6 +1206,9 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) // Load tx records result = std::max(LoadTxRecords(pwallet, *m_batch, upgraded_txs, any_unordered), result); + // Load SPKMs + result = std::max(LoadActiveSPKMs(pwallet, *m_batch), result); + // Get cursor std::unique_ptr cursor = m_batch->GetNewCursor(); if (!cursor) @@ -1236,14 +1254,6 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) result = DBErrors::CORRUPT; } - // Set the active ScriptPubKeyMans - for (auto spk_man_pair : wss.m_active_external_spks) { - pwallet->LoadActiveScriptPubKeyMan(spk_man_pair.second, spk_man_pair.first, /*internal=*/false); - } - for (auto spk_man_pair : wss.m_active_internal_spks) { - pwallet->LoadActiveScriptPubKeyMan(spk_man_pair.second, spk_man_pair.first, /*internal=*/true); - } - if (fNoncriticalErrors && result == DBErrors::LOAD_OK) { result = DBErrors::NONCRITICAL_ERROR; } From 31c033e5ca3b65f4f5345d5aa17aafedd637ef4f Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Mon, 18 Apr 2022 14:43:52 -0400 Subject: [PATCH 13/16] walletdb: refactor defaultkey and wkey loading Instead of dealing with these records when iterating the entire database, find and handle them explicitly. Loading of OLD_KEY records is bumped up to a LOAD_FAIL error as we will not be able to use these types of keys which can lead to users missing funds. --- src/wallet/walletdb.cpp | 44 ++++++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 20df4f589f..d48a5c01db 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -476,22 +476,12 @@ ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, } else if (strType == DBKeys::KEYMETA) { } else if (strType == DBKeys::WATCHMETA) { } else if (strType == DBKeys::DEFAULTKEY) { - // 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 - CPubKey vchPubKey; - ssValue >> vchPubKey; - if (!vchPubKey.IsValid()) { - strErr = "Error reading wallet database: Default Key corrupt"; - return false; - } } else if (strType == DBKeys::POOL) { } else if (strType == DBKeys::CSCRIPT) { } else if (strType == DBKeys::ORDERPOSNEXT) { } else if (strType == DBKeys::DESTDATA) { } else if (strType == DBKeys::HDCHAIN) { } else if (strType == DBKeys::OLD_KEY) { - strErr = "Found unsupported 'wkey' record, try loading with version 0.18"; - return false; } else if (strType == DBKeys::ACTIVEEXTERNALSPK || strType == DBKeys::ACTIVEINTERNALSPK) { } else if (strType == DBKeys::WALLETDESCRIPTOR) { } else if (strType == DBKeys::WALLETDESCRIPTORCACHE) { @@ -794,6 +784,37 @@ static DBErrors LoadLegacyWalletRecords(CWallet* pwallet, DatabaseBatch& batch, }); result = std::max(result, pool_res.m_result); + // Deal with old "wkey" and "defaultkey" records. + // These are not actually loaded, but we need to check for them + + // 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, + [] (CWallet* pwallet, DataStream& key, CDataStream& value, std::string& err) { + CPubKey default_pubkey; + try { + value >> default_pubkey; + } catch (const std::exception& e) { + err = e.what(); + return DBErrors::CORRUPT; + } + if (!default_pubkey.IsValid()) { + err = "Error reading wallet database: Default Key corrupt"; + return DBErrors::CORRUPT; + } + return DBErrors::LOAD_OK; + }); + 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, + [] (CWallet* pwallet, DataStream& key, CDataStream& value, std::string& err) { + err = "Found unsupported 'wkey' record, try loading with version 0.18"; + return DBErrors::LOAD_FAIL; + }); + result = std::max(result, wkey_res.m_result); + if (result <= DBErrors::NONCRITICAL_ERROR) { // Only do logging and time first key update if there were no critical errors pwallet->WalletLogPrintf("Legacy Wallet Keys: %u plaintext, %u encrypted, %u w/ metadata, %u total.\n", @@ -1237,8 +1258,7 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) { // losing keys is considered a catastrophic error, anything else // we assume the user can live with: - if (strType == DBKeys::MASTER_KEY || - strType == DBKeys::DEFAULTKEY) { + if (strType == DBKeys::MASTER_KEY) { result = DBErrors::CORRUPT; } else { // Leave other errors alone, if we try to fix them we might make things worse. From cd211b3b9965b5070d68adc1a03043d82d904d5b Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Mon, 18 Apr 2022 15:08:51 -0400 Subject: [PATCH 14/16] walletdb: refactor decryption key loading Instead of loading decryption keys as we iterate the database, load them explicitly. --- src/wallet/walletdb.cpp | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index d48a5c01db..fb1e4f8fb5 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -471,7 +471,6 @@ ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, } else if (strType == DBKeys::WATCHS) { } else if (strType == DBKeys::KEY) { } else if (strType == DBKeys::MASTER_KEY) { - if (!LoadEncryptionKey(pwallet, ssKey, ssValue, strErr)) return false; } else if (strType == DBKeys::CRYPTED_KEY) { } else if (strType == DBKeys::KEYMETA) { } else if (strType == DBKeys::WATCHMETA) { @@ -1182,6 +1181,21 @@ static DBErrors LoadActiveSPKMs(CWallet* pwallet, DatabaseBatch& batch) EXCLUSIV return result; } +static DBErrors LoadDecryptionKeys(CWallet* pwallet, DatabaseBatch& batch) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) +{ + AssertLockHeld(pwallet->cs_wallet); + + // Load decryption key (mkey) records + LoadResult mkey_res = LoadRecords(pwallet, batch, DBKeys::MASTER_KEY, + [] (CWallet* pwallet, DataStream& key, CDataStream& value, std::string& err) { + if (!LoadEncryptionKey(pwallet, key, value, err)) { + return DBErrors::CORRUPT; + } + return DBErrors::LOAD_OK; + }); + return mkey_res.m_result; +} + DBErrors WalletBatch::LoadWallet(CWallet* pwallet) { CWalletScanState wss; @@ -1230,6 +1244,9 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) // Load SPKMs result = std::max(LoadActiveSPKMs(pwallet, *m_batch), result); + // Load decryption keys + result = std::max(LoadDecryptionKeys(pwallet, *m_batch), result); + // Get cursor std::unique_ptr cursor = m_batch->GetNewCursor(); if (!cursor) @@ -1256,14 +1273,8 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) std::string strType, strErr; if (!ReadKeyValue(pwallet, ssKey, ssValue, wss, strType, strErr)) { - // losing keys is considered a catastrophic error, anything else - // we assume the user can live with: - if (strType == DBKeys::MASTER_KEY) { - result = DBErrors::CORRUPT; - } else { - // Leave other errors alone, if we try to fix them we might make things worse. - fNoncriticalErrors = true; // ... but do warn the user there is something wrong. - } + // Leave other errors alone, if we try to fix them we might make things worse. + fNoncriticalErrors = true; // ... but do warn the user there is something wrong. } if (!strErr.empty()) pwallet->WalletLogPrintf("%s\n", strErr); From 2636844f5353797a0b8e40a879652a0d345172ad Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Mon, 18 Apr 2022 14:56:23 -0400 Subject: [PATCH 15/16] walletdb: Remove loading code where the database is iterated Instead of iterating the database to load the wallet, we now load particular kinds of records in an order that we want them to be loaded. So it is no longer necessary to iterate the entire database to load the wallet. --- src/wallet/walletdb.cpp | 101 ---------------------------------------- 1 file changed, 101 deletions(-) diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index fb1e4f8fb5..2aee750ced 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -298,13 +298,6 @@ bool WalletBatch::EraseLockedUTXO(const COutPoint& output) return EraseIC(std::make_pair(DBKeys::LOCKED_UTXO, std::make_pair(output.hash, output.n))); } -class CWalletScanState { -public: - unsigned int m_unknown_records{0}; - - CWalletScanState() = default; -}; - bool LoadKey(CWallet* pwallet, DataStream& ssKey, DataStream& ssValue, std::string& strErr) { LOCK(pwallet->cs_wallet); @@ -453,61 +446,6 @@ bool LoadHDChain(CWallet* pwallet, DataStream& ssValue, std::string& strErr) return true; } -static bool -ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, - CWalletScanState &wss, std::string& strType, std::string& strErr) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) -{ - try { - // Unserialize - // Taking advantage of the fact that pair serialization - // is just the two items serialized one after the other - ssKey >> strType; - if (pwallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS) && DBKeys::LEGACY_TYPES.count(strType) > 0) { - return true; - } - if (strType == DBKeys::NAME) { - } else if (strType == DBKeys::PURPOSE) { - } else if (strType == DBKeys::TX) { - } else if (strType == DBKeys::WATCHS) { - } else if (strType == DBKeys::KEY) { - } else if (strType == DBKeys::MASTER_KEY) { - } else if (strType == DBKeys::CRYPTED_KEY) { - } else if (strType == DBKeys::KEYMETA) { - } else if (strType == DBKeys::WATCHMETA) { - } else if (strType == DBKeys::DEFAULTKEY) { - } else if (strType == DBKeys::POOL) { - } else if (strType == DBKeys::CSCRIPT) { - } else if (strType == DBKeys::ORDERPOSNEXT) { - } else if (strType == DBKeys::DESTDATA) { - } else if (strType == DBKeys::HDCHAIN) { - } else if (strType == DBKeys::OLD_KEY) { - } else if (strType == DBKeys::ACTIVEEXTERNALSPK || strType == DBKeys::ACTIVEINTERNALSPK) { - } else if (strType == DBKeys::WALLETDESCRIPTOR) { - } else if (strType == DBKeys::WALLETDESCRIPTORCACHE) { - } else if (strType == DBKeys::WALLETDESCRIPTORLHCACHE) { - } else if (strType == DBKeys::WALLETDESCRIPTORKEY) { - } else if (strType == DBKeys::WALLETDESCRIPTORCKEY) { - } else if (strType == DBKeys::LOCKED_UTXO) { - } else if (strType != DBKeys::BESTBLOCK && strType != DBKeys::BESTBLOCK_NOMERKLE && - strType != DBKeys::MINVERSION && strType != DBKeys::ACENTRY && - strType != DBKeys::VERSION && strType != DBKeys::SETTINGS && - strType != DBKeys::FLAGS) { - wss.m_unknown_records++; - } - } catch (const std::exception& e) { - if (strErr.empty()) { - strErr = e.what(); - } - return false; - } catch (...) { - if (strErr.empty()) { - strErr = "Caught unknown exception in ReadKeyValue"; - } - return false; - } - return true; -} - static DBErrors LoadMinVersion(CWallet* pwallet, DatabaseBatch& batch) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) { AssertLockHeld(pwallet->cs_wallet); @@ -1198,8 +1136,6 @@ static DBErrors LoadDecryptionKeys(CWallet* pwallet, DatabaseBatch& batch) EXCLU DBErrors WalletBatch::LoadWallet(CWallet* pwallet) { - CWalletScanState wss; - bool fNoncriticalErrors = false; DBErrors result = DBErrors::LOAD_OK; bool any_unordered = false; std::vector upgraded_txs; @@ -1246,49 +1182,12 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) // Load decryption keys result = std::max(LoadDecryptionKeys(pwallet, *m_batch), result); - - // Get cursor - std::unique_ptr cursor = m_batch->GetNewCursor(); - if (!cursor) - { - pwallet->WalletLogPrintf("Error getting wallet database cursor\n"); - return DBErrors::CORRUPT; - } - - while (true) - { - // Read next record - DataStream ssKey{}; - CDataStream ssValue(SER_DISK, CLIENT_VERSION); - DatabaseCursor::Status status = cursor->Next(ssKey, ssValue); - if (status == DatabaseCursor::Status::DONE) { - break; - } else if (status == DatabaseCursor::Status::FAIL) { - cursor.reset(); - pwallet->WalletLogPrintf("Error reading next record from wallet database\n"); - return DBErrors::CORRUPT; - } - - // Try to be tolerant of single corrupt records: - std::string strType, strErr; - if (!ReadKeyValue(pwallet, ssKey, ssValue, wss, strType, strErr)) - { - // Leave other errors alone, if we try to fix them we might make things worse. - fNoncriticalErrors = true; // ... but do warn the user there is something wrong. - } - if (!strErr.empty()) - pwallet->WalletLogPrintf("%s\n", strErr); - } } catch (...) { // Exceptions that can be ignored or treated as non-critical are handled by the individual loading functions. // Any uncaught exceptions will be caught here and treated as critical. result = DBErrors::CORRUPT; } - if (fNoncriticalErrors && result == DBErrors::LOAD_OK) { - result = DBErrors::NONCRITICAL_ERROR; - } - // Any wallet corruption at all: skip any rewriting or // upgrading, we don't want to make it worse. if (result != DBErrors::LOAD_OK) From 3c83b1d884b419adece95b335b6e956e7459a7ef Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 21 Jun 2023 16:33:12 -0400 Subject: [PATCH 16/16] doc: Add release note for wallet loading changes Co-Authored-By: Ryan Ofsky --- doc/release-notes-24914.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 doc/release-notes-24914.md diff --git a/doc/release-notes-24914.md b/doc/release-notes-24914.md new file mode 100644 index 0000000000..505d356fce --- /dev/null +++ b/doc/release-notes-24914.md @@ -0,0 +1,9 @@ +Wallet +------ + +- Wallet loading has changed in this release. Wallets with some corrupted records that could be + previously loaded (with warnings) may no longer load. For example, wallets with corrupted + address book entries may no longer load. If this happens, it is recommended + load the wallet in a previous version of Bitcoin Core and import the data into a new wallet. + Please also report an issue to help improve the software and make wallet loading more robust + in these cases.