wallet: Store transactions in a separate sqlite table #33034

achow101 commented at 10:52 PM on July 21, 2025: member

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 #32895, #33033

DrahtBot added the label Wallet on Jul 21, 2025

DrahtBot commented at 10:52 PM on July 21, 2025: contributor

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 and AI policy for information on the review process.

Type	Reviewers
Concept ACK	w0xlt, rkrux

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#36126 (wallet, rpc: Implements set key label functionality by polespinasa)
#35998 (wallet: Handle or explicitly ignore WalletBatch write failures by achow101)
#35786 (wallet: drop spent parents redundant cache invalidation and notification by furszy)
#35760 (wallet: make corrupted transaction records fail wallet loading instead of forcing a rescan by achow101)
#34909 (wallet, refactor: modularise wallet by extracting out legacy wallet migration by rkrux)
#33033 (wallet, sqlite: Encapsulate SQLite statements in a RAII class 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.

DrahtBot added the label CI failed on Jul 22, 2025

DrahtBot commented at 12:40 AM on July 22, 2025: contributor

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

<details><summary>Hints</summary>

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.

</details>

achow101 force-pushed on Jul 22, 2025

DrahtBot removed the label CI failed on Jul 22, 2025

achow101 force-pushed on Jul 23, 2025

w0xlt commented at 9:15 PM on July 23, 2025: contributor

Concept ACK

achow101 force-pushed on Jul 24, 2025

DrahtBot added the label Needs rebase on Jul 29, 2025

achow101 force-pushed on Jul 29, 2025

DrahtBot removed the label Needs rebase on Jul 29, 2025

DrahtBot added the label Needs rebase on Aug 8, 2025

achow101 force-pushed on Aug 8, 2025

DrahtBot removed the label Needs rebase on Aug 8, 2025

DrahtBot added the label Needs rebase on Aug 13, 2025

achow101 force-pushed on Aug 14, 2025

DrahtBot removed the label Needs rebase on Aug 14, 2025

DrahtBot added the label Needs rebase on Aug 16, 2025

achow101 force-pushed on Aug 16, 2025

achow101 force-pushed on Aug 19, 2025

DrahtBot removed the label Needs rebase on Aug 19, 2025

DrahtBot added the label Needs rebase on Sep 23, 2025

achow101 force-pushed on Sep 23, 2025

DrahtBot removed the label Needs rebase on Sep 24, 2025

rkrux commented at 8:14 AM on October 9, 2025: contributor

Concept ACK a25557a

I have gone through the PR description and have skimmed over the commit messages as of now. At the outset, I find myself agreeing with the intent here. I am not aware of a strong enough reason to keep using SQLite as a KVS for all purposes, using its relational database properties for wallet transactions seem like a good start. I did notice some serialisation specifics of transaction properties in a couple previous PRs that I didn't fully understand why were required; it could be a lot cleaner and explicit in implementation if using SQLite as an RDBMS can avoid the need for doing those specific serialisations.

DrahtBot added the label Needs rebase on Dec 2, 2025

achow101 force-pushed on Dec 10, 2025

DrahtBot removed the label Needs rebase on Dec 10, 2025

DrahtBot added the label CI failed on Dec 10, 2025

DrahtBot commented at 8:15 PM on December 10, 2025: contributor

🚧 At least one of the CI tasks failed. Task test max 6 ancestor commits: https://github.com/bitcoin/bitcoin/actions/runs/20110042688/job/57704599161 LLM reason (✨ experimental): CMake build failed with exit code 2 during project compilation (gmake reported an error).

<details><summary>Hints</summary>

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.

</details>

achow101 force-pushed on Dec 10, 2025

DrahtBot removed the label CI failed on Dec 11, 2025

DrahtBot added the label Needs rebase on Dec 17, 2025

achow101 force-pushed on Dec 22, 2025

DrahtBot removed the label Needs rebase on Dec 22, 2025

DrahtBot added the label CI failed on Dec 23, 2025

achow101 force-pushed on Jan 3, 2026

DrahtBot removed the label CI failed on Jan 3, 2026

DrahtBot added the label Needs rebase on Jan 19, 2026

achow101 force-pushed on Jan 19, 2026

DrahtBot added the label CI failed on Jan 19, 2026

DrahtBot commented at 7:49 PM on January 19, 2026: contributor

🚧 At least one of the CI tasks failed. Task test max 6 ancestor commits: https://github.com/bitcoin/bitcoin/actions/runs/21148471093/job/60819287525 LLM reason (✨ experimental): Build failed: the CI cmake/make run exited with non-zero status (gmake: all target) after linking bitcoinkernel, indicating a generic build error.

<details><summary>Hints</summary>

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.

</details>

DrahtBot removed the label Needs rebase on Jan 19, 2026

achow101 force-pushed on Jan 19, 2026

achow101 force-pushed on Jan 26, 2026

DrahtBot added the label Needs rebase on Feb 4, 2026

achow101 force-pushed on Feb 4, 2026

DrahtBot removed the label Needs rebase on Feb 4, 2026

DrahtBot added the label Needs rebase on Mar 2, 2026

achow101 force-pushed on Mar 5, 2026

DrahtBot removed the label Needs rebase on Mar 5, 2026

DrahtBot added the label Needs rebase on Mar 11, 2026

achow101 force-pushed on Mar 14, 2026

DrahtBot removed the label Needs rebase on Mar 14, 2026

achow101 force-pushed on Apr 1, 2026

achow101 force-pushed on Apr 2, 2026

DrahtBot added the label Needs rebase on Apr 19, 2026

achow101 force-pushed on Apr 20, 2026

DrahtBot removed the label Needs rebase on Apr 20, 2026

DrahtBot removed the label CI failed on Apr 20, 2026

sedited referenced this in commit db98e357d3 on May 2, 2026

DrahtBot added the label Needs rebase on May 2, 2026

achow101 force-pushed on May 19, 2026

DrahtBot removed the label Needs rebase on May 19, 2026

DrahtBot added the label Needs rebase on May 26, 2026

achow101 force-pushed on May 26, 2026

DrahtBot removed the label Needs rebase on May 26, 2026

DrahtBot added the label CI failed on May 26, 2026

DrahtBot removed the label CI failed on May 27, 2026

DrahtBot added the label Needs rebase on May 28, 2026

achow101 force-pushed on May 29, 2026

DrahtBot removed the label Needs rebase on May 29, 2026

DrahtBot added the label Needs rebase on Jul 10, 2026

achow101 force-pushed on Jul 13, 2026

achow101 force-pushed on Jul 14, 2026

DrahtBot removed the label Needs rebase on Jul 14, 2026

DrahtBot added the label Needs rebase on Aug 4, 2026

achow101 force-pushed on Sep 14, 2026

DrahtBot removed the label Needs rebase on Sep 15, 2026

DrahtBot added the label CI failed on Sep 15, 2026

DrahtBot commented at 1:18 AM on September 15, 2026: contributor

🚧 At least one of the CI tasks failed. Task test ancestor commits: https://github.com/bitcoin/bitcoin/actions/runs/34909942755/job/104194975241 LLM reason (✨ experimental): CI failed during the build because walletdb.cpp did not compile (error: no member named 'tx' in 'wallet::CWalletTx').

<details><summary>Hints</summary>

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.

</details>

achow101 force-pushed on Sep 15, 2026

DrahtBot added the label Needs rebase on Sep 22, 2026

walletdb: Decouple last client record from CLIENT_VERSION

Instead of tying the last client record with the node version, introduce
a separate wallet client version that will be increased as necessary
when new automatic upgrades are introduced.

0f104733cc

wallet: Introduce LastClientFeatures flags and LAST_OPENED_FEATURES record

LastClientFeatures are feature flags indicating what automatic upgrade
features supported by the last client that opened the wallet.

The LAST_OPENED_FEATURES record stores these flags and must be
set to match the exact features supported by the client that opens a
wallet.

47019dfa94

wallet: Record the supported features of the last client to decrypt a wallet 37b318249c

wallet: Set last opened and decrypted features during migration

Set these flags to avoid the automatic upgrade after migrating.

0758310dc3

sqlite: Add SQLiteStatement RAII class

This class will be used to encapsulate a sqlite3_stmt

69776e5663

sqlite: Make Column template function 4bd7eaf4ea

sqlite: Refactor ReadPragmaInteger to use SQLiteStatement dfe5149846

sqlite: Use SQLiteStatement in PRAGMA integrity_check eb8a6400f9

sqlite: Use SQLiteStatement in check_main_stmt e75a0882ca

sqlite: Have SQLiteCursor store SQLiteStatement 9d70269130

sqlite: Replace remaining sqlite3_stmt usage with SQLiteStatement 25385da110

sqlite: Refactor common writing code from WriteKey and ExecStatement

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.

334a232f2f

sqlite: Inline BindBlobToStatement and SpanFromBlob 0743761dc7

sqlite: Construct SQLiteStatements when needed

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.

5868a84b49

util: Add span constructor to (W)Txid 117fe3f5b7

sqlite: Make SQLiteStatement::Bind a template function

We will need to bind data types other than blob

7abbf2875c

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

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

de04df9555

sqlite: Add additional blob types to SQLiteStatement::Column

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

f94545b05c

wallet: Add Un/Serialize to TxState structs d30b2f2440

wallet: Add new serialization format functions for TxState e5936d32c4

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

sqlite: Add functions and statements for transactions table b6f9a9b7fb

sqlite: Iterate the transactions table with SQLiteCursor 202549534b

walletdb: Add functions to modify transactions table 654071625a

wallet: Also write to the new transactions table 79de4d1b58

wallet: Perform automatic upgrade to using the transactions table 7b1c6d4231

walletdb: Load from transactions table and use original tx for upgrade

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.

8122a27816

achow101 force-pushed on Sep 22, 2026

DrahtBot removed the label Needs rebase on Sep 22, 2026

DrahtBot removed the label CI failed on Sep 23, 2026