bLIP-0025: Allow forwarding HTLCs that underpay the onion encoded value

[valentinewallace]

For context, it is expected that many lightning users will be connected to the lightning network via
LSPs (Lightning Service Providers), who will be responsible for managing channel liquidity on end
users' behalf.

Often, users are onboarded to these services via a just-in-time inbound channel when they first
receive a payment. However, this channel open costs money, and so liquidity providers may want to
take an extra fee from the received payment so that end users can help bear the cost of this initial
on-chain fee. This bLIP outlines how they may take said fee.

[valentinewallace]

This needs some fleshing out, see spec meeting notes here lightning/bolts#1082 (comment).

[valentinewallace]

I updated this to disallow MPP unless the receiver has some way of communicating to the penultimate hop when all MPP parts have been aggregated. It'd be nice to add that as a separate bLIP and update this one, in the future. Discussed with @t-bast here: https://gnusha.org/lightning-dev/2023-05-23.log

Will take this out of draft after opening a PR for the implementation in LDK and adding a link.

[TheBlueMatt]

One comment, but not a huge deal.

[tnull]

nit:

Suggested change

	The following table contains extension tlv fields for the `update_add_htlc` message:
	The following table contains extension TLV fields for the `update_add_htlc` message:

[valentinewallace]

Skipped this to keep consistency with the rest of the file.

[JssDWt]

Suggested change

	TLV value of `u64` containing the amount of extra fee they took from the receiver's final received
	TLV value of `tu64` containing the amount of extra fee they took from the receiver's final received

The bolt spec says

For the final value in TLV records, truncated integers may be used. Leading zeros in truncated integers MUST be omitted:

tu64: a 0 to 8 byte truncated unsigned integer

It makes sense to use a tu64 here, since this is the final value of the TLV. And it saves some space for free.

[TheBlueMatt]

Eh, we already shipped this quite a while ago, and its not like this is going to ever go from one packet on the wire to two packets on the wire, so I don't really think this is worth it.

[JssDWt]

Ah, I didn't realize this was already shipped.

[tnull]

Hmm, given this has been used in production for a while by LSPS2 just-in-time channel providers, should we move forward with merging this eventually? (cc @valentinewallace @TheBlueMatt @t-bast)

[t-bast]

I'm using a slightly different one in #36 to also include the related funding_txid, but both can definitely coexist depending on the use-case, so I'm fine with moving forward on this PR.

It is missing a link to the bLIP in README.md and comments should be addressed, but otherwise LGTM.

[valentinewallace]

Addressed feedback and rebased due to merge conflicts, diff prior to rebase was:

diff --git a/README.md b/README.md
index bba6b55..2f8096a 100644
--- a/README.md
+++ b/README.md
@@ -25,3 +25,4 @@ For more detail on the process, please read [bLIP-0001](./blip-0001.md) and
 | [10](./blip-0010.md)     | Podcasting 2.0            | Satoshis Stream             | Active |
 | [11](./blip-0011.md)     | NameDesc                  | Hampus Sjöberg              | Active |
 | [17](./blip-0017.md)     | Hosted Channels           | Anton Kumaigorodskiy        | Active |
+| [25](./blip-0025.md)     | Forward less than onion   | Valentine Wallace           | Active |
diff --git a/blip-0025.md b/blip-0025.md
index 01a8871..3f032d7 100644
--- a/blip-0025.md
+++ b/blip-0025.md
@@ -26,9 +26,9 @@ Penultimate hop on the path:
   value, in millisatoshis

 Receiver:
-* MUST fail the HTLC if they did not expect an extra fee to be taken
+* MUST fail the HTLC if they did not expect an extra fee to be taken or if the extra fee taken is
+  too high
 * MUST either accept or reject the HTLC as if it had received `htlc_value + extra_fee`
-* MUST reject the HTLC if the extra fee taken is too high

 ## Motivation

@@ -47,15 +47,10 @@ that the fee taken by the penultimate hop is as-expected, in practice this may b
 right. If there were a bug in this logic, a sender could exploit it by forwarding less than the
 invoice's expected value, and receive proof-of-payment that they paid the full value.

-In the JIT channel context, MPP should be disallowed unless the receiver can communicate to the LSP
-when all MPP parts have been aggregated, because otherwise the LSP may be at risk of wastefully
-opening a new channel for each MPP part.
-
-## Implementation notes
-If this feature is being used in the context of the penultimate hop opening a just-in-time channel
-to the receiver, MPP should be disabled in the invoice unless the recipient has a way to communicate
-to the penultimate hop when all MPP parts have been received. See
-<https://github.com/BitcoinAndLightningLayerSpecs/lsp/pull/22> "Invoice Generation" section.
+## Implementation Notes
+See
+<https://github.com/BitcoinAndLightningLayerSpecs/lsp/tree/main/LSPS2#3--invoice-generation>
+for invoice requirements if this feature is being used in the JIT channel context.

 ## Reference Implementations
 LDK: <https://github.com/lightningdevkit/rust-lightning/pull/2319>

[t-bast]

We had an interesting discussion in #36 (comment) that is related to this, it's probably worth a read. This isn't a no-go at all for me if you want to go ahead with this TLV to support your existing use-cases, but it points out that it's probably hard to use as a general purpose tool.

[valentinewallace]

Thanks @t-bast, updated with this diff:

diff --git a/blip-0025.md b/blip-0025.md
index 3f032d7..a306ba4 100644
--- a/blip-0025.md
+++ b/blip-0025.md
@@ -20,12 +20,19 @@ This bLIP is licensed under the CC0 license.

 ## Specification

-Penultimate hop on the path:
-* In their outbound `update_add_htlc` message, MUST include a TLV record keyed by type 65537 with a
-  TLV value of `u64` containing the amount of extra fee they took from the receiver's final received
-  value, in millisatoshis
+We define a TLV field for `update_add_htlc` that allows a relaying node to
+relay a smaller amount than the amount encoded in the onion:

-Receiver:
+1. `tlv_stream`: `update_add_htlc_tlvs`
+2. types:
+    1. type: 65537 (`extra_fee`)
+    2. data:
+        - [`u64`:`amount_msat`]
+
+The writer:
+* MUST set `extra_fee` to the amount of extra fee they took from the receiver's final value
+
+The receiver:
 * MUST fail the HTLC if they did not expect an extra fee to be taken or if the extra fee taken is
   too high
 * MUST either accept or reject the HTLC as if it had received `htlc_value + extra_fee`

[valentinewallace]

We had an interesting discussion in #36 (comment) that is related to this, it's probably worth a read. This isn't a no-go at all for me if you want to go ahead with this TLV to support your existing use-cases, but it points out that it's probably hard to use as a general purpose tool.

Hmm so it sounds like this bLIP is not very compatible with splicing and may be deprecated in the near future? I don't feel strongly about what to do about that, maybe @tnull can weigh in. It sounds like this may still be useful for compat with LSPS2?

[TheBlueMatt]

I'll follow up there but I'm really not clear on why we want something different in bLIP 36. In any case, however, this is live in existing LSPS specs so we should at least land a bLIP that describes it.

[tnull]

Hmm so it sounds like this bLIP is not very compatible with splicing and may be deprecated in the near future? I don't feel strongly about what to do about that, maybe @tnull can weigh in. It sounds like this may still be useful for compat with LSPS2?

Yeah, I agree with Matt here: even if we end up going a different route in #36, we'd still want to land this as it's used and deployed by LSPS2 services.

[t-bast]

ACK 1bad9b2, LGTM 👍, feel free to merge this!