search

brave / brave-browser

Brave browser for Android, iOS, Linux, macOS, Windows.
https://brave.com
Mozilla Public License 2.0
17.88k stars 2.34k forks source link

Improve compatibility with MetaMask and allow web3 provider selection #7503

Closed bbondy closed 4 years ago

bbondy commented 4 years ago

Objective

In Brave, if the user has MetaMask installed, we currently show an infobar on a Dapp page that asks the user to disable one of either Crypto Wallets or MetaMask.

Screen Shot 2019-12-20 at 12 08 58 PM

This is over disabling functionality that we don’t need to, when really we should just be asking which web3 provider they want to use for that page. The current implementation also allows for various edge cases where users can be in a stuck broken state when trying to use Dapps.

This is intended to allow Crypto Wallets to remain enabled, and to improve the frustrations that users encounter by having two web3 providers installed. It also enables us to keep other future functionality enabled as we enhance Crypto Wallets for things like exchange integrations.

Future enhancements / iterations

Explicitly not part of this spec:

Targeted platforms

Description

The work can be cleanly divided into 4 areas:

  1. Settings work: Remove the setting in preferences which disables Crypto Wallets. There will be no need for this setting anymore. Replace this setting with an option to pick your web3 provider.
  2. Migration work: How the existing user choice works into the new functionality.
  3. Infobar work: Repurpose our MetaMask infobar to ask the user which Web3 provider to use.
  4. Adding an API to obtain the web3 provider: An API will be made available to both MetaMask and Crypto Wallets to be able to check which web3 provider should be used.

Settings work

The old settings switch to turn off Crypto Wallets will be removed. See the next section on Migration work to see how the existing user choice will be handled.

Screen Shot 2019-12-20 at 12 09 30 PM

The options for the drop down menu will be: Ask, None, Crypto Wallets, and MetaMask if it is installed. The default will be Ask until an explicit option is set. The new preference will be stored with the pref name of brave.wallet.web3_provider in per-profile prefs which are visible in chrome://prefs-internals/. If the user explicitly uses Brave wallet first and opts in and the setting is set to Brave, then even installing MetaMask later won’t change the setting from Brave.

If Ask is selected, UI will be shown as per the section on Infobar work If None is selected, the entire extension will always be lazy loaded, even if it is installed. If Crypto Wallets is selected, only Crypto Wallets will be used as a web3 provider. If MetaMask is selected, only MetaMask will be used as a web3 provider.

Migration work

Some users in the past had seen the old infobar and selected either to use MetaMask or Crypto Wallets. We have only in the past actively disabled Crypto Wallets, so no one will have MetaMask disabled by us.

A one time migration will be done to determine which value should be used for the new setting brave.wallet.web3_provider.

An automated test will be added for each of these migration cases to ensure they work correctly.

Migration code will be added in chrome/browser/prefs/browser_prefs.cc via MigrateObsoleteBrowserPrefs.

Infobar work

If MetaMask is not installed, the existing UX will continue to be valid:

Screen Shot 2019-12-20 at 12 10 17 PM

If MetaMask is installed, the existing infobar UX highlighted in the Objective section will be repurposed to the following.

Screen Shot 2019-12-20 at 12 11 17 PM

Closing the infobar will use neither provider, it will keep the setting mentioned in the last section to “Ask”.

If the user’s setting is “None”, no infobar will be shown.

Depending on which option is selected, the setting mentioned above will be set to either “Crypto Wallets” or “MetaMask”. “None” can only be set from the settings page. Adding an API to obtain the web3 provider A new API will be exposed to both Crypto Wallets and MetaMask: chrome.cryptoWallets.getWeb3Provider((extensionId) => {})

The other APIs available within chrome.cryptoWallets will NOT be accessible to MetaMask.

It will return the extensionId for the provider to use or a blank string. A web3 provider should only inject existing MetaMask code that provides web3 to the page if its extensionId matches the return value from chrome.cryptoWallets.getWeb3Provider.

Possible values for the extensionId in the above callback will be:

QA plan

Test plan being added in wiki here https://github.com/brave/qa-resources/wiki/%5BWIP%5D-Multiple-Web3-provider-support-test

Open Questions

None at this time.

mikedotexe commented 4 years ago

This is such a well-written issue, holy cow.

GeetaSarvadnya commented 4 years ago

Verification passed on

Brave 1.5.109 Chromium: 80.0.3987.132 (Official Build) beta (64-bit)
Revision fcea73228632975e052eb90fcf6cd1752d3b42b4-refs/branch-heads/3987@{#974}
OS Windows 10 OS Version 1803 (Build 17134.1006)

Clean profile:

Upgrade Profile

Verification passed on

Brave 1.5.111 Chromium: 80.0.3987.132 (Official Build) (64-bit)
Revision fcea73228632975e052eb90fcf6cd1752d3b42b4-refs/branch-heads/3987@{#974}
OS Linux

Clean profile:

Upgrade Profile

Verification PASSED on macOS 10.15.3 x64 Catalina using the following build:

Brave | 1.5.112 Chromium: 80.0.3987.132 (Official Build) (64-bit)
-- | --
Revision | fcea73228632975e052eb90fcf6cd1752d3b42b4-refs/branch-heads/3987@{#974}
OS | macOS Version 10.15.3 (Build 19D76)

Basically went through all the cases that @GeetaSarvadnya outlined above and @bbondy outlined via https://github.com/brave/brave-browser/issues/7503#issue-541101168. I had more detailed notes but lost them when attempting to update this post and hit a merge conflict. After I refreshed, everything was gone. Notes were basically similar to @GeetaSarvadnya.

GeetaSarvadnya commented 4 years ago

@bbondy: I don't see new pref name brave.wallet.web3_provider in chrome://prefs-internals/ - I have verified in both clean and upgraded profiles. let me know if I am missing anything.