jsonrpc: Add description_hash parameter to invoice

[laanwj]

Add an optional description_hash parameter to the invoice command. This can be specified instead of the description, to create an invoice that only commits to a 256 bit hash (generally SHA256) of the description.

This is used in the lnurlp protocol. See lightningd/plugins#307 for example usage.

Closes #4705

[vincenzopalazzo]

LGTM, I have only a small comment for the python doc string

[laanwj]

I have pushed a new version with (thanks @vincenzopalazzo).

• slightly extended documentation string for invoice (mention that description needs to be empty)
• add blurb to manual page
• updated invoice python docstring from lightning-cli help output
• added CHANGELOG.md entry

Edit: Somehow it's still saying that the PR is missing a changelog entry 🤷‍♀️ (should be fixed now)

[vincenzopalazzo]

ACK 4788cba

[rustyrussell]

Sorry for delay on this!

I prefer a boolean flag to say "hash description instead": that way lightningd can save the description in the db and show it in listinvoices. Otherwise you've made an unknown commitment (pun intended!) which it has to honor.

We've generally steered away from hashed descriptions, because there was not a transport which used them. Time to bring them back!

[laanwj]

I prefer a boolean flag to say "hash description instead": that way lightningd can save the description in the db and show it in listinvoices

I didnt do this because the descriptions can be extremely long, and in the case of lnurlp they're kind of boring (e.g. it's just the same metadata, which can include images, every time, sometimes with other metadata concatenated). You could store it in the db for every payment but it also seems wasteful in a way.

[rustyrussell]

You control this though, right? You're issuing the invoice, you'd better control what it says. Otherwise use keysend, it's simpler.

[laanwj]

You control this though, right? You're issuing the invoice, you'd better control what it says. Otherwise use keysend, it's simpler.

Yes and no. The creator of the invoice controls it. But in the case of lnurlp (the primary use of this), they have to commit to data in a specific format (the metadata string as specified in LUD-06). Wallets do check the description_hash field and don't allow paying the invoice if it's something else (Something I discovered in ACINQ/phoenix#213).

I don't think there's an option to use keysend here (but I might be wong).

[laanwj]

Anyhow, I'm happy to rewrite this to use the full description and a boolean argument instead.
I think I need help on the "store the description in the database without making it part of the invoice" (with no limit to length) part though. Not sure how to do that.

[erikarvstedt]

LUD-06 allows embedding base64-encoded images in the description which would needlessly bloat the invoice DB.
I'd prefer to let clients decide where and whether to store the description.

[rustyrussell]

Sure, you can always remove old invoices (in fact, autocleaninvoice does this!) though. It doesn't remove paid ones though, that would need enhancement.

We actually already have a 'description' field in the db, and it's wired through (we initially supported creating invoices with description hashes, but nobody used them so we removed the ability).

Just hand the description (instead of b11->description, which will be NULL) to wallet_invoice_create() and it will all Just Work.

[rustyrussell]

LUD-06 allows embedding base64-encoded images in the description which would needlessly bloat the invoice DB. I'd prefer to let clients decide where and whether to store the description.

The invoice is the contract you're committing to. You really want to save it all by strong default; sure, there are scenarios where you can reproduce it, but storage is cheap.

[laanwj]

Just hand the description (instead of b11->description, which will be NULL) to wallet_invoice_create() and it will all Just Work.

Thanks, I'll look into that.

[fiatjaf]

Some reasons why c-lightning storing the full description is a bad idea:

1. Unlike normal invoices, these invoices will never be generated manually, they will be generated automatically by some other program that will hopefully be storing the full description.
2. For most lnurlpay cases the description can be regenerated anytime by just inputting the same parameters to a function.
3. For most lnurlpay cases the description will repeat itself many many times as the paycode is reused -- for example:
   • A donation lnurlpay code will say "donate to Bob" and contain a picture of Bob, base64-encoded, and that will have to be stored again and again every time someone donates to Bob?
   • Someone buying a coffee on a coffeeshop by scanning the lnurlpay QR, the description will contain the name of the coffee item and a picture of the coffee.
4. Most importantly: it's the buyer's responsibility to keep track of what he has paid for. Once the buyer is able to show the full description and the hash of that matches the description hash on the signed invoice, everything is proved and it doesn't matter that the seller has stored the full description or not.

[rustyrussell]

Some reasons why c-lightning storing the full description is a bad idea:

1. Unlike normal invoices, these invoices will never be generated manually, they will be generated automatically by some other program that will hopefully be storing the full description.

That's a big assumption! You'd be surprised what people start doing to make QR codes smaller.

2. For most lnurlpay cases the description can be regenerated anytime by just inputting the same parameters to a function.

3. For most lnurlpay cases the description will repeat itself many many times as the paycode is reused -- for example:
   
   * A donation lnurlpay code will say "donate to Bob" and contain a picture of Bob, base64-encoded, and that will have to be stored again and again every time someone donates to Bob?
   * Someone buying a coffee on a coffeeshop by scanning the lnurlpay QR, the description will contain the name of the coffee item and a picture of the coffee.

Sure. Put 1MB in there, and still nobody cares, as long as you expire unused invoices?

4. Most importantly: it's the buyer's responsibility to keep track of what he has paid for. Once the buyer is able to show the full description and the hash of that matches the description hash on the signed invoice, everything is proved and it doesn't matter that the seller has stored the full description or not.

Sure, in a perfect world! But in practice they do "hey, I paid this " and you go, "um, you did, I have no idea what that is".

The default should never be to discard this information. I'd rather add an option to autoclean to remove it (maybe have delinvoice add a 'desconly' option?).

[rustyrussell]

See #5121 for comparison.

[fiatjaf]

I'm fine with that one considering the new ways to delete invoices. The benefit is that it might be simpler for some users to delegate the description-hashing process to c-lightning.

[rustyrussell]

Merged #5121 instead

[callebtc]

I need this feature as well, not the way it is implemented today (with deschash_only).

There are countless apps out there that can only pass the hash and don't have access to the description itself (because they don't really need to and/or want to avoid unnecessary bloat).

Afaik, CLN is the only implementation that doesn't accept the hash directly and requires special treatment in LNbits. Having the option to just pass the hash would make many things easier and more efficient.

[kiwiidb]

+1 we would like to support CLN as a backend in https://github.com/getAlby/lndhub.go
But this requires a minimum of API overlap with LND and not being able to do this is a complete blocker :(

[jb55]

On Wed, Sep 07, 2022 at 05:29:19AM -0700, kiwiidb wrote: +1 we would like to support CLN as a backend in https://github.com/getAlby/lndhub.go But this requires a minimum of API overlap with LND and not being able to do this is a complete blocker :(

In what situation where you would be paying a deschash invoice and you wouldn't have the description? In lnurl, the static endpoint has the description (metadata). afaik lnurl is the biggest usages of deschash, I'm not sure of others, but we shouldn't be encouraging that usecase as for reasons discussed above.

[kiwiidb]

It's not that we don't have the description. It's just that writing a service with multiple node back-ends is more difficult due to this difference in API. I guess we could also make a breaking change and do it this way.