sqlite: Delete wallet.dat-journal on database close

achow101 commented at 11:58 pm on February 18, 2026: member

The wallet.dat-journal file is SQLite’s hot journal file. This file is either deleted, truncated, or has its header zeroed when a database transaction is committed. However, in restorewallet, we are assuming that sqlite decided to delete the journal file even though it may have truncated or zeroed its header instead.

This PR changes SQLiteDatabase::Close() to remove the journal file if it is 0 bytes, or if the header (first 28 bytes) is all 0’s. Otherwise, it leaves the file untouched.

This fixes one of the issues described in #34354

An alternative is to have RestoreWallet specifically also delete the journal file. However, I thought that this solution would be more robust as I think there are other places where we expect the journal file to be deleted on a clean close of a wallet database.

DrahtBot commented at 11:59 pm on February 18, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	w0xlt

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#31560 (rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script by theStack)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot added the label CI failed on Feb 19, 2026

w0xlt commented at 1:27 am on February 19, 2026: contributor

Concept ACK

in src/wallet/sqlite.cpp:400 in 8f731e6c0f outdated

395+            }
396+        }
397+
398+        // Journal file is truncated or zero'd header, so it can be safely deleted
399+        fs::remove(journal_path);
400+    }

chriszeng1010 commented at 5:19 am on February 19, 2026:

Is there test coverage for the PR?

achow101 commented at 7:09 pm on February 19, 2026:

Not explicitly.

This should fix one of the issues described in #34354 which is an intermittent test failure. I don’t think this behavior needs to have explicit testing as it largely doesn’t matter whether the journal does or does not exist when it is invalidated.

in src/wallet/sqlite.cpp:383 in 8f731e6c0f outdated

374@@ -374,6 +375,29 @@ void SQLiteDatabase::Close()
375         throw std::runtime_error(strprintf("SQLiteDatabase: Failed to close database: %s\n", sqlite3_errstr(res)));
376     }
377     m_db = nullptr;
378+
379+    const fs::path& journal_path = fs::PathFromString(Filename() + "-journal");
380+    if (fs::exists(journal_path)) {
381+        // If the journal file is more than 0 bytes, check if the 28 byte header is zero'd
382+        std::error_code ec;
383+        auto size = fs::file_size(journal_path, ec);

chriszeng1010 commented at 5:20 am on February 19, 2026:

Should add a LogDebug to make debugging more transparent if silent failures were to happen here.

achow101 commented at 7:10 pm on February 19, 2026:

I don’t think that is necessary as any failures here should have occurred much earlier.

in src/wallet/sqlite.cpp:389 in 8f731e6c0f

384+        if (ec) return;
385+        if (size > 0) {
386+            std::ifstream file{journal_path.std_path(), std::ios::binary};
387+            if (!file.is_open()) return;
388+
389+            char header[28];

chriszeng1010 commented at 5:21 am on February 19, 2026:

28 magic number.

achow101 commented at 7:22 pm on February 19, 2026:

Made a constant

in src/wallet/sqlite.cpp:390 in 8f731e6c0f

385+        if (size > 0) {
386+            std::ifstream file{journal_path.std_path(), std::ios::binary};
387+            if (!file.is_open()) return;
388+
389+            char header[28];
390+            file.read(header, 28);

chriszeng1010 commented at 5:34 am on February 19, 2026:

file.reader(header,28) maybe read fewer than 28 bytes in the file is shorter than expected. Remaining bytes in header would be uninitialized. std::any_of would read them — which is UB.

0file.read(header, 28);
1if (file.gcount() != 28) return;

This bails out if we didn’t get a full header, which is the safe default

achow101 commented at 7:23 pm on February 19, 2026:

Changed the earlier size check to check for the header size.

If the file is less than 28 bytes, it cannot have a valid journal header and the file can therefore be deleted. This does not need to fail safe.

in src/wallet/sqlite.cpp:379 in 8f731e6c0f

374@@ -374,6 +375,29 @@ void SQLiteDatabase::Close()
375         throw std::runtime_error(strprintf("SQLiteDatabase: Failed to close database: %s\n", sqlite3_errstr(res)));
376     }
377     m_db = nullptr;
378+
379+    const fs::path& journal_path = fs::PathFromString(Filename() + "-journal");

chriszeng1010 commented at 5:39 am on February 19, 2026:

is there any good reason to use const fs::path& over const fs::path?

achow101 commented at 7:23 pm on February 19, 2026:

It doesn’t particularly matter.

sqlite: Delete wallet.dat-journal on database close

The wallet.dat-journal file can be deleted if it is 0 bytes or the
header is all 0'd. Sqlite may also delete it for us when shutting down,
but we can do our own check to ensure it is cleaned up.

161ac5dfbb

achow101 force-pushed on Feb 19, 2026

furszy commented at 4:32 pm on February 20, 2026: member

I understand the reasoning behind the changes; adapting RestoreWallet to handle all db files would mean moving MakeWalletDatabase up and passing it to LoadWallet(), which is annoying. But, I’m doubtful on digging deep into sqlite internals and relying on inspecting journal files directly. These files are internal implementation details, and sqlite may handle them differently across versions. Will investigate more and see if there is anything else we can do through the sqlite API.

achow101 commented at 10:56 pm on February 20, 2026: member

I think this is actually a red herring

achow101 closed this on Feb 20, 2026

sqlite: Delete wallet.dat-journal on database close #34619

Reviews

Conflicts