Invalid bolt11: h: does not match description

[jsarenik]

Issue and Steps to Reproduce

Get an invoice with description_hash and try to pay it without supplying description.

Introduced by d5c736f and I think it is wrong. The payer does not have to set the funny JSON description that was set by LNURL parameter metadata.

@rustyrussell please fix before release, it totally breaks UX

[jsarenik]

Example:

I want to send 1sat to [email protected]. The invoice I get via Lightning Address is:

lnbc10n1p3yc5q0pp5j9dyzz4vszrncd2q47undmypx6xy4n8qx5gduxp7zqpf782pw7gqhp5t7ygdl8ft532fa2gpgxhh4rg9afsn9yp22yfpnglluevg6ey6ytqcqzpgxqyz5vqsp5jzrmdt7ryzzde3hchnza2xnx5ws37nj2emrz0gny28dxnyclaqgs9qyyssqlx4xxqs2d9cgsrl9kl95spdq5p683wdgnq4asz0gj36rw3u26ymsk7lhg5t033jkcntp055wu7z2gs8pj0nnetzr7n7grrfcx4kdcjqqypmn4z

But can not pay it until I cryptically supply the description via RPC interface to be [[\"text/plain\",\"paying anyone\"]].

$ lightning-cli pay lnbc10n1p3yc5q0pp5j9dyzz4vszrncd2q47undmy
px6xy4n8qx5gduxp7zqpf782pw7gqhp5t7ygdl8ft532fa2gpgxhh4rg9afsn9yp22yfpnglluevg6ey
6ytqcqzpgxqyz5vqsp5jzrmdt7ryzzde3hchnza2xnx5ws37nj2emrz0gny28dxnyclaqgs9qyyssqlx
4xxqs2d9cgsrl9kl95spdq5p683wdgnq4asz0gj36rw3u26ymsk7lhg5t033jkcntp055wu7z2gs8pj0
nnetzr7n7grrfcx4kdcjqqypmn4z
{
   "code": -32602,
   "message": "bolt11 uses description_hash, but you did not provide description parameter"
}

[jsarenik]

The intention of d5c736f is good, but it actually breaks things. On our side we should not check if we possess the plain-text description when paying an already generated invoice (where the server was checking this). @fiatjaf, what do you think about this?

[jb55]

On Tue, Apr 05, 2022 at 07:16:57AM -0700, Ján Sáreník wrote: The payer does not have to set the funny JSON description that was set by LNURL parameter `metadata`.

I'm not sure I agree with 'The payer does not have to set the funny JSON description'. Isn't this the entire point of lnurl's description_hash usage? To be able to ensure that the payer is paying the invoice associated with the payment metadata presented from the initial call to the lnurl?

[jsarenik]

OK. It's just unusable for me.

[fiatjaf]

Indeed it is not very usable if you're doing everything on the CLI, but these steps were always intended to be abstracted by a wallet or some other thing that exists on top of lightningd.

You can probably write a simple script or plugin paytolnurl that would abstract everything.

Would it work to have a plugin with an rpc_command hook intercept the pay command so it could accept LNURLs as invoices, decode and get the underlying bolt11 and then pay it?

[jb55]

On Tue, Apr 05, 2022 at 08:08:55AM -0700, Ján Sáreník wrote:

For me I mean.

It breaks lnlink too but it wasn't really following spec anyways... will
have to update. but now I'm concerned about making large (>64k) pay
requests (that include large images) over commando+lnsocket. afaik there is no
COMMANDO_REQ_CONTINUES and COMMANDO_REQ_TERM... cc @rustyrussell

[rustyrussell]

Should still be allowed, unless you disabled deprecated-apis?

(Checked, indeed this is true).

But you can add a literal description using lightning-cli pay bolt11=lnbc10n1p3yc5q0pp5j9dyzz4vszrncd2q47undmy px6xy4n8qx5gduxp7zqpf782pw7gqhp5t7ygdl8ft532fa2gpgxhh4rg9afsn9yp22yfpnglluevg6ey 6ytqcqzpgxqyz5vqsp5jzrmdt7ryzzde3hchnza2xnx5ws37nj2emrz0gny28dxnyclaqgs9qyyssqlx 4xxqs2d9cgsrl9kl95spdq5p683wdgnq4asz0gj36rw3u26ymsk7lhg5t033jkcntp055wu7z2gs8pj0 nnetzr7n7grrfcx4kdcjqqypmn4z description='"<complicated description here>"'. (The ' protects the " from the shell, and the " means it gets included literally).

[rustyrussell]

On Tue, Apr 05, 2022 at 08:08:55AM -0700, Ján Sáreník wrote:

For me I mean.

It breaks lnlink too but it wasn't really following spec anyways... will have to update. but now I'm concerned about making large (>64k) pay requests (that include large images) over commando+lnsocket. afaik there is no COMMANDO_REQ_CONTINUES and COMMANDO_REQ_TERM... cc @rustyrussell

lightningd/plugins#354 seems to do the trick?

[jsarenik]

Should still be allowed, unless you disabled deprecated-apis?

Yes it works. Thanks. To catch things early, I used to run lightningd with allow-deprecated-apis=false. Now I removed that option. Simply because a valid bolt11 invoice should be payable. The description_hash checks are LNURL-related which is out of context on lower (bolt11) level.

@rustyrussell This is actually a good example of what you were talking about during the meeting on Monday. The change was a good try, with good intentions, but should not go into deprecation and I would strongly suggest it to be un-deprecated in the next release (in the meanwhile any issues with deprecated-apis will get ignored by me as they will be practically not noticed).

[jsarenik]

Works for me now. Closing.

[jsarenik]

Just to add some proposals and up-to-date comments to this. In plugins/pay.c there is this:

 /* BOLT #11:
  * A reader:
  *...
  * - MUST check that the SHA2 256-bit hash in the `h` field
  *   exactly matches the hashed description.
  */

From the user point of view, the functionality has changed. Imagine a web browser not opening a URL containing a hash simply because the user does not present it with a clear-text data of the hash. No real limitation, HTTPS could be done and would work, but the browser refuses to open a connection…

Two proposals here:

1. I understand it is not about to go away and the allow-deprecated-apis "hack" will not work for long. Nevertheless I would suggest implementing at least a force option which if set to true would restore the old behavior of not checking if user has the plaintext of something that is not even settable by them (since it comes as part of LNURLp JSON metadata and they have to be passively fetched before any callback is being called — that could contain some additional data set by the user).

2. Similar to adding deschashonly was fine because it is fase by default, requiring BOLT11 magical h field (no confusion, it's just a description_hash) to be verified against lightningd-RPC-operator-set description (by default now; was not done before) should be done by another extra parameter like checkhash which would have to be explicitly set to true. Here I suggest changing BOLT11 spec to use word MAY instead of MUST.

[jb55]

One example where this would have saved me is generating an invoice on https://expensive-relay.fiatjaf.com, it gave me a deschash-only invoice which I paid. It turns out there was a bug and his service didn't work, since cln blindly paid the invoice, I have no description showing what I bought. fiatjaf's node could simply deny that they see my nostr pubkey in their database and I have no recourse to prove otherwise.

[fiatjaf]

I agree the contents of the invoice are important and that having a proof of having paid something is meaningless if that proof doesn't specify what was paid for, but I also agree that your node software refusing to pay an invoice you know is good or you don't care is super annoying.

An argument could be made that an invoice like this which I paid today is as meaningless as an invoice with a description hash: lnbc6300n1p3xk5hypp59udvtkc8k59mqgml3ap2hccxza5wgpq5lcnxz3evlr3wcr6kfceqdpd2pskjepqw3hjq4rpd3jhxgrxwfhk6gr5dpjjqsmj09c8gcqzpgxqzuysp5u0muek5ph3rsh32k2zjt94q0c2q9z0jy8c063zcgz6zvx92fcwps9qy9qsqycqqre3en2hyet3vk3nw2xmz303al6ptvh27f7vafau760v3k60rl979xvh5xvd9r0ckwfcq32tfzae2mafny3e3gu7hdy49xaja2pgpddzamv

On the other hand one can also say that there is actually no fundamental restriction, as @jsarenik can use sendpay directly or write his own pay plugin.

As a last point, I don't think the spec mandates the behavior that the current pay plugin has chosen explicitly, because "reader" there can be interpreted as being the user or some other client sitting in front of lightningd and not lightningd directly (although it could be argued that the pay plugin is such a client).

[jsarenik]

Closing, thanks for comments!

#5747