[WIP] Enable syncing of vault/preferences between multiple browsers

[heyellieday]

This PR is to address issue #1459. Implementation details will be added to this PR, but for now, the linked issue has good amount of info.

Incomplete list of TODOS:

• Decide if enforcing strict rate-limiting due to extension's own limits is worth added complexity.
• Add ExtensionStore onChange listener to update in mem store as updates to extension store come in
• Add error handling when calling extensionStore methods
• Add actually spec coverage (by actually being able to run specs locally)
• Gracefully fallback to only localStorage for incompatible browsers (Opera, Firefox versions 52 and lower)
• Add isEnabled flag (with a default of false) and corresponding settings option

[kumavis]

hello/ :fox_face:
welcome to development on metamask
your writeups on this feature have been very thorough 👌

I am a bit concerned about the worst cases for the conflict resolution, and debugging against the blackboxed nature of the chrome/firefox syncing infrastructure

the ideal case seems like it would allow parts of the state to merge, or we would just import something like a redux action stream for the backend.

also need to consider:

• when users intend to have different vaults on different computers and not sync them
• security implications of sending the vaults out to the network

hope im not overly complicating this feature, just don't want user's vaults to get blown out by a bad sync

[heyellieday]

Hi @kumavis, thanks for the warm welcome! :)

Let me try and address your comments.

I am a bit concerned about the worst cases for the conflict resolution, and debugging against the
blackboxed nature of the chrome/firefox syncing infrastructure

I agree that Chrome and Firefox's syncing feature isn't the most transparent and I'm also finding that it isn't the most well documented. However, the fact that it is built in might outweigh those points.

the ideal case seems like it would allow parts of the state to merge, or we would just import
something like a redux action stream for the backend.

Are you talking about only syncing parts of the top level state, or having even more granularity by filtering out keys of each controller-based state slice? (like KeyringController and PreferencesController). I currently have it set up to only grab from a select set of top level keys:

const KEYS_TO_SYNC = ['KeyringController', 'PreferencesController'].

when users intend to have different vaults on different computers and not sync them

The plan is to handle this case via an option in the settings section that a user can select to enable or disable syncing. It would be disabled by default.

security implications of sending the vaults out to the network

I agree that there are some concerns with the security of this feature. I stumbled across this in the docs for chrome's implementation of syncing: "Confidential user information should not be stored! The storage area isn't encrypted.". However, I'm unsure how accurate the warning to not store confidental info is as, again, the docs don't seem to be maintained very well. I can look and see if there are any documented use cases of popular extensions using this feature to see if specific security concerns are mentioned.

Let me know if you have any more thoughts or questions! In the meantime, I'll be working on the existing TODOs at the top of this PR.

[danfinlay]

Some thoughts on this:

As one of our first bounties, I think we learned that slightly ambiguous, research-y bounties are probably unfair to bounty hunters, and we're pretty embarrassed that we learned so much about this feature while it was under development. We're going to pay a bounty for Ellie's work because we're using it in another feature too, but since this one has had so much scope creep, there's some apprehension that makes us want to be very sure before ever fully adopting this feature.

"Confidential user information should not be stored! The storage area isn't encrypted.". However, I'm unsure how accurate the warning to not store confidental info is

The current local APIs have the same problem, and we somewhat mitigate it by encrypting the local data with the user's password ourselves. We could probably always seek better encryption for this, though (more hash cycles before generating the key, longer minimum passwords).

One necessary feature for this to work well:

• If there are two MetaMasks open and have syncing enabled, and one writes to shared storage, the other should adopt that storage.

Another Q:

• If two MetaMasks are open on the same chrome profile, and syncing is enabled on one, is it enabled on the other automatically?

[danfinlay]

Related to #2547.

Another key that would be useful to sync would be the tokens (preferences controller).