lnd icon indicating copy to clipboard operation
lnd copied to clipboard

Aux Closer: Move coop-close aux finalization to chain watcher

Open GeorgeTsagk opened this issue 3 months ago • 5 comments

Description

Moves the aux closer finalization hook call site from ReceiveClosingSigned, which relies on our peer staying online and sending us the message, to the chain watcher where we instead wait for the coop close tx to be detected onchain.

This implies that the finalization will be completed after some delay, but this also comes coupled with the guarantee of not having to rely on our peer behaving correctly.

Circular dependency

The aux closer is used in lnwallet/chancloser and now also in contractcourt (by chain watcher). This caused a circular dependency which disallowed us from importing some definitions directly from the former package. The direct solution was to replicate some of the types and use an adapter on the server for our new slim interface. Not an ideal solution, will keep searching for alternatives -- although this is non blocking for the purpose of this PR.

GeorgeTsagk avatar Oct 13 '25 13:10 GeorgeTsagk

Summary of Changes

Hello @GeorgeTsagk, I'm Gemini Code Assist[^1]! 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 refactors the cooperative channel closing mechanism to enhance its reliability. It shifts the responsibility of finalizing auxiliary channel closures from relying on a peer's message to monitoring the blockchain for the confirmed cooperative close transaction. This change ensures that finalization is guaranteed, albeit with a slight delay, by removing the dependency on the peer's online status or correct behavior. The implementation also addresses a resulting circular dependency by introducing new types and an adapter.

Highlights

  • Aux Closer Relocation: The finalization hook call for the aux closer has been moved from ReceiveClosingSigned (which relied on peer messages) to the chain watcher. This means finalization now occurs when the cooperative close transaction is detected on-chain, ensuring robustness even if the peer is offline or misbehaves.
  • Circular Dependency Resolution: A circular dependency between lnwallet/chancloser and contractcourt was introduced due to the aux closer's new usage. This was resolved by replicating necessary types in contractcourt and implementing an adapter in server.go to bridge the interfaces.
  • New Interface and Types: New types (CloseOutput, AuxShutdownReq, AuxCloseDesc) and an interface (AuxChanCloser) have been defined within contractcourt to support the new on-chain finalization logic and manage the circular dependency.
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 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 :thumbsup: and :thumbsdown: 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.

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.

[^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.

gemini-code-assist[bot] avatar Oct 13 '25 13:10 gemini-code-assist[bot]

Replying to @gijswijs's comment:

If node restarts after close tx confirms but before finalization, what happens? Do we re-detect the spend on restart?

I believe that this is the case, since this is code that is called for any channel in LND (with the finalizeCoopClose being optionally called).

I'm missing an unit test for finalizeCoopClose., although the original ReceiveClosingSigned func also doesn't have any testing iiiuc. But this might be a good time to increase that coverage.

Yeah there's not much to test as this is an external hook. We can make a basic-level assertion on a mock implementation of it -- i.e just make sure that it is being called with non-nil parameters.

GeorgeTsagk avatar Oct 20 '25 10:10 GeorgeTsagk

I will re-request review although 2 main things remain to figure out:

  • Do we keep both call-sites for better UX? (https://github.com/lightningnetwork/lnd/pull/10289#discussion_r2439129067)
  • Add test coverage (probably in LiT) to expose the behavior of a peer dropping the connection before sending the closing signed message.

GeorgeTsagk avatar Oct 30 '25 12:10 GeorgeTsagk

Rebased

GeorgeTsagk avatar Dec 04 '25 11:12 GeorgeTsagk

@gijswijs: review reminder @georgetsagk, remember to re-request review from reviewers when ready

lightninglabs-deploy avatar Dec 17 '25 02:12 lightninglabs-deploy