bolts icon indicating copy to clipboard operation
bolts copied to clipboard

final_incorrect_htlc_amount failure too strict?

Open joostjager opened this issue 3 years ago • 4 comments

The spec dictates that the htlc amount for the final hop must match the amount in the onion payload exactly:

https://github.com/lightning/bolts/blob/f32c6ddb5f11b431c9bb4f501cdec604172a90de/04-onion-routing.md?plain=1#L1118

Is there a reason why the final hop must not accept an htlc with an amount that is higher than the payload amount?

Relaxing this requirement could be helpful to distinguish in a route description between the intended payment amount and the actual amount delivered to the final hop.

For example when the final channel has a min_htlc policy of 100 sat that is above the payment amount of let's say 80 sat, the sender could decide to use the channel anyway. That they are giving away 20 sat extra isn't visible in the route description. For the two last nodes, both will have an amt_to_forward of 100 sat. WIth MPP, there is the total amount, but it doesn't tell you how much each htlc is overpaying.

joostjager avatar Sep 14 '22 07:09 joostjager

But the sender knows how much it's overpaying, so there's no issue with the onion needing to be flexible. I agree I'm not aware of a reason why this needs to be strict, but absent a good reason to change it...

TheBlueMatt avatar Sep 15 '22 21:09 TheBlueMatt

The sender knows how much it's overpaying, but it is something that needs to be tracked separately. It can't be determined by just looking at a single outgoing htlc and associated onion payload(s).

I think the advantage of making the spec less strict is that the current amount_to_forward field in the onion can carry the overpayment information, so that no separate internal field is required.

joostjager avatar Sep 19 '22 09:09 joostjager

IIUC you're suggesting overpaying by reducing the amount in the onion. You shouldn't do this - this allows the second-to-last node to steal the overpayment amount. This isn't a huge deal on its own, but it does mean nodes are incentivized to always try this, which will slow down HTLC forwarding.

TheBlueMatt avatar Sep 21 '22 13:09 TheBlueMatt

Not overpaying by reducing the onion amount, but overpaying by increasing the instructed forward amount for the previous node. Your comment does get me thinking.

If you want to pay 10 sat through a channel with a 100 sat min_htlc policy, you'll have to overpay. But because the sending side of a channel can violate it's own min_htlc policy, they can indeed only forward 10 and keep the rest. The final node can protect against this by doing the current exact match with the onion amount.

For routing nodes though, there is no such protection. If a channel somewhere in the middle of route has a high min_htlc policy and there is no alternative route, the sender may decide to overpay to meet min_htlc. But also in that case the sending side of that channel can steal the overpayment. Not sure if that matters to the sender though? For them the only thing that probably matters is that the payment succeeds?

joostjager avatar Sep 21 '22 14:09 joostjager

Without looking at it, I thought the htlc-min was set by a node and applied for incoming HTLCs. ie you're always protected cause you apply it when you receive an HTLC, though you have to forward an HTLC that is below your own htlc-min, it just has to comply with your peer's HTLC-min.

TheBlueMatt avatar Sep 29 '22 22:09 TheBlueMatt

There are two different properties. The one that you are talking about is htlc_minimum_msat as agreed when the channel was opened. It is indeed applied by the party who accepts the htlc.

The one I was referring to is in the channel_update message, here. It is applied only by the routing node that sets it, and only to the outgoing channel.

joostjager avatar Oct 08 '22 07:10 joostjager

Then I don't understand your original comment - a node can require overpayment through the htlc_min in the channel_update, yes, but no matter what it cannot under-forward because the values are committed to in the onion - whatever the origin node intended to do is what you have to forward. I still fail to see any reason why we should change this.

TheBlueMatt avatar Oct 08 '22 21:10 TheBlueMatt

Acceptance criteria have been relaxed in https://github.com/lightning/bolts/pull/1040, https://github.com/lightning/bolts/pull/1032 and https://github.com/lightning/bolts/pull/1031.

Closing issue.

joostjager avatar Dec 08 '22 15:12 joostjager