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

MM and Crypto wallet breaks web3 sites #6301

Closed
srirambv opened this issue Oct 3, 2019 · 8 comments
Closed

MM and Crypto wallet breaks web3 sites #6301

srirambv opened this issue Oct 3, 2019 · 8 comments
Labels

Comments

@srirambv
Copy link
Contributor

srirambv commented Oct 3, 2019

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

  1. Clean profile on 0.69.132
  2. Enable Crypto Wallet
  3. Install MM from webstore
  4. Open a new tab and load cryptokitties.co
  5. 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

@srirambv srirambv added bug priority/P3 The next thing for us to work on. It'll ride the trains. QA/Yes release-notes/exclude feature/web3/wallet Integrating Ethereum+ wallet support labels Oct 3, 2019
@dpazdan
Copy link

dpazdan commented Oct 4, 2019

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
Copy link
Contributor Author

srirambv commented Oct 7, 2019

@Brave-Matt
Copy link

@balresch
Copy link

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
Copy link

dpazdan commented Nov 12, 2019

@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
Copy link

ppeinsold commented Nov 25, 2019

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
Copy link

jcrain commented Dec 12, 2019

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, MetaMask/metamask-extension@5ce94e6

@bbondy
Copy link
Member

bbondy commented Dec 20, 2019

This will be fixed with the implementation of #7503 which is coming soon. Closing this issue in favor of that.

@bbondy bbondy closed this as completed Dec 20, 2019
@bbondy bbondy added this to the Closed / Invalid milestone Jun 3, 2020
@srirambv srirambv added feature/ethereum-remote-client and removed feature/web3/wallet Integrating Ethereum+ wallet support labels Sep 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants