invoice description hash support

[rustyrussell]

No description provided.

[erikarvstedt]

No thorough review, these are just some doc errors that stuck out.

As stated before, I'd prefer if there was an option for hash-only storage.
clightning should be unintrusive, efficient base layer tech. Forcing invoice creators to store descriptions and then deleting them in a separate command is inconvenient and slow.

[jb55]

Concept ACK, going to test this with https://github.com/jb55/lnurl-commando

[jb55]

If we now have a deschashonly option why not allow just passing a description hash? Otherwise when generating invoices remotely I have to send the full png over commando every time. not a big deal but it's slightly annoying...

[jb55]

If we now have a deschashonly option why not allow just passing a description hash? Otherwise when generating invoices remotely I have to send the full png over commando every time. not a big deal but it's slightly annoying...

ah I see #4892 implements it this way already 🤔

[rustyrussell]

No thorough review, these are just some doc errors that stuck out.

As stated before, I'd prefer if there was an option for hash-only storage. clightning should be unintrusive, efficient base layer tech. Forcing invoice creators to store descriptions and then deleting them in a separate command is inconvenient and slow.

Then... don't delete them? If you're regularly issuing invoices you should be deleting expired ones, and if you have thousands of successful payments you can afford a few GB of storage.

[rustyrussell]

To be clear: I really want to see what invoices I have issued by my node. I suspect that the ability to mint invoices with only a description hash will get abused by plugins pretty badly, and disk space is cheap.

[vincenzopalazzo]

LGTM, the only comment that I have is that some of these procedure parameters may be declared constant!

[vincenzopalazzo]

Suggested change

	struct invoices *invoices,
	struct invoice invoice)
	const struct invoices *invoices,
	const struct invoice invoice)

[niftynei]

@rustyrussell can you outline what abuse you have in mind here?

I suspect that the ability to mint invoices with only a description hash will get abused by plugins pretty badly,

They make invoices smaller...

[cdecker]

I have to say that I agree with having the description be part of the invoice data is the correct thing to do here:

• Consistency: being able to provide the description without referring to an external system is very valuable, especially if those external systems may end up malleating the description (image compression, PDF OCR modifications, editing descriptions) which would unlink the hash from the doc
• Completeness and auditability: as @niftynei points out, there is a point in making sure that the information is as self-contained as possible. Plugins can rely on the information being present, and it reduces the variations that have to be dealt with.

I think ultimately, the fact that the description string can be used as an escape hatch by just including a URL and a hash should serve the use-cases where the description is managed externally.

[jb55]

all good justifications and I see that the description is stored even when deschashonly is set which makes sense. I still don't want to permanently store a copy of my lnaddress thumbnail for every invoice requested in an lnaddress payment, so perhaps I should just remove my png to become spec compliant or keep it and become not lnaddress spec compliant. I'm leaning toward the latter since lnlink skips the check anyways...