wallet: Store transactions in a separate sqlite table

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 #32763, #32895, #32984, #32985, #33031, #33032, #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 for information on the review process.

Type	Reviewers
Concept ACK	w0xlt

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#33043 ([POC] wallet: Enable non-electronic (paper-based) wallet backup with codex32 by w0xlt)
#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)
#32977 (wallet: Remove wallet version and several legacy related functions by w0xlt)
#32966 (Silent Payments: Receiving by Eunovo)
#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)
#32489 (wallet: Add exportwatchonlywallet RPC to export a watchonly version of a wallet by achow101)
#32273 (wallet: Fix relative path backup during migration. by davidgumberg)
#29641 (scripted-diff: Use LogInfo over LogPrintf [WIP, NOMERGE, DRAFT] by maflcko)
#28201 (Silent Payments: sending by josibake)
#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 typos and grammar issues:

encapsualtes -> encapsulates [correct spelling of “encapsulates”]

drahtbot_id_4_m

DrahtBot added the label CI failed on Jul 22, 2025

DrahtBot commented at 0: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.

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.

achow101 force-pushed on Jul 22, 2025

DrahtBot removed the label CI failed on Jul 22, 2025

wallet: Pass replaces_txid to CommitTransaction outside of mapValue

Instead of updating mapValue with the "replaces_txid" value and passing
the updated mapValue to CommitTransaction, pass the replaces_txid
directly to CommitTransaction which updates mapValue as necessary.

This is prepration for removing mapValue.

14868cf68a

wallet: Pass comment and comment_to to CommitTransaction

Instead of passing these by setting them in mapValue, pass them directly
to CommitTransaction.

This is preparation for removing mapValue.

353cbaf53d

wallet: Drop mapValue from CommitTransaction

The values previously passed in mapValue are now parameters to
CommitTransaction so there is no need for mapValue to be passed.

a2367e7b7a

wallet: Make CWalletTx "from" and "message" member variables

Instead of storing "from" and "message" inside of mapValue, store these
explicitly as members of CWalletTx.

fe4974793f

wallet: Make CWalletTx "comment" and "to" member variables

Instead of storing "comment" and "to" inside of mapValue, store these
expliclty as members of CWalletTx.

4f2754374c

wallet: Make CWalletTx "replaces_txid" and "replaced_by_txid" member variables

Instead of storing "replaces_txid" and "replaced_by_txid" as strings inside of
mapValue, store these expliclty as members of CWalletTx.

6fbac4020a

wallet: Drop mapValue from CWalletTx

It doesn't make sense to be storing relevant metadata variables inside
of a string map in CWalletTx. All of the fields have been pulled out
into separate members, so there is no need for mapValue to stick around.

0373e6683b

wallet: Drop vOrderForm from CommitTransaction

This parameter is only used to pass in the "Messages" from the GUI.
Instead of making it opaque by putting those into vOrderForm, use a
specific dedicated parameter for providing the messages.

7fdc5a8b23

wallet: Replace CWalletTx's vOrderForm with specific fields

vOrderForm contained 2 kinds of strings: BIP 21 messages, and BIP 70
Payment Requests. Instead of having both inside of a single vOrderForm
field that is opaque, split them into separate std::vector<std::string>
to contain this metadata.

27515609a5

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.

668889d8e8

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.

84d7f2e64c

wallet: Record the supported features of the last client to decrypt a wallet 5413a37338

wallet: Set last opened and decrypted features during migration

Set these flags to avoid the automatic upgrade after migrating.

97d7a914e1

wallet: Set migrated wallet name only on success

After a wallet is migrated and we are trying to load it, if it could not be
loaded, don't try to set the wallet name.

eeec30b2b9

wallet: Always rewrite tx records during migration

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.

e83033dfa3

tests: Check that the last hardened cache upgrade occurs

When loading an older wallet without the last hardened cache, an
automatic upgrade should be performed. Check this in
wallet_backwards_compatibility.py

When migrating a wallet, the migrated wallet should always have the last
hardened cache, so verify in wallet_migration.py

9c77c9383f

wallet: Always write last hardened cache flag in migrated wallets 7dbddfee7c

achow101 force-pushed on Jul 23, 2025

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

Concept ACK

bench, wallet: Make WalletMigration's setup WalletBatch scoped

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

92227989d9

test: Make duplicating MockableDatabases use cursor and batch

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.

0c7072d591

wallet: Make Mockable{Database,Batch} subclasses of SQLite classes

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.

c65de169ba

walletdb: Remove m_mock from SQLiteDatabase 6b8d032575

sqlite: Add SQLiteStatement RAII class

This class will be used to encapsulate a sqlite3_stmt

64e9e24e42

sqlite: Make Column template function 8cd387c3ac

sqlite: Refactor ReadPragmaInteger to use SQLiteStatement a85ca9a8e5

sqlite: Use SQLiteStatement in PRAGMA integrity_check 44e50d3f2c

sqlite: Use SQLiteStatement in check_main_stmt 716b9f0c03

sqlite: Have SQLiteCursor store SQLiteStatement 497c03f5ad

sqlite: Replace remaining sqlite3_stmt usage with SQLiteStatement 6315ed4bb1

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.

5f2c4a15b9

sqlite: Inline BindBlobToStatement and SpanFromBlob 9436c60484

util: Add span constructor to (W)Txid 5f426159e5

sqlite: Make SQLiteStatement::Bind a template function

We will need to bind data types other than blob

dd4d7e74a2

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.

d11cbf561f

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.

4a0c7d93ba

wallet: Add Un/Serialize to TxState structs 4c306f78d1

wallet: Add new serialization format functions for TxState d99eda56b7

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

sqlite: Add functions and statements for transactions table eda1cdc6f6

sqlite: Iterate the transactions table with SQLiteCursor edb46a3583

walletdb: Add functions to modify transactions table 82cb55e2bd

test: Include sql transactions table in DuplicateMockDatabase a23d0bb143

wallet: Also write to the new transactions table a32e373f67

wallet: Perform automatic upgrade to using the transactions table 49b809a20e

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.

678c2ea5f7

achow101 force-pushed on Jul 24, 2025

wallet: Store transactions in a separate sqlite table #33034

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)