#33034 wallet: Store transactions in a separate sqlite table
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #33034 wallet: Store transactions in a separate sqlite table | Needs rebase | 0 | Strong | 1,036 + 9 tests | |||
|
|
| |||||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | |||||||
Goal
Transitions SQLite wallet transaction storage from flat key-value pairs in the main database table to a dedicated relational transactions table where transaction fields occupy separate columns. Wallets load transactions ordered by wallet position (ORDER BY pos) rather than arbitrary database iteration order. Automatic upgrades from legacy key-value transaction records are managed through wallet client feature flags while dual-writing maintains compatibility across upgrade-downgrade cycles.
Problem: SQLite is used purely as a flat key-value store in Bitcoin Core wallets, forcing composite binary serialization and yielding transaction records in arbitrary database iteration order. Out-of-order loading complicates wallet transaction state reconstruction and makes reasoning about sequential transaction dependencies difficult.
Evidence of importance:
- PR description: '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.'
- PR description: '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.'
Scope: The commit stack bundles commits from SQLite statement RAII encapsulation (#33033) and last-opened client feature flag tracking (#32895).
Risk of a mistake: high (wallet, wallet_storage). Flaws in table schema migration or transaction serialization logic could corrupt wallet state, fail wallet loading, or lose transaction history during client upgrades and downgrades.
Changed since the previous assessment: The underlying approach is unchanged; the branch has been rebased and incorporates revisions from its dependencies.
What the assessment lacked:
- rest_of_diff
Reviewability
Ready · Needs rebase
Conflicts with master; review the logic now, the rebase is mechanical.
Agreement
Strong
substantive support, no open objection (rkrux)
Thread notes: Initial record. Both w0xlt and rkrux provided Concept ACKs with no objections raised.
Objections, suggestions, and questions: none recorded.
Support:
- w0xlt (contributor) (Concept ACK, 2025-07-23): Concept ACK [not substantive]
- rkrux (member) (Concept ACK, 2025-10-09): Agrees with utilizing SQLite's relational database properties for transactions rather than a key-value store, which can make transaction serialization cleaner and more explicit
Participants:
- w0xlt (contributor, support, 1 statements 2025-07-23 to 2025-07-23): Concept ACK
- rkrux (member, support, 1 statements 2025-10-09 to 2025-10-09): Concept ACK; agrees with leveraging SQLite relational capabilities over KVS to simplify serialization
Dependencies
Files
12 lines under test/bench/ci.
- src/wallet/sqlite.cpp +476/-195
- src/wallet/walletdb.cpp +220/-25
- src/wallet/sqlite.h +86/-15
- src/wallet/transaction.h +87/-0
- src/wallet/transaction.cpp +66/-0
- src/wallet/wallet.cpp +60/-4
- src/wallet/walletdb.h +16/-0
- src/wallet/walletutil.h +13/-1
- test/functional/wallet_backwards_compatibility.py +8/-2
- src/wallet/wallet.h +6/-0
- src/primitives/transaction_identifier.h +3/-0
- test/functional/wallet_createwallet.py +1/-1
- src/wallet/db.h +1/-0
- src/wallet/export.cpp +1/-0
- src/wallet/migrate.h +1/-0
Card
The short text the ranking pass compares across PRs.
This PR moves SQLite wallet transaction storage from generic key-value records into a dedicated relational table with columns for each transaction attribute. Storing transactions in their own table guarantees sequential loading by wallet position order via SQL queries, simplifying transaction state reconstruction and UTXO tracking. This change unblocks downstream improvements such as tracking no-longer-spendable TXOs in #27865. It depends on #32895 for feature-flagged database upgrades and #33033 for SQLite statement management.
Data
thread read through 2025-10-09 (2 statements), head 8122a27816
Discussion read under schema 1 rules: suggestions and questions may be missing, and an objection may combine several harms (schema history).
labels: Wallet, Needs rebase · draft
Categories
Wallet
P2 · unblocks #27865 (ranking pass: Moved up to keep the #33033 -> #33034 -> #27865 chain contiguous and in base-first order.)
Transitioning SQLite transaction storage from flat key-value pairs to dedicated relational columns provides major architectural maintainability and unblocks sequential transaction ordering in #27865. As the PR notes, out-of-order database iteration causes 'numerous headaches due to the need to serialize multiple fields together' and loading in assigned transaction order simplifies reasoning about sequential transaction dependencies.
Membership: Labeled Wallet and modifies SQLite wallet storage and schema across src/wallet/sqlite.cpp and src/wallet/walletdb.cpp.
Factors: security/stability 1, bug 0, performance 1, user value 1, leverage 2