wallet: Store transactions in a separate sqlite table #33034
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33034. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
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:
drahtbot_id_4_m |
|
🚧 At least one of the CI tasks failed. HintsTry to run the tests locally, according to the documentation. However, a CI failure may still
Leave a comment here, if you need help tracking down a confusing failure. |
b36fd77 to
3d61435
Compare
3d61435 to
80e1937
Compare
80e1937 to
18035d1
Compare
|
Concept ACK |
18035d1 to
678c2ea
Compare
678c2ea to
3c31700
Compare
WalletBatch needs to be in a scope so that it is destroyed before the database is closed during migration.
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.
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.
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.
c2efcd6 to
b0e81e6
Compare
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 likeSELECT * FROM transactions ORDER BY poswhich 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, #33031, #33032, #33033