MetaMask / metamask-extension

:globe_with_meridians: :electric_plug: The MetaMask browser extension enables browsing Ethereum blockchain enabled websites
https://metamask.io
Other
11.9k stars 4.85k forks source link

Move Encryptor outside of KeyringController #4088

Open danfinlay opened 6 years ago

danfinlay commented 6 years ago

Once #4034 is completed, the next function that could be moved outside of the KeyringController is the encryption functions.

Currently the KeyringController stores accounts and their schema, but these aren't actually the only portions of MetaMask state that might need to be encrypted.

Instead, we should move our encryption related functions into their own module at the MetaMaskController level, so that it can combine encryption-needing state as needed.

Two functions involving the password currently pass through the KeyringController:

Also, a couple methods that assume keyringController manages the password encryption exist:

Since we are using obs-store for most of our state objects now, it would probably make sense to take advantage of this architecture for the new encryption strategy.

One implementation idea: SecureComposableObservableStore

We could maybe achieve this goal with a subclass of ComposableObservableStore, as the metamaskController.store instead.

The SecureComposableObservableStore class

danfinlay commented 6 years ago

No. Just recording likely long term architecture changes. The vault may not need to be in charge of encryption. Just ignore if it's distracting.