fix stuck payment#10141
Conversation
There was a problem hiding this comment.
Summary of Changes
Hello @ziggie1984, 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!
I've addressed an issue where payments could get stuck by ensuring that all outstanding payment attempts are properly resolved when the overall payment lifecycle is canceled due to a timeout or context cancellation. This prevents orphaned payment states and improves the robustness of the payment routing system.
Highlights
- Payment Lifecycle Robustness: I've adjusted the payment lifecycle logic to proactively check for context cancellation or timeouts. This ensures that all outstanding payment attempts are resolved promptly, preventing payments from getting stuck in an unresolved state.
- Release Notes Update: I've added a release note to document this fix, informing users about the improved reliability of payment resolution.
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 or fill out our survey to provide feedback.
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.
Code Review
This pull request addresses a bug where a payment could become stuck if it timed out while waiting for an HTLC attempt to resolve. The fix correctly moves the context check to the beginning of the payment lifecycle loop, ensuring that timeouts are handled promptly on each iteration. The logic is sound and effectively resolves the issue. A corresponding release note has also been added. I have one minor suggestion to improve a comment for better readability, adhering to the style guide's emphasis on clarity.
56c503c to
36e427c
Compare
We check the context of the payment lifecycle at the beginning of the `resumepayment` loop. This will make sure we have always the latest state of the payment before deciding on the next steps in the function `decideNextStep`.
36e427c to
524d1b4
Compare
|
@saubyk need reviewers here. |
bitromortac
left a comment
There was a problem hiding this comment.
Nice LGTM 🎉
So just to ensure my understanding of the bug is that checkContext marks the payment failed and that creates an inconsistency with currentPayment, which leads to a failure later when registering the attempt in the db, aborting the payment lifecycle and keeping the payment in flight.
524d1b4 to
dc7bea1
Compare
Exactly that's the right analysis. Updated the release notes in the last push. |
1 similar comment
Exactly that's the right analysis. Updated the release notes in the last push. |
Fixes #10130