lightningd: disallow msatoshi arg to sendpay unless exact when non-…
#3470
Conversation
…MPP. Using it with a different value to the amount sent causes a crash in 0.8.0, which is effectively deprecating it, so let's disallow it now. Changelog-Changed: If the optional `msatoshi` param to sendpay for non-MPP is set, it must be the exact amount sent to the final recipient. Signed-off-by: Rusty Russell <[email protected]>
rustyrussell
force-pushed
the
fix-sendpay-msat-no-partid
branch
from
aac1023
to
e6b04f9
Compare
| @@ -27,8 +27,8 @@ definite failure. | |||
| The *label* and *bolt11* parameters, if provided, will be returned in | |||
| *waitsendpay* and *listsendpays* results. | |||
|
|
|||
| 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 | |||
| The *msatoshi* amount must be provided if *partid* is non-zero, otherwise | |||
There was a problem hiding this comment.
This parameter used to be used for value randomization, where we overpay the payee, but record the exact amount indicated in the invoice in our database rather than the overpaid amount (i.e. treating the overpayment as a fee). We have since replaced value randomization with shadow routing, also overpaying the payee, but it seems this was no longer used to override the database record. I think it is better if our permanent records did not reveal the use of overpayment to obscure the location of the payee, but well. In any case changing msatoshi to make it signal multipath is a breaking change in the previous interface.
There was a problem hiding this comment.
Yes, the original intent with mpp was to only override the semantic for msatoshi for this case, but since it's broken in 0.8.0 anyway :(
The original use has been largely superceded by the fact that we now save the bolt11 string with the payment.
| """sendpay msatoshi arg was used for non-MPP to indicate the amount | ||
| they asked for. But using it with anything other than the final amount | ||
| caused a crash in 0.8.0, so we then disallowed it. | ||
| """ |
There was a problem hiding this comment.
Nit: comments should be a short single line description (<80 chars), followed by a blank line, and then a multiline description (if any) that is indented to match the """, finally a """ on it's own on a last line. You comment would become this:
"""Verify that the msatoshi arg is checked for `sendpay`.
The `sendpay` `msatoshi` argument was used for non-MPP to indicate the amount
they asked for. But using it with anything other than the final amount caused a crash
in 0.8.0, so we then disallowed it.
"""
…MPP.
Using it with a different value to the amount sent causes a crash in 0.8.0,
which is effectively deprecating it, so let's disallow it now.
Changelog-Changed: If the optional
msatoshiparam to sendpay for non-MPP is set, it must be the exact amount sent to the final recipient.Signed-off-by: Rusty Russell [email protected]
Fixes: #3431
Closes: #3450