wallet: Store transactions in a separate sqlite table by achow101 · Pull Request #33034 · bitcoin/bitcoin

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, #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:

#34838 (interfaces: typed receive-request APIs by MkDev11)
#34502 (wallet: remove most asserts of WALLET_FLAG_DESCRIPTORS flag by rkrux)
#34490 (wallet: Follow-ups to create/load split (Split CWallet::Create() into CreateNew and LoadExisting #32636) by davidgumberg)
#33160 (bench: Add more realistic Coin Selection Bench by murchandamus)
#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)
#32895 (wallet: Prepare for future upgrades by recording versions of last client to open and decrypt by achow101)
#32489 (wallet: Add exportwatchonlywallet RPC to export a watchonly version of a wallet by achow101)
#27865 (wallet: Track no-longer-spendable TXOs separately by achow101)

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 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

^{2026-03-14 01:35:29}

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

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.

Instead of constructing all SQLiteStatements when SQLiteBatch is constructed, construct them once when they are needed before each read, write, or erase operation. Once constructed, the statement will persist for the lifetime of the SQLiteBatch to be reused across multiple statements.

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 27 commits March 13, 2026 15:50

wallet, bench: Use TestingSetup in CoinSelection benchmark

e0a5e75

walletdb: Remove m_mock from SQLiteDatabase

13e89b5

sqlite: Add SQLiteStatement RAII class

d6a91f5

This class will be used to encapsulate a sqlite3_stmt

sqlite: Make Column template function

5884161

sqlite: Refactor ReadPragmaInteger to use SQLiteStatement

a9aa1c8

sqlite: Use SQLiteStatement in PRAGMA integrity_check

3968016

sqlite: Use SQLiteStatement in check_main_stmt

7e3bc30

sqlite: Have SQLiteCursor store SQLiteStatement

646f26a

sqlite: Replace remaining sqlite3_stmt usage with SQLiteStatement

2a90679

sqlite: Inline BindBlobToStatement and SpanFromBlob

fbe4f8a

util: Add span constructor to (W)Txid

e52ca4d

sqlite: Make SQLiteStatement::Bind a template function

9ac5e03

We will need to bind data types other than blob

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

5b4e2fa

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

2902e54

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

33e5280

wallet: Add new serialization format functions for TxState

41d4324

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

affa71b

sqlite: Add functions and statements for transactions table

9a6b608

sqlite: Iterate the transactions table with SQLiteCursor

981c799

walletdb: Add functions to modify transactions table

f4bd7ef

test: Include sql transactions table in DuplicateMockDatabase

fe637d6

wallet: Also write to the new transactions table

79872ca

wallet: Perform automatic upgrade to using the transactions table

f6c2f2e

achow101 force-pushed the wallet-sql-txs-table branch from 2888c10 to cecfc0c Compare March 14, 2026 01:34

DrahtBot removed the Needs rebase label Mar 14, 2026

DrahtBot mentioned this pull request Mar 18, 2026

interfaces: typed receive-request APIs #34838

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wallet: Store transactions in a separate sqlite table#33034

wallet: Store transactions in a separate sqlite table#33034
achow101 wants to merge 42 commits intobitcoin:masterfrom
achow101:wallet-sql-txs-table

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

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