wallet: Store transactions in a separate sqlite table #33034

achow101 · 2025-07-21T22:52:54Z

The wallet uses SQLite as a key-value store even though SQLite is a powerful relational database engine. This causes us numerous headaches due to the need to serialize multiple fields together, and since record read from the database during loading can come in any order.

Notably, for transactions, if we were able to load them in the transaction order assigned by the wallet, PRs like #27865 would be a bit simpler and easier to reason about.

This PR makes it possible for us to do that by storing transactions in a separate table. This table has a column for each field in CWalletTx, which lets us use a SQL statement like SELECT * FROM transactions ORDER BY pos which guarantees us the order in which the transactions are loaded into the wallet.

The eventual goal is to use SQLite as a relational database with tables that store our keys and other metadata that can reference each other so that the wallet doesn't just use the database as a storage mechanism. But that is still a work in progress.

This PR makes use of the last client opened features introduced in #32895 to determine when the old record style needs to be upgraded to the new transactions table. This should give us sufficient upgrade-downgrade-upgrade handling to not lose any funds.

Depends on #32763, #32895, #32985, #33031, #33032, #33033

DrahtBot · 2025-07-21T22:52:59Z

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33034.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	w0xlt, rkrux

If your review is incorrectly listed, please copy-paste  into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#33033 (wallet, sqlite: Encapsulate SQLite statements in a RAII class by achow101)
#33032 (wallet, test: Replace MockableDatabase with in-memory SQLiteDatabase by achow101)
#33008 (wallet: support bip388 policy with external signer by Sjors)
#32985 (wallet: Always rewrite tx records during migration by achow101)
#32895 (wallet: Prepare for future upgrades by recording versions of last client to open and decrypt by achow101)
#32685 (wallet: Allow read-only database access for info and dump commands by PeterWrighten)
#32636 (Split CWallet::Create() into CreateNew and LoadExisting by davidgumberg)
#32489 (wallet: Add exportwatchonlywallet RPC to export a watchonly version of a wallet by achow101)
#31650 (refactor: Avoid copies by using const references or by move-construction by maflcko)
#27865 (wallet: Track no-longer-spendable TXOs separately by achow101)
#19461 (multiprocess: Add bitcoin-gui -ipcconnect option by ryanofsky)
#19460 (multiprocess: Add bitcoin-wallet -ipcconnect option by ryanofsky)
#10102 (Multiprocess bitcoin by ryanofsky)

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.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

"a upgrade-downgrade-upgrade" -> "an upgrade-downgrade-upgrade" [Incorrect article before a vowel-starting word "upgrade".]
"LastClientFEatures" -> "LastClientFeatures" [Typo / incorrect capitalization in identifier name within comment; should read "Features".]

Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

sqlite3_prepare_v2(&m_db, stmt_text.c_str(), -1, &m_stmt, nullptr) in src/wallet/sqlite.cpp
MockableSQLiteDatabase::MockableSQLiteDatabase() : SQLiteDatabase(fs::PathFromString("mock/"), fs::PathFromString("mock/wallet.dat"), DatabaseOptions(), SQLITE_OPEN_MEMORY) in src/wallet/test/util.cpp

^2026-01-03

DrahtBot · 2025-07-22T00:40:33Z

🚧 At least one of the CI tasks failed.
_{Task lint: https://github.com/bitcoin/bitcoin/runs/46429203587}
_{LLM reason (✨ experimental): The CI failure is caused by Python lint errors due to unresolved 'self' references in test files.}

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

w0xlt · 2025-07-23T21:15:06Z

Concept ACK

Since loading a wallet may change some parts of tx records (e.g. adding nOrderPos), we should rewrite the records instead of copying them so that the automatic upgrade does not need to be performed again when the wallet is loaded.

WalletBatch needs to be in a scope so that it is destroyed before the database is closed during migration.

Instead of directly copying the stored records map when duplicating a MockableDatabase, use a Cursor to read the records, and a Batch to write them into the new database. This prepares for using SQLite as the database backend for MockableDatabase.

The mocking functionality of MockableDatabase, MockableBatch, and MockableCursor was not really being used. These are changed to be subclasses of their respective SQLite* classes and will use in-memory SQLite databases so that the tests are more representative of actual database behavior. MockableCursor is removed as there are no overrides needed in SQLiteCursor for the tests.

This class will be used to encapsulate a sqlite3_stmt

WriteKey and ExecStatement use the same code for the actual execution of the statement. This is refactored into a separate function, also called ExecStatement, and the original ExecStatement renamed to ExecEraseStatement as it is only used by the erase functions.

We will need to bind data types other than blob

To handle columns containing NULL values, Column needs to return some value representing NULL, so make it a std::optional.

Not all blob data types fit in ColumnBlob, so we need additional template type requirements to match those.

When loading a wallet, always load from the transactions table, except when loading a legacy wallet for migration. The original 'tx' records are only used to upgrade to the transactions table if an upgraded is necessary.

DrahtBot added the Wallet label Jul 21, 2025

DrahtBot added the CI failed label Jul 22, 2025

achow101 force-pushed the wallet-sql-txs-table branch from b36fd77 to 3d61435 Compare July 22, 2025 06:16

achow101 force-pushed the wallet-sql-txs-table branch from 3d61435 to 80e1937 Compare July 22, 2025 17:22

DrahtBot removed the CI failed label Jul 22, 2025

achow101 force-pushed the wallet-sql-txs-table branch from 80e1937 to 18035d1 Compare July 23, 2025 00:05

DrahtBot mentioned this pull request Jul 23, 2025

[POC] wallet: Enable non-electronic (paper-based) wallet backup with codex32 #33043

Draft

achow101 force-pushed the wallet-sql-txs-table branch from 18035d1 to 678c2ea Compare July 24, 2025 20:48

DrahtBot mentioned this pull request Jul 25, 2025

scripted-diff: Use LogInfo over LogPrintf #29641

Merged

DrahtBot added the Needs rebase label Jul 29, 2025

achow101 force-pushed the wallet-sql-txs-table branch from 678c2ea to 3c31700 Compare July 29, 2025 20:11

DrahtBot removed the Needs rebase label Jul 29, 2025

achow101 added 28 commits January 2, 2026 17:24

wallet: Always rewrite tx records during migration

f8c130c

Since loading a wallet may change some parts of tx records (e.g. adding nOrderPos), we should rewrite the records instead of copying them so that the automatic upgrade does not need to be performed again when the wallet is loaded.

bench, wallet: Make WalletMigration's setup WalletBatch scoped

72e177e

WalletBatch needs to be in a scope so that it is destroyed before the database is closed during migration.

walletdb: Remove m_mock from SQLiteDatabase

baee477

sqlite: Add SQLiteStatement RAII class

4a863ed

This class will be used to encapsulate a sqlite3_stmt

sqlite: Make Column template function

16202b0

sqlite: Refactor ReadPragmaInteger to use SQLiteStatement

69b9300

sqlite: Use SQLiteStatement in PRAGMA integrity_check

202b79e

sqlite: Use SQLiteStatement in check_main_stmt

f5a0bcd

sqlite: Have SQLiteCursor store SQLiteStatement

14b6ac8

sqlite: Replace remaining sqlite3_stmt usage with SQLiteStatement

5df4355

sqlite: Inline BindBlobToStatement and SpanFromBlob

4145a6f

util: Add span constructor to (W)Txid

eb77682

sqlite: Make SQLiteStatement::Bind a template function

02e361b

We will need to bind data types other than blob

sqlite: Make SQLiteDatabase::Column an std::optional

51c613b

To handle columns containing NULL values, Column needs to return some value representing NULL, so make it a std::optional.

sqlite: Add additional blob types to SQLiteStatement::Column

b7c00ce

Not all blob data types fit in ColumnBlob, so we need additional template type requirements to match those.

wallet: Add Un/Serialize to TxState structs

9f5f4bd

wallet: Add new serialization format functions for TxState

a51082f

sqlite: Create a 'transactions' table if it does not exist

98fefb1

sqlite: Add functions and statements for transactions table

0915609

sqlite: Iterate the transactions table with SQLiteCursor

8379ef9

walletdb: Add functions to modify transactions table

13a834c

test: Include sql transactions table in DuplicateMockDatabase

abc25eb

wallet: Also write to the new transactions table

dadc930

wallet: Perform automatic upgrade to using the transactions table

f996525

achow101 force-pushed the wallet-sql-txs-table branch from e479564 to 9e093b2 Compare January 3, 2026 01:24

DrahtBot removed the CI failed label Jan 3, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

wallet: Store transactions in a separate sqlite table #33034

wallet: Store transactions in a separate sqlite table #33034

achow101 commented Jul 21, 2025 •

edited

Loading

Uh oh!

DrahtBot commented Jul 21, 2025 •

edited

Loading

Uh oh!

DrahtBot commented Jul 22, 2025

Uh oh!

w0xlt commented Jul 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

wallet: Store transactions in a separate sqlite table #33034

Are you sure you want to change the base?

wallet: Store transactions in a separate sqlite table #33034

Conversation

achow101 commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)

Uh oh!

DrahtBot commented Jul 22, 2025

Uh oh!

w0xlt commented Jul 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

achow101 commented Jul 21, 2025 •

edited

Loading

DrahtBot commented Jul 21, 2025 •

edited

Loading