Fix missing check for pay_invoice for NWC recv #2365
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
In #2169, I forgot to copy the existing validation for NWC receive to the new code.
This means we didn't check that
pay_invoiceis not supported.Additional Context
I downloaded a CSV file for all currently saved NWC receive protocols to check if any of them do support
pay_invoice. Then we should delete them.I also went through the diff of #2169 to check if I forgot that for other protocols, too, but I didn't. But most of them didn't even include a way to test if payments are not supported (see LNbits, Phoenixd).
This could be fixed via #1341. Instead of only testing if a payment is possible, we can do the same when attaching a receive wallet to make sure it's not possible (we always cancel the HODL invoice).
Checklist
Are your changes backward compatible? Please answer below:
yes, but we have to manually delete any existing NWC wallet for receiving that supports payments
On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:
10. Attaching NWC send for NWC receive fails now. Attaching NWC receive for NWC receive still works.For frontend changes: Tested on mobile, light and dark mode? Please answer below:
n/a
Did you introduce any new environment variables? If so, call them out explicitly here:
no