lnurl / luds

lnurl specifications
584 stars 138 forks source link

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

Open michaelWuensch opened 8 months ago

michaelWuensch commented 8 months ago

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

hsjoberg commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

@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 https://github.com/lightningnetwork/lnd/commit/02757f67350297811615cc2d7cf7a1fe85a34bb7

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 commented 8 months ago

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.