DRAFT: multi: add static address loop in monitor#1165
Conversation
Summary of ChangesHello, 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 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 AssistThe 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
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 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
|
|
Looking for ConceptACK/suggestions. Still need to test. |
There was a problem hiding this comment.
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.
| if loopSwap.HtlcAddressP2TR != nil { | ||
| htlcAddressP2TR = loopSwap.HtlcAddressP2TR.EncodeAddress() | ||
| htlcAddress = htlcAddressP2TR | ||
| } else { | ||
| htlcAddressP2WSH = loopSwap.HtlcAddressP2WSH.EncodeAddress() | ||
| htlcAddress = htlcAddressP2WSH | ||
| } |
There was a problem hiding this comment.
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().
| 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
left a comment
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
Static address is always a P2TR, never a P2WSH, so we can just assert that loopSwap.HtlcAddressP2TR is set and return an error otherwise.
| if (swapInfo.SwapType == swap.TypeIn || | ||
| swapInfo.SwapType == swap.TypeStaticAddressLoopIn) && |
There was a problem hiding this comment.
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.
| LOOP_IN = 2; | ||
|
|
||
| // STATIC_ADDRESS_LOOP_IN indicates a static address loop in swap. | ||
| STATIC_ADDRESS_LOOP_IN = 3; |
There was a problem hiding this comment.
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.
|
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 Keeping future swap types in mind maybe it would make sense to send lightweight notifications Maybe we could have a small monitor publisher in each existing and new swap config that does something like: What do you think @GustavoStingelin @starius ? |
Implements monitor support for static address loop-in swaps.