From 0de4e329bb48b403b8e7cf64b6ce642fc82d0f70 Mon Sep 17 00:00:00 2001 From: brunoerg Date: Tue, 10 Dec 2024 17:52:19 -0300 Subject: [PATCH 1/4] wallet: add option to create in-memory SQLite DB in the migration --- src/bench/wallet_migration.cpp | 2 +- src/wallet/db.h | 2 ++ src/wallet/sqlite.cpp | 2 +- src/wallet/wallet.cpp | 28 +++++++++++----------------- src/wallet/wallet.h | 4 ++-- src/wallet/walletdb.cpp | 2 +- 6 files changed, 18 insertions(+), 22 deletions(-) diff --git a/src/bench/wallet_migration.cpp b/src/bench/wallet_migration.cpp index 524839e387..a6398518c2 100644 --- a/src/bench/wallet_migration.cpp +++ b/src/bench/wallet_migration.cpp @@ -65,7 +65,7 @@ static void WalletMigration(benchmark::Bench& bench) } bench.epochs(/*numEpochs=*/1).run([&context, &wallet] { - util::Result res = MigrateLegacyToDescriptor(std::move(wallet), /*passphrase=*/"", context, /*was_loaded=*/false); + util::Result res = MigrateLegacyToDescriptor(std::move(wallet), /*passphrase=*/"", context, /*was_loaded=*/false, /*in_memory=*/false); assert(res); assert(res->wallet); assert(res->watchonly_wallet); diff --git a/src/wallet/db.h b/src/wallet/db.h index e8790006a4..44bcec206d 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -186,11 +186,13 @@ enum class DatabaseFormat { SQLITE, BERKELEY_RO, BERKELEY_SWAP, + MOCK }; struct DatabaseOptions { bool require_existing = false; bool require_create = false; + bool in_memory = false; std::optional require_format; uint64_t create_flags = 0; SecureString create_passphrase; diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp index a8c9f8a8ab..a2dad030ab 100644 --- a/src/wallet/sqlite.cpp +++ b/src/wallet/sqlite.cpp @@ -695,7 +695,7 @@ std::unique_ptr MakeSQLiteDatabase(const fs::path& path, const D { try { fs::path data_file = SQLiteDataFile(path); - auto db = std::make_unique(data_file.parent_path(), data_file, options); + auto db = std::make_unique(data_file.parent_path(), data_file, options, options.in_memory ? true : false); if (options.verify && !db->Verify(error)) { status = DatabaseStatus::FAILED_VERIFY; return nullptr; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index dd9dfcd516..ede66910e0 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3989,7 +3989,7 @@ ScriptPubKeyMan* CWallet::AddWalletDescriptor(WalletDescriptor& desc, const Flat return spk_man; } -bool CWallet::MigrateToSQLite(bilingual_str& error) +bool CWallet::MigrateToSQLite(bilingual_str& error, bool in_memory) { AssertLockHeld(cs_wallet); @@ -4040,6 +4040,7 @@ bool CWallet::MigrateToSQLite(bilingual_str& error) DatabaseOptions opts; opts.require_create = true; opts.require_format = DatabaseFormat::SQLITE; + opts.in_memory = in_memory; DatabaseStatus db_status; std::unique_ptr new_db = MakeDatabase(wallet_path, opts, db_status, error); assert(new_db); // This is to prevent doing anything further with this wallet. The original file was deleted, but a backup exists. @@ -4451,10 +4452,10 @@ util::Result MigrateLegacyToDescriptor(const std::string& walle return util::Error{Untranslated("Wallet loading failed.") + Untranslated(" ") + error}; } - return MigrateLegacyToDescriptor(std::move(local_wallet), passphrase, context, was_loaded); + return MigrateLegacyToDescriptor(std::move(local_wallet), passphrase, context, was_loaded, /*in_memory=*/false); } -util::Result MigrateLegacyToDescriptor(std::shared_ptr local_wallet, const SecureString& passphrase, WalletContext& context, bool was_loaded) +util::Result MigrateLegacyToDescriptor(std::shared_ptr local_wallet, const SecureString& passphrase, WalletContext& context, bool was_loaded, bool in_memory) { MigrationResult res; bilingual_str error; @@ -4516,7 +4517,7 @@ util::Result MigrateLegacyToDescriptor(std::shared_ptr { LOCK(local_wallet->cs_wallet); // First change to using SQLite - if (!local_wallet->MigrateToSQLite(error)) return util::Error{error}; + if (!local_wallet->MigrateToSQLite(error, in_memory)) return util::Error{error}; // Do the migration of keys and scripts for non-blank wallets, and cleanup if it fails success = local_wallet->IsWalletFlagSet(WALLET_FLAG_BLANK_WALLET); @@ -4553,7 +4554,7 @@ util::Result MigrateLegacyToDescriptor(std::shared_ptr } if (!success) { // Migration failed, cleanup - // Before deleting the wallet's directory, copy the backup file to the top-level wallets dir + // Copy the backup to the actual wallet dir fs::path temp_backup_location = fsbridge::AbsPathJoin(GetWalletDir(), backup_filename); fs::copy_file(backup_path, temp_backup_location, fs::copy_options::none); @@ -4590,24 +4591,17 @@ util::Result MigrateLegacyToDescriptor(std::shared_ptr } // Restore the backup - // Convert the backup file to the wallet db file by renaming it and moving it into the wallet's directory. - // Reload it into memory if the wallet was previously loaded. - bilingual_str restore_error; - const auto& ptr_wallet = RestoreWallet(context, temp_backup_location, wallet_name, /*load_on_start=*/std::nullopt, status, restore_error, warnings, /*load_after_restore=*/was_loaded); - if (!restore_error.empty()) { - error += restore_error + _("\nUnable to restore backup of wallet."); + DatabaseStatus status; + std::vector warnings; + if (!RestoreWallet(context, temp_backup_location, wallet_name, /*load_on_start=*/std::nullopt, status, error, warnings)) { + error += _("\nUnable to restore backup of wallet."); return util::Error{error}; } - // The wallet directory has been restored, but just in case, copy the previously created backup to the wallet dir + // Move the backup to the wallet dir fs::copy_file(temp_backup_location, backup_path, fs::copy_options::none); fs::remove(temp_backup_location); - // Verify that there is no dangling wallet: when the wallet wasn't loaded before, expect null. - // This check is performed after restoration to avoid an early error before saving the backup. - bool wallet_reloaded = ptr_wallet != nullptr; - assert(was_loaded == wallet_reloaded); - return util::Error{error}; } return res; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 0afe71a40e..a59e8e0278 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1045,7 +1045,7 @@ public: * A backup is not created. * May crash if something unexpected happens in the filesystem. */ - bool MigrateToSQLite(bilingual_str& error) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool MigrateToSQLite(bilingual_str& error, bool in_memory = false) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! Get all of the descriptors from a legacy wallet std::optional GetDescriptorsForLegacy(bilingual_str& error) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); @@ -1136,7 +1136,7 @@ struct MigrationResult { //! Do all steps to migrate a legacy wallet to a descriptor wallet [[nodiscard]] util::Result MigrateLegacyToDescriptor(const std::string& wallet_name, const SecureString& passphrase, WalletContext& context); //! Requirement: The wallet provided to this function must be isolated, with no attachment to the node's context. -[[nodiscard]] util::Result MigrateLegacyToDescriptor(std::shared_ptr local_wallet, const SecureString& passphrase, WalletContext& context, bool was_loaded); +[[nodiscard]] util::Result MigrateLegacyToDescriptor(std::shared_ptr local_wallet, const SecureString& passphrase, WalletContext& context, bool was_loaded, bool in_memory); } // namespace wallet #endif // BITCOIN_WALLET_WALLET_H diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 801c2ab97b..c06489b884 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -1401,7 +1401,7 @@ std::unique_ptr MakeDatabase(const fs::path& path, const Databas { bool exists; try { - exists = fs::symlink_status(path).type() != fs::file_type::not_found; + exists = fs::symlink_status(path).type() != fs::file_type::not_found && !options.in_memory; } catch (const fs::filesystem_error& e) { error = Untranslated(strprintf("Failed to access database path '%s': %s", fs::PathToString(path), fsbridge::get_filesystem_error_message(e))); status = DatabaseStatus::FAILED_BAD_PATH; From 88ceaddf9154fd9d3ac944a8580ba8ed37ebd54c Mon Sep 17 00:00:00 2001 From: brunoerg Date: Tue, 10 Dec 2024 18:18:21 -0300 Subject: [PATCH 2/4] wallet: sqlite: there is no need to have exclusive locking when mocking For in-memory SQLite databases, exclusive locking is generally not necessary because in-memory dbs are private to the connection that created them. --- src/wallet/sqlite.cpp | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp index a2dad030ab..e3ce34882a 100644 --- a/src/wallet/sqlite.cpp +++ b/src/wallet/sqlite.cpp @@ -278,15 +278,18 @@ void SQLiteDatabase::Open() // Acquire an exclusive lock on the database // First change the locking mode to exclusive - SetPragma(m_db, "locking_mode", "exclusive", "Unable to change database locking mode to exclusive"); - // Now begin a transaction to acquire the exclusive lock. This lock won't be released until we close because of the exclusive locking mode. - int ret = sqlite3_exec(m_db, "BEGIN EXCLUSIVE TRANSACTION", nullptr, nullptr, nullptr); - if (ret != SQLITE_OK) { - throw std::runtime_error("SQLiteDatabase: Unable to obtain an exclusive lock on the database, is it being used by another instance of " CLIENT_NAME "?\n"); - } - ret = sqlite3_exec(m_db, "COMMIT", nullptr, nullptr, nullptr); - if (ret != SQLITE_OK) { - throw std::runtime_error(strprintf("SQLiteDatabase: Unable to end exclusive lock transaction: %s\n", sqlite3_errstr(ret))); + int ret; + if (!m_mock) { + SetPragma(m_db, "locking_mode", "exclusive", "Unable to change database locking mode to exclusive"); + // Now begin a transaction to acquire the exclusive lock. This lock won't be released until we close because of the exclusive locking mode. + ret = sqlite3_exec(m_db, "BEGIN EXCLUSIVE TRANSACTION", nullptr, nullptr, nullptr); + if (ret != SQLITE_OK) { + throw std::runtime_error("SQLiteDatabase: Unable to obtain an exclusive lock on the database, is it being used by another instance of " CLIENT_NAME "?\n"); + } + ret = sqlite3_exec(m_db, "COMMIT", nullptr, nullptr, nullptr); + if (ret != SQLITE_OK) { + throw std::runtime_error(strprintf("SQLiteDatabase: Unable to end exclusive lock transaction: %s\n", sqlite3_errstr(ret))); + } } // Enable fullfsync for the platforms that use it From 34d03a102a4b89c631679c3932bfc9f03d223507 Mon Sep 17 00:00:00 2001 From: brunoerg Date: Tue, 10 Dec 2024 19:02:20 -0300 Subject: [PATCH 3/4] wallet: skip backup restoration when using in-memory dbs By using in-memory databases, we do not have dbs file to delete and restore. --- src/wallet/wallet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ede66910e0..37a873f70f 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4552,7 +4552,7 @@ util::Result MigrateLegacyToDescriptor(std::shared_ptr success = reload_wallet(res.solvables_wallet); } } - if (!success) { + if (!success && !in_memory) { // Migration failed, cleanup // Copy the backup to the actual wallet dir fs::path temp_backup_location = fsbridge::AbsPathJoin(GetWalletDir(), backup_filename); From cbffd662ae6f2bbbd983b3980488835fcf63613d Mon Sep 17 00:00:00 2001 From: brunoerg Date: Sat, 9 Mar 2024 06:22:50 -0300 Subject: [PATCH 4/4] fuzz: wallet: add target for spkm migration --- src/wallet/test/fuzz/scriptpubkeyman.cpp | 141 +++++++++++++++++++++++ 1 file changed, 141 insertions(+) diff --git a/src/wallet/test/fuzz/scriptpubkeyman.cpp b/src/wallet/test/fuzz/scriptpubkeyman.cpp index 0c17a6bf7a..239a2fcc04 100644 --- a/src/wallet/test/fuzz/scriptpubkeyman.cpp +++ b/src/wallet/test/fuzz/scriptpubkeyman.cpp @@ -21,7 +21,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -50,6 +52,13 @@ void initialize_spkm() MOCKED_DESC_CONVERTER.Init(); } +void initialize_spkm_migration() +{ + static const auto testing_setup{MakeNoLogFileContext()}; + g_setup = testing_setup.get(); + SelectParams(ChainType::MAIN); +} + /** * Key derivation is expensive. Deriving deep derivation paths take a lot of compute and we'd rather spend time * elsewhere in this target, like on actually fuzzing the DescriptorScriptPubKeyMan. So rule out strings which could @@ -203,5 +212,137 @@ FUZZ_TARGET(scriptpubkeyman, .init = initialize_spkm) (void)spk_manager->GetKeyPoolSize(); } +FUZZ_TARGET(spkm_migration, .init = initialize_spkm_migration) +{ + SeedRandomStateForTest(SeedRand::ZEROS); + FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; + SetMockTime(ConsumeTime(fuzzed_data_provider)); + const auto& node{g_setup->m_node}; + Chainstate& chainstate{node.chainman->ActiveChainstate()}; + WalletContext context; + auto& args{g_setup->m_args}; + context.args = const_cast(&args); + context.chain = g_setup->m_node.chain.get(); + + std::unique_ptr wallet_ptr{std::make_unique(node.chain.get(), "", CreateMockableWalletDatabase())}; + wallet_ptr->chainStateFlushed(ChainstateRole::NORMAL, CBlockLocator{}); + CWallet& wallet{*wallet_ptr}; + wallet.m_keypool_size = 1; + { + LOCK(wallet.cs_wallet); + wallet.UnsetWalletFlag(WALLET_FLAG_DESCRIPTORS); + wallet.SetLastBlockProcessed(chainstate.m_chain.Height(), chainstate.m_chain.Tip()->GetBlockHash()); + } + + auto& legacy_data{*wallet.GetOrCreateLegacyDataSPKM()}; + + std::vector keys; + LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 30) { + const auto key{ConsumePrivateKey(fuzzed_data_provider)}; + if (!key.IsValid()) return; + auto pub_key{key.GetPubKey()}; + if (!pub_key.IsFullyValid()) return; + if (legacy_data.LoadKey(key, pub_key) && std::find(keys.begin(), keys.end(), key) == keys.end()) keys.push_back(key); + } + + bool add_hd_chain{fuzzed_data_provider.ConsumeBool() && !keys.empty()}; + CHDChain hd_chain; + CKey hd_key; + if (add_hd_chain) { + hd_key = PickValue(fuzzed_data_provider, keys); + hd_chain.nVersion = fuzzed_data_provider.ConsumeBool() ? CHDChain::VERSION_HD_CHAIN_SPLIT : CHDChain::VERSION_HD_BASE; + hd_chain.seed_id = hd_key.GetPubKey().GetID(); + legacy_data.LoadHDChain(hd_chain); + } + + bool good_data{true}; + LIMITED_WHILE(good_data && fuzzed_data_provider.ConsumeBool(), 30) { + CallOneOf( + fuzzed_data_provider, + [&] { + CKey private_key{ConsumePrivateKey(fuzzed_data_provider)}; + if (!private_key.IsValid()) return; + const auto& dest{GetDestinationForKey(private_key.GetPubKey(), OutputType::LEGACY)}; + (void)legacy_data.LoadWatchOnly(GetScriptForDestination(dest)); + }, + [&] { + CKey key; + if (!keys.empty()) { + key = PickValue(fuzzed_data_provider, keys); + } else { + key = ConsumePrivateKey(fuzzed_data_provider, /*compressed=*/fuzzed_data_provider.ConsumeBool()); + } + if (!key.IsValid()) return; + auto pub_key{key.GetPubKey()}; + CScript script; + CallOneOf( + fuzzed_data_provider, + [&] { + script = GetScriptForDestination(CTxDestination{PKHash(pub_key)}); + }, + [&] { + script = GetScriptForDestination(WitnessV0KeyHash(pub_key)); + }, + [&] { + std::optional script_opt{ConsumeDeserializable(fuzzed_data_provider)}; + if (!script_opt) { + good_data = false; + return; + } + script = script_opt.value(); + } + ); + (void)legacy_data.AddCScript(script); + }, + [&] { + CKey key; + if (!keys.empty()) { + key = PickValue(fuzzed_data_provider, keys); + } else { + key = ConsumePrivateKey(fuzzed_data_provider, /*compressed=*/fuzzed_data_provider.ConsumeBool()); + } + if (!key.IsValid()) return; + const auto num_keys{fuzzed_data_provider.ConsumeIntegralInRange(1, MAX_PUBKEYS_PER_MULTISIG)}; + std::vector pubkeys; + for (size_t i = 0; i < num_keys; i++) { + if (fuzzed_data_provider.ConsumeBool()) { + pubkeys.emplace_back(key.GetPubKey()); + } else { + CKey private_key{ConsumePrivateKey(fuzzed_data_provider, /*compressed=*/fuzzed_data_provider.ConsumeBool())}; + if (!private_key.IsValid()) return; + pubkeys.emplace_back(private_key.GetPubKey()); + } + } + if (pubkeys.size() < num_keys) return; + CScript multisig_script{GetScriptForMultisig(num_keys, pubkeys)}; + (void)legacy_data.AddCScript(multisig_script); + } + ); + } + + good_data = true; + LIMITED_WHILE(fuzzed_data_provider.ConsumeBool() && good_data, 50) { + std::optional mtx{ConsumeDeserializable(fuzzed_data_provider, TX_WITH_WITNESS)}; + if (!mtx) { + good_data = false; + return; + } + if (!mtx->vout.empty() && fuzzed_data_provider.ConsumeBool()) { + const auto idx{fuzzed_data_provider.ConsumeIntegralInRange(0, mtx->vout.size() - 1)}; + const auto output_type{fuzzed_data_provider.PickValueInArray({OutputType::BECH32, OutputType::LEGACY})}; + const auto label{fuzzed_data_provider.ConsumeRandomLengthString()}; + CScript spk{GetScriptForDestination(*Assert(wallet.GetNewDestination(output_type, label)))}; + mtx->vout[idx].scriptPubKey = spk; + } + if (mtx->vin.size() != 1 || !mtx->vin[0].prevout.IsNull()) { + wallet.AddToWallet(MakeTransactionRef(*mtx), TxStateInactive{}, /*update_wtx=*/nullptr, /*fFlushOnClose=*/false, /*rescanning_old_block=*/true); + } + } + + MigrationResult result; + bilingual_str error; + (void)MigrateLegacyToDescriptor(std::move(wallet_ptr), /*passphrase=*/"", context, /*was_loaded=*/false, /*in_memory=*/true); +} + } // namespace } // namespace wallet