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

Allow creation and import of multiple wallet types #328

Closed
danfinlay opened this issue Jun 26, 2016 · 11 comments
Closed

Allow creation and import of multiple wallet types #328

danfinlay opened this issue Jun 26, 2016 · 11 comments

Comments

@danfinlay
Copy link
Contributor

danfinlay commented Jun 26, 2016

Current behavior

Right now MetaMask's wallet system is entirely BIP-44 seed-word based.

Whenever MetaMask is first set up, the user gets a twelve-word phrase, and it's used to generate their first account.

When restoring MetaMask, the only key-pairs you can import are ones derived from a BIP-44 compliant, twelve-word seed-word phrase like MetaMask uses at setup.

This means that when you press the + button under the account list, it always generates the same accounts in order.

The problem

The ecosystem is full of alternative key signing methods, and we want the user to get to use one of their choosing.

For some users, it will be obvious that they want to import their old wallet file into MetaMask, not to mention it was a feature in the original MetaMask preview.

Since our entire UX right now is designed around these twelve-word seed phrases, which are hard to explain to normal people, there's a question of how to represent an account that is derived from your main seed words vs from a wallet file.

Proposal: Organized vaults

The account list section could be divided into sections. A section would represent either a single key-pair wallet file, or a seed-word phrase.

The seed-word phrase sections would be unique, because they would have their own "add account" section.

vault sections

Right now we're thinking these sub-sections would be views provided by that account type, so any features or UI elements that are specific to that keychain would show up in its collapsable section, or maybe it has an options gear button that navigates to a view it controls.

From this custom view, it could do as much interaction as it wanted, and then could pass it back to MetaMask when it was done.

@danfinlay danfinlay changed the title Design interaction for importing wallet files. Design interaction for importing wallet files and beyond Jun 26, 2016
@danfinlay
Copy link
Contributor Author

In some ways, we've thought about addressing this domain of concerns before, in #67.

#67 includes a sketch of a design where you would choose a wallet/vault when unlocking, which especially makes sense when considering the constraint that each wallet needs its own password. We don't store user passwords, so every time they add a new vault or wallet type, we need them to enter a password to encrypt it with.

I think I like the potential simplicity of listing all available accounts in one list, so if we did the above mockup instead, we would simply have to include in our design considerations for how we need the user to enter their password to add a vault or single account.

We can verify they reuse the same password for all their vaults, and that way when unlocking metamask, we could decrypt all of their vaults.

This means if a user imports an old wallet file that's encrypted with a different password, we'll probably just re-encrypt it with their MetaMask password for local storage, and if they ask to export it, we'll give them the metamask-passworded copy.

@danfinlay
Copy link
Contributor Author

danfinlay commented Jun 27, 2016

Long term "vaults" may be a very modular thing. We may have lots of vault types. Other types of account MetaMask might eventually include:

  • Coinbase or other centralized key holders.
  • Trezor or other hardware signing wallet.
  • uPort mobile phone based signing (maybe with QR-codes)

@danfinlay danfinlay added this to the Public Beta milestone Jul 15, 2016
@niran
Copy link

niran commented Aug 6, 2016

One idea is to draw a clear distinction between identities and the backups needed to recover them. This is similar to your mockups, except each identity would have an icon or label that indicates which backup is required for it. Instead of a Seed Vault section in the accounts list, each account tile would have something like ":closed_lock_with_key: 1". The geth wallet tile would have ":closed_lock_with_key: 2". There'd be another screen that lists backups with their names and creation or import dates. When you create a new identity, it tells you which backup is needed to recover it. I'm not sure this is better than visually separating identities from different backups, just an idea.

Another concept that would be nice to support is contract-based identities. Key-based identities are a dead-end because losing a key is too easy. Contract-based identities make it easier to design recovery strategies and use dapps with communal identities, like BoardRoom and multisig wallets.

With contract identity support, importing a seed or a wallet kicks off a registry lookup for any identities the key controls. Contract identities are typically created from a single factory, so it'd be easy to maintain an off-chain index of the identities each key controls. Once control of a contract identity changes, the factory is no longer a reliable source, but only BoardRoom currently allows that. Dapp UIs should record changes of control in a recovery registry to make it easy to see if a recovered key controls contract identities.

As long as contract identities can be recovered by recovering the keys that control them, there's nothing special about the UI for contract identities. It's just like another identity attached to a seed, and the recovery process is identical.

@nmushegian
Copy link

With contract identity support, importing a seed or a wallet kicks off a registry lookup for any identities the key controls. Contract identities are typically created from a single factory, so it'd be easy to maintain an off-chain index of the identities each key controls. Once control of a contract identity changes, the factory is no longer a reliable source, but only BoardRoom currently allows that. Dapp UIs should record changes of control in a recovery registry to make it easy to see if a recovered key controls contract identities.

I don't think factories need to track ownership at all, you can just ask the contract itself for its history, since you know it is trustworthy because it came from the factory =]

@danfinlay
Copy link
Contributor Author

New type of hardware wallet: https://www.ledgerwallet.com/products/12-ledger-nano-s

@danfinlay
Copy link
Contributor Author

@VladTod if you have some free time, I'd appreciate you taking a visual pass on the above proposal.

The idea is that the account-list view is now a keychain-list view, and it has addable and collapsable keychains on it, which each can show whatever they want under them, usually a list of accounts, but also include details for adding new accounts.

@danfinlay danfinlay changed the title Design interaction for importing wallet files and beyond Allow creation and import of multiple wallet types Oct 7, 2016
@danfinlay
Copy link
Contributor Author

I've begun a branch for working on this feature:
https://github.com/MetaMask/metamask-plugin/tree/i328-MultiVault

Its first commit is a document planning its architecture:
https://github.com/MetaMask/metamask-plugin/blob/i328-MultiVault/docs/multi_vault_planning.md

@danfinlay
Copy link
Contributor Author

danfinlay commented Oct 7, 2016

Tasks:

  • Break up current functions' responsibilities to divide nicely along the new organization structure.
    • createVault should be two methods: createEncryptedVault(password), then createBipVault(entropy).
  • Move KeyChain methods to generic dispatcher pattern, to allow dynamic function names.
  • Create tests for these newly discrete functions.
  • Create idStore 2.0.
  • Create Bip44Keychain module for backend.
  • Ensure Bip44Keychain imports old format nicely.
  • UI Separate vault (password encryption) creation from keychain (entropy) creation views.
  • Organize Bip44KeychainComponent views into their own folder, isolate them organizationally into a keychains folder.
  • Make places where Bip44KeychainComponent views are used agnostic to the type of KeychainComponent being used.

We will then have paved the way to theoretically easily:

@danfinlay danfinlay self-assigned this Oct 7, 2016
@danfinlay danfinlay modified the milestones: Multi Vault Support, Public Release Oct 11, 2016
@danfinlay
Copy link
Contributor Author

danfinlay commented Oct 12, 2016

Me and Kevin had a simplified MVP for this, that avoided many custom views, and uses a new tab for keychain creation.

The task list for that roadmap is this:

  • Consult with UI/UX people.
  • Remove seed word confirmation code
  • Create minimal KeychainManager & Bip44 keychain class (with init method)
  • Define vault creation tab protocol
  • Add BIP44 keychain creation page (for tab)
  • That should do it, right?

Keychain.init {
a method to digest the output of the new keychain tab.
}

Open questions:

What does first time usage look like with multi-vault?
- Keep it simple, encourage a single type of vault by default?
- Provide a list of options for first time users?
- Hybrid? Encourage a obvious default, with a subtle advanced option?
- uPort default?

@danfinlay
Copy link
Contributor Author

We have some new designs in this issue's Quip designs folder. I've assembled a nearly comprehensive proposal for comments.

I'm starting to think that we actually need to make Keyring creation happen in a new tab, the benefits are just too considerable to ignore, so we could use design mockups for what an account creation browser page might look like.

It should have

  • Metamask branding
  • A security image
  • Support for creating or importing.

This page could either be for a single Keyring type, or it could allow selecting keyrings within its UI, whatever we decide feels the most natural.

@danfinlay
Copy link
Contributor Author

I'm now closing this as it's better represented with a milestone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants