-
Notifications
You must be signed in to change notification settings - Fork 122
loop: add resume manager #1029
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
loop: add resume manager #1029
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.
Code Review
This pull request introduces a recovery manager to handle unfinished swaps due to routing failures. It adds a NotificationManager interface and a resumeManager struct to manage and recover unfinished swaps. The changes include modifications to client.go, loopd/daemon.go, loopout.go, notifications/manager.go, and swapserverrpc/server.proto to incorporate the new recovery mechanism. The review focuses on identifying potential issues related to correctness, efficiency, and maintainability, with a severity level of medium or higher.
| message SubscribeNotificationsRequest { | ||
| enum ListenerVersion { | ||
| LEGACY = 0; | ||
| V1 = 1; | ||
| } | ||
|
|
||
| ListenerVersion version = 1; | ||
| } |
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.
medium: Adding a ListenerVersion enum and version field to the SubscribeNotificationsRequest message. It's good practice to add a comment explaining the purpose of this versioning and when new versions might be introduced. This will help future developers understand the intent and maintain the code more effectively.
11bffcb to
7706173
Compare
7706173 to
5cffa6e
Compare
3dda018 to
c93f1ab
Compare
c93f1ab to
c411679
Compare
c411679 to
79af5b3
Compare
This PR enables resumption of a stuck swap due to routing failures.
Pull Request Checklist
release_notes.mdif your PR contains major features, breaking changes or bugfixes