-
Couldn't load subscription status.
- Fork 109
Add custom channels liquidity edge cases itest #842
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
Add custom channels liquidity edge cases itest #842
Conversation
d85b327 to
d77401e
Compare
| select { | ||
| case <-done: | ||
| case <-timeoutChan: | ||
| t.Fatalf("Payment didn't fail within expected time duration") |
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.
I'm doing this because both timeout and actual payment error result in lnrpc.Payment_FAILED above, so I wanna make sure we failed but not because we got stuck in a loop (behavior prior to bug fix)
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 to have a specific test for these edge cases 👍
|
|
||
| } | ||
|
|
||
| logBalance(t.t, nodes, assetID, "after failed 50 assets") |
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.
Would be nice if we could also assert the payment's failure reason, that it's actually the INSUFFICIENT_BALANCE code. I think we return that somewhere.
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.
yeap good idea, it's already available here through the SendPayment RPC response, result.FailureReason
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.
Since this is meant to fail in the paymentLifecycle phase, we should get a NO_ROUTE instead
The reason the failure happens there is because that's when we actually provide the 500 sats amount to the PaymentBandwidth hook. See more on this comment
d77401e to
fcd4227
Compare
|
|
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.
Linter and itest failed, other than that looks good 🎉
fcd4227 to
15e694a
Compare
|
|
15e694a to
6a2097a
Compare
6a2097a to
7f75248
Compare
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 catches, LGTM 🎉
| fn.None[lnrpc.PaymentFailureReason](), | ||
| ) | ||
|
|
||
| logBalance(t.t, nodes, assetID, "after 50 sats backwards") |
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.
nit: after 50 assets backwards, not sats
This PR adds an itest that exposes the liquidity related edge cases of custom channels.
The following PRs need to be merged first, in order to bump the dependencies on this PR:
Closes lightninglabs/taproot-assets#1060
Closes lightninglabs/taproot-assets#1073
Closes lightninglabs/taproot-assets#1013