From ee43743f126120b1dc2a601f149a88052f39152b Mon Sep 17 00:00:00 2001 From: Pablo Martin Date: Sat, 4 Jul 2026 15:30:13 -0300 Subject: [PATCH 1/3] wallet: store m_additional_flags in SQLiteDatabase to fix reopen path SQLiteDatabase::Open() (the public override) always reopens the database with no additional flags. If SQLiteBatch::Close() triggers the force_conn_refresh path (TxnAbort failed), it calls Open() which drops the original additional_flags, causing in-memory databases to be reopened as on-disk instead. Store additional_flags as a member and use it in Open() so the reconnect preserves the original flags. For in-memory databases, connection recovery makes no sense as all data would be lost; both the force_conn_refresh path and the public Open() now throw instead. Co-authored-by: Sjors Provoost --- src/wallet/sqlite.cpp | 12 +++++++++--- src/wallet/sqlite.h | 4 ++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp index 8d39fbae2e1..28123ecf4f4 100644 --- a/src/wallet/sqlite.cpp +++ b/src/wallet/sqlite.cpp @@ -116,7 +116,7 @@ SQLiteDatabase::SQLiteDatabase(const fs::path& dir_path, const fs::path& file_pa {} SQLiteDatabase::SQLiteDatabase(const fs::path& dir_path, const fs::path& file_path, const DatabaseOptions& options, int additional_flags) - : WalletDatabase(), m_dir_path(dir_path), m_file_path(fs::PathToString(file_path)), m_write_semaphore(1), m_use_unsafe_sync(options.use_unsafe_sync) + : WalletDatabase(), m_dir_path(dir_path), m_file_path(fs::PathToString(file_path)), m_additional_flags(additional_flags), m_write_semaphore(1), m_use_unsafe_sync(options.use_unsafe_sync) { { LOCK(g_sqlite_mutex); @@ -139,7 +139,7 @@ SQLiteDatabase::SQLiteDatabase(const fs::path& dir_path, const fs::path& file_pa } try { - Open(additional_flags); + Open(m_additional_flags); } catch (const std::runtime_error&) { // If open fails, cleanup this object and rethrow the exception Cleanup(); @@ -247,7 +247,10 @@ bool SQLiteDatabase::Verify(bilingual_str& error) void SQLiteDatabase::Open() { - Open(/*additional_flags*/0); + if (m_additional_flags & SQLITE_OPEN_MEMORY) { + throw std::runtime_error("SQLiteDatabase: Cannot reopen an in-memory database"); + } + Open(m_additional_flags); } void SQLiteDatabase::Open(int additional_flags) @@ -448,6 +451,9 @@ void SQLiteBatch::Close() } if (force_conn_refresh) { + if (m_database.m_additional_flags & SQLITE_OPEN_MEMORY) { + throw std::runtime_error("SQLiteDatabase: Cannot recover in-memory database connection"); + } m_database.Close(); try { m_database.Open(); diff --git a/src/wallet/sqlite.h b/src/wallet/sqlite.h index fb0fa39c8bd..3a35bb24bba 100644 --- a/src/wallet/sqlite.h +++ b/src/wallet/sqlite.h @@ -103,10 +103,14 @@ public: class SQLiteDatabase : public WalletDatabase { private: + friend class SQLiteBatch; + const fs::path m_dir_path; const std::string m_file_path; + const int m_additional_flags; + /** * This mutex protects SQLite initialization and shutdown. * sqlite3_config() and sqlite3_shutdown() are not thread-safe (sqlite3_initialize() is). From d1e7f8c986fbe78e5a9d986ea05ca43cff796b5d Mon Sep 17 00:00:00 2001 From: Pablo Martin Date: Sat, 4 Jul 2026 15:30:37 -0300 Subject: [PATCH 2/3] wallet: use in-memory SQLite for temporary wallet in exportwatchonlywallet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The intermediate watchonly wallet created during exportwatchonlywallet is a pure build artifact — it is always discarded once BackupWallet() copies it to the destination. Creating it as an in-memory SQLiteDatabase (SQLITE_OPEN_MEMORY) removes the need to write files to the wallets directory and eliminates the cleanup handler that deleted those files on both success and failure paths. Introduces InMemoryWalletDatabase (a minimal SQLiteDatabase subclass) and MakeInMemoryWalletDatabase() factory in sqlite.h/cpp, following the same pattern as MockableSQLiteDatabase / CreateMockableWalletDatabase() in the test utilities. MockableSQLiteDatabase now derives from InMemoryWalletDatabase, removing its redundant Files() override. The wallet is named after the source wallet ("_watchonly_temp") so concurrent exports of different wallets use distinct names and log lines remain traceable to the source wallet. --- src/wallet/export.cpp | 32 ++++---------------------------- src/wallet/sqlite.cpp | 9 +++++++++ src/wallet/sqlite.h | 11 +++++++++++ src/wallet/test/util.cpp | 2 +- src/wallet/test/util.h | 3 +-- 5 files changed, 26 insertions(+), 31 deletions(-) diff --git a/src/wallet/export.cpp b/src/wallet/export.cpp index 6f170dd728e..1df51b6b260 100644 --- a/src/wallet/export.cpp +++ b/src/wallet/export.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -69,40 +70,15 @@ util::Result ExportWatchOnlyWallet(const CWallet& wallet, const fs: return util::Error{_("Error: Wallet has no descriptors to export")}; } - // Setup DatabaseOptions to create a new sqlite database - DatabaseOptions options; - options.require_existing = false; - options.require_create = true; - options.require_format = DatabaseFormat::SQLITE; - // Make the wallet with the same flags as this wallet, but without private keys - options.create_flags = wallet.GetWalletFlags() | WALLET_FLAG_DISABLE_PRIVATE_KEYS; + const uint64_t create_flags = wallet.GetWalletFlags() | WALLET_FLAG_DISABLE_PRIVATE_KEYS; - // Make the watchonly wallet - DatabaseStatus status; + // Create the temporary watchonly wallet in memory to avoid leaving files on disk std::vector warnings; - std::string wallet_name = wallet.GetName() + "_watchonly_temp"; bilingual_str error; - std::unique_ptr database = MakeWalletDatabase(wallet_name, options, status, error); - if (!database) { - return util::Error{strprintf(_("Wallet file creation failed: %s"), error)}; - } - - // Always remove the temporary wallet files, even when returning early on error. - std::shared_ptr watchonly_wallet; - fs::path wallet_path = fs::PathFromString(database->Filename()).parent_path(); - std::vector cleanup_files = database->Files(); - auto cleanup_watchonly_wallet = interfaces::MakeCleanupHandler([&watchonly_wallet, &wallet_path, &cleanup_files] { - if (watchonly_wallet) watchonly_wallet.reset(); - for (const auto& file : cleanup_files) { - fs::remove(file); - } - fs::remove(wallet_path); - }); - WalletContext empty_context; empty_context.args = context.args; - watchonly_wallet = CWallet::CreateNew(empty_context, wallet_name, std::move(database), options.create_flags, /*born_encrypted=*/false, error, warnings); + std::shared_ptr watchonly_wallet = CWallet::CreateNew(empty_context, /*name=*/wallet.GetName() + "_watchonly_temp", MakeInMemoryWalletDatabase(), create_flags, /*born_encrypted=*/false, error, warnings); if (!watchonly_wallet) { return util::Error{strprintf(_("Error: Failed to create new watchonly wallet. %s"), error)}; } diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp index 28123ecf4f4..fa2abc93a51 100644 --- a/src/wallet/sqlite.cpp +++ b/src/wallet/sqlite.cpp @@ -722,6 +722,15 @@ std::unique_ptr MakeSQLiteDatabase(const fs::path& path, const D } } +InMemoryWalletDatabase::InMemoryWalletDatabase() + : SQLiteDatabase(fs::path{}, fs::path{":memory:"}, DatabaseOptions(), SQLITE_OPEN_MEMORY) +{} + +std::unique_ptr MakeInMemoryWalletDatabase() +{ + return std::make_unique(); +} + std::string SQLiteDatabaseVersion() { return std::string(sqlite3_libversion()); diff --git a/src/wallet/sqlite.h b/src/wallet/sqlite.h index 3a35bb24bba..f3268b4b490 100644 --- a/src/wallet/sqlite.h +++ b/src/wallet/sqlite.h @@ -175,8 +175,19 @@ public: bool m_use_unsafe_sync; }; +/** An in-memory SQLiteDatabase. Used as a temporary build artifact where no + * on-disk persistence is needed. */ +class InMemoryWalletDatabase : public SQLiteDatabase +{ +public: + InMemoryWalletDatabase(); + std::vector Files() override { return {}; } +}; + std::unique_ptr MakeSQLiteDatabase(const fs::path& path, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error); +std::unique_ptr MakeInMemoryWalletDatabase(); + std::string SQLiteDatabaseVersion(); } // namespace wallet diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp index 43cfd40fabd..19281be63bd 100644 --- a/src/wallet/test/util.cpp +++ b/src/wallet/test/util.cpp @@ -116,7 +116,7 @@ CTxDestination getNewDestination(CWallet& w, OutputType output_type) } MockableSQLiteDatabase::MockableSQLiteDatabase() - : SQLiteDatabase(fs::PathFromString("mock/"), fs::PathFromString("mock/wallet.dat"), DatabaseOptions(), SQLITE_OPEN_MEMORY) + : InMemoryWalletDatabase() {} std::unique_ptr CreateMockableWalletDatabase() diff --git a/src/wallet/test/util.h b/src/wallet/test/util.h index 9a407d31025..0af46e5577a 100644 --- a/src/wallet/test/util.h +++ b/src/wallet/test/util.h @@ -56,7 +56,7 @@ public: /** A WalletDatabase whose contents and return values can be modified as needed for testing **/ -class MockableSQLiteDatabase : public SQLiteDatabase +class MockableSQLiteDatabase : public InMemoryWalletDatabase { public: MockableSQLiteDatabase(); @@ -64,7 +64,6 @@ public: bool Backup(const std::string& strDest) const override { return true; } std::string Filename() override { return "mockable"; } - std::vector Files() override { return {}; } std::string Format() override { return "sqlite-mock"; } std::unique_ptr MakeBatch() override { return std::make_unique(*this); } }; From 777d23f25c7215b8e329ef040b7b2632ffd0e76f Mon Sep 17 00:00:00 2001 From: Pablo Martin Date: Tue, 7 Jul 2026 00:00:34 -0300 Subject: [PATCH 3/3] test: add regression test for in-memory SQLiteDatabase reopen InMemoryWalletDatabase::Open() now throws to prevent silently returning a fresh empty connection after close, which would discard all data. Add a test to pin this behaviour. Co-authored-by: Jan B <608446+janb84@users.noreply.github.com> --- src/wallet/test/db_tests.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/wallet/test/db_tests.cpp b/src/wallet/test/db_tests.cpp index 532bdce3847..818b176cefa 100644 --- a/src/wallet/test/db_tests.cpp +++ b/src/wallet/test/db_tests.cpp @@ -297,5 +297,14 @@ BOOST_AUTO_TEST_CASE(concurrent_txn_dont_interfere) BOOST_CHECK_EQUAL(read_value, value2); } +BOOST_AUTO_TEST_CASE(in_memory_database_cannot_reopen) +{ + // Reopening an in-memory database would create a fresh empty connection, + // silently losing all data. Open() must throw instead. + InMemoryWalletDatabase database; + database.Close(); + BOOST_CHECK_THROW(database.Open(), std::runtime_error); +} + BOOST_AUTO_TEST_SUITE_END() } // namespace wallet