staticaddr: migrate deposit swap hashes #988

hieblmi · 2025-07-30T12:53:16Z

To allow for versatile queries when handling deposits related to swaps this PR adds a column swap_hash to the deposits table that maps deposit ids to their respective swap hashes.

A migration maps the deposits that are stored in a loop-in entry to that new table.
Loop-In getter methods pull related deposit records and attach them to the swap object.

gemini-code-assist

Summary of Changes

Hello @hieblmi, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a significant database schema change and a corresponding migration to improve how deposit IDs are associated with static address loop-in swaps. By creating a dedicated deposit_to_swap table, the system gains enhanced flexibility for querying and managing deposit-related data, ensuring that historical and future swap deposits are properly normalized and easily retrievable.

Highlights

Database Schema Enhancement: A new deposit_to_swap table is introduced to explicitly map deposit_id to swap_hash, enabling more versatile queries for deposit-related swap information.
Data Migration: A one-time migration (MigrateDepositNormalization) is implemented and integrated into the daemon's startup. This migration populates the new deposit_to_swap table by extracting deposit outpoint information from existing static address loop-in swaps.
New Data Access Layer: New SQL queries and corresponding Go methods are added to the loopdb and staticaddr/loopin/sql_store to facilitate the creation, retrieval, and batch processing of these new deposit-to-swap mappings.
Improved Data Normalization: The changes normalize the storage of deposit-to-swap relationships, moving away from potentially less structured storage within loop-in entries to a dedicated, queryable table.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces a new database table deposit_to_swap to create a mapping between deposits and swaps, which will help in querying. A database migration is included to populate this new table for existing swaps. The changes include new SQL schema, queries, and Go code to interact with the new table, as well as the migration logic and tests.

My review has identified a critical issue in the sqlc-generated code that will likely cause runtime errors, along with several medium-severity issues related to maintainability, such as a function name typo, an incorrect comment, use of magic numbers, and inconsistent naming conventions in generated code. Addressing these points will improve the quality and robustness of the code.

loopdb/sqlc/static_address_loopin.sql.go

loopd/daemon.go

loopdb/sqlc/static_address_loopin.sql.go

staticaddr/loopin/sql_store.go

hieblmi · 2025-07-31T14:56:35Z

staticaddr/loopin/deposit_normalization_migration.go

+const (
+	// depositNormalizationMigrationID is the identifier for the deposit
+	// normalization migration.
+	depositNormalizationMigrationID = "deposit_normalization"


rename "deposit normalization" to something like deposit swap hash migration...

starius · 2025-07-31T17:19:00Z

To allow for versatile queries when handling deposits related to swaps this PR introduces table deposit_to_swap that maps deposit ids to their respective swap hashes.

A migration maps the deposits that are stored in a loop-in entry to that new table.

The PR description seems outdated. It no longer introduces a new table, but a new column in an existing table.

starius

LGTM! 🎾
Added few comments.

starius · 2025-07-31T17:27:03Z

loopdb/sqlc/queries/static_address_loopin.sql

+    swap_hash = $1;
+
+
+


Too many empty lines in this file. I propose to have 1 empty line between the queries.

starius · 2025-07-31T17:30:28Z

staticaddr/loopin/deposit_normalization_migration.go

+	// First we'll fetch all past and pending static loop in swaps from the
+	// database.
+	swaps, err := swapStore.GetStaticAddressLoopInSwapsByStates(
+		ctx, append(PendingStates, FinalStates...),


You can use AllStates.

Also I propose to redefine AllStates not to append to another public variable:

var AllStates = append( append([]fsm.StateType{}, PendingStates...), FinalStates..., )

starius · 2025-07-31T17:37:39Z

staticaddr/loopin/deposit_normalization_migration.go

+					"not found", outpoint)
+			}
+
+			depositsToSwapHashes[deposit.ID] = swap.SwapHash


I propose to add a check that depositsToSwapHashes[deposit.ID] didn't exist already.

added, and log a warning if its a duplicate, but not interrupt migration. Deposit IDs are unique so this should not happen.

if _, ok := depositsToSwapHashes[deposit.ID]; !ok { depositsToSwapHashes[deposit.ID] = swap.SwapHash } else { log.Warnf("Duplicate deposit ID %s found for "+ "outpoint %s, skipping", deposit.ID, outpoint) }

starius · 2025-07-31T17:39:33Z

staticaddr/loopin/sql_store.go

+	for _, id := range depositIDs {
+		swapHash, err := s.baseDB.SwapHashForDepositID(ctx, id[:])
+		if err != nil {
+			if errors.Is(err, sql.ErrNoRows) {


|| errors.Is(err, pgx.ErrNoRows) {

starius · 2025-07-31T19:34:10Z

staticaddr/loopin/fsm.go

 	OnHtlcTimeoutSweepPublished = fsm.EventType("OnHtlcTimeoutSweepPublished")
 	OnHtlcTimeoutSwept          = fsm.EventType("OnHtlcTimeoutSwept")
 	OnPaymentReceived           = fsm.EventType("OnPaymentReceived")
-	OnPaymentDeadlineExceeded   = fsm.EventType("OnPaymentDeadlineExceeded")


starius · 2025-07-31T20:07:49Z

loopdb/sqlc/queries/static_address_loopin.sql

+        LIMIT 1
+    )
+WHERE
+    d.swap_hash = $1;


I propose to try to use LATERAL JOIN. It should work faster than an embedded query.

SELECT d.*, u.update_state, u.update_timestamp FROM deposits d LEFT JOIN LATERAL ( SELECT * FROM deposit_updates WHERE deposit_updates.deposit_id = deposits.deposit_id ORDER BY deposit_updates.timestamp DESC LIMIT 1 ) u ON true WHERE d.swap_hash = $1;

Please make sure to have this index:

CREATE INDEX ... ON deposit_updates(deposit_id, timestamp DESC);

LATERAL JOIN is not supported by sqlite.

starius · 2025-07-31T20:10:51Z

staticaddr/loopin/sql_store_test.go

+func TestGetStaticAddressLoopInSwapsByStates(t *testing.T) {
+	// Set up test context objects.
+	ctxb := context.Background()
+	testDb := loopdb.NewTestDB(t)


starius · 2025-07-31T20:13:23Z

staticaddr/loopin/sql_store_test.go

+	require.Equal(t, deposit.LoopedIn, finalizedDeposits[0].GetState())
+}
+
+// TestCreateLoopIn ...


Please complete the godoc.

since deposits have been normalized in the db in the context of a static address swap this commit now retrieves the deposit information as part of GetLoopInByHash and GetStaticAddressLoopInSwapsByStates.

sputn1ck

LGTM!

hieblmi self-assigned this Jul 30, 2025

hieblmi added the static-loop-in label Jul 30, 2025

hieblmi marked this pull request as draft July 30, 2025 12:53

gemini-code-assist bot reviewed Jul 30, 2025

View reviewed changes

hieblmi force-pushed the normalize-deposit-swap branch 4 times, most recently from 8be67b3 to deeed97 Compare July 31, 2025 11:20

hieblmi marked this pull request as ready for review July 31, 2025 11:43

hieblmi requested review from sputn1ck and starius July 31, 2025 11:43

hieblmi force-pushed the normalize-deposit-swap branch from deeed97 to f5377ca Compare July 31, 2025 14:34

hieblmi commented Jul 31, 2025

View reviewed changes

starius approved these changes Jul 31, 2025

View reviewed changes

staticaddr: don't append to public variable PendingStates

35901d1

hieblmi force-pushed the normalize-deposit-swap branch from f5377ca to 6e2fce5 Compare August 1, 2025 09:02

hieblmi changed the title ~~staticaddr: normalize swapped deposits~~ staticaddr: migrate deposit swap hashes Aug 1, 2025

hieblmi force-pushed the normalize-deposit-swap branch from 6e2fce5 to 62e649b Compare August 1, 2025 09:24

hieblmi added 5 commits August 1, 2025 11:26

staticaddr: migrate swap hashes to deposits

6e7441b

staticaddr: remove unused fsm transition

a1af860

staticaddr: relax scope of ToDeposit

d7ac314

loopdb: add query DepositsForSwapHash

d5e3075

staticaddr: fetch deposits as part of swap retrieval

fe7622d

since deposits have been normalized in the db in the context of a static address swap this commit now retrieves the deposit information as part of GetLoopInByHash and GetStaticAddressLoopInSwapsByStates.

hieblmi force-pushed the normalize-deposit-swap branch from 62e649b to fe7622d Compare August 1, 2025 09:26

sputn1ck approved these changes Aug 1, 2025

View reviewed changes

hieblmi merged commit 2f21a48 into lightninglabs:master Aug 1, 2025
4 checks passed

staticaddr: migrate deposit swap hashes #988

staticaddr: migrate deposit swap hashes #988

Uh oh!

Conversation

hieblmi commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

starius commented Jul 31, 2025

Uh oh!

starius left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sputn1ck left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hieblmi commented Jul 30, 2025 •

edited

Loading