Skip to content
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

Fix sendpayment without amount argument, BIP21 only #1357

Merged

Conversation

kristapsk
Copy link
Member

Fixes #1356. Bug was introduced with #1316.

@AdamISZ
Copy link
Member

AdamISZ commented Sep 25, 2022

Agreed, that's what I did to fix it too :) utACK.

As I noted a couple of years ago, our sendpayment code got way too difficult to parse (I mean: there are a lot of complex conditional branchings), because we kept adding new options piecemeal.

If someone wants to rationalize it, so that it's easier to follow, and therefore easier to manage, that would be great.

@kristapsk
Copy link
Member Author

It's bad that there is no test coverage, as that code is complex enough to accidentally break things from time to time. :(

@kristapsk kristapsk merged commit cd61051 into JoinMarket-Org:master Sep 25, 2022
@kristapsk kristapsk deleted the sendpayment-fix-no-amount branch September 25, 2022 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New BIP21 parsing can crash without amount argument
2 participants