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

[WIP] LNURL pay #141

Merged
merged 37 commits into from
Aug 4, 2021
Merged

[WIP] LNURL pay #141

merged 37 commits into from
Aug 4, 2021

Conversation

dylancom
Copy link
Contributor

@dylancom dylancom commented Jul 21, 2021

Can be tested with: https://lnurl.fiatjaf.com/

TODO:

  • Additionally, a payment dialog must include:
  • Domain name extracted from LNURL query string.
  • A way to view the metadata sent of text/plain format.
  • A text input where user can enter a comment string (max character count is equal or less than commentAllowed value)

@bumi
Copy link
Collaborator

bumi commented Jul 21, 2021

wondering if we should just call it lnurl and have all the lnurl features and options implemented there. (later also lnurl-auth #7 )

@dylancom
Copy link
Contributor Author

@bumi makes sense!

import Donation from "./donation";

if (document) {
window.webln = new WebLNProvider();
window.lnurl = new LNURLProvider();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually I think we should stick with WebLN here and propose a new lnul function on the webln object.

so if it is a lnurl link we call the webln.lnurl() function which sends the message to lnurl action. There we decode the lnurl, do the GET request and then continue depending on the type of the lnurl (pay, auth, etc.)

what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lnurl action sounds good to me. (Moving the decoding and GET away from React) I'm only not sure about if lnurl should be part of webln as they are different directions?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that needs to be discussed. Though I am not sure if it is different directions. WebLN describes a way how to interact with lightning from the web/JS.
Instead of passing a lightning invoice we are passing in a lnurl.

(somewhat related discussion around extending webln with keysend: joule-labs/webln#33)

try {
// Get the invoice
const params = {
amount: value, // user specified sum in MilliSatoshi
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have to be careful here currently we use satoshis but we get MilliSatoshi here.


// Once payment is fulfilled LN WALLET executes a non-null successAction
// LN WALLET should also store successAction data on the transaction record
if (successAction) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wondering if we should handle the successAction in the component?

displaying the URL (with link to open) and the decrypted message could be easier in the component? or we have to open another prompt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bumi Yes indeed, was already planning to move this to the component. That way we can easily show custom modals etc.

dylancom and others added 5 commits July 25, 2021 22:48
A decrypted account is only available after the prompt.
If the account is not unlocked the prompt asks for the password and unlocks the account
bumi added 4 commits July 28, 2021 16:08
To auto-login we need to make sure the account is unlocked. Otherwise we need to show the prompt which handles unlocking
@bumi bumi mentioned this pull request Jul 28, 2021
successCallback,
// Once payment is fulfilled LN WALLET executes a non-null successAction
// LN WALLET should also store successAction data on the transaction record
successCallback: successAction
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting, didn't even think of passing a function through those calls. good that it works.

@bumi bumi mentioned this pull request Aug 3, 2021
<div className="p-6">
<dl className="shadow p-4 rounded-lg mb-8">
<dt className="font-semibold text-gray-500">Send payment to</dt>
<dd className="mb-6">{origin.name}</dd>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here we should also display details.url.host

@bumi
Copy link
Collaborator

bumi commented Aug 4, 2021

whoa! yay! let's merge it, the missing parts we can improve in new PRs.
so good! 🎉

@bumi bumi merged commit a5c7a27 into getAlby:master Aug 4, 2021
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