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

Legacy multi-trampoline mpp fails due to wrong total amount #2127

Closed
bitromortac opened this issue Jan 11, 2022 · 1 comment
Closed

Legacy multi-trampoline mpp fails due to wrong total amount #2127

bitromortac opened this issue Jan 11, 2022 · 1 comment

Comments

@bitromortac
Copy link

I'm testing mpp payments with Electrum and we want to enable splitting across multiple trampoline forwarders.
Consider a splitting of (S: sender, R: receiver, T1: trampoline 1, T2: trampoline 2):

S ->- T1 ->- R (500/2000)
      T1 ->- R (500/2000)
S ->- T2 ->- R (1000/2000)

In this case, the sender tries to send 2000 sat to R, splitting accross T1 and T2. This already works for E2E trampoline payments, because the trampoline onions that arrive at the receiver have the information about the total amount from the trampoline onions.

In the legacy case, the trampolines T1, T2 carry out a payment to R and they can know about the total amount from their trampoline onion. Right now, T1 encodes a total amount of 1000 sat in the normal onion, causing a legacy receiver to fail the payment, because the total amount doesn't match the invoice amount.

I communicated with @t-bast, and the culprit lies in

val payment = SendMultiPartPayment(payFsmAdapters, paymentSecret, payloadOut.outgoingNodeId, payloadOut.amountToForward, payloadOut.outgoingCltv, nodeParams.maxPaymentAttempts, routingHints, routeParams)

discarding the total amount from the trampoline onion and sending a different total amount.

Receiving trampoline onions will probably still take a while which is why I think it's important to support legacy mpp. I'll check whether I can find some time to fix this, if not anybody else wants to.

@t-bast
Copy link
Member

t-bast commented Jul 29, 2024

I don't think we need to support MPP across multiple trampoline nodes towards nodes that don't support trampoline. Using the trampoline-to-legacy relay is already a hack, that we don't want people to rely on (because trampoline nodes learn the recipient's identity and may steal over-payments or the whole payment in case of an amount-less invoice).

We've already waited for years without this being a real issue in practice, we can wait longer to get recipient to either support trampoline or Bolt 12 (which fixes these issues as well: see the last commit from lightning/bolts#836).

@t-bast t-bast closed this as completed Jul 29, 2024
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

No branches or pull requests

2 participants