-
Notifications
You must be signed in to change notification settings - Fork 47
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
bLIP-0025: Allow forwarding HTLCs that underpay the onion encoded value #25
bLIP-0025: Allow forwarding HTLCs that underpay the onion encoded value #25
Conversation
4241675
to
d17353d
Compare
d17353d
to
e435847
Compare
This needs some fleshing out, see spec meeting notes here lightning/bolts#1082 (comment). |
e435847
to
3692c46
Compare
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment, but not a huge deal.
3692c46
to
ed6f44e
Compare
@@ -101,6 +101,13 @@ The following table contains extension tlv fields for the `payment_onion_payload | |||
| 7629169 | `podcasting_2_0` | [bLIP 10](./blip-0010.md) | | |||
| 5482373484 | `keysend_preimage` | [bLIP 3](./blip-0003.md) | | |||
|
|||
#### `update_add_htlc` | |||
|
|||
The following table contains extension tlv fields for the `update_add_htlc` message: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
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: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skipped this to keep consistency with the rest of the file.
blip-0025.md
Outdated
|
||
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I didn't realize this was already shipped.
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) |
I'm using a slightly different one in #36 to also include the related It is missing a link to the bLIP in |
99b5fa1
to
cb889f3
Compare
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> |
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. |
cb889f3
to
1bad9b2
Compare
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` |
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? |
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. |
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 1bad9b2, LGTM 👍, feel free to merge this!
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.