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

New default wallet handling for Desktop #18213

Closed
bbondy opened this issue Sep 20, 2021 · 1 comment · Fixed by brave/brave-core#10191
Closed

New default wallet handling for Desktop #18213

bbondy opened this issue Sep 20, 2021 · 1 comment · Fixed by brave/brave-core#10191

Comments

@bbondy
Copy link
Member

bbondy commented Sep 20, 2021

  • Change the pref:
Ethereum provider for using Dapps To: Default wallet
  • Remove Dapp detection for Crypto Wallets
  • Remove lazy loading of Crypto Wallets
  • Remove the Load Crypto Wallets on startup option.

When brave://flags/#native-brave-wallet is disabled:

  • The default wallet setting being set to None will let MetaMask insert window.web3 but not Crytpo Wallets
  • The default wallet setting being set to Ask option will work the same as none (It is only preserved for now because we'll migrate Ask settings to the new wallet later, but not None settings.
  • As before, installing MetaMask will automatically change the default wallet to MetaMask.
  • If Crypto Wallets is not installed, navigating to brave://wallet will ask the user to install Crypto Wallets
  • Navigating to brave://wallet will load the Crypto Wallets UI (pre extension install and ask the user to opt in)

When brave://flags/#native-brave-wallet is enabled:

  • Brave Wallet is the default for new installs
  • There will be no Ask option for the default wallet anymore
  • As before, installing MetaMask will automatically change the default wallet to MetaMask.
  • User will be given the option in the new wallet UI to switch back to Crypto Wallets

If the default wallet is deprecated Crypto Wallets extension:

  • Hamburger menu for Wallet opens up Crpyto Wallets
  • window.ethereum is provided by Crypto Wallets
  • Content scripts on MetaMask are disabled.
  • brave://wallet resolves to the Crypto Wallets UI
  • *Note: If the wallet starts off as Brave Wallet, and then the user only goes into settings to change it to Crypto Wallets. It will not work with dapps until you go to brave://wallet at least once (and install if it hasn't been installed yet). I.e. a settings change doesn't currently actively load the wallet.

If the default wallet is None or MetaMask:

  • Hamburger menu for Wallet opens up the new Brave Wallet.
  • window.ethereum is not provided by Brave Wallet
  • Content scripts on Crypto Wallets are disabled.
  • Crypto Wallets cannot be loaded unless it is already loaded

If the default wallet is Brave Wallet:

  • Hamburger menu for Wallet opens up the new Brave Wallet
  • window.ethereum is provided by Brave Wallet and content scripts are disabled for MetaMask.
  • Content scripts on MetaMask and Crypto Wallets are disabled. The only way to have Crypto Wallets actually running here is to have changed the setting within a restart.

QA note:

When testing changes to default wallet app, close the active tab and open a new one to check which window.ethereum is available. I usually test with https://www.cryptokitties.co/ and click the Start button to see which wallet is running. You sometimes need to wait a few seconds too.

@srirambv
Copy link
Contributor

srirambv commented Oct 6, 2021

Verification passed on

Brave 1.31.71 Chromium: 94.0.4606.71 (Official Build) beta (64-bit)
Revision 1d32b169326531e600d836bd395efc1b53d0f6ef-refs/branch-heads/4606@{#1256}
OS Windows 11 Version Dev (Build 22471.1000)
Verified test plan from issue description
  • Verified settings is updated from Ethereum provider for Dapps to Default cryptocurrency wallet
  • Verified Dapp detection for Crypto Wallets (deprecated extension) is removed
  • Verified lazy loading of Crypto Wallets (deprecated extension) is removed
  • Verified Load Crypto Wallets on startup option is also removed
    image

When brave://flags/#native-brave-wallet is disabled:

  • Verified when native wallet flag is set to default/disabled, default provider is set to Ask
  • Verified installing MM automatically sets it as default Dapp provider
  • Verified when Crypto Wallet Extension is installed and flag is set to default/disabled, Crypto Wallet is set as default Dapp provider
  • Verified with flag disabled, visiting brave://wallet loads the CW Extension UI and installs it when the user opts-in

When brave://flags/#native-brave-wallet is enabled:

  • Verified Brave Wallet is default when flag is enabled
  • Verified no info bar is shown to setup or choose provider or enable Wallet similar to CW Extensions
  • Verified enabling Brave Wallet and then installing MM, sets MM as default Dapp provider
  • Verified with Brave Wallet enabled via flag, settings still allows to switch to CW Extension as default and install the component via brave://wallet

When default wallet is deprecated Crypto Wallets extension:

  • Verified clicking on wallet in hamburger menu opens brave://wallet
  • Verified brave://components load the latest released version on Crypto Wallet
  • Verified window.ethereum returns promise and has the Wallet address linked to the account from CW
  • Verified content scripts from MM are disabled
  • Verified even with flag enabled for native wallet, setting CW extension as default, visiting brave://wallet in a new tab loads CW UI

When default wallet is None or MetaMask:

  • Verified clicking on wallet in hamburger menu opens brave://wallet/crypto/portfolio
  • Verified window.ethereum returns undefined when default wallet is set to None
  • Verified window.ethereum returns a promise when MM is set as default
  • Verified content scripts from Crypto Wallet are disabled
  • Verified Crypto Wallets can be loaded only when the extension is already set as default and loaded before changing the setting

When default wallet is Brave Wallet:

  • Verified clicking on wallet in hamburger menu opens brave://wallet/crypto/onboarding if not on-boarded
  • Verified clicking on wallet in hamburger menu opens brave://wallet/crypto/portfolio if already on-boarded/wallet created
  • Verified content scripts from MM/CW is not injected when Brave Wallet is set as default. Closing tab and reopening a new one worked

Verification passed on

Brave 1.31.71 Chromium: 94.0.4606.71 (Official Build) beta (64-bit)
Revision 1d32b169326531e600d836bd395efc1b53d0f6ef-refs/branch-heads/4606@{#1256}
OS Linux
Verified test plan from issue description
  • Verified settings is updated from Ethereum provider for Dapps to Default cryptocurrency wallet
  • Verified Dapp detection for Crypto Wallets (deprecated extension) is removed
  • Verified lazy loading of Crypto Wallets (deprecated extension) is removed
  • Verified Load Crypto Wallets on startup option is also removed
    image

When brave://flags/#native-brave-wallet is disabled:

  • Verified when native wallet flag is set to default/disabled, default provider is set to Ask
  • Verified installing MM automatically sets it as default Dapp provider
  • Verified when Crypto Wallet Extension is installed and flag is set to default/disabled, Crypto Wallet is set as default Dapp provider
  • Verified with flag disabled, visiting brave://wallet loads the CW Extension UI and installs it when the user opts-in

When brave://flags/#native-brave-wallet is enabled:

  • Verified Brave Wallet is default when flag is enabled
  • Verified no info bar is shown to setup or choose provider or enable Wallet similar to CW Extensions
  • Verified enabling Brave Wallet and then installing MM, sets MM as default Dapp provider
  • Verified with Brave Wallet enabled via flag, settings still allows to switch to CW Extension as default and install the component via brave://wallet

When default wallet is deprecated Crypto Wallets extension:

  • Verified clicking on wallet in hamburger menu opens brave://wallet
  • Verified brave://components load the latest released version on Crypto Wallet
  • Verified window.ethereum returns promise and has the Wallet address linked to the account from CW
  • Verified content scripts from MM are disabled
  • Verified even with flag enabled for native wallet, setting CW extension as default, visiting brave://wallet in a new tab loads CW UI

When default wallet is None or MetaMask:

  • Verified clicking on wallet in hamburger menu opens brave://wallet/crypto/portfolio
  • Verified window.ethereum returns undefined when default wallet is set to None
  • Verified window.ethereum returns a promise when MM is set as default
  • Verified content scripts from Crypto Wallet are disabled
  • Verified Crypto Wallets can be loaded only when the extension is already set as default and loaded before changing the setting

When default wallet is Brave Wallet:

  • Verified clicking on wallet in hamburger menu opens brave://wallet/crypto/onboarding if not on-boarded
  • Verified clicking on wallet in hamburger menu opens brave://wallet/crypto/portfolio if already on-boarded/wallet created
  • Verified content scripts from MM/CW is not injected when Brave Wallet is set as default. Closing tab and reopening a new one worked

Verification passed on

Brave 1.31.71 Chromium: 94.0.4606.71 (Official Build) beta (x86_64)
Revision 1d32b169326531e600d836bd395efc1b53d0f6ef-refs/branch-heads/4606@{#1256}
OS macOS Version 11.5.2 (Build 20G95)
Verified test plan from issue description
  • Verified settings is updated from Ethereum provider for Dapps to Default cryptocurrency wallet
  • Verified Dapp detection for Crypto Wallets (deprecated extension) is removed
  • Verified lazy loading of Crypto Wallets (deprecated extension) is removed
  • Verified Load Crypto Wallets on startup option is also removed
    image

When brave://flags/#native-brave-wallet is disabled:

  • Verified when native wallet flag is set to default/disabled, default provider is set to Ask
  • Verified installing MM automatically sets it as default Dapp provider
  • Verified when Crypto Wallet Extension is installed and flag is set to default/disabled, Crypto Wallet is set as default Dapp provider
  • Verified with flag disabled, visiting brave://wallet loads the CW Extension UI and installs it when the user opts-in

When brave://flags/#native-brave-wallet is enabled:

  • Verified Brave Wallet is default when flag is enabled
  • Verified no info bar is shown to setup or choose provider or enable Wallet similar to CW Extensions
  • Verified enabling Brave Wallet and then installing MM, sets MM as default Dapp provider
  • Verified with Brave Wallet enabled via flag, settings still allows to switch to CW Extension as default and install the component via brave://wallet

When default wallet is deprecated Crypto Wallets extension:

  • Verified clicking on wallet in hamburger menu opens brave://wallet
  • Verified brave://components load the latest released version on Crypto Wallet
  • Verified window.ethereum returns promise and has the Wallet address linked to the account from CW
  • Verified content scripts from MM are disabled
  • Verified even with flag enabled for native wallet, setting CW extension as default, visiting brave://wallet in a new tab loads CW UI

When default wallet is None or MetaMask:

  • Verified clicking on wallet in hamburger menu opens brave://wallet/crypto/portfolio
  • Verified window.ethereum returns undefined when default wallet is set to None
  • Verified window.ethereum returns a promise when MM is set as default
  • Verified content scripts from Crypto Wallet are disabled
  • Verified Crypto Wallets can be loaded only when the extension is already set as default and loaded before changing the setting

When default wallet is Brave Wallet:

  • Verified clicking on wallet in hamburger menu opens brave://wallet/crypto/onboarding if not on-boarded
  • Verified clicking on wallet in hamburger menu opens brave://wallet/crypto/portfolio if already on-boarded/wallet created
  • Verified content scripts from MM/CW is not injected when Brave Wallet is set as default. Closing tab and reopening a new one worked

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