loopout: handle htlc confirmed notification#1120
loopout: handle htlc confirmed notification#1120starius wants to merge 5 commits intolightninglabs:masterfrom
Conversation
Add ServerHtlcConfirmedNotification message to SubscribeNotificationsResponse oneof. This new notification type allows the server to inform the client that a loop out HTLC has been confirmed on-chain, including the confirmed outpoint.
Add NotificationTypeHtlcConfirmed and SubscribeHtlcConfirmed so consumers can subscribe to HTLC-confirmed notifications. Also add dispatch in handleNotification to forward htlc_confirmed messages to subscribers.
Log unknown server notification variants at Info level instead of Warn to avoid noisy logs during feature rollout mismatches.
Add a parallel HTLC-confirmed notification path in waitForConfirmedHtlc that competes with the existing chain notifier path. Add swap-scoped deduplication for retry notifications and validate outpoint index plus pkscript before accepting notification-based confirmation. Update unit tests to cover success, invalid inputs, output mismatch, cancellation, nil notifier, and deduplication behavior.
Wire the notification manager as the HTLC confirmed notifier through the client config to swapConfig so that individual loop out swaps can subscribe to server-side HTLC confirmed notifications.
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 significantly enhances the reliability and responsiveness of HTLC confirmation in Loop by introducing an alternative, server-driven notification mechanism. This new path runs in parallel with the existing chain-based confirmation, aiming to accelerate the detection of confirmed HTLCs. It also incorporates sophisticated deduplication and validation logic to ensure the integrity and efficiency of these notifications, making the system more resilient to network conditions and server retries. Highlights
🧠 New Feature in Public Preview: You can now enable Memory 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 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 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. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism for loop out swaps to receive HTLC confirmation notifications from the server. It adds a new HtlcConfirmedNotifier interface, updates the notification manager to handle HtlcConfirmed notifications, and implements a parallel listener in loopOutSwap that registers for on-chain confirmations upon receiving a server notification. The implementation includes an atomic state machine to handle retries and deduplication. Feedback was provided regarding the log level for unknown notification types, suggesting a change from Infof to Debugf to minimize log noise during normal operation.
| log.Infof("Received unknown notification type: %v", | ||
| ntfn) |
There was a problem hiding this comment.
The log level for unknown notification types has been changed from Warnf to Infof. While this is a reasonable change to avoid spamming warnings for new, unsupported notification types, it might be better to use Debugf. An unknown notification type is not something that requires operator attention at an Info level, but it is useful for debugging. Using Debugf would keep the logs cleaner for normal operation while still providing the necessary information when debugging.
| log.Infof("Received unknown notification type: %v", | |
| ntfn) | |
| log.Debugf("Received unknown notification type: %v", | |
| ntfn) |
1 participant
Add a parallel HTLC-confirmed notification path in waitForConfirmedHtlc that competes with the existing chain notifier path.
Add swap-scoped deduplication for retry notifications and validate outpoint index plus pkscript before accepting notification-based confirmation.
Update unit tests to cover success, invalid inputs, output mismatch, cancellation, nil notifier, and deduplication behavior.
Pull Request Checklist
release_notes.mdif your PR contains major features, breaking changes or bugfixes