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

bolt11: drop requirement to check descriptionhash #1064

Closed
wants to merge 1 commit into from

Conversation

kiwiidb
Copy link

@kiwiidb kiwiidb commented Apr 1, 2023

For some context to the discussion see ElementsProject/lightning#6092.

Today there are hundreds of wallets, node front-ends and services that support Lightning Address, and if LND starts enforcing this bolt11 requirement they will all need to update or break, which is not going to happen, which is why I think this requirement should be dropped from bolt11, regardless if the requirement itself is a good thing or not. Like it or not, but if LND does not enforce this requirement, clients that do enforce it will just brick themselves by enforcing. In the LNURL Pay Spec, the requirement to check that hash and preimage match falls on the "Wallet" component, which is in most cases not the paying LN node but a client-side application. It is true that having the description is useful for showing historical payments and remembering what they were about, but I think you could claim the node not knowing the description is better for privacy as well in the case of a custodial service.

There are thousands of invoices generated each day which only have a description hash, are passed on from clients to servers (or to other clients) that are maintained by different people, with no way to easily pass the description as well. I believe the best thing to do is to accept that this is a lost battle, drop the requirement from bolt11 and wait for the 2nd coming of Christ (bolt12) to fix this.

@TheBlueMatt
Copy link
Collaborator

You absolutely should never be agreeing to pay for something you don't know. That doesn't, strictly speaking, have to happen in lnd, it could happen in upstream software driving lnd. But it definitely needs to happen. If a protocol is based on the sender not knowing what it's paying for that protocol is broken (I don't believe lnurl requires this, or don't recall it, I thought it hashed known data).

@rustyrussell
Copy link
Collaborator

Yeah, this a strong nack from me.

Clearly there's no point having a commitment scheme if you don't check the commitment. What's next, not checking signatures? Clearly we are the wallet, since we control the secret keys, and all else is sophistry.

Sure, some software is broken today. But that's a reason to fix the software which is clearly doing the wrong thing, not to break the spec.

@t-bast
Copy link
Collaborator

t-bast commented Apr 11, 2023

Agreed with Rusty, this is a NACK for me as well. Fortunately Bolt 12 forces you to input a "full" description which removes this edge case entirely.

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

Successfully merging this pull request may close these issues.

None yet

4 participants