diff --git a/src/bench/CMakeLists.txt b/src/bench/CMakeLists.txt index 8a52980e072..45b65100446 100644 --- a/src/bench/CMakeLists.txt +++ b/src/bench/CMakeLists.txt @@ -71,6 +71,7 @@ if(ENABLE_WALLET) wallet_create_tx.cpp wallet_loading.cpp wallet_ismine.cpp + wallet_migration.cpp ) target_link_libraries(bench_bitcoin bitcoin_wallet) endif() diff --git a/src/bench/wallet_migration.cpp b/src/bench/wallet_migration.cpp new file mode 100644 index 00000000000..eff6c6b526d --- /dev/null +++ b/src/bench/wallet_migration.cpp @@ -0,0 +1,80 @@ +// Copyright (c) 2024 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php. + +#include // IWYU pragma: keep + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#if defined(USE_BDB) && defined(USE_SQLITE) // only enable benchmark when bdb and sqlite are enabled + +namespace wallet{ + +static void WalletMigration(benchmark::Bench& bench) +{ + const auto test_setup = MakeNoLogFileContext(); + + WalletContext context; + context.args = &test_setup->m_args; + context.chain = test_setup->m_node.chain.get(); + + // Number of imported watch only addresses + int NUM_WATCH_ONLY_ADDR = 20; + + // Setup legacy wallet + DatabaseOptions options; + options.use_unsafe_sync = true; + options.verify = false; + DatabaseStatus status; + bilingual_str error; + auto database = MakeWalletDatabase(fs::PathToString(test_setup->m_path_root / "legacy"), options, status, error); + uint64_t create_flags = 0; + auto wallet = TestLoadWallet(std::move(database), context, create_flags); + + // Add watch-only addresses + std::vector scripts_watch_only; + for (int w = 0; w < NUM_WATCH_ONLY_ADDR; ++w) { + CKey key = GenerateRandomKey(); + LOCK(wallet->cs_wallet); + const CScript& script = scripts_watch_only.emplace_back(GetScriptForDestination(GetDestinationForKey(key.GetPubKey(), OutputType::LEGACY))); + bool res = wallet->ImportScriptPubKeys(strprintf("watch_%d", w), {script}, + /*have_solving_data=*/false, /*apply_label=*/true, /*timestamp=*/1); + assert(res); + } + + // Generate transactions and local addresses + for (int j = 0; j < 400; ++j) { + CMutableTransaction mtx; + mtx.vout.emplace_back(COIN, GetScriptForDestination(*Assert(wallet->GetNewDestination(OutputType::BECH32, strprintf("bench_%d", j))))); + mtx.vout.emplace_back(COIN, GetScriptForDestination(*Assert(wallet->GetNewDestination(OutputType::LEGACY, strprintf("legacy_%d", j))))); + mtx.vout.emplace_back(COIN, scripts_watch_only.at(j % NUM_WATCH_ONLY_ADDR)); + mtx.vin.resize(2); + wallet->AddToWallet(MakeTransactionRef(mtx), TxStateInactive{}, /*update_wtx=*/nullptr, /*fFlushOnClose=*/false, /*rescanning_old_block=*/true); + } + + // Unload so the migration process loads it + TestUnloadWallet(std::move(wallet)); + + bench.epochs(/*numEpochs=*/1).run([&] { + util::Result res = MigrateLegacyToDescriptor(fs::PathToString(test_setup->m_path_root / "legacy"), "", context); + assert(res); + assert(res->wallet); + assert(res->watchonly_wallet); + }); +} + +BENCHMARK(WalletMigration, benchmark::PriorityLevel::LOW); + +} // namespace wallet + +#endif // end USE_SQLITE && USE_BDB diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index af0c78f0d9e..f3fe8a19c19 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -208,6 +208,7 @@ class BerkeleyBatch : public DatabaseBatch bool TxnBegin() override; bool TxnCommit() override; bool TxnAbort() override; + bool HasActiveTxn() override { return activeTxn != nullptr; } DbTxn* txn() const { return activeTxn; } }; diff --git a/src/wallet/db.h b/src/wallet/db.h index 049af8dd19e..e8790006a4d 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -122,6 +122,7 @@ class DatabaseBatch virtual bool TxnBegin() = 0; virtual bool TxnCommit() = 0; virtual bool TxnAbort() = 0; + virtual bool HasActiveTxn() = 0; }; /** An instance of this class represents one database. diff --git a/src/wallet/migrate.h b/src/wallet/migrate.h index 58c8c0adf4e..16eadeb019d 100644 --- a/src/wallet/migrate.h +++ b/src/wallet/migrate.h @@ -115,6 +115,7 @@ class BerkeleyROBatch : public DatabaseBatch bool TxnBegin() override { return false; } bool TxnCommit() override { return false; } bool TxnAbort() override { return false; } + bool HasActiveTxn() override { return false; } }; //! Return object giving access to Berkeley Read Only database at specified path. diff --git a/src/wallet/salvage.cpp b/src/wallet/salvage.cpp index 04c02b0dcc6..0ac1b66897e 100644 --- a/src/wallet/salvage.cpp +++ b/src/wallet/salvage.cpp @@ -44,6 +44,7 @@ class DummyBatch : public DatabaseBatch bool TxnBegin() override { return true; } bool TxnCommit() override { return true; } bool TxnAbort() override { return true; } + bool HasActiveTxn() override { return false; } }; /** A dummy WalletDatabase that does nothing and never fails. Only used by salvage. diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 46ec5dc1acc..62384056dc6 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -1807,6 +1807,12 @@ std::optional LegacyDataSPKM::MigrateToDescriptor() keyid_it++; } + WalletBatch batch(m_storage.GetDatabase()); + if (!batch.TxnBegin()) { + LogPrintf("Error generating descriptors for migration, cannot initialize db transaction\n"); + return std::nullopt; + } + // keyids is now all non-HD keys. Each key will have its own combo descriptor for (const CKeyID& keyid : keyids) { CKey key; @@ -1837,8 +1843,8 @@ std::optional LegacyDataSPKM::MigrateToDescriptor() // Make the DescriptorScriptPubKeyMan and get the scriptPubKeys auto desc_spk_man = std::make_unique(m_storage, w_desc, /*keypool_size=*/0); - desc_spk_man->AddDescriptorKey(key, key.GetPubKey()); - desc_spk_man->TopUp(); + WITH_LOCK(desc_spk_man->cs_desc_man, desc_spk_man->AddDescriptorKeyWithDB(batch, key, key.GetPubKey())); + desc_spk_man->TopUpWithDB(batch); auto desc_spks = desc_spk_man->GetScriptPubKeys(); // Remove the scriptPubKeys from our current set @@ -1883,8 +1889,8 @@ std::optional LegacyDataSPKM::MigrateToDescriptor() // Make the DescriptorScriptPubKeyMan and get the scriptPubKeys auto desc_spk_man = std::make_unique(m_storage, w_desc, /*keypool_size=*/0); - desc_spk_man->AddDescriptorKey(master_key.key, master_key.key.GetPubKey()); - desc_spk_man->TopUp(); + WITH_LOCK(desc_spk_man->cs_desc_man, desc_spk_man->AddDescriptorKeyWithDB(batch, master_key.key, master_key.key.GetPubKey())); + desc_spk_man->TopUpWithDB(batch); auto desc_spks = desc_spk_man->GetScriptPubKeys(); // Remove the scriptPubKeys from our current set @@ -1950,9 +1956,9 @@ std::optional LegacyDataSPKM::MigrateToDescriptor() if (!GetKey(keyid, key)) { continue; } - desc_spk_man->AddDescriptorKey(key, key.GetPubKey()); + WITH_LOCK(desc_spk_man->cs_desc_man, desc_spk_man->AddDescriptorKeyWithDB(batch, key, key.GetPubKey())); } - desc_spk_man->TopUp(); + desc_spk_man->TopUpWithDB(batch); auto desc_spks_set = desc_spk_man->GetScriptPubKeys(); desc_spks.insert(desc_spks.end(), desc_spks_set.begin(), desc_spks_set.end()); @@ -2019,13 +2025,26 @@ std::optional LegacyDataSPKM::MigrateToDescriptor() // Make sure that we have accounted for all scriptPubKeys assert(spks.size() == 0); + + // Finalize transaction + if (!batch.TxnCommit()) { + LogPrintf("Error generating descriptors for migration, cannot commit db transaction\n"); + return std::nullopt; + } + return out; } bool LegacyDataSPKM::DeleteRecords() +{ + return RunWithinTxn(m_storage.GetDatabase(), /*process_desc=*/"delete legacy records", [&](WalletBatch& batch){ + return DeleteRecordsWithDB(batch); + }); +} + +bool LegacyDataSPKM::DeleteRecordsWithDB(WalletBatch& batch) { LOCK(cs_KeyStore); - WalletBatch batch(m_storage.GetDatabase()); return batch.EraseRecords(DBKeys::LEGACY_TYPES); } diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index cf7b7eaf31d..d8b6c90178a 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -366,8 +366,9 @@ class LegacyDataSPKM : public ScriptPubKeyMan, public FillableSigningProvider /** Get the DescriptorScriptPubKeyMans (with private keys) that have the same scriptPubKeys as this LegacyScriptPubKeyMan. * Does not modify this ScriptPubKeyMan. */ std::optional MigrateToDescriptor(); - /** Delete all the records ofthis LegacyScriptPubKeyMan from disk*/ + /** Delete all the records of this LegacyScriptPubKeyMan from disk*/ bool DeleteRecords(); + bool DeleteRecordsWithDB(WalletBatch& batch); }; // Implements the full legacy wallet behavior @@ -582,6 +583,7 @@ class LegacySigningProvider : public SigningProvider class DescriptorScriptPubKeyMan : public ScriptPubKeyMan { + friend class LegacyDataSPKM; private: using ScriptPubKeyMap = std::map; // Map of scripts to descriptor range index using PubKeyMap = std::map; // Map of pubkeys involved in scripts to descriptor range index diff --git a/src/wallet/sqlite.h b/src/wallet/sqlite.h index 6b84f34366f..78a3accf890 100644 --- a/src/wallet/sqlite.h +++ b/src/wallet/sqlite.h @@ -95,6 +95,7 @@ class SQLiteBatch : public DatabaseBatch bool TxnBegin() override; bool TxnCommit() override; bool TxnAbort() override; + bool HasActiveTxn() override { return m_txn; } }; /** An instance of this class represents one SQLite3 database. diff --git a/src/wallet/test/util.h b/src/wallet/test/util.h index ba12f5f6bf1..c8a89c0e642 100644 --- a/src/wallet/test/util.h +++ b/src/wallet/test/util.h @@ -95,6 +95,7 @@ class MockableBatch : public DatabaseBatch bool TxnBegin() override { return m_pass; } bool TxnCommit() override { return m_pass; } bool TxnAbort() override { return m_pass; } + bool HasActiveTxn() override { return false; } }; /** A WalletDatabase whose contents and return values can be modified as needed for testing diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index de565102cc2..652d6e9f6d8 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1701,10 +1701,16 @@ bool CWallet::CanGetAddresses(bool internal) const } void CWallet::SetWalletFlag(uint64_t flags) +{ + WalletBatch batch(GetDatabase()); + return SetWalletFlagWithDB(batch, flags); +} + +void CWallet::SetWalletFlagWithDB(WalletBatch& batch, uint64_t flags) { LOCK(cs_wallet); m_wallet_flags |= flags; - if (!WalletBatch(GetDatabase()).WriteWalletFlags(m_wallet_flags)) + if (!batch.WriteWalletFlags(m_wallet_flags)) throw std::runtime_error(std::string(__func__) + ": writing wallet flags failed"); } @@ -2382,8 +2388,21 @@ DBErrors CWallet::LoadWallet() util::Result CWallet::RemoveTxs(std::vector& txs_to_remove) { AssertLockHeld(cs_wallet); - WalletBatch batch(GetDatabase()); - if (!batch.TxnBegin()) return util::Error{_("Error starting db txn for wallet transactions removal")}; + bilingual_str str_err; // future: make RunWithinTxn return a util::Result + bool was_txn_committed = RunWithinTxn(GetDatabase(), /*process_desc=*/"remove transactions", [&](WalletBatch& batch) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { + util::Result result{RemoveTxs(batch, txs_to_remove)}; + if (!result) str_err = util::ErrorString(result); + return result.has_value(); + }); + if (!str_err.empty()) return util::Error{str_err}; + if (!was_txn_committed) return util::Error{_("Error starting/committing db txn for wallet transactions removal process")}; + return {}; // all good +} + +util::Result CWallet::RemoveTxs(WalletBatch& batch, std::vector& txs_to_remove) +{ + AssertLockHeld(cs_wallet); + if (!batch.HasActiveTxn()) return util::Error{strprintf(_("The transactions removal process can only be executed within a db txn"))}; // Check for transaction existence and remove entries from disk using TxIterator = std::unordered_map::const_iterator; @@ -2392,38 +2411,30 @@ util::Result CWallet::RemoveTxs(std::vector& txs_to_remove) for (const uint256& hash : txs_to_remove) { auto it_wtx = mapWallet.find(hash); if (it_wtx == mapWallet.end()) { - str_err = strprintf(_("Transaction %s does not belong to this wallet"), hash.GetHex()); - break; + return util::Error{strprintf(_("Transaction %s does not belong to this wallet"), hash.GetHex())}; } if (!batch.EraseTx(hash)) { - str_err = strprintf(_("Failure removing transaction: %s"), hash.GetHex()); - break; + return util::Error{strprintf(_("Failure removing transaction: %s"), hash.GetHex())}; } erased_txs.emplace_back(it_wtx); } - // Roll back removals in case of an error - if (!str_err.empty()) { - batch.TxnAbort(); - return util::Error{str_err}; - } - - // Dump changes to disk - if (!batch.TxnCommit()) return util::Error{_("Error committing db txn for wallet transactions removal")}; - - // Update the in-memory state and notify upper layers about the removals - for (const auto& it : erased_txs) { - const uint256 hash{it->first}; - wtxOrdered.erase(it->second.m_it_wtxOrdered); - for (const auto& txin : it->second.tx->vin) - mapTxSpends.erase(txin.prevout); - mapWallet.erase(it); - NotifyTransactionChanged(hash, CT_DELETED); - } + // Register callback to update the memory state only when the db txn is actually dumped to disk + batch.RegisterTxnListener({.on_commit=[&, erased_txs]() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { + // Update the in-memory state and notify upper layers about the removals + for (const auto& it : erased_txs) { + const uint256 hash{it->first}; + wtxOrdered.erase(it->second.m_it_wtxOrdered); + for (const auto& txin : it->second.tx->vin) + mapTxSpends.erase(txin.prevout); + mapWallet.erase(it); + NotifyTransactionChanged(hash, CT_DELETED); + } - MarkDirty(); + MarkDirty(); + }, .on_abort={}}); - return {}; // all good + return {}; } bool CWallet::SetAddressBookWithDB(WalletBatch& batch, const CTxDestination& address, const std::string& strName, const std::optional& new_purpose) @@ -3736,22 +3747,30 @@ DescriptorScriptPubKeyMan& CWallet::SetupDescriptorScriptPubKeyMan(WalletBatch& return *out; } -void CWallet::SetupDescriptorScriptPubKeyMans(const CExtKey& master_key) +void CWallet::SetupDescriptorScriptPubKeyMans(WalletBatch& batch, const CExtKey& master_key) { AssertLockHeld(cs_wallet); - - // Create single batch txn - WalletBatch batch(GetDatabase()); - if (!batch.TxnBegin()) throw std::runtime_error("Error: cannot create db transaction for descriptors setup"); - for (bool internal : {false, true}) { for (OutputType t : OUTPUT_TYPES) { SetupDescriptorScriptPubKeyMan(batch, master_key, t, internal); } } +} + +void CWallet::SetupOwnDescriptorScriptPubKeyMans(WalletBatch& batch) +{ + AssertLockHeld(cs_wallet); + assert(!IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER)); + // Make a seed + CKey seed_key = GenerateRandomKey(); + CPubKey seed = seed_key.GetPubKey(); + assert(seed_key.VerifyPubKey(seed)); - // Ensure information is committed to disk - if (!batch.TxnCommit()) throw std::runtime_error("Error: cannot commit db transaction for descriptors setup"); + // Get the extended key + CExtKey master_key; + master_key.SetSeed(seed_key); + + SetupDescriptorScriptPubKeyMans(batch, master_key); } void CWallet::SetupDescriptorScriptPubKeyMans() @@ -3759,16 +3778,10 @@ void CWallet::SetupDescriptorScriptPubKeyMans() AssertLockHeld(cs_wallet); if (!IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER)) { - // Make a seed - CKey seed_key = GenerateRandomKey(); - CPubKey seed = seed_key.GetPubKey(); - assert(seed_key.VerifyPubKey(seed)); - - // Get the extended key - CExtKey master_key; - master_key.SetSeed(seed_key); - - SetupDescriptorScriptPubKeyMans(master_key); + if (!RunWithinTxn(GetDatabase(), /*process_desc=*/"setup descriptors", [&](WalletBatch& batch) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet){ + SetupOwnDescriptorScriptPubKeyMans(batch); + return true; + })) throw std::runtime_error("Error: cannot process db transaction for descriptors setup"); } else { ExternalSigner signer = ExternalSignerScriptPubKeyMan::GetExternalSigner(); @@ -4055,15 +4068,14 @@ std::optional CWallet::GetDescriptorsForLegacy(bilingual_str& err return res; } -bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) +util::Result CWallet::ApplyMigrationData(WalletBatch& local_wallet_batch, MigrationData& data) { AssertLockHeld(cs_wallet); LegacyDataSPKM* legacy_spkm = GetLegacyDataSPKM(); if (!Assume(legacy_spkm)) { // This shouldn't happen - error = Untranslated(STR_INTERNAL_BUG("Error: Legacy wallet data missing")); - return false; + return util::Error{Untranslated(STR_INTERNAL_BUG("Error: Legacy wallet data missing"))}; } // Get all invalid or non-watched scripts that will not be migrated @@ -4077,16 +4089,15 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) for (auto& desc_spkm : data.desc_spkms) { if (m_spk_managers.count(desc_spkm->GetID()) > 0) { - error = _("Error: Duplicate descriptors created during migration. Your wallet may be corrupted."); - return false; + return util::Error{_("Error: Duplicate descriptors created during migration. Your wallet may be corrupted.")}; } uint256 id = desc_spkm->GetID(); AddScriptPubKeyMan(id, std::move(desc_spkm)); } // Remove the LegacyScriptPubKeyMan from disk - if (!legacy_spkm->DeleteRecords()) { - return false; + if (!legacy_spkm->DeleteRecordsWithDB(local_wallet_batch)) { + return util::Error{_("Error: cannot remove legacy wallet records")}; } // Remove the LegacyScriptPubKeyMan from memory @@ -4095,22 +4106,21 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) m_internal_spk_managers.clear(); // Setup new descriptors - SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + SetWalletFlagWithDB(local_wallet_batch, WALLET_FLAG_DESCRIPTORS); if (!IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { // Use the existing master key if we have it if (data.master_key.key.IsValid()) { - SetupDescriptorScriptPubKeyMans(data.master_key); + SetupDescriptorScriptPubKeyMans(local_wallet_batch, data.master_key); } else { // Setup with a new seed if we don't. - SetupDescriptorScriptPubKeyMans(); + SetupOwnDescriptorScriptPubKeyMans(local_wallet_batch); } } // Get best block locator so that we can copy it to the watchonly and solvables CBlockLocator best_block_locator; - if (!WalletBatch(GetDatabase()).ReadBestBlock(best_block_locator)) { - error = _("Error: Unable to read wallet's best block locator record"); - return false; + if (!local_wallet_batch.ReadBestBlock(best_block_locator)) { + return util::Error{_("Error: Unable to read wallet's best block locator record")}; } // Check if the transactions in the wallet are still ours. Either they belong here, or they belong in the watchonly wallet. @@ -4119,21 +4129,23 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) std::unique_ptr watchonly_batch; if (data.watchonly_wallet) { watchonly_batch = std::make_unique(data.watchonly_wallet->GetDatabase()); + if (!watchonly_batch->TxnBegin()) return util::Error{strprintf(_("Error: database transaction cannot be executed for wallet %s"), data.watchonly_wallet->GetName())}; // Copy the next tx order pos to the watchonly wallet LOCK(data.watchonly_wallet->cs_wallet); data.watchonly_wallet->nOrderPosNext = nOrderPosNext; watchonly_batch->WriteOrderPosNext(data.watchonly_wallet->nOrderPosNext); // Write the best block locator to avoid rescanning on reload if (!watchonly_batch->WriteBestBlock(best_block_locator)) { - error = _("Error: Unable to write watchonly wallet best block locator record"); - return false; + return util::Error{_("Error: Unable to write watchonly wallet best block locator record")}; } } + std::unique_ptr solvables_batch; if (data.solvable_wallet) { + solvables_batch = std::make_unique(data.solvable_wallet->GetDatabase()); + if (!solvables_batch->TxnBegin()) return util::Error{strprintf(_("Error: database transaction cannot be executed for wallet %s"), data.solvable_wallet->GetName())}; // Write the best block locator to avoid rescanning on reload - if (!WalletBatch(data.solvable_wallet->GetDatabase()).WriteBestBlock(best_block_locator)) { - error = _("Error: Unable to write solvable wallet best block locator record"); - return false; + if (!solvables_batch->WriteBestBlock(best_block_locator)) { + return util::Error{_("Error: Unable to write solvable wallet best block locator record")}; } } for (const auto& [_pos, wtx] : wtxOrdered) { @@ -4152,8 +4164,7 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) ins_wtx.CopyFrom(to_copy_wtx); return true; })) { - error = strprintf(_("Error: Could not add watchonly tx %s to watchonly wallet"), wtx->GetHash().GetHex()); - return false; + return util::Error{strprintf(_("Error: Could not add watchonly tx %s to watchonly wallet"), wtx->GetHash().GetHex())}; } watchonly_batch->WriteTx(data.watchonly_wallet->mapWallet.at(hash)); // Mark as to remove from the migrated wallet only if it does not also belong to it @@ -4165,31 +4176,21 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) } if (!is_mine) { // Both not ours and not in the watchonly wallet - error = strprintf(_("Error: Transaction %s in wallet cannot be identified to belong to migrated wallets"), wtx->GetHash().GetHex()); - return false; + return util::Error{strprintf(_("Error: Transaction %s in wallet cannot be identified to belong to migrated wallets"), wtx->GetHash().GetHex())}; } } - watchonly_batch.reset(); // Flush + // Do the removes if (txids_to_delete.size() > 0) { - if (auto res = RemoveTxs(txids_to_delete); !res) { - error = _("Error: Could not delete watchonly transactions. ") + util::ErrorString(res); - return false; + if (auto res = RemoveTxs(local_wallet_batch, txids_to_delete); !res) { + return util::Error{_("Error: Could not delete watchonly transactions. ") + util::ErrorString(res)}; } } // Pair external wallets with their corresponding db handler std::vector, std::unique_ptr>> wallets_vec; - for (const auto& ext_wallet : {data.watchonly_wallet, data.solvable_wallet}) { - if (!ext_wallet) continue; - - std::unique_ptr batch = std::make_unique(ext_wallet->GetDatabase()); - if (!batch->TxnBegin()) { - error = strprintf(_("Error: database transaction cannot be executed for wallet %s"), ext_wallet->GetName()); - return false; - } - wallets_vec.emplace_back(ext_wallet, std::move(batch)); - } + if (data.watchonly_wallet) wallets_vec.emplace_back(data.watchonly_wallet, std::move(watchonly_batch)); + if (data.solvable_wallet) wallets_vec.emplace_back(data.solvable_wallet, std::move(solvables_batch)); // Write address book entry to disk auto func_store_addr = [](WalletBatch& batch, const CTxDestination& dest, const CAddressBookData& entry) { @@ -4236,39 +4237,27 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) continue; } - error = _("Error: Address book data in wallet cannot be identified to belong to migrated wallets"); - return false; + return util::Error{_("Error: Address book data in wallet cannot be identified to belong to migrated wallets")}; } } // Persist external wallets address book entries for (auto& [wallet, batch] : wallets_vec) { if (!batch->TxnCommit()) { - error = strprintf(_("Error: address book copy failed for wallet %s"), wallet->GetName()); - return false; + return util::Error{strprintf(_("Error: Unable to write data to disk for wallet %s"), wallet->GetName())}; } } // Remove the things to delete in this wallet - WalletBatch local_wallet_batch(GetDatabase()); - local_wallet_batch.TxnBegin(); if (dests_to_delete.size() > 0) { for (const auto& dest : dests_to_delete) { if (!DelAddressBookWithDB(local_wallet_batch, dest)) { - error = _("Error: Unable to remove watchonly address book data"); - return false; + return util::Error{_("Error: Unable to remove watchonly address book data")}; } } } - local_wallet_batch.TxnCommit(); - - // Connect the SPKM signals - ConnectScriptPubKeyManNotifiers(); - NotifyCanGetAddressesChanged(); - - WalletLogPrintf("Wallet migration complete.\n"); - return true; + return {}; // all good } bool CWallet::CanGrindR() const @@ -4379,10 +4368,14 @@ bool DoMigration(CWallet& wallet, WalletContext& context, bilingual_str& error, } // Add the descriptors to wallet, remove LegacyScriptPubKeyMan, and cleanup txs and address book data - if (!wallet.ApplyMigrationData(*data, error)) { - return false; - } - return true; + return RunWithinTxn(wallet.GetDatabase(), /*process_desc=*/"apply migration process", [&](WalletBatch& batch) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet){ + if (auto res_migration = wallet.ApplyMigrationData(batch, *data); !res_migration) { + error = util::ErrorString(res_migration); + return false; + } + wallet.WalletLogPrintf("Wallet migration complete.\n"); + return true; + }); } util::Result MigrateLegacyToDescriptor(const std::string& wallet_name, const SecureString& passphrase, WalletContext& context) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index d3a7208b15e..d869f031bbf 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -422,6 +422,9 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati // Same as 'AddActiveScriptPubKeyMan' but designed for use within a batch transaction context void AddActiveScriptPubKeyManWithDb(WalletBatch& batch, uint256 id, OutputType type, bool internal); + /** Store wallet flags */ + void SetWalletFlagWithDB(WalletBatch& batch, uint64_t flags); + //! Cache of descriptor ScriptPubKeys used for IsMine. Maps ScriptPubKey to set of spkms std::unordered_map, SaltedSipHasher> m_cached_spks; @@ -791,6 +794,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati /** Erases the provided transactions from the wallet. */ util::Result RemoveTxs(std::vector& txs_to_remove) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + util::Result RemoveTxs(WalletBatch& batch, std::vector& txs_to_remove) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool SetAddressBook(const CTxDestination& address, const std::string& strName, const std::optional& purpose); @@ -1018,9 +1022,12 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati //! Create new DescriptorScriptPubKeyMan and add it to the wallet DescriptorScriptPubKeyMan& SetupDescriptorScriptPubKeyMan(WalletBatch& batch, const CExtKey& master_key, const OutputType& output_type, bool internal) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! Create new DescriptorScriptPubKeyMans and add them to the wallet - void SetupDescriptorScriptPubKeyMans(const CExtKey& master_key) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + void SetupDescriptorScriptPubKeyMans(WalletBatch& batch, const CExtKey& master_key) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void SetupDescriptorScriptPubKeyMans() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + //! Create new seed and default DescriptorScriptPubKeyMans for this wallet + void SetupOwnDescriptorScriptPubKeyMans(WalletBatch& batch) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + //! Return the DescriptorScriptPubKeyMan for a WalletDescriptor if it is already in the wallet DescriptorScriptPubKeyMan* GetDescriptorScriptPubKeyMan(const WalletDescriptor& desc) const; @@ -1044,7 +1051,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati //! Adds the ScriptPubKeyMans given in MigrationData to this wallet, removes LegacyScriptPubKeyMan, //! and where needed, moves tx and address book entries to watchonly_wallet or solvable_wallet - bool ApplyMigrationData(MigrationData& data, bilingual_str& error) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + util::Result ApplyMigrationData(WalletBatch& local_wallet_batch, MigrationData& data) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! Whether the (external) signer performs R-value signature grinding bool CanGrindR() const; diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 597a4ef9a4c..b09a0e40eb4 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -1335,10 +1335,8 @@ bool WalletBatch::WriteWalletFlags(const uint64_t flags) bool WalletBatch::EraseRecords(const std::unordered_set& types) { - return RunWithinTxn(*this, "erase records", [&types](WalletBatch& self) { - return std::all_of(types.begin(), types.end(), [&self](const std::string& type) { - return self.m_batch->ErasePrefix(DataStream() << type); - }); + return std::all_of(types.begin(), types.end(), [&](const std::string& type) { + return m_batch->ErasePrefix(DataStream() << type); }); } @@ -1349,12 +1347,34 @@ bool WalletBatch::TxnBegin() bool WalletBatch::TxnCommit() { - return m_batch->TxnCommit(); + bool res = m_batch->TxnCommit(); + if (res) { + for (const auto& listener : m_txn_listeners) { + listener.on_commit(); + } + // txn finished, clear listeners + m_txn_listeners.clear(); + } + return res; } bool WalletBatch::TxnAbort() { - return m_batch->TxnAbort(); + bool res = m_batch->TxnAbort(); + if (res) { + for (const auto& listener : m_txn_listeners) { + listener.on_abort(); + } + // txn finished, clear listeners + m_txn_listeners.clear(); + } + return res; +} + +void WalletBatch::RegisterTxnListener(const DbTxnListener& l) +{ + assert(m_batch->HasActiveTxn()); + m_txn_listeners.emplace_back(l); } std::unique_ptr MakeDatabase(const fs::path& path, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error) diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index bffcc87202a..32c3c29b5e3 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -180,6 +180,11 @@ class CKeyMetadata } }; +struct DbTxnListener +{ + std::function on_commit, on_abort; +}; + /** Access to the wallet database. * Opens the database and provides read and write access to it. Each read and write is its own transaction. * Multiple operation transactions can be started using TxnBegin() and committed using TxnCommit() @@ -292,9 +297,18 @@ class WalletBatch bool TxnCommit(); //! Abort current transaction bool TxnAbort(); + bool HasActiveTxn() { return m_batch->HasActiveTxn(); } + + //! Registers db txn callback functions + void RegisterTxnListener(const DbTxnListener& l); + private: std::unique_ptr m_batch; WalletDatabase& m_database; + + // External functions listening to the current db txn outcome. + // Listeners are cleared at the end of the transaction. + std::vector m_txn_listeners; }; /**