multi: add static address loop in monitor by GustavoStingelin · Pull Request #1165 · lightninglabs/loop

GustavoStingelin · 2026-07-06T02:26:51Z

Implements monitor support for static address loop-in swaps.

gemini-code-assist · 2026-07-06T02:27:36Z

Summary of Changes

Hello, 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 implements support for static address loop-in swaps within the monitor system. It extends the existing swap type definitions to include a new variant, updates the RPC interface to allow clients to interact with and filter these swaps, and ensures that the internal marshalling and filtering logic correctly processes these transactions.

Highlights

New Swap Type: Introduced 'StaticAddressLoopIn' as a new swap type across the codebase, including updates to internal types, RPC definitions, and Swagger documentation.
RPC and Filtering Updates: Updated the gRPC client and server to support the new swap type, including the ability to filter by 'STATIC_ADDRESS_LOOP_IN' in list swap requests.
Logic Enhancements: Updated swap marshalling and filtering logic to correctly handle the new static address loop-in swap type, including support for P2TR and P2WSH addresses.

New Features

🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

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 by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

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 pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize the 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 counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

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. ↩

GustavoStingelin · 2026-07-06T02:28:21Z

Looking for ConceptACK/suggestions.

Still need to test.

gemini-code-assist

Code Review

This pull request introduces support for a new swap type, STATIC_ADDRESS_LOOP_IN, updating the internal swap structures, marshalling logic, filtering, and RPC definitions. The feedback highlights a potential nil pointer dereference in marshallSwap when both HtlcAddressP2TR and HtlcAddressP2WSH are nil, and suggests adding a nil check for HtlcAddressP2WSH to prevent a panic.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-07-06T02:29:30Z

+		if loopSwap.HtlcAddressP2TR != nil {
+			htlcAddressP2TR = loopSwap.HtlcAddressP2TR.EncodeAddress()
+			htlcAddress = htlcAddressP2TR
+		} else {
+			htlcAddressP2WSH = loopSwap.HtlcAddressP2WSH.EncodeAddress()
+			htlcAddress = htlcAddressP2WSH
+		}


In the swap.TypeStaticAddressLoopIn case, if loopSwap.HtlcAddressP2TR is nil, the code falls back to the else block and calls loopSwap.HtlcAddressP2WSH.EncodeAddress(). However, if loopSwap.HtlcAddressP2WSH is also nil (which is highly likely for static address loop-in swaps that primarily use Taproot/P2TR), this will result in a nil pointer dereference panic.

To prevent this, we should add a nil check for loopSwap.HtlcAddressP2WSH before calling EncodeAddress().

Suggested change

if loopSwap.HtlcAddressP2TR != nil {

htlcAddressP2TR = loopSwap.HtlcAddressP2TR.EncodeAddress()

htlcAddress = htlcAddressP2TR

} else {

htlcAddressP2WSH = loopSwap.HtlcAddressP2WSH.EncodeAddress()

htlcAddress = htlcAddressP2WSH

}

if loopSwap.HtlcAddressP2TR != nil {

htlcAddressP2TR = loopSwap.HtlcAddressP2TR.EncodeAddress()

htlcAddress = htlcAddressP2TR

} else if loopSwap.HtlcAddressP2WSH != nil {

htlcAddressP2WSH = loopSwap.HtlcAddressP2WSH.EncodeAddress()

htlcAddress = htlcAddressP2WSH

}

starius

Concept ACK for API shape.

One implementation note. Monitor snapshots and subscribes only to s.swaps, populated from loop.Client.FetchSwaps and live statusChan updates. Static address loop-ins live under staticLoopInManager.GetAllSwaps() and (ListStaticAddressSwaps), so the current changes alone will not make loop monitor see them.

Some bridging is needed to deliver static loop-in updates into monitor's SwapStatus stream: initial snapshot plus live state updates. That also requires an explicit state mapping from static loop-in FSM states to generic SwapState, and filling the generic SwapInfo fields safely, especially HTLC address, amount, label, last hop, timestamps, and costs.

starius · 2026-07-06T06:13:04Z

+			htlcAddressP2TR = loopSwap.HtlcAddressP2TR.EncodeAddress()
+			htlcAddress = htlcAddressP2TR
+		} else {
+			htlcAddressP2WSH = loopSwap.HtlcAddressP2WSH.EncodeAddress()


Static address is always a P2TR, never a P2WSH, so we can just assert that loopSwap.HtlcAddressP2TR is set and return an error otherwise.

starius · 2026-07-06T06:17:27Z

+	if (swapInfo.SwapType == swap.TypeIn ||
+		swapInfo.SwapType == swap.TypeStaticAddressLoopIn) &&


We can use !swapType.IsOut() or even swapType.IsIn() instead of (swapInfo.SwapType == swap.TypeIn || swapInfo.SwapType == swap.TypeStaticAddressLoopIn)

So we can keep two distinct SwapType instances for loop-in and static loop-in, but use IsOut/IsIn in situations where both loop-in and static loop-in work the same way.

I kind of agree. My only concern is that the difference between TypeStaticAddressLoopIn, TypeIn, and type.IsIn() could become a bit brittle.

Internally, is it common in the codebase to classify TypeStaticAddressLoopIn as a generic In swap? If yes, I am okay with that.

If not, I think we should establish a clear separation in how we name and classify these swap types, so we do not introduce ambiguity later.

starius · 2026-07-06T06:19:31Z

        LOOP_IN = 2;
+
+        // STATIC_ADDRESS_LOOP_IN indicates a static address loop in swap.
+        STATIC_ADDRESS_LOOP_IN = 3;


This change seems unrelated to the loop monitor integration of static loop-ins. If we need it for completeness, I propose to do it in a separate commit or in a follow-up PR.

The thing here is, how can we tell the monitor that this is a static loop in swap and that we are sending a new notification type, not the classic swap in or swap out notification?

SwapStatus is currently a flat struct returned in the rpc Monitor, and SwapType is the field that carries this information. Adding this option here was the best design I found so far.

Are you thinking about another option? How would that look?

hieblmi · 2026-07-06T08:33:50Z

Thanks @GustavoStingelin for putting up this draft PR!

I am looking a bit ahead of where we'd update the monitor with the static state, and it should probably be updateLoopIn which is triggered by the state machine after every fsm action. Given that this is called
frequently we should probably not do too much work for the monitor here and slow down the state
transitions.

Keeping future swap types in mind maybe it would make sense to send lightweight notifications
with minimum swap info to the monitor instance, and then let the monitor instance retrieve and assemble
the full swap info.

Maybe we could have a small monitor publisher in each existing and new swap config that does something like:

f.cfg.MonitorPublisher.Notify(MonitorRef{
      Type: MonitorStaticLoopIn,
      ID:   f.loopIn.SwapHash,
  })

What do you think @GustavoStingelin @starius ?

GustavoStingelin · 2026-07-08T14:15:44Z

@hieblmi

Maybe we could have a small monitor publisher in each existing and new swap config that does something like

I like the idea, but I have two questions:

Is this expected to be included in this PR?
IIUC could this create a race between the notification and the monitor full swap info assemble? For example:

state changes, notification is sent
state changes again, another notification is sent
monitor is still assembling the first notification
monitor reads the latest state, so the first event is shown with the second state
monitor then processes the second notification and shows the same state again

In that case, we could show the second state twice in the monitor and lose the first state transition from the monitor log.

GustavoStingelin · 2026-07-08T14:47:03Z

I am thinking that, in the future, we could have something like this:

message MonitorEvent {
  string id = 1;
  bytes id_bytes = 2;
  SwapKind kind = 3;
  int64 amount = 4;
  int64 initiation_time = 5;
  int64 last_update_time = 6;
  string label = 7;

  // Human friendly normalized lifecycle.
  MonitorPhase phase = 8;

  // Machine specific exact state.
  string state = 9;

  oneof details {
    LoopInMonitorDetails loop_in = 20;
    LoopOutMonitorDetails loop_out = 21;
    StaticLoopInMonitorDetails static_loop_in = 22;
    // Future swap types can be added here.
  }
}

Where:

kind answers, what is this?

LOOP_IN
LOOP_OUT
STATIC_LOOP_IN
future: INSTANT_OUT, ASSET_LOOP_OUT, etc.

phase answers, where is it broadly in the lifecycle?

INITIATING
WAITING_FOR_PAYMENT
WAITING_FOR_CHAIN
SWEEPING
SUCCESS
FAILED

state answers, what is the exact implementation state?

SIGN_HTLC_TX
MONITOR_INVOICE_HTLC_TX
PREIMAGE_REVEALED
etc.

The idea is to separate the high level swap category, the normalized monitor lifecycle, and the exact internal state. That should make the monitor API easier to extend without overloading one field with multiple meanings.

WDYT @hieblmi @starius?

hieblmi · 2026-07-09T11:43:26Z

I am thinking that, in the future, we could have something like this:

message MonitorEvent {
  string id = 1;
  bytes id_bytes = 2;
  SwapKind kind = 3;
  int64 amount = 4;
  int64 initiation_time = 5;
  int64 last_update_time = 6;
  string label = 7;

from SwapKind to label all fields should be available in the swap details below. Do we need them here?

// Human friendly normalized lifecycle.
MonitorPhase phase = 8;

// Machine specific exact state.
string state = 9;

state here too is part of the swap details.

oneof details {
LoopInMonitorDetails loop_in = 20;
LoopOutMonitorDetails loop_out = 21;
StaticLoopInMonitorDetails static_loop_in = 22;
// Future swap types can be added here.
}
}

hieblmi · 2026-07-09T12:28:28Z

@hieblmi

Maybe we could have a small monitor publisher in each existing and new swap config that does something like

I like the idea, but I have two questions:

Is this expected to be included in this PR?

I tend to not assemble info in the static address state machine and then send it to the monitor, rather just send a lightweight reference which the monitor can assemble from.

IIUC could this create a race between the notification and the monitor full swap info assemble? For example:
state changes, notification is sent
state changes again, another notification is sent
monitor is still assembling the first notification
monitor reads the latest state, so the first event is shown with the second state
monitor then processes the second notification and shows the same state again
In that case, we could show the second state twice in the monitor and lose the first state transition from the monitor log.

This is a good point! We might cache the swap state at that point in time to not miss anything, but that might defeat
the purpose of being performant.

I think as long as the data we are returning to the monitor is already in memory we can think about this optimization later.

GustavoStingelin · 2026-07-09T13:50:43Z

@hieblmi

from SwapKind to label all fields should be available in the swap details below. Do we need them here?

Cool, I totally agree.

I think doing that broader refactor in this PR would be too much, but I’m also not very happy with my current local code. It has some ugly workarounds to support both state models.

So what do you think about shifting to the new monitor approach now? Or would you prefer to take a look at my current code first and decide from there?

Also, just to confirm, since we release the client and loopd together, a breaking protobuf API change should not be a problem, right?

GustavoStingelin · 2026-07-09T15:55:15Z

+          "state": "INITIATED",
+          "static_loop_in_state": "SIGN_HTLC_TX",


@hieblmi It is the thing that I'm mostly concerned about

gemini-code-assist Bot reviewed Jul 6, 2026

View reviewed changes

starius reviewed Jul 6, 2026

View reviewed changes

GustavoStingelin added 2 commits July 8, 2026 23:35

looprpc: expose static loop-in monitor state

365af4c

staticaddr/loopin: preserve loop-in last update time

8720b72

levmi assigned GustavoStingelin Jul 9, 2026

GustavoStingelin force-pushed the feat/monitor-static-address branch from c93d2dd to db63a8b Compare July 9, 2026 15:53

GustavoStingelin commented Jul 9, 2026

View reviewed changes

GustavoStingelin added 5 commits July 9, 2026 14:07

loopd: map static address loop-ins to swap status

f29460a

loopd: include static loop-ins in monitor snapshots

81be5ad

staticaddr/loopin: publish status updates after persistence

0007002

cmd/loop: display static loop-in monitor stages

09dce72

cmd/loop: add static loop-in monitor replay

73bc60a

GustavoStingelin force-pushed the feat/monitor-static-address branch from db63a8b to 73bc60a Compare July 9, 2026 17:07

GustavoStingelin changed the title ~~DRAFT: multi: add static address loop in monitor~~ multi: add static address loop in monitor Jul 9, 2026

GustavoStingelin marked this pull request as ready for review July 9, 2026 17:30

GustavoStingelin requested review from hieblmi and starius July 9, 2026 17:30

		if (swapInfo.SwapType == swap.TypeIn \|\|
		swapInfo.SwapType == swap.TypeStaticAddressLoopIn) &&

		"state": "INITIATED",
		"static_loop_in_state": "SIGN_HTLC_TX",

Uh oh!

Conversation

GustavoStingelin commented Jul 6, 2026

Uh oh!

gemini-code-assist Bot commented Jul 6, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

GustavoStingelin commented Jul 6, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

starius left a comment

Choose a reason for hiding this comment

Uh oh!

starius Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

hieblmi Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

starius Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

GustavoStingelin Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

starius Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

GustavoStingelin Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

hieblmi commented Jul 6, 2026

Uh oh!

GustavoStingelin commented Jul 8, 2026

Uh oh!

GustavoStingelin commented Jul 8, 2026

Uh oh!

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

Uh oh!

Uh oh!

hieblmi commented Jul 9, 2026

Uh oh!

GustavoStingelin commented Jul 9, 2026

Uh oh!

GustavoStingelin Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hieblmi commented Jul 9, 2026 •

edited

Loading