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

Update Auth protocol description, add example values, fix bug. #249

Open
wants to merge 2 commits into
base: luds
Choose a base branch
from

Conversation

michaelWuensch
Copy link
Contributor

Hi,
I am in the process of implementing LNURL Auth for BitBanana. It works and it gives the same linking key as Zeus for the same node.
During the process of implementations I stumbled upon some difficulties, that I now try to overcome with this PR so implementation for other users is easier.

First big thing I noticed is, that both Zeus and Blixt do NOT follow the spec.
In LUD-04 at step 2 it said canonical phrase is hashed one time before signing, but Zeus and Blixt hash it two times. (the called LND endpoint hashes it two times automatically, unless you explicitly tell it to only hash it once, which these wallets did not do.)
I now did the same and adapted the spec accordingly. I am unsure though how other wallets do it.

I further

  • clarified how domain name is converted into bytes.
  • removed PrivateKey(...) which looked like a function and felt confusing and unnecessary for me.
  • linked LUD 05 and LUD 13 in LUD 4
  • added an example with values to easily debug an implementation. I verified these values with the Zeus implementation (https://github.com/ZeusLN/zeus/blob/master/views/LnurlAuth.tsx#L112), BitBanana gives the same results now.

@hsjoberg
Copy link
Collaborator

hsjoberg commented Nov 26, 2023

Just a short comment before looking into this further, the single_hash arg to signMessage did definitely not exist back when I implemented LUD-04/13 for Blixt.
In fact, I tried to make them remove the hashing requirement, before we wrote LUD-13.
So I'm not sure when did double-hashing stuff arrived.

@michaelWuensch
Copy link
Contributor Author

Ah, okay.
As double hashing is the default now I just assumed is has always been that way and they only later added the option to do single hashing. At least I can say that I needed to do the double hashing to get the same result as Zeus, I could not test Blixt as Blixt cannot remote control the same node.

@michaelWuensch
Copy link
Contributor Author

@hsjoberg
I now looked at the other implementations.
Alby and Balance of satoshi seem to do double hashing as well.
I am not sure about CLAMS. It just calls the signMessage Endpoint from core lightning with the plain message as input, but I couldn't find out how often this is hashed be core lightning before signing.

This means:
Blixt, Zeus, Balance of Satoshi & Alby seem to do double hashing. I will join this for BitBanana so I am compatible and others do not have to change their implementation which is bad UX for their users.

References:
Here is the commit that added double hashing for lnd
lightningnetwork/lnd@02757f6

Alby:
https://github.com/getAlby/lightning-browser-extension/blob/master/src/extension/background-script/actions/lnurl/auth.ts#L81
https://github.com/getAlby/lightning-browser-extension/blob/master/src/extension/background-script/connectors/lnd.ts#L334

Balance of Satoshi:
https://github.com/alexbosworth/balanceofsatoshis/blob/master/lnurl/auth.js#L109

Clams:
https://github.com/clams-tech/App/blob/master/src/routes/lnurl/utils.ts#L13

@hsjoberg
Copy link
Collaborator

Thank you so much for your research @michaelWuensch.
Yeah I would've expected this if lnd had a subtle breaking change like this.
I think we should go ahead with the changes you're proposing in this PR.

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.

2 participants