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

LUD-18 Wallet implementation #531

Merged
merged 6 commits into from
Oct 3, 2023
Merged

Conversation

SatsAllDay
Copy link
Contributor

Query the lightning address provider client-side to learn of capabilities

Conditionally render LUD-12 and LUD-18 fields based on what the remote server says is supported

Allow identifier, name, and email to be sent from the SN side during the withdrawal flow. Auth seems too complicated for our use case, and idk about pubkey?

Query the lightning address provider client-side to learn of capabilities

Conditionally render LUD-12 and LUD-18 fields based on what the remote
server says is supported

Allow identifier, name, and email to be sent from the SN side during the withdrawal flow. Auth seems too complicated for our use case, and idk about pubkey?
@huumn
Copy link
Member

huumn commented Sep 29, 2023

Allow identifier, name, and email to be sent from the SN side during the withdrawal flow.

I haven't looked at the code yet, but I'd prefer this be optional.

Auth seems too complicated for our use case, and idk about pubkey?

Hmmm I don't even know what this refers to. I'll take a look tomorrow.

@SatsAllDay
Copy link
Contributor Author

Every piece of payer data is opt in by default. Lightning address providers can specify whether certain pieces of payer data are required, but it's still ultimately up to the sender if they want to satisfy those requirements by supplying payer data or not. I have yet to encounter a lightning address provider that requires any payer data.

Regarding auth, it is whether you want to sign the random challenge provided by the lightning address provider with your private key. Very much like LNAuth. Since stacker news does not manage private keys for users, I don't think it's worth trying to implement Wallet implementation for auth payer data.

* dynamic client-side validation for required payer data

* don't re-init form state on error

* correct min and max amount values

* only send applicable data to graphql api based on payerdata schema

* input type for numeric values (amount, max fee)

* update step for amount and max fee
@SatsAllDay
Copy link
Contributor Author

I'll record a demo of this a little later on and attach it to this PR for reference. I think that would help to review, once it's ready.

@SatsAllDay
Copy link
Contributor Author

Two recordings to show functionality. I partially mocked the API response in the shorter recording to simulate the identifier use case.

Alby supports name, email. WoS doesn't support any LUD18 fields. You can also see the dynamic LUD12 comment length when I switch between different lightning address providers.

The could not decode invoice errors are just because I'm trying to process mainnet invoices using a regtest LND node locally.

stacker.news.lud18.wallet.mov
stacker.news.lud18.wallet.2.mov

@huumn
Copy link
Member

huumn commented Sep 30, 2023

This looks great!

@SatsAllDay SatsAllDay marked this pull request as ready for review September 30, 2023 23:44
@SatsAllDay
Copy link
Contributor Author

I think this is ready for review now. Of course, now that I've marked it as such, I'll probably realize something else that I want to do with it 😅

@hsjoberg
Copy link

hsjoberg commented Oct 2, 2023

FYI, I've added identifier (aka Lightning Address) support to the https://chat.blixtwallet.com playground now. In case you guys wanna test it there too.

@huumn huumn merged commit 362f95a into stackernews:master Oct 3, 2023
1 check passed
@SatsAllDay
Copy link
Contributor Author

darth coin is going to be so happy

@Darth-Coin
Copy link

darth coin is going to be so happy

Now I can send sats "from" Christine Lagarde :)

@SatsAllDay SatsAllDay deleted the lud-18-wallet branch October 4, 2023 19:48
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.

4 participants