joule-extension icon indicating copy to clipboard operation
joule-extension copied to clipboard

[WIP] Implement allowances that auto-pay invoices

Open wbobeirne opened this issue 5 years ago • 18 comments

Closes #42

Description

This adds a new module, appconf that can be used to add customizations to each webapp. The one implemented here is allowances, the idea of setting up rules that allow automatic payments to happen without user intervention.

This also adds a status bar that gives you quick actions you can take on the current page you're on.

A more thorough write-up of this PR is up on Medium: https://medium.com/p/2b08bec75e3a

TODOs

  • Make more chain agnostic (hard-coded use of sats in a few places)
  • Find good places to provide fiat equivalents
  • ???

Steps to Test

  1. Go to a WebLN page that you've enabled
  2. Confirm you see the enabled status bar
  3. Click the disable button, confirm it disables WebLN for app
  4. Re-enable it, and click on the piggy bank, confirm it created a default allowance for you
  5. Attempt to make a payment that fits the constraints, confirm it happened automatically for you
  6. Attempt to make a payment that does not fit the constraints, confirm you were prompted instead
  7. Configure the settings and make sure they all work as expected
  8. Add a new allowance directly from the allowances UI, confirm it correctly works when manually entered

Screenshots

Status Bar

Screen Shot 2019-06-11 at 4 05 02 PM

Allowance page

Screen Shot 2019-06-06 at 5 42 29 PM

Notifications

Screen Shot 2019-06-06 at 5 40 31 PM Screen Shot 2019-06-06 at 5 40 38 PM

wbobeirne avatar Jun 13 '19 17:06 wbobeirne

I was experimenting a bit with this PR. First of all: the UX is great! Super intuitive and nice to use. Can not wait to get this released :)

Here are a few things that I've noticed:

send payment for allowances is done in the context of the website

It seems currently the send payment call (POST /v1/channels/transactions) is done in a content script thus in the context of the current website. This does not work for me because of blocked cross origin calls. (calling the rest API of a lnd node without CORS). (debug screenshot) Somehow I feel like the check if it is a "allowed" domain and handling the request could be done in the same "handlePrompt" receiver?

failed browser notifications

For whatever reason browser.notifications is undefined and thus browser.notifications.create and browser.notifications.clear fails. (screenshot I could not find out why this is the case. (I was using Chrome 75.0.3 on Linux)

bolt11 (and coinfo/bitcoinjs-lib) currently does not support simnet network

And the bolt11 package adds some dependencies and I was wondering if we can use the /v1/payreq API call to decode the payment request.

bumi avatar Jun 26 '19 15:06 bumi

send payment for allowances is done in the context of the website

Yep, this is definitely a hacked together version. I think I'm going to have to setup a communication channel with the background script and do all node interactions in there. I definitely don't want node communications outside of the extension sandbox. Thanks for noting this.

failed browser notifications

This is possibly a permissions issue? I'll have to dig a little deeper to see why it might be missing. But I should definitely be handling cases where notifications go wrong.

I was wondering if we can use the /v1/payreq API call to decode the payment request.

I wanted to avoid this for latency reasons, my remote node can sometimes respond a little slow and I didn't want to add 500ms+ delays to payment prompts to decode the invoice. I'd rather contribute to bolt11 and make that library more robust than require a network request.

wbobeirne avatar Jul 01 '19 18:07 wbobeirne

fyi: I was experimenting a bit with this and also moving parts into the background script. My changes are in my fork - in case you want to follow: https://github.com/joule-labs/joule-extension/compare/autopay...bumi:autopay (includes loads of debug stuff for me to understand what is going on and get more familiar with the code)

bumi avatar Jul 01 '19 18:07 bumi

it seems the state of crypto.isRequestingPassword is always true screenshot - do you have an idea why that is the case? Shouldn't that be false once one payment is done and the checkbox is set to store the password for the session?

bumi avatar Jul 04 '19 14:07 bumi

In that screenshot, password is still null so it would seem that the password hasn't yet been entered or fetched from the background script. The hasSetPassword indicates that they have ever set a password, not that they've entered it that session.

The password is only filled into the store state again when you've checked the remember checkbox once it's needed. You can see that logic over in the crypto saga.

wbobeirne avatar Jul 04 '19 19:07 wbobeirne

ok, thanks for the hint. somehow it seems this is always the case. inspecting the state here or here gives me an empty password.

Though I don't have to enter the password when I confirm the prompt. Is this related to the autopay changes? or some bug? I am currently stuck there any hints would be awesome :) - sadly I don't know the redux-saga concept yet.

update: after some more debugging I don't find/understand how the password should be stored. also I failed to try to add it. any hints are welcome :) thx.

bumi avatar Jul 04 '19 22:07 bumi

Hey @bumi, sorry for leaving you hanging so long on this one. The password saving behavior is definitely a little tricky, and sagas are doubly tricky. The way it works is once you enter your password with the save password checkbox set, it send a message to the background script (sending code here, saving code here) to hold onto it in memory.

Whenever a password request happens, we first check in with the background script if it has the password cached (Request from background here, background sending back here). Now, my understanding is that when that happens, it should fire the ENTER_PASSWORD action that sets it all in state. I'm not entirely sure why you're not seeing what you're seeing in your logs, I'll do a little investigating of my own to see if I can reproduce it.

wbobeirne avatar Jul 19 '19 18:07 wbobeirne

@wbobeirne I see you requested a review on this PR. I'm looking forward to playing with it. Is it ready? I should have time over the weekend.

jamaljsr avatar Feb 29 '20 04:02 jamaljsr

Haha, oh man I totally forgot I had tagged you ages ago. Finally scratching out some free time to work on Joule again.

Planning on getting this in a stable state and trying to entice people to run it as a beta for me for some sats in exchange. Still has some work to go after the refactor in #244, but it should be pretty close.

Happily using Polar to do my testing with this right now.

wbobeirne avatar Feb 29 '20 06:02 wbobeirne

No worries. I just saw a notification and wasn't sure if that was new or old. Feel free to let me know if you need any help with anything.

Once you go Polar, it's hard to go back to the terminal. Lol

jamaljsr avatar Feb 29 '20 07:02 jamaljsr

<3 I am also jumping back on this. let me know if I can help you with something.

bumi avatar Mar 02 '20 10:03 bumi

Unfortunately in spite of moving the requests to the background context, we still get stopped with ERR_SSL_PROTOCOL_ERROR errors when trying to fetch to self signed certificate nodes. This is doubly bad because unlike the current workaround of going to the URL with the self signed cert and manually approving it, the background script will error out no matter what.

Unfortunately this type of automatic node communication may be dead in the water unless a native bridge is built (#106).

wbobeirne avatar Mar 03 '20 03:03 wbobeirne

hmm. does that mean current master also has that issue? I could try working on a native proxy for a showcase. But native extension can not be bundled with the extension, can they? I think it should be prevented as official requirement as much as possible.

bumi avatar Mar 31 '20 11:03 bumi

Yeah, that issue is currently present in develop (master is always the latest release, develop is the working branch.) I may revert those commits given these findings if I start working on something else, so that a new release doesn't break people's existing installations.

Native programs can't be bundled with extensions, they have to be downloaded and installed separately. This isn't an uncommon setup with some applications, like 1Password, where they have a version of the extension that requires a native application to be paired with it.

Not having native access has been a blocker for a ton of issues, such as switching to use the gRPC API, dealing with invalid TLS cers, being able to implement channel close, being able to handle lightning: URIs, and a ton of other things. While I've tried very hard to avoid going native, it definitely seems like it unlocks a ton of possibilities for Joule.

wbobeirne avatar Mar 31 '20 15:03 wbobeirne

fyi. this is the proxy app I've built to work around the invalid cert error: https://github.com/bumi/lnd-proxy it runs a local proxy and proxies requests from localhost to the remote LND node.

bumi avatar Sep 29 '20 15:09 bumi

Dude, this is very slick! I'd be happy to throw something in the readme after I look it over. Nice work.

wbobeirne avatar Sep 29 '20 17:09 wbobeirne

it is a pretty early version and I have to learn some Golang for it :) And I am wondering about the UX and the usefulness of it. If the plan is to move towards a native bridge, then that could be a first step maybe.

What is your plan with the allowance feature? As the communication through the background script should get reverted do you see any chance to implement the allowances? Could it maybe be optional?

bumi avatar Sep 30 '20 15:09 bumi

webpack.config.js

curtcurt87 avatar Mar 31 '21 05:03 curtcurt87