MM and Crypto wallet breaks web3 sites

srirambv commented 5 years ago

Description

As a consequence of #6181, if you install MM and enable crypto wallet on browser, web3 sites like cryptokitties fail to load

Steps to Reproduce

Clean profile on 0.69.132
Enable Crypto Wallet
Install MM from webstore
Open a new tab and load cryptokitties.co
Open dev tools for the page you can see MM conflicts with another crypto wallet

Actual result:

Sites fail to load due to 2 crypto wallet being enabled. The workaround is to either uninstall MM or disable browser crypto wallet

Expected result:

Should prompt to disable the extension and continue using browser crypto wallet

Reproduces how often:

Easy

Brave version (brave://version info)

Brave	0.69.132 Chromium: 77.0.3865.90 (Official Build) (64-bit)
Revision	58c425ba843df2918d9d4b409331972646c393dd-refs/branch-heads/3865@{#830}
OS	Linux

Version/Channel Information:

Can you reproduce this issue with the current release? Yes
Can you reproduce this issue with the beta channel? Yes
Can you reproduce this issue with the dev channel? Yes
Can you reproduce this issue with the nightly channel? Yes

Other Additional Information:

Does the issue resolve itself when disabling Brave Shields? N/A
Does the issue resolve itself when disabling Brave Rewards? N/A
Is the issue reproducible on the latest version of Chrome? N/A

Miscellaneous Information:

cc: @bbondy

dpazdan commented 5 years ago

Expected result: Should prompt to disable the extension and continue using browser crypto wallet

Maybe users can have a choice to pick their default, and/or select on each site (if they want to use multiple solutions).

srirambv commented 5 years ago

+1 from https://community.brave.com/t/crypto-wallet-conflicts-with-metamask/86427/2

Brave-Matt commented 5 years ago

+1 from reddit: https://www.reddit.com/r/brave_browser/comments/df8fii/opening_brave_wallet_on_mac_os_breaks_metamask/

balresch commented 4 years ago

I have issues using MM even when the brave crypto wallet is disabled. I'd love it if Metamask could still be used (without the need to re-import the key into the Brave crypto wallet).

dpazdan commented 4 years ago

@balresch after disabling Brave wallet, make sure to also close and reopen your browser. If you do not do this step, MetaMask will not function properly. Thanks!

ppeinsold commented 4 years ago

Let me add a little bit of a code example. The following code is to connect to web3 browser using web3js library:

        // Check for injected web3 (mist/metamask)             
        // Modern dapp browsers...
        let web3Provider = null;
        if (window.ethereum) {
            web3Provider = window.ethereum;
            try {
                // Request account access
                await window.ethereum.enable();
            } catch (error) {
                // User denied account access...
                console.error("User denied account access to metamask");
                return;
            }
        }
        // Legacy dapp browsers...
        else if (window.web3) {
            web3Provider = window.web3.currentProvider;
        }
        else {
            console.error("Unable to connect to metamask");
            return;
        }

        let web3 = new Web3(web3Provider);
        ...

This works fine for all browsers but in brave it crashes when "crypto wallets" is enabled. The problem here is, that a check is missing. On the line where I have " console.error("Unable to connect to metamask");" I need to detect "crypto wallets" because I end up here if only "crypto wallets" is installed. Does anyone know how to make this check?

jcrain commented 4 years ago

Also experiencing this. I'd love to be able to prompt the user. This error gets thrown in the console but I haven't found a good way to handle it yet, https://github.com/MetaMask/metamask-extension/commit/5ce94e69b311428d9d2f9b5502c02d3b960e380e

bbondy commented 4 years ago

This will be fixed with the implementation of https://github.com/brave/brave-browser/issues/7503 which is coming soon. Closing this issue in favor of that.

brave / brave-browser