-
Notifications
You must be signed in to change notification settings - Fork 94
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
[r2r] Lightning Swaps Locktimes + Lightning Maker Swap #1557
Conversation
…e that generated the invoice, add unit tests for this case
…er swap locktime for other coin 1.5x taker swap locktime
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.
Awesome as usual!
I have mostly questions due to the algorithm complexity
…_log_with_msg when we need to log info too
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.
LGTM, thank you!
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.
Great work 🔥 I only have a few minor notes and some questions
mm2src/coins/lightning.rs
Outdated
Ok(()) | ||
}, | ||
Ok(None) => MmError::err(ValidatePaymentError::UnexpectedPaymentState(format!( | ||
"Payment {} should be found on the database", |
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.
How about "Payment {} could not be found on the database"
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.
Because the error variant is UnexpectedPaymentState
, I wanted to highlight the expected state in the error message. At this stage (validating a swap payment), the payment info should be in the database. I can modify the message to be
"Payment {} is not in the database when it should be"
What do you think?
de0fa05
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.
Re-approve
#1045