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

Display TLV data in listinvoices #4470

Open
Sjors opened this issue Apr 7, 2021 · 18 comments
Open

Display TLV data in listinvoices #4470

Sjors opened this issue Apr 7, 2021 · 18 comments
Assignees

Comments

@Sjors
Copy link
Contributor

Sjors commented Apr 7, 2021

When receiving a keysend payment I can see it under listinvoices. But this doesn't show any custom TLV fields.

I'm particularly interested in the fields used by Podcasting 2.0: https://github.com/satoshisstream/satoshis.stream/blob/main/TLV_registry.md#field-7629169

@cdecker cdecker self-assigned this Apr 13, 2021
@rustyrussell rustyrussell added this to the v0.10.1 milestone Apr 16, 2021
@cdecker
Copy link
Member

cdecker commented Apr 24, 2021

We have one issue with this, namely that despite the name, TLV fields are not typed as such. They are just blobs of binary data, that are interpreted as integers, hashes, signatures, etc by any program that has more knowledge about the T in the acronym.

So a generic implementation would just show type, length (redundant really in JSON results) and a hex-encoded binary value. Would that still be acceptable?

@Sjors
Copy link
Contributor Author

Sjors commented Apr 24, 2021

It's a start. I could probably pipe that through JQ and then some tools to decipher the hex value. Maybe plugins can do that too.

@daveajones
Copy link

We have one issue with this, namely that despite the name, TLV fields are not typed as such. They are just blobs of binary data, that are interpreted as integers, hashes, signatures, etc by any program that has more knowledge about the T in the acronym.

So a generic implementation would just show type, length (redundant really in JSON results) and a hex-encoded binary value. Would that still be acceptable?

The hex encoded strings are what we are parsing when pulling out the tlv payload on incoming keysend transactions for the Podcast Index node. It’s not difficult (bin2hex —> json_decode) and provides tremendous listener stats to see what podcast/episode was being listened to, and exactly what time stamp within it that the payment was sent.

The podcast industry currently works on “download stats” which are highly inaccurate. It’s basically guesswork. This would revolutionize that as a standard for listener metrics.

@cdecker
Copy link
Member

cdecker commented Jul 4, 2021

#4610 is the first step towards this. It adds experimental support for extratlvs, i.e., hex-encoded TLVs that are not interpreted and stored internally yet, with opt-in to even TLV types on the command line. They are currently only being exposed in the invoice_payment hook for testing, but if the format looks ok to users we can add them to the corresponding notification, and start storing them in the DB as well, so listinvoices can retrieve and display them.

@cdecker cdecker modified the milestones: v0.10.1, v0.10.2 Jul 30, 2021
@cdecker cdecker modified the milestones: v0.10.2, next Oct 8, 2021
@rustyrussell rustyrussell modified the milestones: v0.11, v0.12 Mar 28, 2022
@niftynei niftynei modified the milestones: v0.12, v22.10 Aug 2, 2022
dennisreimann added a commit to dennisreimann/BTCPayServer.Lightning that referenced this issue Sep 21, 2022
dennisreimann added a commit to dennisreimann/BTCPayServer.Lightning that referenced this issue Sep 21, 2022
NicolasDorier pushed a commit to btcpayserver/BTCPayServer.Lightning that referenced this issue Sep 26, 2022
* Add custom TLV records to invoice

Custom [TLV records](https://github.com/satoshisstream/satoshis.stream/blob/main/TLV_registry.md) only work for LND right now, Core Lightning support might come, see ElementsProject/lightning#4470

* Format files
@cnixbtc
Copy link

cnixbtc commented Oct 4, 2022

a generic implementation would just show type, length (redundant really in JSON results) and a hex-encoded binary value. Would that still be acceptable?

I think that would be fine. It should be easy for consuming apps to interpret the TLV type and decode the hex values accordingly.

They are currently only being exposed in the invoice_payment hook for testing

That's already really helpful -- thanks for that! :)

I've written a small plugin for lightningd that watches for Podasting 2.0 TLVs in the invoice_payment hook and stores them in a local db so that they can be retrieved again later on: cln-podcast-payments.

It's not much but should be an acceptable workaround until Core Lightning can natively do that.

but if the format looks ok to users we can add them to the corresponding notification, and start storing them in the DB as well, so listinvoices can retrieve and display them.

That'd be great, imo! As I said, I feel like the binary format is fine enough. Having access to TLVs in listinvoices would make the use of a separate plugin obsolete and by that make everything easier for people to use.

@cdecker
Copy link
Member

cdecker commented Nov 19, 2022

I wonder if this can now be considered as solved by #5619. That PR adds the longest string extracted from TLVs into the invoice we use to track spontaneous payments.

@jb55
Copy link
Collaborator

jb55 commented Nov 20, 2022

yes I see podcast keysend data in listinvoices now which is pretty cool
Nov20-083149

@Sjors
Copy link
Contributor Author

Sjors commented Nov 23, 2022

Nice, I'll give this a try after the final v22.11 release.

@dennisreimann
Copy link
Contributor

Great news, looking forward to integrate this into our BTCPay Lightning lib.

@cdecker cdecker modified the milestones: v22.11, v23.02 Dec 1, 2022
@dennisreimann
Copy link
Contributor

I wonder if this can now be considered as solved by #5619

@cdecker As far as I see, the description with the keysend: part contains only the 133773310 TLV key. Is there a way to also get access to the other custom TLV records?

@Sjors
Copy link
Contributor Author

Sjors commented Dec 12, 2022

This jq query gets a long list of podcast sats:

lightning-cli listinvoices | jq -c '[.invoices[] | select(.status == "paid") | select(.label | startswith("keysend-")) | select(.description | startswith("keysend:"))] | .[].description[9:] | fromjson' | jq --slurp

This query doesn't seem to find all boosts though, e.g. adding | select(.action == "boost") only returns one result, even though the episode in question has 6 boosts.

I'm guessing the other boosts live under 7629173.

@dennisreimann
Copy link
Contributor

dennisreimann commented Jan 6, 2023

@Sjors Are you able to access TLV records other than 133773310 at all? Just checked again with CLN 22.11 and I only get those. Pointers appreciated :)

Also curious to know if the accept-htlc-tlv-types option has to list the types one wants to accept, e.g.

accept-htlc-tlv-types=696969,112111100

@rustyrussell
Copy link
Contributor

Like, seriously... Should we just put every damn TLV in the invoice label? Standards FFS people. Or are there TLVs which are ACTUALLY used which we should just always include?

Otherwise, we may need an explicit new field for this garbage, and give you all the TLVs in hex and let you sort it out :(

Removing this from milestone.

@rustyrussell rustyrussell removed this from the v23.02 milestone Feb 6, 2023
@dennisreimann
Copy link
Contributor

@rustyrussell It'd be nice to include the well known ones. I came across this testing the Podcasting 2.0 fields (7629173, 7629175, 133773310) + wallet integration (696969, 112111100) …

@Sjors
Copy link
Contributor Author

Sjors commented Feb 6, 2023

every damn TLV

That's probably the easiest. Having a parameter to specify one or more is fine by me too, though perhaps that more suitable for a transaction filter (plugin?).

Also curious to know if the accept-htlc-tlv-types option has to list the types one wants to accept

Accepting or rejecting TLV types seems out of scope for here.

@dennisreimann
Copy link
Contributor

every damn TLV

That's probably the easiest. Having a parameter to specify one or more is fine by me too

Maybe combining it with accept-htlc-tlv-types would be an option, I haven't looked at how it works internally though.

Unfortunately the current state of only listing certain TLV records isn't intuitive and as described above one cannot support e.g. the Podcasting 2.0 features fully as there is no API access to most of the records.

@dennisreimann
Copy link
Contributor

Sorry to bump this, but including either the well-known TLV records or the ones listed in accept-htlc-tlv-types (see above) would give us a chance to implement Podcasting 2.0 features on Core Lightning.

If you don't see it that way that's also fine, but then let's manage expectations and close this issue. /cc @cdecker @rustyrussell

@Sjors
Copy link
Contributor Author

Sjors commented Dec 4, 2023

As podcaster I like to stack sats, as the song goes...

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

8 participants