[sql-43] Add support for sqldb/v2 in litd
#1085
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
@ViktorTigerstrom - maybe change the base branch so we only see the relevant commits here? |
ViktorT-11
force-pushed
the
2025-05-sqldb-v2
branch
from
45d36a7 to
f153c65
Compare
ViktorT-11
force-pushed
the
2025-05-sqldb-v2
branch
4 times, most recently
from
6354999 to
16cf42e
Compare
ViktorT-11
force-pushed
the
2025-05-sqldb-v2
branch
from
16cf42e to
c53e85b
Compare
ViktorT-11
force-pushed
the
2025-05-sqldb-v2
branch
from
c53e85b to
7792848
Compare
ViktorT-11
added
the
no-changelog
Contributor
Author
|
@bitromortac & @ellemouton, I just requested reviews from both of you as I think we're ready to start reviewing this PR now. Therefore we can't merge this PR yet, as the dependency link will change when the above has merged. The actual code in this PR is final though, so it is worth reviewing IMO :) |
ellemouton
reviewed
Sep 4, 2025
Member
ellemouton
left a comment
ellemouton
left a comment
There was a problem hiding this comment.
wow, this PR is absolutely impeccable: really great descriptions, super modular commits, very easy to review - great work! 🔥
| @@ -1,311 +1,5 @@ | |||
| package db | |||
|
|
|||
| import ( | |||
bitromortac
reviewed
Sep 5, 2025
Contributor
bitromortac
left a comment
bitromortac
left a comment
There was a problem hiding this comment.
wow, this PR is absolutely impeccable: really great descriptions, super modular commits, very easy to review - great work! 🔥
100%, only a first pass for me (and I don't have a good view of the upstream work as well). Thanks for the great commit structure! 🚀
ViktorT-11
force-pushed
the
2025-05-sqldb-v2
branch
2 times, most recently
from
9956b37 to
027a495
Compare
ViktorT-11
force-pushed
the
2025-05-sqldb-v2
branch
10 times, most recently
from
017d3d8 to
827ca57
Compare
ViktorT-11
force-pushed
the
2025-05-sqldb-v2
branch
from
827ca57 to
cfb6140
Compare
ViktorT-11
force-pushed
the
2025-05-sqldb-v2
branch
from
cfb6140 to
5dcd396
Compare
|
@ellemouton: review reminder |
ellemouton
approved these changes
Jan 29, 2026
Member
ellemouton
left a comment
ellemouton
left a comment
There was a problem hiding this comment.
LGTM!! epic epic epic.
Let's rebase the base branch to match master, then merge this, then open a draft pr from the base branch against master that we then continue to merge into & occasionally rebase on master.
Excited to get the ball rolling here 🔥
Comment on lines
+35
to
+37
| github.com/lightninglabs/taproot-assets v0.7.1-0.20260127120359-89b1ea3a7888 | ||
| github.com/lightninglabs/taproot-assets/taprpc v1.0.12-0.20260121192339-ac2ec3419cec | ||
| github.com/lightningnetwork/lnd v0.20.0-beta.rc4.0.20251127014118-f8b5cb0e8918 |
Member
There was a problem hiding this comment.
these would all be bumped to tagged versions before releasing lit right?
bitromortac
approved these changes
Feb 4, 2026
Contributor
bitromortac
left a comment
bitromortac
left a comment
There was a problem hiding this comment.
Awesome changes, LGTM 🚀🚀
In the upcoming commits, we'll updating litd to use the new sqldb/v2 package. In order for the migrations run by the sqldb/v2 package to work correctly, we need to bump the `migrate` dependency to the latest version. When doing so, we also need to bump taproot-assets to a version which uses that migrate dependency version, which currently only the `main` branch on taproot-assets does. As the /universerpc.Universe/Info call now requires a macaroon, we need to add the `taproot-assets.allow-public-uni-proof-courier` to the node config in order to whitelist the call.
In upcoming commits, we will introduce support for sqldb/v2 in Lightning Terminal, in order to support code migrations. This is needed, so that we can ensure that the kvdb to sql migration is run exactly after an exact sql migration version, and not after all sql migrations have been executed. The reason why that's important is that sql table definition may change with future sql migrations, while the kvdb to sql migration will remain the same and always expect the same table definition. Therefore, if the kvdb to sql migration is run after all sql migrations, it may fail due to a table definition mismatch. This commit adds the sqldb/v2 dependency to `litd`, so that we can implement it's usage and execution of the kvdb to sql migration correctly in the upcoming commits. NOTE: currently the sqldb/v2 dependency hasn't been shipped in any lnd release, so this dependency currently a forked version of sqldb/v2.
A core component of `sqldb/v2` useage, is that the package allows and requires that the callsite defines a `sqldb.MigrationStream` that will be run during the initialization of the `sqldb/v2 database instance. The `sqldb.MigrationStream` defines the exact sql migrations to run, as well as additional code migrations will be run after each individual migration version. This commit introduces the core definition of the migration stream for `litd`, and will be further exteded in upcoming commits that will introduce kvdb to sql code migration. Note that as of this commit, as no part of the `litd` codebase uses the `sqldb/v2` database instances, this migration stream is not yet used.
The `sqldb/v2` package now provides a definition for the `BackendType` type. As useage of `sqldb/v2` requires useage of that type, we update `litd` to use the `BackendType` definition from `sqldb/v2`, instead of it's own definition.
The upcoming implementation of `sqldb/v2` will extensively create a `Queries` object on the fly. To make more intuitive how to create the queries object for specific database types, we introduce a `NewForType` helper method. This also mimics how `tapd` creates the `Queries` object, and in order not let `litd` have it's own definition of how `Queries` object are created on the fly, the upcoming `sqldb/v2` usage will utilize this helper method.
As we will change the `accounts`, `session` & `firewalldb` packages to use the `sqldb/v2` package, we need to make those packages use the `sqldb/v2` `PostgresStore` when setting up their test postgres databases, instead of `litd`'s own `PostgresStore` version. In order to enable that functionality, we add a new helper function that creates a `PostgresStore` using the `sqldb/v2` package, in addition to helper function that creates a `PostgresStore` using the `litd` version. Once we have shifted all of `litd`'s code to use the `sqldb/v2` definition, we will remove the `litd` version.
Update the accounts package to use the `sqldb/v2` package instead of the older version.
Update the session package to use the `sqldb/v2` package instead of the older version.
Previous commits had forgotten to add the `ListAllKVStoresRecords` query to the `firewalldb.SQLKVStoreQueries` interface. As that is required to make the query useable when defining the `sqldb/v2` `TransactionExecutor` for the `firewalldb` package, this commit adds it to the interface.
rename `sqlStore` to `store` in the firewalldb sql migration test file, to make the name shorted. This is done in preparation for future commits which will lengthen the lines where `sqlStore` is used, which otherwise would make the lines exceed the 80 character limit.
Update the firewalldb package to use the `sqldb/v2` package instead of the older version.
As we've now switched over to using sqldb v2 for most of the db objects, we can remove a lot of deprecated code that's no longer used in the litd project. This commit removes that code.
As the legacy `NewTestPostgresDB` function is no longer used and has been removed, it no longer makes sense to have a `V2` suffix on the `NewTestPostgresV2DB` function. This commit renames it to `NewTestPostgresDB`, to indicate that this function now replaces the legacy function.
ViktorT-11
force-pushed
the
2025-05-sqldb-v2
branch
from
5dcd396 to
f525a7f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently based on #1079
Implements step 1. of "Phase 3" in #917.
This is a draft PR to updates litd to use the new
sqldd/v2package. Note that this with just this PR,litdwill not utilize the capabilities of sqldb v2 to run specific post migrations steps (such as migrating the kvdb to SQL). That functionality will be added in upcoming PRs.Instead, this PR just focuses on adding support for the new sqldb v2 package, and the functionality of the SQL stores are expected to remain the same as prior to this commit.
Note that this is only a draft PR for now, as the
sqldb/v2is not released, and we therefore need to add a forked dependency in order to run this. Once we have an official release of thesqldb/v2package, I'll update this PR to use that dependency.