'invoice' command returning preimage.

[fiatjaf]

I'm sorry for this silly PR, but this has been bugging me for a long time. Being able to see the preimages of invoices issued is good for overall understanding of the protocol and for easy checking of payment proofs from others, not to mention other advanced uses (which may be covered by passing your own preimage to invoice as the docs imply, but still).

The point is that this doesn't hurt and makes everything better.

---

Some useless context

On https://t.me/lntxbot I like to show a lot of debugging information (because I'm crazy and I think users will want to understand what is happening):

However, because I can't get the preimages my invoices are using from the invoice command to show these preimages I have to generate them on my own using a probably-not-so-safe generator and store them in not-so-optimal ways instead of having lightningd generate them for me.

edit: just realized the above image is from the sender side, not the receiver, so completely unrelated.

[cdecker]

For context: we aren't displaying the preimage mainly because it currently serves also as a proof-of-payment ("I have the preimage so I have paid you, how else could I have gotten it?"). If we were to show the preimage in the cli output, then the user could be tricked into sharing that with the sender, thus giving him the proof.

Worse, if any node on the path between sender and recipient learns about the preimage before the payment is attempted it can grab the transfer, short-circuiting it using the preimage. This would then be a really hard to debug situation (sender sends payment successfully, but the payment never arrives at the recipient, but sender still has proof that he paid).

A middle ground could be to show the preimage after the payment has completed and the invoice was paid, at which point users could compare the preimage and just see that it matches on both ends.

[fiatjaf]

Well, but doesn't this seem like a big user failure from someone who doesn't know what they're doing? A user like that can do a lot of harm to himself already in other ways. Like being tricked into issuing pay on an invoice he didn't inspect first and end up paying 1000000 sat instead of 1000 (actually I'm very afraid of doing that in a moment of distraction).

Anyway, ignore the above paragraph. The middle ground you suggested is a big improvement, I think. And it even solves my specific issue now that I'm thinking about it. I'll change this PR to do that.

[darosior]

About the middle ground: waitsendpay, listpays and listsendpays already contains the preimage. https://lightning.readthedocs.io/lightning-waitsendpay.7.html, https://lightning.readthedocs.io/lightning-listpays.7.html, https://lightning.readthedocs.io/lightning-listsendpays.7.html. There are also a hook and a notification https://lightning.readthedocs.io/PLUGINS.html#invoice-payment sent when an invoice was paid and containing the payment preimage

[cdecker]

About the middle ground: waitsendpay, listpays and listsendpays already contains the preimage. https://lightning.readthedocs.io/lightning-waitsendpay.7.html, https://lightning.readthedocs.io/lightning-listpays.7.html, https://lightning.readthedocs.io/lightning-listsendpays.7.html. There are also a hook and a notification https://lightning.readthedocs.io/PLUGINS.html#invoice-payment sent when an invoice was paid and containing the payment preimage

All of those RPC calls however are on the sender side, and I think @fiatjaf is talking about the recipient side.

I'll implement the middle solution ASAP :-)

[cdecker]

I went ahead and added a couple of fixup commits that I'll squash before merging, but I wanted to get @fiatjaf's sign-off first:

• Good work on adding json_add_preimage and consolidating it's use
• I removed the payment_preimage in the return of invoice again as discussed before
• I added a test for the preimage when listing paid invoices

The commit message for the first commit doesn't really match anymore, if you'd like I can reword that before squashing ^^

[fiatjaf]

I think this is great, thanks!

Please reword the commit while squashing, yes.

Also thank you for the compliment, it means a lot to my insufficient C skills.

[rustyrussell]

I share @cdecker's reluctance to share the preimage, but this could be made a parameter to listinvoices (default off).

[cdecker]

I share @cdecker's reluctance to share the preimage, but this could be made a parameter to listinvoices (default off).

I had thought about this, however if we assume the attacker can trick the user into sharing the output of listinvoices it'd be easy for him to also trick him to use the extra parameter ("My payment isn't working? That's strange! Can you share the output of lightning-cli listinvoices [payment_hash] true?"). By gating the reveal of the preimage on the status of the invoice we at least make sure it was paid first, which is in line with the "don't reuse invoices" practice.

[cdecker]

Rebased and squashed, should be good to go, unless @rustyrussell wants to guard the payment_preimage for paid invoices using a flag.

[cdecker]

Ack d0905df

[ Changed ACK to Ack to see if ackbot will recognize it? --RR ]

[rustyrussell]

Ack d0905df

[rustyrussell]

Ack d0905df

[ Changed ACK to Ack to see if ackbot will recognize it? --RR ]

That didn't work, but once I acked it recognized @cdecker's ack too. Weird.

[cdecker]

Ack d0905df
[ Changed ACK to Ack to see if ackbot will recognize it? --RR ]

That didn't work, but once I acked it recognized @cdecker's ack too. Weird.

Yeah, the bot had caught it, but calling the status RPC on Github failed, so changes to the ack message were bumping against the uniqueness constraint and not retrying the Github RPC call. Will fix 😉