-
Notifications
You must be signed in to change notification settings - Fork 911
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
Check whether the recipient supports keysend #4236
Conversation
9854082
to
9dc9a51
Compare
Ack 9dc9a51 But you have to fix up tests which broke... |
0c9bbd4
to
1ee3c64
Compare
Rolled up the fixups. The remaining issue is unrelated (flaky Ready to merge ^^ |
1ee3c64
to
25936ae
Compare
We were blindly initiating the keysend payment, which could lead to confusing outcomes. This adds a very specific error message to the error returned. Changelog-Fixed: keysend: Keysend now checks whether the destination supports keysend before attempting a payment. If not a more informative error is returned.
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.
OK, I'll rebase on master and drop first commit now we have get_gossmap()
plugins/libplugin-pay.h
Outdated
|
||
/* A way for a plugin to interrogate the gossip map for things | ||
* it needs to know. */ | ||
const struct gossmap *gossmap; |
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.
No, this looks like it predates get_gossmap() which returns this? That would explain why this doesn't compile, too.
Stashing it in the struct payment is really ugly, too...
25936ae
to
ac9f7dd
Compare
ACK ac9f7dd |
We weren't checking whether the recipient supports
keysend
or not before trying to pay. This could lead to rather unhelpful 16399 errors, because the recipient simply doesn't understand what we're trying to do.This might be the cause for #3968, but I can't verify that that's the case.
Fixes #3968