Skip to content

Add JoinMarket WebUI#1216

Merged
lukechilds merged 18 commits into
getumbrel:masterfrom
theborakompanioni:joinmarket-webui
Feb 28, 2022
Merged

Add JoinMarket WebUI#1216
lukechilds merged 18 commits into
getumbrel:masterfrom
theborakompanioni:joinmarket-webui

Conversation

@theborakompanioni

@theborakompanioni theborakompanioni commented Jan 31, 2022

Copy link
Copy Markdown
Contributor

This is a draft PR until following TODOs are finished on our side:

  • change to a non-development version before merge (v0.0.3)
  • APP_PASSWORD/APP_SEED support
  • test on "Umbrel OS on a Raspberry Pi 4"
  • test on "Custom Umbrel install on Linux"
  • Gallery images
  • 256x256 SVG icon

App Submission

App name

JoinMarket Web UI

Version

v0.0.3

One line description of the app

(max 50 characters)

A user-friendly UI for JoinMarket.

Summary of the app

(50 to 200 words)

JoinMarket WebUI is a user-interface for JoinMarket with a focus on user-friendliness.
It is time for top-notch privacy for your bitcoin. Widespread use of JoinMarket improves bitcoin's fungibility and privacy for all.

The app provides sensible defaults and is easy to use for beginners while still providing the features advanced users expect.

Developer name

JoinMarket WebUI Organisation

Developer website

https://github.com/joinmarket-webui

Source code link

(Link to your app's source code repository.)

https://github.com/joinmarket-webui/joinmarket-webui

Support link

(Link to your Telegram support channel, GitHub issues/discussions, support portal, or any other place where users could contact you for support.)

https://github.com/joinmarket-webui/joinmarket-webui/issues

Requires

  • Bitcoin Core
  • Electrum server
  • LND

256x256 SVG icon

(Submit an icon with no rounded corners as it will be dynamically rounded with CSS. GitHub doesn't allow uploading SVGs directly, so please upload your icon to an alternate service, like https://svgur.com, and paste the link below.)

https://svgur.com/i/dsx.svg

Gallery images

(Upload 3 to 5 high-quality gallery images (1440x900px) of your app in PNG format, or just upload 3 to 5 screenshots of your app and we'll help you design the gallery images.)

image
image
image
image
image

I have tested my app on:

@lukechilds

Copy link
Copy Markdown
Member

Wow exciting stuff @theborakompanioni, great work!

I realise this is still a draft but just checking, is this supposed to work in its current state? I'm trying to mix but keep hitting the same error Not enough counterparties, aborting.. I set the counterparties to 3 and it looks like 3 makers connected from the logs so it's not too clear to me what went wrong. I also see lots of errors related to IRC connecitons, not sure if that's normal.

2022-02-06 15:44:42,085 [INFO]  IRC connection failed
2022-02-06 15:44:42,086 [INFO]  Attempting to reconnect...
2022-02-06 15:44:48,820 [INFO]  IRC connection failed
2022-02-06 15:44:48,821 [INFO]  Attempting to reconnect...
2022-02-06 15:45:09,230 [INFO]  joined: #joinmarket-pit guybrush.hackint.org
2022-02-06 15:45:09,232 [INFO]  Could not connect to *ALL* servers yet, waiting up to 60 more seconds.
2022-02-06 15:45:15,342 [INFO]  IRC connection failed
2022-02-06 15:45:15,343 [INFO]  Attempting to reconnect...
2022-02-06 15:45:16,457 [INFO]  JM daemon setup complete
2022-02-06 15:45:54,640 [INFO]  IRC connection failed
2022-02-06 15:45:54,641 [INFO]  Attempting to reconnect...
2022-02-06 15:46:16,476 [INFO]  INFO:Received offers from joinmarket pit
2022-02-06 15:46:17,013 [INFO]  INFO:Preparing bitcoin data..
2022-02-06 15:46:17,051 [INFO]  Using bitcoin network feerate for 3 block confirmation target (randomized for privacy): 10568 sat/vkB (10.5 sat/vB)
2022-02-06 15:46:17,064 [INFO]  Using bitcoin network feerate for 3 block confirmation target (randomized for privacy): 10951 sat/vkB (10.9 sat/vB)
2022-02-06 15:46:17,066 [INFO]  total estimated amount spent = 0.00043266 BTC (43266 sat)
2022-02-06 15:46:17,086 [INFO]  INFO:Commitment sourced OK
2022-02-06 15:46:43,735 [INFO]  IRC connection failed
2022-02-06 15:46:43,736 [INFO]  Attempting to reconnect...
2022-02-06 15:46:44,567 [INFO]  Makers responded with: {'J5DfPt2XABWoi9BC': [['85183201273a02ad1276673bc024bec9123df732202cd731ac611572fe6a3742:12'], '02268ac28648b94e0e464b6b42a6f9f0a70ed9e57ed4db110f5ab653a0ad3b51d8', 'bc1q53cfduwfcyajm67cn4s2eqm8s4ryxjwylmpyuj', 'bc1q5x5dg7ehsf7tclx5nk5tgejeynhwzx7f4pvsg6', 'MEUCIQCNceca08+7Wc7/pmM4JW80TuP36Mtz7Cj2r+x+E+MbQwIgGccQvAG/fFKzcgER16oGeOuDvdSAI/UPGwI44sF1nY8=', '08d993a81b820df1f940f7f52409e5513ec6dd483b7a28de66bbb4fe7df66f02'], 'J53RrgKr8sijY2me': [['2bffdf6b86e41679f2cb84f0211a7449b76e5daeaec08f3c5393d98230ff1df7:8'], '02ae9584cc12dee4bbd373f0d7abfbee7cd02e2641aad95b5c3aed064411f98640', 'bc1q8xl5n7g0yjzg0cmpq6fps2cevty3g8zl68fhle', 'bc1q29raamv642nf8dm9p3f4hp8yjjd7ecyunaqu38', 'MEQCIFb+lZZmv32xNZxT81V5VvhxAcwsTabrDMVklvLwcBkbAiA9eioryqR5qLElTQb1IqgJSFMxSqRp9xCeAuvaPb6xRA==', 'd96aff34776a5bf36ec3fd0d4c7260ee2921fc559ddf6ee4b9987d26fe250e70'], 'J577TXyD3b1sE9tq': [['891a78e59c7b5567cb3c7951b960de19ea970455910de177123a5a557ca2ae63:12'], '0281496c136f181e0100be24338a0d2cef2449e8049a32f3400c824d1e0f090f39', 'bc1qglv2prq444n45heuyq0xhth7lwysdnuyudwwz4', 'bc1q7ud08mvp03sf2998yq8ktfljg6h8v76n27k2cm', 'MEUCIQCz22Hj5/fKlQIF/HaL9WD7Z7hncTztOAp2tjVvpqIrFQIgX+MvKmHngzuWrvL+/wCvtOVfdFfpLJmT+yuox478X5k=', 'f5429141e58fb3f52e6ad3f5bc6f63276f5a25d73e61fec5af85a88ec064c601']}
2022-02-06 15:46:44,654 [INFO]  fee breakdown for J5DfPt2XABWoi9BC totalin=56146291 cjamount=30000 txfee=0 realcjfee=72
2022-02-06 15:46:44,655 [INFO]  fee breakdown for J53RrgKr8sijY2me totalin=1814343 cjamount=30000 txfee=0 realcjfee=3994
2022-02-06 15:46:44,656 [INFO]  fee breakdown for J577TXyD3b1sE9tq totalin=2174447 cjamount=30000 txfee=0 realcjfee=282
2022-02-06 15:46:44,657 [INFO]  INFO:Not enough counterparties, aborting.
2022-02-06 15:46:44,659 [INFO]  Taker is not continuing, phase 2 abandoned.
2022-02-06 15:46:44,659 [INFO]  Reason: Not enough counterparties responded to fill, giving up
2022-02-06 15:46:44,660 [INFO]  Coinjoin did not complete successfully.

I tried about 5 coinjoins, hit the same error every time. Any ideas?

@theborakompanioni

theborakompanioni commented Feb 6, 2022

Copy link
Copy Markdown
Contributor Author

Hey @lukechilds !

I realise this is still a draft but just checking, is this supposed to work in its current state?

It is built from the respective master branches of joinmarket-clientserver and joinmarket-webui, so these are definitely not considered stable! However, it should generally work, yes.

I tried about 5 coinjoins, hit the same error every time. Any ideas?

I have personally not tried this on mainnet yet, pinging @AdamISZ who's the most capable of answering these questions.

Wow exciting stuff

Thank you for checking it out and your valuable feedback. Highly appreciated!
I'm looking forward to when the PR is ready for review.
Hopefully a version can be released soon!

@AdamISZ

AdamISZ commented Feb 6, 2022

Copy link
Copy Markdown

@lukechilds we have a default setting in the config minimum_makers under [POLICY] that disallows anything less than 4, unfortunately (sorry there is no informative error for that, PRs welcome etc.). This setting is really there to prevent fallbacks to less than 4, when the original choice is higher. But it also serves as a kind of 'idiot proof' function, that we don't want people accidentally choosing very small numbers without realising it doesn't have a decent outcome. Arguably we shouldn't do that, but oh well.

Meanwhile the current setup via the RPC, while it does have a configset method allowing you to change a config setting, I don't think the joinmarket-webui app yet has exposed that. (Edit: if you know where joinmarket.cfg is living, and it usually lives in ~/.joinmarket/joinmarket.cfg, you can edit it yourself manually).

Also yeah you can do 30K but consider bumping it up to like 100K if at all possible, just to avoid edge cases.

Even better, test on signet, but you need to have people running signet maker bots for that.

@AdamISZ

AdamISZ commented Feb 6, 2022

Copy link
Copy Markdown

Oh I forgot to mention, backend needs restart if you edit the config manually, otherwise it would not be updated. There's a bit of an issue around config updates generally, for now, editing the file is simplest.

@lukechilds lukechilds added the app submission A brand new app submission label Feb 7, 2022
@lukechilds

Copy link
Copy Markdown
Member

Got it, thanks guys!

Looking forward to the stable release, ping me when you're ready for review, super exited for this one!

And of course let me know if you need anything or if there's anything I can do to help.

@theborakompanioni

theborakompanioni commented Feb 13, 2022

Copy link
Copy Markdown
Contributor Author

Looking forward to the stable release, ping me when you're ready for review, super exited for this one!

🙏

And of course let me know if you need anything or if there's anything I can do to help.

Thanks @lukechilds, your help is highly appreciated.
I got a question or two..

I am currently trying to make use of the APP_PASSWORD env variable.
The web UI is served as static content from an nginx webserver. All api requests are proxied to the joinmarket server, but access should be limited as some unauthenticated endpoints contain sensitive information. It seems that the easiest way to keep serving the ui as static files would be Basic Authentication from nginx with password ${APP_PASSWORD} (and e.g. user umbrel).

Is this okay for the first version or should we go a different route?
I do not know if any other Umbrel app uses Basic Authentication or if using it is discouraged.
There are some drawbacks and the prompt is not visually appealing...

Otherwise we have to come up with our own solution before the first release.
If possible, we want the app to stay a static page - but that is not set in stone.
I can also imagine a custom endpoint in the nginx config that can be checked and - if present - ask for a password.

However, there may be a completely different best practice you can advise.

Edit: Okay, found that at least one app (lightning-shell) is using Basic Authentication.

@dergigi

dergigi commented Feb 17, 2022

Copy link
Copy Markdown

I think basic auth should be fine for now. We can always upgrade it to something more fancy at a later stage, but I'd say basic auth is good enough for the first release (which is basically an "alpha" version anyway).

@lukechilds

Copy link
Copy Markdown
Member

Yep totally cool ot use HTTP basic auth, we have a number of apps using it currently.

Also, without giving too much secret sauce away, $APP_PASSWORD is more of a hacky temporary solution to locking down apps. We're working on a much cleaner solution at the Umbrel level that doesn't involve any custom handling from apps. So totally makes sense to go with HTTP basic auth for now until we have that ready.

A nice thing is that the browser caches the HTTP basic auth credentials after a sucessful login. So while it doesn't look too pretty, it's relatively uninvasive. Most users will only see it the first time they use the app, then the credentials will be cached and they won't see it again.

@theborakompanioni

theborakompanioni commented Feb 18, 2022

Copy link
Copy Markdown
Contributor Author

So totally makes sense to go with HTTP basic auth for now until we have that ready.

Great to hear. Thanks for your feedback.

All upstream app versions are updated and ready to go.

One last question before signing off the submission: Are there any problems with changing to a non-root user in a subsequent release, or is it safe to do so? Just asking if we should tackle this before or after. Otherwise, we would be ready. 🚀

@lukechilds

Copy link
Copy Markdown
Member

Are there any problems with changing to a non-root user in a subsequent release

It's fine to ship as root. It's just a reccomendation that apps don't run as root but it's not strictly enforced. You can implmement that in a future update.

One small issue is that the persistent data that lives in the volume from existing installs will probably not be readable by the new user after the update since it would have been created by root. But that's easily fixable by adding a migration step that chowns the files to the new user during the update process. We have stuff in place to handle that, so don't worry about it, I can talk you through it when we get to that point.

@theborakompanioni

Copy link
Copy Markdown
Contributor Author

We have stuff in place to handle that, so don't worry about it, I can talk you through it when we get to that point.

Nice. Thank you 🙏
PR is ready for review now 🚀

@theborakompanioni theborakompanioni marked this pull request as ready for review February 19, 2022 14:20
@dergigi

dergigi commented Feb 22, 2022

Copy link
Copy Markdown

Awesome! Thanks for getting this ready @theborakompanioni 🧡

PR looks good to me, let us know if there is anything that still needs to be done @lukechilds

@lukechilds

lukechilds commented Feb 25, 2022

Copy link
Copy Markdown
Member

I see this in the description:

  • APP_PASSWORD/APP_SEED support

But I don't see $APP_SEED being passed through anywhere. Does this app make use of $APP_SEED for deterministic wallets?

@theborakompanioni

Copy link
Copy Markdown
Contributor Author

Does this app make use of $APP_SEED for deterministic wallets?

No, it doesn't. Sorry, I wrote the checklist before a decision was made. Just $APP_PASSWORD is supported.

@lukechilds

Copy link
Copy Markdown
Member

No, it doesn't. Sorry, I wrote the checklist before a decision was made. Just $APP_PASSWORD is supported.

Got it, no worries, we don't need to block the intitial release for this but it would be a really awesome feature to support.

Main benefit of $APP_SEED is there's no need for the user to back anything up and minimal chance of loss of funds. Currently if you uninstall the JoinMarket app and don't have your seed phrase written down then you nuke your funds. And even if you do have the seed phrase written down afaik you can't recover it via the UI. Using $APP_SEED the JM wallet would be deterministically derived from the main Umbrel seed which the user already has backed up. If the user uninstalled the app, when they reinstall at a later date the same wallet would be determinisitcally derived and any previous funds would still be in it.

If their Umbrel get destroyed, they could setup a new one and use their previous Umbrel seed and installing JM would also recover their previous funds.

@lukechilds

Copy link
Copy Markdown
Member

Ok so just had a play with this, it's looking really really good! The UI is 😘👌. Magic wallet mode is a thing of beauty!

One issue I had is that it took me quite a few attempts to get a (taker) coinjoin to work. I think I tried maybe 10 attempts at coinjoining before I managed to suceed. Is this something you've also experienced or am I doing something wrong?

One other things was that it seemed like the state of the UI and the state of JM were kind of out of sync at times. I could see stuff failing by tailing the JM logs on the filesystem but there wasn't any feedback in the UI that the CJ had failed or why. This was easy enough for me to figure out but I don't think many Umbrel users would be comfortable hunting down log files, so there may be some confusion with users there. It woulds be great if the UI could handle the different ways CJs can fail and display a simple but helpful error to users so they have some idea what's going wrong.

Of course again that's an improvement that doesn't need to delay the intial release, just a suggestion.

@lukechilds

Copy link
Copy Markdown
Member

Btw guys how do you feel about naming the app just "JoinMarket" instead of "JoinMarket Web UI"? I'm not sure how closely affiliated with upstream JM devs you are or if they would be ok with that. But assuming they are I think it's a much cleaner name, thoughts?

@AdamISZ

AdamISZ commented Feb 27, 2022

Copy link
Copy Markdown

One issue I had is that it took me quite a few attempts to get a (taker) coinjoin to work. I think I tried maybe 10 attempts at coinjoining before I managed to suceed. Is this something you've also experienced or am I doing something wrong?

It's hard to say without knowing all the details of what you're doing, which of course you shouldn't say, on mainnet. With default number of counterparties ~ 9-10 we can expect:

  • If you choose a really low amount there can be problems finding liquidity
  • It's going to take several minutes, which maybe surprising. I think 3 minutes is normal but can be easily a minute or two longer. A big part of that is simply waiting: first waiting to get connected to all IRC servers, then waiting for all makers to send you their offers. Then the interactive part also takes a bit of time, then waiting to see it on the bitcoin network (usually another counterparty does the tx broadcast). Please get people to test this PR, it will solve so many of these problems, but as usual I can get nearly no one to test or review these really important things ...
  • It's not uncommon for there to be one or two counterparties drop out in the first phase, but it should only lead to a slightly lowe anon set. It is certainly possible for them to block right at the end by not sending the signature, but it isn't seen often.
  • If you set very low max_rel_cj_fee,max_abs_cj_fee you may experience difficulties with liquidity.

I generally experience success first time, but to say any more than vague generalities, would need to at least know, what error is shown on the console. For this reason agree strongly that we need feedback to the user on a failure at the end of a Taker event.

@dergigi

dergigi commented Feb 28, 2022

Copy link
Copy Markdown

how do you feel about naming the app just "JoinMarket" instead of "JoinMarket Web UI"?

We are in the process of coming up with a better name. Our thinking was that we could change it later on, but since this issue came up here and we have a couple of ACKs on "Jam" - which is short for Joinmarket's awesome, man - we might as well go with it straight away.

@lukechilds

lukechilds commented Feb 28, 2022

Copy link
Copy Markdown
Member

Haha, love Jam!

So rename to Jam and then good to merge?

@theborakompanioni

theborakompanioni commented Feb 28, 2022

Copy link
Copy Markdown
Contributor Author

Haha, love Jam!

So rename to Jam and then good to merge?

Jep, will rename everything, except including the env variables - is that okay @lukechilds ?


services:
jam:
image: ghcr.io/joinmarket-webui/joinmarket-webui-standalone:v0.0.3-clientserver-v0.9.5@sha256:d33817e4daec4ddaaf95a1c08e54f50cff4893b7d8c5b3bbe8b179ced74bbaa2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Did you miss the name change in the Docker image or is it not renamed to jam-standalone (or something like that)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, image is already pinned. Everything will be renamed (repos, support group, hyperlinks in general, etc.) within the next releases. Since it is not user-facing - is it okay to leave it that way?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah that's fine.

@lukechilds

Copy link
Copy Markdown
Member

Perfect!

Ok lemme just quickly just run through another test with all the naming changes and make sure nothing's broken, then we can merge this. 🚀

@lukechilds

Copy link
Copy Markdown
Member

Its Happening Ron Paul GIF

Excellent work gentlemen!

@lukechilds lukechilds merged commit 94aeabd into getumbrel:master Feb 28, 2022
@mayankchhabra

Copy link
Copy Markdown
Member

Really amazing work, everyone! The app's UI is very well done. Congrats on the launch, and thank you for finally making JoinMarket with web UI a reality. ❤️

Here are the gallery images and app icon for Jam. If you ever want to change/update them, feel free to directly open a PR at getumbrel/umbrel-apps-gallery.

@theborakompanioni theborakompanioni deleted the joinmarket-webui branch March 8, 2022 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app submission A brand new app submission

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants