Merge cbffd662ae into 85f96b01b7

src/bench/wallet_migration.cpp

@@ -65,7 +65,7 @@ static void WalletMigration(benchmark::Bench& bench)
     }
 
     bench.epochs(/*numEpochs=*/1).run([&context, &wallet] {
-        util::Result<MigrationResult> res = MigrateLegacyToDescriptor(std::move(wallet), /*passphrase=*/"", context, /*was_loaded=*/false);
+        util::Result<MigrationResult> res = MigrateLegacyToDescriptor(std::move(wallet), /*passphrase=*/"", context, /*was_loaded=*/false, /*in_memory=*/false);
         assert(res);
         assert(res->wallet);
         assert(res->watchonly_wallet);

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<DatabaseFormat> require_format;
     uint64_t create_flags = 0;
     SecureString create_passphrase;

src/wallet/sqlite.cpp

@@ -278,9 +278,11 @@ void SQLiteDatabase::Open()
 
     // Acquire an exclusive lock on the database
     // First change the locking mode to exclusive
+    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.
-    int ret = sqlite3_exec(m_db, "BEGIN EXCLUSIVE TRANSACTION", nullptr, nullptr, nullptr);
+        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");
         }
@@ -288,6 +290,7 @@ void SQLiteDatabase::Open()
         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
     SetPragma(m_db, "fullfsync", "true", "Failed to enable fullfsync");
@@ -695,7 +698,7 @@ std::unique_ptr<SQLiteDatabase> MakeSQLiteDatabase(const fs::path& path, const D
 {
     try {
         fs::path data_file = SQLiteDataFile(path);
-        auto db = std::make_unique<SQLiteDatabase>(data_file.parent_path(), data_file, options);
+        auto db = std::make_unique<SQLiteDatabase>(data_file.parent_path(), data_file, options, options.in_memory ? true : false);
         if (options.verify && !db->Verify(error)) {
             status = DatabaseStatus::FAILED_VERIFY;
             return nullptr;

src/wallet/test/fuzz/scriptpubkeyman.cpp

@@ -21,7 +21,9 @@
 #include <util/check.h>
 #include <util/time.h>
 #include <util/translation.h>
+#include <util/string.h>
 #include <validation.h>
+#include <wallet/context.h>
 #include <wallet/scriptpubkeyman.h>
 #include <wallet/test/util.h>
 #include <wallet/types.h>
@@ -50,6 +52,13 @@ void initialize_spkm()
     MOCKED_DESC_CONVERTER.Init();
 }
 
+void initialize_spkm_migration()
+{
+    static const auto testing_setup{MakeNoLogFileContext<const TestingSetup>()};
+    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<ArgsManager*>(&args);
+    context.chain = g_setup->m_node.chain.get();
+
+    std::unique_ptr<CWallet> wallet_ptr{std::make_unique<CWallet>(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<CKey> 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<CScript> script_opt{ConsumeDeserializable<CScript>(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<size_t>(1, MAX_PUBKEYS_PER_MULTISIG)};
+                std::vector<CPubKey> 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<CMutableTransaction> mtx{ConsumeDeserializable<CMutableTransaction>(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<size_t>(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

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<WalletDatabase> 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<MigrationResult> 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<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet> local_wallet, const SecureString& passphrase, WalletContext& context, bool was_loaded)
+util::Result<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet> local_wallet, const SecureString& passphrase, WalletContext& context, bool was_loaded, bool in_memory)
 {
     MigrationResult res;
     bilingual_str error;
@@ -4516,7 +4517,7 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet>
     {
         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);
@@ -4551,9 +4552,9 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet>
             success = reload_wallet(res.solvables_wallet);
         }
     }
-    if (!success) {
+    if (!success && !in_memory) {
         // 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<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet>
         }
 
         // Restore the backup
-        // Convert the backup file to the wallet db file by renaming it and moving it into the wallet's directory.
+        DatabaseStatus status;
-        // Reload it into memory if the wallet was previously loaded.
+        std::vector<bilingual_str> warnings;
-        bilingual_str restore_error;
+        if (!RestoreWallet(context, temp_backup_location, wallet_name, /*load_on_start=*/std::nullopt, status, error, warnings)) {
-        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);
+            error += _("\nUnable to restore backup of wallet.");
-        if (!restore_error.empty()) {
-            error += restore_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;

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<MigrationData> 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<MigrationResult> 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<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet> local_wallet, const SecureString& passphrase, WalletContext& context, bool was_loaded);
+[[nodiscard]] util::Result<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet> local_wallet, const SecureString& passphrase, WalletContext& context, bool was_loaded, bool in_memory);
 } // namespace wallet
 
 #endif // BITCOIN_WALLET_WALLET_H

src/wallet/walletdb.cpp

@@ -1401,7 +1401,7 @@ std::unique_ptr<WalletDatabase> 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;