Skip to content

[sql-42] accounts: Update SQL migration to iterate over buckets + ensure proper closure of SQL test stores #1084

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

ViktorTigerstrom
Copy link
Contributor

This PR addresses review comment #1051 (comment), and changes the implementation of the accounts SQL migration to instead iterate over the Bbolt Buckets, rather than using the public BoltStore struct members.

Additionally, we add also add a createStore helper function for the test SQL db creation methods, that ensures that the SQL db is properly closed during the test cleanup. We will add a similar function for the sessions + firewalldb packages in their respective PRs, and also build upon that function in the upcoming PR that adds support for the sqldb/v2 implementation, which is why that function may feel a bit too verbose for just this PR alone.

Update the SQL migration to instead of using the public functions of the
BoltStore struct, we instead iterate over the buckets directly. This
ensures that the BoltStore struct Acccounts & LastIndexes functions of
the BoltStore implementation can be changed, without effecting the
SQL migration.
We add a helper function to the functions that creates the test SQL
stores, in order to ensure that the store is properly closed when the
test is cleaned up.
@ViktorTigerstrom ViktorTigerstrom self-assigned this Jun 9, 2025
@ViktorTigerstrom ViktorTigerstrom added the no-changelog This PR is does not require a release notes entry label Jun 9, 2025
Copy link
Member

@ellemouton ellemouton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! thanks

Copy link
Contributor

@bitromortac bitromortac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ✔️

@ellemouton ellemouton merged commit d593098 into lightninglabs:master Jun 10, 2025
21 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog This PR is does not require a release notes entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants