-
Notifications
You must be signed in to change notification settings - Fork 2.2k
multi: add mode for external payment lifecycle management #10178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
multi: add mode for external payment lifecycle management #10178
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @calvinrzachman, 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 introduces a new configuration option to support external management of payment lifecycles. It allows a node to preserve the history of HTLC attempts across restarts by preventing the local ChannelRouter from automatically cleaning up the dispatcher's attempt store. This is crucial for scenarios where a remote router or external entity is responsible for managing in-flight payments via an RPC interface, ensuring state is maintained and preventing loss of in-flight payment state.
Highlights
- External Payment Lifecycle Management: Introduced a new configuration flag "routing.managed-externally" to enable external entities to manage HTLC attempt lifecycles.
- Conditional HTLC Store Cleanup: Modified the ChannelRouter to skip automatic cleanup of the HTLC attempt store on startup when "routing.managed-externally" is true, preserving state for external controllers.
- Configuration Integration: The new flag is integrated into the default configuration, the routing configuration struct, and passed to the router's internal configuration.
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. 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.
Footnotes
-
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. ↩
There was a problem hiding this comment.
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 a new configuration option, routing.managed-externally
, to support external payment lifecycle management. When enabled, it prevents the local ChannelRouter
from cleaning up the HTLC attempt store on startup, preserving state for external controllers. The changes are well-contained and correctly implemented across the configuration, server, and routing packages. My review includes a minor style suggestion to improve code alignment for better readability.
Make use of remote router conditional upon building lnd with the switchrpc server. Either all payments are managed by the onboard router, or they're managed by an external payment life-cycle managing entity (eg: a "remote router").
To support scenarios where an external entity, such as a remote router, manages the payment lifecycle via the Switch RPC server, the node must preserve the history of HTLC attempts across restarts. Currently, the local ChannelRouter cleans up this history on startup, which would cause the external entity to lose state and fail to properly manage in-flight payments. This commit makes this behavior conditional on how the lnd binary is built. When compiled with the switchrpc build tag, the local ChannelRouter's automatic cleanup of the dispatcher's (Switch) attempt store on startup is disabled. This shifts the responsibility of state cleanup to the external controller, which is expected to use an RPC interface (e.g., switchrpc) to manage the lifecycle of attempts. Tying this behavior to a build tag, rather than a runtime flag, makes the binary's purpose explicit and prevents potential misconfigurations.
This prepares for modifying the behavior of the switch depending on how lnd has been deployed in the following commit.
Prevent silent loss of htlc attempt information and resulting stuck payments via introduction of a db flag which indicates whether lnd has been used with a remote router (switchrpc) configuration. The switch will refuse to start if lnd is built without the switchrpc server while this db flag is present. To revert back to using the local router, allow the remote router to complete all active payment attempts it has in-flight with the switch. The safety check during Switch creation prevents start up of the Switch if it contains attempt data from use with a remote router. This avoids lnd's onboard channel router from clearing state needed to track payment life-cycle for payments that it did not inititate. NOTE: This may be unnecessary once the Switch stores are namespaced and can be shared by multiple routers at once.
01b4543
to
5f7a1ad
Compare
@calvinrzachman, remember to re-request review from reviewers when ready |
Change Description
To support scenarios where an external entity, such as a remote router, manages the payment lifecycle via the Switch RPC server, the node must preserve the history of HTLC attempts across restarts.
Currently, the local ChannelRouter cleans up this history on startup, which would cause the external entity to lose state and fail to properly manage in-flight payments.
This commit makes this behavior conditional on how the lnd binary is built. When compiled with the
switchrpc
build tag, the local ChannelRouter's automatic cleanup of the dispatcher's (Switch) attempt store on startup is disabled. This shifts the responsibility of state cleanup to the external controller, which is expected to use an RPC interface (e.g., switchrpc) to manage the lifecycle of attempts. Tying this behavior to a build tag, rather than a runtime flag, makes the binary's purpose explicit and prevents potential misconfigurations.IMPORTANT: It is currently only safe to allow a single entity (either the local router or one external router) to dispatch attempts via the Switch at any given time. Running multiple controllers concurrently will lead to undefined behavior and potential loss of funds.