json-rpc: Check that sendpay with mpp has all the details it needs #3450
Conversation
This file contains 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
ZmnSCPxj
reviewed
Jan 29, 2020
There was a problem hiding this comment.
Probably the doc/lightning-sendpay.7.md should be updated as well, text reads:
The *msatoshi* amount, if provided, is the amount that will be recorded
as the target payment value. If not specified, it will be the final
amount to the destination. By default it is in millisatoshi precision; it can be a whole number, or a whole number
ending in *msat* or *sat*, or a number with three decimal places ending
in *sat*, or a number with 1 to 11 decimal places ending in *btc*.Nary a mention of multipath in the document, yet this is supposed to signal multipath.
|
So, prior to mpp, msatoshi could be used to indicate the amount we wanted recorded as paid (e.g. the amount before fuzz or fees). We asserted that it was <= the actual amount sent. These days we keep the invoice string around for that anyway. So this was actually a compat break. Let's just remove msatoshi argument unless mpp; I tried coding up both old and new semantics and it's a confusing mess. Will send patch... |
We were implicitly assuming a multi-part payment if the `msatoshi` argument was specified, but then didn't check that we have all the pieces in place for mpp. This adds a couple of additional checks to the arguments and makes it more explicit what it means to do an mpp. Fixes ElementsProject#3431 Reported-by: Sergei Tikhomirov <@s-tikhomirov>
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.
We were implicitly assuming a multi-part payment if the
msatoshiargumentwas specified, but then didn't check that we have all the pieces in place for
mpp. This adds a couple of additional checks to the arguments and makes it
more explicit what it means to do an mpp.
Fixes #3431
Reported-by: Sergei Tikhomirov <@s-tikhomirov>