Skip to content

Ability to add custom TLV to update_add_htlc message #6663

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

Open
2 of 3 tasks
JssDWt opened this issue Sep 11, 2023 · 4 comments
Open
2 of 3 tasks

Ability to add custom TLV to update_add_htlc message #6663

JssDWt opened this issue Sep 11, 2023 · 4 comments
Assignees

Comments

@JssDWt
Copy link
Contributor

JssDWt commented Sep 11, 2023

The LSPS2 specification requires the LSP to add a custom extra_fee TLV to the update_add_htlc message. An LSP run on top of CLN will intercept htlcs using the htlc_accepted hook. For some htlcs, LSP fees will be deducted from the outgoing amount, by modifying the onion payload.

  • It will modify the next hop id record (6)
  • It will modify the amount to forward record (2)
  • It has to add an extra_fee record. (65537)

I tried adding the extra_fee record to the returned payload in the htlc_accepted hook, but I get an error: Failing HTLC because of an invalid payload. I assume this is because the TLV is not recognized. The only option currently available appears to be to enable all unknown TLVs on the node, but that is not what I want.

Relevant section in LSPS2: https://github.com/BitcoinAndLightningLayerSpecs/lsp/blob/a1d8ca8439e730d6a097585f7e1d091f12f9afb0/LSPS2/README.md?plain=1#L950-L966

So: Add support for custom TLVs for the update_add_htlc message.

@cdecker cdecker self-assigned this Sep 11, 2023
@cdecker cdecker added this to the v23.11 milestone Sep 11, 2023
@cdecker
Copy link
Member

cdecker commented Sep 11, 2023

I tried adding the extra_fee record to the returned payload in the htlc_accepted hook, but I get an error: Failing HTLC because of an invalid payload. I assume this is because the TLV is not recognized.

That is expected, since the payload is the TLV stream for the local node. As such lightningd will fail the HTLC if the payload contains any field that is not understood by lightningd. The onion would contain the payload for the next hop (but is unmodifiable due to the encrypted and HMACd nature of the onion). There is currently no way return value that allows adding fields to the TLVs in the update_add_htlc because it was not used by anyone so far.

I will add this new return field to the htlc_accepted hook call, and wire it through to the appropriate place.

@nepet nepet removed this from the v23.11 milestone Dec 3, 2023
@JssDWt
Copy link
Contributor Author

JssDWt commented Apr 17, 2025

I'm revisiting this subject with the extra-fee TLV. It's still useful.
The extra_fee TLV would actually come from the local node's payload. Just like the amount to forward and next hop id.
It's set as a TLV on the update_add_htlc message, so there's no need to hack the onion.

If there would be a setting in CLN to allow adding extra TLVs to the htlc_accepted hook payload, and CLN would pass these TLVs around up to sending the update_add_htlc message this can work. The problem is right now CLN picks the fields it knows and forwards. Let's walk through it:

  • The TLV would end up in the payload in htlc_accepted_hook_deserialize if the extra TLV is in the accept-htlc-tlv-type config.
  • Then only the fields CLN knows about are passed along to forward_htlc. Passing the additional TLVs here could be an option, all the way up to sending update_add_htlc.

@andrei-21
Copy link

Hey team.
Just wanted to bump this issue as it's pretty important for enabling LSPS2 server implementations on CLN. The extra_fee TLV field is a key piece needed to make this work.
This also ties into the new bLIP-36 protocol, which will rely on similar TLV fields for on-the-fly funding. Having this in place would open up more flexibility for node operators and service providers.

Would love to see this prioritized if possible! Thanks for all the work you’re already putting in.

@cdecker
Copy link
Member

cdecker commented May 8, 2025

@nepet and I had a good discussion on how this could be implemented, since he's also working on an LSPS implementation. So this should happen soon ☺️

@nepet nepet self-assigned this May 14, 2025
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

4 participants