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.
If your review is incorrectly listed, please copy-paste <code><!--meta-tag:bot-skip--></code> into the comment that the bot should ignore.
<!--174a7506f384e20aa4161008e828411d-->
Conflicts
Reviewers, this pull request conflicts with the following ones:
#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)
#35716 (wallet: Replace mapWallet and wtxOrdered with a boost::multi_index by achow101)
#35501 (wallet: store all witness variants of a transaction by achow101)
#35445 (wallet, descriptor: Revert StringType::COMPAT for Miniscript expressions and drop the concept of a Descriptor ID that can be validated by achow101)
#35302 (Silent Payments: Sending (take 2) by Eunovo)
#34909 (wallet, refactor: modularise wallet by extracting out legacy wallet migration by rkrux)
#34502 (wallet: remove most asserts of WALLET_FLAG_DESCRIPTORS flag by rkrux)
#33033 (wallet, sqlite: Encapsulate SQLite statements in a RAII class by achow101)
#32895 (wallet: Prepare for future upgrades by recording versions of last client to open and decrypt by achow101)
#27865 (wallet: Track no-longer-spendable TXOs separately by achow101)
#25722 (refactor: Use util::Result class for wallet loading 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.
<!--5faf32d7da4f0f540f40219e4f7537a3-->
LLM Linter (✨ experimental)
Possible typos and grammar issues:
InMempool -> InMempool [the comment says “written as InActive(abandoned=false)”; “InActive” is misspelled and should be “Inactive”]
<sup>2026-07-14 00:25:55</sup>
DrahtBot added the label CI failed on Jul 22, 2025
DrahtBot
commented at 12:40 AM on July 22, 2025:
contributor
<!--85328a0da195eb286784d51f73fa0af9-->
🚧 At least one of the CI tasks failed.
<sub>Task lint: https://github.com/bitcoin/bitcoin/runs/46429203587</sub>
<sub>LLM reason (✨ experimental): The CI failure is caused by Python lint errors due to unresolved 'self' references in test files.</sub>
<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
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 ACKa25557a
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
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
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
<!--85328a0da195eb286784d51f73fa0af9-->
🚧 At least one of the CI tasks failed.
<sub>Task test max 6 ancestor commits: https://github.com/bitcoin/bitcoin/actions/runs/21148471093/job/60819287525</sub>
<sub>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.</sub>
<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
achow101 force-pushed on Apr 2, 2026
achow101 force-pushed on Apr 2, 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
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.
fd5b99417d
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.
48d5f5b274
wallet: Record the supported features of the last client to decrypt a wallet9cd32e3868
wallet: Set last opened and decrypted features during migration
Set these flags to avoid the automatic upgrade after migrating.
61c6eb387f
sqlite: Add SQLiteStatement RAII class
This class will be used to encapsulate a sqlite3_stmt
e3505774bc
sqlite: Make Column template function23216d2a09
sqlite: Refactor ReadPragmaInteger to use SQLiteStatement4d7ce54d9f
sqlite: Use SQLiteStatement in PRAGMA integrity_check199ce1f1e2
sqlite: Use SQLiteStatement in check_main_stmt657cd4fdda
sqlite: Have SQLiteCursor store SQLiteStatement1d227f9523
sqlite: Replace remaining sqlite3_stmt usage with SQLiteStatementfdc6b38f4a
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.
0500a0aceb
sqlite: Inline BindBlobToStatement and SpanFromBlobb59c01bb6c
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.
313286015e
util: Add span constructor to (W)Txid8feb55d363
sqlite: Make SQLiteStatement::Bind a template function
We will need to bind data types other than blob
938254f6f9
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.
6bdb50bf3e
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.
fd30af2271
wallet: Add Un/Serialize to TxState structs4a95c33801
wallet: Add new serialization format functions for TxStatee37984a619
sqlite: Create a 'transactions' table if it does not existc2808d7d8d
sqlite: Add functions and statements for transactions tablea55ed823b5
sqlite: Iterate the transactions table with SQLiteCursorfe187d75f4
walletdb: Add functions to modify transactions table0095ce0751
wallet: Also write to the new transactions table3bb591da7c
wallet: Perform automatic upgrade to using the transactions table36dcf7f766
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.
372e21d7fa
achow101 force-pushed on Jul 14, 2026
DrahtBot removed the label Needs rebase on Jul 14, 2026
This is a metadata mirror of the GitHub repository
bitcoin/bitcoin.
This site is not affiliated with GitHub.
Content is generated from a GitHub metadata backup.
generated: 2026-07-30 05:51 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me