Skip to content

DRAFT: multi: add static address loop in monitor#1165

Draft
GustavoStingelin wants to merge 1 commit into
lightninglabs:masterfrom
GustavoStingelin:feat/monitor-static-address
Draft

DRAFT: multi: add static address loop in monitor#1165
GustavoStingelin wants to merge 1 commit into
lightninglabs:masterfrom
GustavoStingelin:feat/monitor-static-address

Conversation

@GustavoStingelin

Copy link
Copy Markdown

Implements monitor support for static address loop-in swaps.

@gemini-code-assist

Copy link
Copy Markdown

Summary of Changes

Hello, I'm Gemini Code Assist1! 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.

Footnotes

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

Copy link
Copy Markdown
Author

Looking for ConceptACK/suggestions.

Still need to test.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Comment on lines +418 to +424
if loopSwap.HtlcAddressP2TR != nil {
htlcAddressP2TR = loopSwap.HtlcAddressP2TR.EncodeAddress()
htlcAddress = htlcAddressP2TR
} else {
htlcAddressP2WSH = loopSwap.HtlcAddressP2WSH.EncodeAddress()
htlcAddress = htlcAddressP2WSH
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

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 starius left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

+1

Comment on lines +716 to +717
if (swapInfo.SwapType == swap.TypeIn ||
swapInfo.SwapType == swap.TypeStaticAddressLoopIn) &&

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

Comment thread looprpc/client.proto
LOOP_IN = 2;

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

@hieblmi

hieblmi commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

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 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants