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

Make one wallet the standard #228

Open
editwentyone opened this issue Apr 21, 2022 · 18 comments
Open

Make one wallet the standard #228

editwentyone opened this issue Apr 21, 2022 · 18 comments
Labels
blocked Merging this pull request is blocked until another issue is resolved concept Wild idea, or too many details unknown yet UI/UX Issue related to cosmetics, design, or user experience
Milestone

Comments

@editwentyone
Copy link

editwentyone commented Apr 21, 2022

what do you think about sliming down the overall use of passwords for JAM and wallets by giving the user in the normal journey the following:

  • password protected JAM app preview with Logout Button on settings page
  • automatically creating one wallet without password after intro (is this even possible with JM API?)
    (no preview, created screens would be wired differently)
  • and move "switch wallet" / "create another wallet" into the settings for more advanced people. preview

we are not getting rid of the screens and functionality that is build already, we just link them differently into another flow.

later we could also maybe build in import/ export of wallets for different cross usage like joininbox or backups if needed.

the default user would have an easier experience without too many passwords to remember/ secure and there is no lock/unlock issue with running processes in the background.

@openoms
Copy link
Contributor

openoms commented Apr 22, 2022

On most systems the wallet password is the only encryption standing between needing a password or having the bitcoin keys readily available for anyone with physical access to the disk.

Be aware that getting rid of wallet password there will be no security left.
A slightly better solution could be to reuse the wallet password to be the app password so then it can be stored in the memory to unlock the wallet also and disappears when the node is switched off.
See the raspiblitz LND wallet for example:
the apps are locked with the PasswordB (which is the bitcoin password reused)
the LND wallet has it's own PasswordC which is not stored by default and locks all funds on a restart.

Think of what happens when someone takes the node away.
Passwords not stored (or only in RAM) are flushed and strong encryption saves the user from losing funds.

The problem with loosening the security is that JM is meant for higher amounts by nature so autounlocks and passwordless setups risk a lot potentially.

@ghost
Copy link

ghost commented Apr 22, 2022

I wouldn't be comfortable with implementing completely passwordless wallets for the reasons @openoms mentioned above.

I do like the general idea of having a single wallet flow by default (with the option to create more wallets if needed). I feel like most of our users would be perfectly fine with having only one wallet. Currently the "choose wallet" screen is really prominent and I don't think it needs to be such a central thing in the UX when most users (I assume; might be wrong of course) have a single wallet only.

We could, for example, build a guided onboarding flow that is shown the first time the user opens the app. There we could guide the user to create a wallet and then make this wallet the default. After unlocking the app (and the wallet) this wallet would then be opened directly. A pro user could still reach the "switch / create wallets" screen via settings for example. There they could also choose which wallet (if they have several) is the default one. Does that make sense from a UX point of view?

@editwentyone
Copy link
Author

editwentyone commented Apr 24, 2022

nice, I like the insights. if its a possible way, I would love to see the user punching in the chosen wallet password as the jam login password. we need to optimize the wording for the user to understand that a login is the same as unlock the wallet in general. after that we are in a single wallet environment.

all the other features like adding another wallet, switch wallets, make a wallet the new default, lock and unlock wallets, maybe even delete wallets and export and import plus rescan wallets should live under settings. imo that makes more sense.

@ghost ghost added concept Wild idea, or too many details unknown yet UI/UX Issue related to cosmetics, design, or user experience labels Apr 25, 2022
@ghost
Copy link

ghost commented Apr 26, 2022

all the other features like adding another wallet, switch wallets, make a wallet the new default, lock and unlock wallets, maybe even delete wallets and export and import plus rescan wallets should live under settings. imo that makes more sense.

💯 agree on this!


About reusing the wallet password as the app password: While I'd love to have this from a Jam point-of-view, I'm not sure we can do that in a clean way considering the bigger picture. It might turn out to be more confusing in the end.
My thinking is that by convention we're forced to use the shared app password that the nodes provide to unlock the app. But we cannot reuse this password to unlock the wallet for security reasons. That in turn means that we basically have to go down the two passwords route. I'll explain in more detail below, please tell me if I'm wrong. I might easily be miss something!

(Sorry in advance for the wall of text.)

App Password vs. Wallet Password

Currently, there's two different passwords:

  1. The app password: Protects the React app and the JoinMarket API from unauthorized access while being served on the network. This password can be stored on disk.
  2. The wallet password: Protects the funds in the JoinMarket wallet from unauthorized access even when the node is turned off. This password cannot be stored on disk since it should still protect the wallet e.g. in case of theft.

Like pointed out by openoms, a way of maybe achieving a single-password flow would be to reuse the wallet password for the app password. However, since nodes like RaspiBlitz or Umbrel work with a single shared app password ("Password B" on a RaspiBlitz) to unlock the apps (e.g. RTL, Thunderhub) running on these nodes, it would be "non-standard" in a way for us to deviate from this scheme. That could be confusing for users who are used to the shared app-password.

I feel like it's the safer route to stick with the way the nodes want us to use their app-password. We also can't reuse this app-password for the wallet of course, since it's stored on disk e.g. on the RaspiBlitz.

So assuming all my assumptions above are true (please tell me if I'm wrong!) we basically have to keep the two passwords around:

  1. The shared app password provided by the node to unlock the app
  2. A user chosen wallet password that is not stored on disk

I do see how this is confusing. We somehow have to engineer the UI in a way that it isn't weird to enter two passwords at once. Maybe we could do it like a 2FA flow since users are already accustomed to entering two passwords there. 🤔 Another option might be to stick with Basic Auth (our current approach on Umbrel) since browsers cache these passwords so it would feel like a one-password flow most of the time.

I'd love to see a simpler, one-password approach though if anyone has any ideas! Hope this is helpful background. Let me know what you think everyone!

@AdamISZ
Copy link
Contributor

AdamISZ commented May 2, 2022

Currently the "choose wallet" screen is really prominent and I don't think it needs to be such a central thing in the UX when most users (I assume; might be wrong of course) have a single wallet only.

This is an interesting point and intuitively, correct (arguable though!).

It's worth remembering that one of the main distinguishing features of a Joinmarket wallet is that it has 5 (by default, which most don't change) accounts and they function very much like separate wallets to all intents and purposes.

There is definitely a use case for switching up to a new wallet file at least from time to time. Depending on .. stuff .., a user can end up having thousands of used addresses in a JM wallet, unlike most wallets (don't forget - even failed coinjoins "use up" addresses, because they were communicated to counterparties and are therefore considered unusable in future - notice how different that is, from most wallets). This causes more scanning slowdown, and our wallet sync is not the fastest, in the first place!

(Of course there are other reasons to switch to a new wallet file; I just mention that one as it's less obvious)

So the obvious thing I guess is to hide 'new wallet' away from the main workflow, but do keep it and don't hide it too well :)

@editwentyone editwentyone added this to the v0.0.7 - Sticky Jam milestone May 24, 2022
@editwentyone
Copy link
Author

🎨Figma Settings
🎨Figma Wallet

  • settings categories
  • login/ logout
  • removed switch wallets in header (top right)
  • rewired screens / flow for switch wallet and create wallet
  • new "back" header coming from settings page, when inside "switch wallet" flow

image

image

@editwentyone editwentyone added this to JAM May 24, 2022
@editwentyone editwentyone moved this to Todo 📝 in JAM May 24, 2022
@dergigi dergigi changed the title Make one wallet the standard ?! Make one wallet the standard Jun 14, 2022
@dergigi
Copy link
Contributor

dergigi commented Jun 14, 2022

Resolved, at least partially, with #296 and #316

@dergigi dergigi moved this from Todo 📝 to Done ✅ in JAM Jun 14, 2022
@ghost
Copy link

ghost commented Jun 14, 2022

Agreed, it's partially resolved (also relevant: #325). For the scope of v0.0.7 we can definitely consider it resolved.

The meat of this feature will be replacing the "home" screen which is currently the "select which wallet to unlock screen" with something more sleek that just asks you to input the wallet password for the default wallet. The screen to switch wallets will then only be accessible from the settings.

@editwentyone If you're motivated to think about how this could look, feel free to open a new issue with any UI ideas! 🙏 ✨

@editwentyone
Copy link
Author

editwentyone commented Jun 20, 2022

lets use this to unlock the default wallet. also it would be nice to declare another wallet the "default", is this possible through settings screen?

image

https://www.figma.com/file/kfejZJFlwBywvLEnPEmJo1/?node-id=3994%3A73345

@ghost
Copy link

ghost commented Jun 20, 2022

also it would be nice to declare another wallet the "default", is this possible through settings screen?

@editwentyone I'd say it'll be easiest to slap that onto the "wallet switcher" screen:

Screenshot 2022-06-20 at 17 09 29

What do you think? Any ideas on how it could look like? I'm thinking a "make default wallet" button would probably be sufficient. 🤔

@ghost ghost mentioned this issue Jun 20, 2022
@MaxHillebrand
Copy link
Contributor

How about a star icon?

@editwentyone
Copy link
Author

"make default wallet" button was also my idea, but there will be too many buttons on each row.

star icon is more the "favorite" route

@editwentyone
Copy link
Author

editwentyone commented Jun 20, 2022

I came up with this 2 ideas to drag drop a tag or drag drop a line to the top , let me know what you think:

Figma 🎨

image

or 3rd option with radio buttons, saved when chosen/ selected, should be the easiest one

Figma

image

@ghost
Copy link

ghost commented Jun 21, 2022

or 3rd option with radio buttons, saved when chosen/ selected, should be the easiest one

I like this direction the most. Drap and drop implies an order between the elements which is not there in our case. What about the radio buttons option but make them ⭐ shaped?

@editwentyone
Copy link
Author

because there can only be one default wallet, I would stay with radios. stars are more favorites and you can have more then one

@ghost
Copy link

ghost commented Jun 28, 2022

See also #337

@ghost ghost moved this from Done ✅ to In Progress ⌛ in JAM Jun 28, 2022
@ghost
Copy link

ghost commented Jul 26, 2022

Finishing this is blocked by #337.

@ghost ghost added the blocked Merging this pull request is blocked until another issue is resolved label Jul 26, 2022
@ghost
Copy link

ghost commented Aug 2, 2022

Moving this issue out of v0.0.10 since we need a decision / designs for #337 first.

@ghost ghost removed this from the v0.0.10 - Jam it in there! milestone Aug 2, 2022
@editwentyone editwentyone removed the status in JAM Sep 29, 2023
@editwentyone editwentyone modified the milestones: v0.4.0, v0.5.0 Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Merging this pull request is blocked until another issue is resolved concept Wild idea, or too many details unknown yet UI/UX Issue related to cosmetics, design, or user experience
Projects
Status: No status
Development

No branches or pull requests

5 participants