Handle onion creation failure #314
Conversation
| @@ -14,6 +14,7 @@ sealed class FinalFailure { | |||
|
|
|||
| // @formatter:off | |||
| object AlreadyPaid : FinalFailure() { override fun toString(): String = "this invoice has already been paid" } | |||
| object InvoiceTooBig : FinalFailure() { override fun toString(): String = "this invoice contains too much metadata to be paid" } | |||
There was a problem hiding this comment.
@dpad85 @robbiehanson what do you think of this error? I believe we need to surface to the user that this problem is caused by this specific invoice that they are trying to pay, and that some of the fields it contains force us to send too much data to the recipient, which we cannot do.
This shouldn't happen often, but when it does, it's good that it's explicitly handled. We currently use an inefficient trampoline scheme where the onion is capped at 400 bytes, we will change that to a variable-size onion which should improve the issue if we see it happening regularly once option_payment_metadata starts being deployed.
There was a problem hiding this comment.
This makes sense. The error message would get surfaced to the user. On iOS, this info gets displayed in the payment details screen. For a developer, or anybody helping to debug, this would be very useful information.
There was a problem hiding this comment.
Agreed, the UI needs as much info as it can get. We don't have to display everything right away, but this is a good thing to have and we can also reformat those messages easily if needed.
There was a problem hiding this comment.
Cool, then this should find its way into lightning-kmp soon (I've stacked a few PRs on top of each other)
If we have too much data to include in the onion, its creation will fail. This wasn't handled explicitly before, as we thought there was no reason we would overflow the onion space. However, it happened when invoices contained too many route hints, and it can now happen if invoices contain too much payment metadata, so we need to handle it explicitly and provide a clear error to the user.
t-bast
force-pushed
the
handle-onion-creation-failure
branch
from
574df26
to
6bb2712
Compare
|
Closing this PR, as we may be able to implement a final spec version of trampoline, which would make this easier to check before-hand that we won't overflow the onion and won't need that weird additional code. |
If we have too much data to include in the onion, its creation will fail.
This wasn't handled explicitly before, as we thought there was no reason we would overflow the onion space.
However, it happened when invoices contained too many route hints, and it can now happen if invoices contain too much payment metadata (or too many features), so we need to handle it explicitly and provide a clear error to the user.
I'm not a huge fan of the complexity it brings to the
OutgoingPaymentHandler, if you have suggestions on how we could improve it that would be great.EDIT: once we move to variable-size trampoline onions, we probably don't need that change anymore and can instead set a hard-coded limit on invoice data before creating the onion.