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

Add WebLN support #215

Merged
merged 8 commits into from
Aug 25, 2022
Merged

Conversation

KoalaSat
Copy link
Member

@KoalaSat KoalaSat commented Aug 22, 2022

Fixes #176

This PR implements the automatic detection of WebLN providers on user's browser.

It also includes a workaround to deal with locked payments, since WebLN does not support yet this feature, we just wait for the back-end to communicate a new status.

The main workflow will behave as follows:

  1. The user with a WebLN app takes/creates an order.

  2. Robosats detects WebLN and automatically tries to contact with the provider, if this is the first time, the provider ask for permissions to the user:

  3. Robosats creates a payment with the BOLT-11 invoice received from back-end, a notification is shown to help the user understand the next step:

  4. The user accepts the payment on the provider popup, at this point WebLN does not contemplate locking payments so for this case (Alby) it'll keep loading forever, until the user cancels or the order expires, Robosats updates the message to help the user understand the next step:

  5. The user closes the provider popup and continue with the standard order process.

I tried to create several payment locks with the same wallet and despite the loading button problem, all payments are successfully locked in the network.

@Reckless-Satoshi - I need some help testing this on Tor Browser, looks like my Alby extension does not work.

@KoalaSat KoalaSat force-pushed the add-webln-support branch 2 times, most recently from cde3e08 to 959380d Compare August 22, 2022 15:00
} else {
this.setState({ waitingWebNL: false });
}
});
Copy link
Member Author

@KoalaSat KoalaSat Aug 22, 2022

Choose a reason for hiding this comment

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

Please, confirm these are the right combinations of user role + status + invoice

@KoalaSat KoalaSat marked this pull request as ready for review August 22, 2022 15:10
@KoalaSat KoalaSat mentioned this pull request Aug 22, 2022
}

orderDetailsReceived = (data) =>{
if (data.status !== this.state.status) { this.handleWebLN(data) }
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the only way I found to manage status changes without using Hooks, happy to hear any other suggestions I might have missed out.

@Reckless-Satoshi
Copy link
Collaborator

Reckless-Satoshi commented Aug 22, 2022

This looks absolutely fantastic!! Congrats! 🚀
Haven't tested it, hope to find some time for it during this week.

Screenshot 2022-08-22 at 16 15 56

I assume there is about 5-10 second when Alby shows the spinner and RoboSats still shows "please check your WebLN wallet"? Does WebLN send any signal the moment the payment is initiated? If so, it could be used to trigger a text change in the dialog "Waiting for your bond to be accepted".

@Reckless-Satoshi - I need some help testing this on Tor Browser, looks like my Alby extension does not work.

I also was not able to get the extension to work. I cannot get pass the password creation step, the "confirm password" button simply does not work. Have you tried testing on Brave with Tor or on a torified Mozilla session? We should report about the TOR Browser bug anyway.

Can WebLN generate and autofill the Bolt11 invoice requested to buyers in (data.status == 6 || data.status == 8) and on the "Rewards and Compensations" payout?

@KoalaSat
Copy link
Member Author

KoalaSat commented Aug 22, 2022

I assume there is about 5-10 second

On average for me took 1-2 seconds, never 5.

Does WebLN send any signal the moment the payment is initiated?

No, there are 0 signals from the provider until the payment is completed, so never. WebLN is really restrictive.

Have you tried testing on Brave with Tor or on a torified Mozilla session?

I tried Brave but looks like it has a strong CORS block, even for localhost, Tor Browser has the same limitation and I manage to make it work, but I had no luck with brave. I never tried the torified Mozilla session.

We should report about the TOR Browser bug anyway.

I think I can create tomorrow an issue for them.

Can WebLN generate and autofill the Bolt11 invoice requested to buyers in (data.status == 6 || data.status == 8) and on the "Rewards and Compensations" payout?

I think what you want is to use this function right? https://webln.dev/#/api/make-invoice
So the idea is to automatically generate an invoice and send it to the back-end?

@Reckless-Satoshi
Copy link
Collaborator

No, there are 0 signals from the provider until the payment is completed, so never. WebLN is really restrictive.

Ah, I see. Well, then this is the best approach possible at the moment :)

I think I can create tomorrow an issue for them.

Very appreciated!

I think what you want is to use this function right? https://webln.dev/#/api/make-invoice So the idea is to automatically generate an invoice and send it to the back-end?

It looks exactly like what is needed, with an explicit amount. Once the user confirms creating the invoice on the extension RoboSats could directly proceed with the API request.

@KoalaSat
Copy link
Member Author

KoalaSat commented Aug 22, 2022

I included the invoice generation for the buyer status and the rewards program.

Rewards program

  1. If the user has a WebLN provider and claims a reward, the "Generate with WebLN" button is displayed.
    Screenshot 2022-08-22 at 23 21 28
  2. If the user clicks on it, a provider's popup shows the invoice form with the reward amount.
    Screenshot 2022-08-22 at 23 21 45
  3. If the user generates the invoice, it's sent to the back-end the same way as the manual submit.
    Screenshot 2022-08-22 at 23 23 22

Since this is a regular invoice generation contemplated on WebLN, the popup closes automatically once the invoice is generated.

Canceling on the provider's popup won't make any change and the user will still be able to click again or submit it manually.

Buyer Invoice

It's not possible to test this steps on the testnet, so the only way I can imagine to check is to actually generate a real order. Anyways I hacked the code to reach to that step and the invoice request looks as good as in the reward request.

Signed-off-by: KoalaSat <111684255+KoalaSat@users.noreply.github.com>
@KoalaSat
Copy link
Member Author

getAlby/lightning-browser-extension#1328

@Reckless-Satoshi
Copy link
Collaborator

Reckless-Satoshi commented Aug 23, 2022

Been testing it for a little while, it's amazing! I need more time to test it but I am super tight these days. I left a few comments.

This feature will be the best companion with the release of the Umbrel / Citadel apps as TOR browser won't be a limiting factor for those that self host the RoboSats client.

@@ -90,6 +93,18 @@ const ProfileDialog = ({
copyToClipboard(`http://${host}/ref/${referralCode}`);
};

const handleWeblnInvoiceClicked = (e: any) =>{
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const handleWeblnInvoiceClicked = (e: any) =>{
const handleWeblnInvoiceClicked = async (e: any) =>{

webln.makeInvoice(earnedRewards).then((response) => {
handleSubmitInvoiceClicked(e, response.paymentRequest);
})
});
Copy link
Contributor

Choose a reason for hiding this comment

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

What about the following suggestion to avoid the nested callbacks?

const webln = await getWebln();

const response = await webln.makeInvoice(earnedRewards);

handleSubmitInvoiceClicked(e, response.paymentRequest)

Copy link
Member Author

@KoalaSat KoalaSat Aug 24, 2022

Choose a reason for hiding this comment

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

If that's the patterns you apply it's fine, but I'm curious now, what's the difference?

@@ -113,6 +122,44 @@ class OrderPage extends Component {
this.getOrderDetails(this.state.orderId);
}

handleWebln = (data) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

The same concept could be applied here, so we can await the getWebln function call.

} else {
this.setState({ waitingWebNL: false });
}
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

These conditions exactly match the rol+status that is expected to lock an invoice.
https://github.com/Reckless-Satoshi/robosats/blob/67a542cf31d36933a0bbb5bb1832110ba0070e12/api/models.py#L272-L279
The use of data.bond_invoice and escrow_invoice is also correct.

@@ -300,6 +300,9 @@
"This order has been cancelled collaborativelly":"This order has been cancelled collaboratively",
"This order is not available":"This order is not available",
"The Robotic Satoshis working in the warehouse did not understand you. Please, fill a Bug Issue in Github https://github.com/reckless-satoshi/robosats/issues":"The Robotic Satoshis working in the warehouse did not understand you. Please, fill a Bug Issue in Github https://github.com/reckless-satoshi/robosats/issues",
"WebNL Payment": "WebNL Payment",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just small typo WebNL -> WebLN (happens in every translation file and javascript source)

this.setState({openWebNLDialog: false});
}

webNLDialog =() =>{
Copy link
Member Author

Choose a reason for hiding this comment

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

Yes I noticed yesterday and I tried to change it everywhere, on the last version of this PR all NL typos should be fixed

{this.state.waitingWebln ?
<>
<CircularProgress size={16} thickness={5} style={{ marginRight: 10 }}/>
{data.is_buyer ? t("Invoice not received, please check your WebLN wallet.") : t("Payment not received, please check your WebLN wallet.")}
Copy link
Collaborator

Choose a reason for hiding this comment

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

data.is_buyer does not exist. Therefore the app crashes when creating a maker / buy order. It should be replaced with this.state.is_buyer

getWebln()
.then((webln) => {
this.setState({ weblnEnabled: webln !== undefined })
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is probably not the best location to introduce the webln enabling routine. In case webln is not enabled, it will keep checking everytime setAppState is called (that's very often!).

I tested to directly define the state variable as

weblnEnabled: getWebln().then((webln) => {webln !== undefined })

However webln enabling tends to fail the first few times. On componentDidMount does also not work for me. Ideas?

    componentDidMount() {
      getWebln()
      .then((webln) => {
        this.setState({ weblnEnabled: webln !== undefined })
      });
    };

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok on that case let's keep it on the component using it (just the rewards), I move it there it was more convenient for future uses, anyways with the implementation of typescripts eventually it might be created a Context for this one-shot global data.

</DialogContentText>
</DialogContent>
<DialogActions>
<Button onClick={this.handleCloseWeblnDialog} autoFocus>{t("Go back")}</Button>
Copy link
Collaborator

Choose a reason for hiding this comment

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

"Done" might be more appropriate than "Go back" when the text is "You can close now your WebLN wallet popup."

this.setState({ waitingWebln: true, openWeblnDialog: true});
} else if (data.is_buyer & (data.status == 6 || data.status == 8 )) {
webln.makeInvoice(data.amount).then((invoice) => {
this.sendWeblnInvoice(invoice)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Submitting a payout invoice as a buyer. I get this error immediately after clicking the

Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'id')
    at OrderPage.eval [as sendWeblnInvoice] (OrderPage.js:216:61)

In case the user rejects the creation of the invoice ("Cancel" button in Aldy) the "User Rejected" response could be used to close the WebLN Dialog.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Pop up with the spinner also stays open when the invoice is successfully submitted and the order moves to the next state.

Copy link
Member Author

@KoalaSat KoalaSat Aug 24, 2022

Choose a reason for hiding this comment

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

I think a catch will do the trick, but I can't confirm

Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume you cannot test it since this feature can only be tested on mainnet. While it is a bit of a hassle, it possible to test the submission of the invoice payout on the mainnet platform by taking your own order with another Robot. After the escrow is locked (20K Sats), you can collaboratively cancel to unlock every hodl invoice.

'invoice': invoice,
}),
};
fetch('/api/order/' + '?order_id=' + this.props.data.id, requestOptions)
Copy link
Collaborator

Choose a reason for hiding this comment

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

this.props.data.id -> this.state.orderId
invoice': invoice, -> invoice': invoice.paymentRequest, ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Invoice submission as buyer works with these fixes.

webln.sendPayment(data.escrow_invoice);
this.setState({ waitingWebln: true, openWeblnDialog: true});
} else if (data.is_buyer & (data.status == 6 || data.status == 8 )) {
webln.makeInvoice(data.amount).then((invoice) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

data.amount is the amount of fiat. I believe we want to use data.trade_satoshis .Otherwise the invoice is rejected by the backend for having the wrong amount of Sats.

this.setState({openWebNLDialog: false});
}

webNLDialog =() =>{
Copy link
Member Author

Choose a reason for hiding this comment

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

Yes I noticed yesterday and I tried to change it everywhere, on the last version of this PR all NL typos should be fixed

@KoalaSat
Copy link
Member Author

Everything ready! thanks for the feedback @Reckless-Satoshi @fernandoporazzi

{this.state.is_buyer ? t("Invoice not received, please check your WebLN wallet.") : t("Payment not received, please check your WebLN wallet.")}
</>
: <>
{t("You can close now your WebLN wallet popup.")}
Copy link
Collaborator

Choose a reason for hiding this comment

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

To highlight the barely noticeable change in the dialog when the bond is locked, after CircularProgress spinner is gone we could place a green checkmark

import CheckIcon from '@mui/icons-material/Check';
...
<CheckIcon color="success"/>
{t("You can close now your WebLN wallet popup.")}
...

webln.sendPayment(data.escrow_invoice);
this.setState({ waitingWebln: true, openWeblnDialog: true});
} else if (data.is_buyer & (data.status == 6 || data.status == 8 )) {
webln.makeInvoice(data.trade_satoshis)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move this.setState({ waitingWebln: true, openWeblnDialog: true}); here

webln.makeInvoice(data.trade_satoshis)
.then((invoice) => {
if (invoice) {
this.sendWeblnInvoice(invoice.paymentRequest);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this.setState({ waitingWebln: true, openWeblnDialog: true}); from here. The Dialog must be opened earlier.

this.setState({ waitingWebln: true, openWeblnDialog: true});
}
}).catch(() => {
this.setState({ waitingWebln: false, openWeblnDialog: false });
Copy link
Collaborator

Choose a reason for hiding this comment

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

The catch does not close the dialog if the invoice is successfully submitted and the order advances status. The dialog only closes with this catch when the Alby invoice generation is Canceled.

Copy link
Member Author

Choose a reason for hiding this comment

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

ON that case we need to set some special conditions here, so the dialog is closed automatically with invoices but keeps open with payment, makes sense.

@Reckless-Satoshi
Copy link
Collaborator

It's looking ready to be merged! There is only a couple of things to polish with the invoice submission (comments above).

Please @KoalaSat , send a LN invoice for 200K Sats with a long expiration time from a proxy wallet (not your node pubkey)

Thanks a lot @fernandoporazzi for reviewing

@KoalaSat
Copy link
Member Author

KoalaSat commented Aug 25, 2022

Last changes applied, thanks for the extra support!

lnbc2m1p3swgpjpp5an5elzvn6r76rhf6m2vx0klz7eu6gqccyak6qcmnndgklj7uqftqdq5g9kxy7fqd9h8vmmfvdjscqzpgxqyz5vqsp5q0jmu9wpdww93xg08yqnyv78dfcw66v7d5wtey22r3gdcv6sv05s9qyyssqqcz2yxw3c8jsh9a2z3jtwk3vafwj75muwd6s2qzl55t8ep2jvn5p6h6q7t79jel8hvnpgkhgx8s6wsjllk4c602l6mzdkrml3cgax6gp526kjq

@Reckless-Satoshi Reckless-Satoshi merged commit 7083423 into RoboSats:main Aug 25, 2022
@Reckless-Satoshi
Copy link
Collaborator

lnbc2m1p3swgpjpp5an5elzvn6r76rhf6m2vx0klz7eu6gqccyak6qcmnndgklj7uqftqdq5g9kxy7fqd9h8vmmfvdjscqzpgxqyz5vqsp5q0jmu9wpdww93xg08yqnyv78dfcw66v7d5wtey22r3gdcv6sv05s9qyyssqqcz2yxw3c8jsh9a2z3jtwk3vafwj75muwd6s2qzl55t8ep2jvn5p6h6q7t79jel8hvnpgkhgx8s6wsjllk4c602l6mzdkrml3cgax6gp526kjq

3662393239386161343530323765363265366431643766303565653164383237

@KoalaSat KoalaSat deleted the add-webln-support branch August 25, 2022 21:16
@KoalaSat KoalaSat restored the add-webln-support branch August 25, 2022 21:16
KoalaSat added a commit to KoalaSat/robosats that referenced this pull request Aug 26, 2022
* Add WebLN support

* Fix Variable Typo

* Invoice Generation

Signed-off-by: KoalaSat <111684255+KoalaSat@users.noreply.github.com>

* Code Review

* Second CR

* Catch cancelations

* Final Review

Signed-off-by: KoalaSat <111684255+KoalaSat@users.noreply.github.com>
KoalaSat added a commit to KoalaSat/robosats that referenced this pull request Aug 26, 2022
* Add WebLN support

* Fix Variable Typo

* Invoice Generation

Signed-off-by: KoalaSat <111684255+KoalaSat@users.noreply.github.com>

* Code Review

* Second CR

* Catch cancelations

* Final Review

Signed-off-by: KoalaSat <111684255+KoalaSat@users.noreply.github.com>
KoalaSat added a commit to KoalaSat/robosats that referenced this pull request Aug 26, 2022
* Add WebLN support

* Fix Variable Typo

* Invoice Generation

Signed-off-by: KoalaSat <111684255+KoalaSat@users.noreply.github.com>

* Code Review

* Second CR

* Catch cancelations

* Final Review

Signed-off-by: KoalaSat <111684255+KoalaSat@users.noreply.github.com>
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.

Add WebLN support
3 participants