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

InjectedConnector activation opening MetaMask and Coinbase Wallet pop-ups when the user has both extensions #300

Closed
tomaszbakula opened this issue Oct 21, 2021 · 22 comments
Labels
bug Something isn't working

Comments

@tomaszbakula
Copy link

Bug Description
Activation of the InjectedConnector results in displaying pop-ups from both wallets at the same time when the user has both extensions installed. When we use WalletLinkConnector only the Coinbase Wallet is activated.

Reproduction
CodeSandbox reproduction

Steps to reproduce:

  1. Install MetaMask and Coinbase Wallet chrome extensions.
  2. Open the CodeSandbox example and then run the code in a new tab (when we try it within the CodeSandbox editor it seems to work properly) - direct link.
  3. Click the "Connect Wallet" button.

Expected Behavior
Only the MetaMask wallet popup should be displayed when we use InjectedConnector.

Additional Context
Using Google Chrome 94.0.4606.81 on Mac

@tomaszbakula tomaszbakula added the bug Something isn't working label Oct 21, 2021
@sambacha
Copy link
Contributor

This is a known issue for a long time with respect to Injected providers. This is why people use a modal screen so that users can select which wallet instead of automatically trying to connect to a wallet provider.

Here are two popular solutions:

@tomaszbakula
Copy link
Author

@sambacha thank you for your reply.

Hmm, I'm not trying to automatically connect to a wallet provider. I would rather like to add two separate buttons in my app one for MetaMask (using InjectedConnector) and the other one for Coinbase (using WalletLinkConnector) and connect only after the user clicks in one of them. But from what I understand it is currently not possible because of the issue with respect to Injected providers.

@sambacha
Copy link
Contributor

sambacha commented Nov 6, 2021

@tomaszbakula - if this is a fresh browser instance (i.e. user has not logged into either one), I believe the coinbase extension would most likely have priority as it does not require you to login to active its web3 engine (it uses the QR code for 2FA to establish walletlink) vs metamask needing user login first.

Walletconnect lets you define a parameters for 'favoured wallet' for providing a choice in their web3modal example

@ritvij14
Copy link

@tomaszbakula did you find any solution to this? I am in the same situation and in my dapp I need to give users choice to either connect to Metamask extension or Coinbase wallet extension.

Since injected connector is the only one that supports connection for browser wallet extensions, I think there should be some way to sort this out and give the user a choice between multiple browser wallet extensions, right?

@tomaszbakula
Copy link
Author

@ritvij14 - Unfortunately, I didn't manage to find a solution to this problem.

@sambacha - Switching to web3modal won't solve it cause the package seems to have the same issue. When we try to connect the MetaMask wallet and have both extensions installed, then two modals popups. Please, have a look at the attached screenshot.

Screenshot 2021-11-15 at 23 22 39

I am not sure how the InjectedConnector works under the hood, but as @ritvij14 said, there probably should be some way to specify which wallet we want to connect.

@mattcasey
Copy link

mattcasey commented Dec 8, 2021

I am working on this problem myself, and it unfortunately doesn't seem like something that can be fix on the developer side. These extensions work by following a standard set of protocols which were designed for a case where only a single wallet is installed. You essentially have two apps listening to the same events, and the protocol doesn't supply a means to specify which one should respond.

One thing I'd like to see in web3-react would be support for multiple injected providers. I discovered that when you have more than one, they are all accessible under window.ethereum.providers, and the Metamask one has a special property isMetaMask. This allows me to at least make sure I'm reading from Metamask in case someone also has Coinbase also installed.

@japhex
Copy link

japhex commented Dec 10, 2021

I've just encountered this issue today myself...I'm guessing this would also be the case for any browser extension based wallets? Its only when I added coinbase that the issue surfaced. I guess what would be great is passing the type into the injected called:

activate(injected('MetaMask')) activate(injected('CoinBase'))

@hieronymus777
Copy link

hieronymus777 commented Dec 13, 2021

Hi, I can talk about this a bit.

Most dapps look for the window.ethereum object as the 'injected' provider.

When we created the Coinbase Wallet extension, we needed a way to support users having both Coinbase Wallet extension and Metamask installed at the same time - without updating dapps and without making the user enable / disable one of the extensions each time they use a dapp.

The tl;dr is that the multiple popups actually do give the user a choice - whichever wallet they approve is the one that is selected as the ethereum provider.

The details:
In the case where the user has both Coinbase Wallet extension and Metamask installed, the current approach is to inject a 'multi' provider at window.ethereum that has the following logic:

  1. It keeps a list of providers if it discovers more than one is being registered at window.ethereum. (i.e. [Coinbase wallet extension, Metamask].
  2. If one of those providers is already connected (window.ethereum.selectedAddress is defined), it selects that provider
  3. When the dapp requests ethereum accounts (eip1102), it will initiate the eip1102 request on all providers it has registered (hence the multiple popups).
  4. Whichever one the user approves first, becomes the selected provider.
  5. It also implements the isMetaMask method, which returns true if one of the providers has isMetaMask set to true (this is necessary since some dapps will not work if window.ethereum has isMetaMask set to false).
  6. For chainId it returns 0x1 unless there is a selected provider (2 or 4 above) in which case it returns that selected provider's chain id.

This is one solution to the problem of having a single window.ethereum interface. The benefit is mainly that dapps continue to work without being aware that there are multiple wallets at window.ethereum.

I'd be curious to understand @mattcasey what the use case is where you only want to support Metamask at window.ethereum?

Open to suggestions on better solutions. One thought:

  1. web3 react & web3 modal can look for window.ethereum.providers and list them out in the UI.
  2. We can expose a method on window.ethereum multi provider like selectProvider that accepts the result of what the user selects in the web3 react UI.

@tomaszbakula
Copy link
Author

@hieronymus777 Thank you for the detailed description of how things work. It was really helpful.

In my case, where I have separate buttons, one to connect to MetaMask and another one to connect CoinBase Wallet, I've managed to fix multiple popups issue by setting window.ethereum.selectedProvider just before activating the selected connector.

export function activateInjectedProvider(providerName: 'MetaMask' | 'CoinBase') {
    const { ethereum } = window;

    if (!ethereum?.providers) {
        return undefined;
    }

    let provider;
    switch (providerName) {
        case 'CoinBase':
            provider = ethereum.providers.find(({ isCoinbaseWallet }) => isCoinbaseWallet);
            break;
        case 'MetaMask':
            provider = ethereum.providers.find(({ isMetaMask }) => isMetaMask);
            break;
    }

    if (provider) {
        ethereum.setSelectedProvider(provider);
    }
}

activateInjectedProvider('MetaMask');
activate(injected);

As @japhex mentioned it would be great if we could somehow pass the desired provider as a parameter to the injected connector.

@NoahZinsmeister
Copy link
Contributor

hi! library author here, sorry for the (extremely) slow responses; for an update on the state of this library, please see #163 (comment)

TLDR: v6 is deprecated and going into maintenance mode. i expect not to make any non-security related changes going forward (however, you're always welcome to! the beauty of open source)

i'll be closing a lot of issues with this message right now, but please feel free to re-open if you think the topic is still relevant!

thanks for using web3-react <3

@Adamj1232
Copy link

Hi, I can talk about this a bit.

Most dapps look for the window.ethereum object as the 'injected' provider.

When we created the Coinbase Wallet extension, we needed a way to support users having both Coinbase Wallet extension and Metamask installed at the same time - without updating dapps and without making the user enable / disable one of the extensions each time they use a dapp.

The tl;dr is that the multiple popups actually do give the user a choice - whichever wallet they approve is the one that is selected as the ethereum provider.

The details: In the case where the user has both Coinbase Wallet extension and Metamask installed, the current approach is to inject a 'multi' provider at window.ethereum that has the following logic:

1. It keeps a list of providers if it discovers more than one is being registered at `window.ethereum`. (i.e. [`Coinbase wallet extension`, `Metamask`].

2. If one of those providers is already connected (`window.ethereum.selectedAddress` is defined), it selects that provider

3. When the dapp requests ethereum accounts (eip1102), it will initiate the eip1102 request on all providers it has registered (hence the multiple popups).

4. Whichever one the user approves first, becomes the selected provider.

5. It also implements the `isMetaMask` method, which returns true if one of the providers has `isMetaMask` set to true (this is necessary since some dapps will not work if `window.ethereum` has `isMetaMask` set to false).

6. For `chainId` it returns `0x1` unless there is a selected provider (2 or 4 above) in which case it returns that selected provider's chain id.

This is one solution to the problem of having a single window.ethereum interface. The benefit is mainly that dapps continue to work without being aware that there are multiple wallets at window.ethereum.

I'd be curious to understand @mattcasey what the use case is where you only want to support Metamask at window.ethereum?

Open to suggestions on better solutions. One thought:

1. web3 react & web3 modal can look for `window.ethereum.providers` and list them out in the UI.

2. We can expose a method on `window.ethereum` multi provider like `selectProvider` that accepts the result of what the user selects in the web3 react UI.

@tomaszbakula thank you very much for the response! Can you comment on if this copying of a provider into the provider.providers array happens with any other injected wallets aside from MetaMask? I am only seeing this with MM currently. Thank you!

@tomaszbakula
Copy link
Author

@Adamj1232 - I'm not sure cause I've tested this only with MM and Coinbase Wallet.

@Nir-Cohen
Copy link

Any progress with this?

@sambacha
Copy link
Contributor

sambacha commented May 9, 2022

Hi, I can talk about this a bit.

Most dapps look for the window.ethereum object as the 'injected' provider.

When we created the Coinbase Wallet extension, we needed a way to support users having both Coinbase Wallet extension and Metamask installed at the same time - without updating dapps and without making the user enable / disable one of the extensions each time they use a dapp.

The tl;dr is that the multiple popups actually do give the user a choice - whichever wallet they approve is the one that is selected as the ethereum provider.

The details: In the case where the user has both Coinbase Wallet extension and Metamask installed, the current approach is to inject a 'multi' provider at window.ethereum that has the following logic:

1. It keeps a list of providers if it discovers more than one is being registered at `window.ethereum`. (i.e. [`Coinbase wallet extension`, `Metamask`].
2. If one of those providers is already connected (`window.ethereum.selectedAddress` is defined), it selects that provider
3. When the dapp requests ethereum accounts (eip1102), it will initiate the eip1102 request on all providers it has registered (hence the multiple popups).
4. Whichever one the user approves first, becomes the selected provider.
5. It also implements the `isMetaMask` method, which returns true if one of the providers has `isMetaMask` set to true (this is necessary since some dapps will not work if `window.ethereum` has `isMetaMask` set to false).
6. For `chainId` it returns `0x1` unless there is a selected provider (2 or 4 above) in which case it returns that selected provider's chain id.

This is one solution to the problem of having a single window.ethereum interface. The benefit is mainly that dapps continue to work without being aware that there are multiple wallets at window.ethereum.

I'd be curious to understand @mattcasey what the use case is where you only want to support Metamask at window.ethereum?

Open to suggestions on better solutions. One thought:

1. web3 react & web3 modal can look for `window.ethereum.providers` and list them out in the UI.
2. We can expose a method on `window.ethereum` multi provider like `selectProvider` that accepts the result of what the user selects in the web3 react UI.

@tomaszbakula thank you very much for the response! Can you comment on if this copying of a provider into the provider.providers array happens with any other injected wallets aside from MetaMask? I am only seeing this with MM currently. Thank you!

They give them a choice in the sense that they get to choose their own race condition.

Use case for only supporting metamask: using eth_sign which you no longer support and alot of dapps are starting to have to use.

Brave and another wallet also do this same behavior. Here is how wagmi (web3 react hook library) handles this via s shim:

wevm/wagmi@632f63a

@Adamj1232
Copy link

This behavior is handled and supported with Blocknative's Onboard solution here and here Very slick UX/UI and actively supported and in development with active dev support through discord

@eth-skywalker
Copy link

@Adamj1232 any plans to add documentation on how to use Onboard with popular hooks libs like useDapp, wagmi, etc. ?

@Adamj1232
Copy link

Adamj1232 commented May 10, 2022

@eth-skywalker what kind of hooks do you use most from those libs?
Blocknative provides a react hooks library and makes interacting with a wallet provider straight forward and simple.
Blocknative also supports many hardware wallets along with injected and SDK wallets.
Only add the wallet packages you want to support and keep your DApp light and fast!
React lib
Vanilla JS core lib

@eth-skywalker
Copy link

@Adamj1232 ah so not rly looking to migrate libraries entirely. more just to resolve the issue noted on this thread where MM / CB wallets both pop up, but still using web3-react. re: your question, some common hooks we use in useDapp are hooks like useEthers, useContractFunction, useCall

@Adamj1232
Copy link

@eth-skywalker ahh gotcha, yeah web3-onboard would be a replacement for those. To that point might be worth considering if you look at active development and maintenance of the different packages and who they are backed by.

@TroubleSeven
Copy link

for me, my conflict extension are BitKeep and MetaMask. When i use window.ethereum.providers it's come out undefined. but i solved the problem with this way . Hope its can help sb.
if (!window?.bitkeep) return undefined window.ethereum = window.bitkeep?.ethereum activate(injected);

@Adamj1232
Copy link

@TroubleSeven window.ethereum.providers is a specific pattern to Coinbase wallet although I hope other wallets use a similar pattern. That is to say with Coinbase wallet the dev/users has access to other wallets injected at window.ethereum
Web3-Onboard handles this natively and allows multiple wallets to connect to a site.

dmitry-yudakov added a commit to momentum-xyz/ui-client that referenced this issue Apr 10, 2023
@moondayli
Copy link

@hieronymus777 Thank you for the detailed description of how things work. It was really helpful.

In my case, where I have separate buttons, one to connect to MetaMask and another one to connect CoinBase Wallet, I've managed to fix multiple popups issue by setting window.ethereum.selectedProvider just before activating the selected connector.

export function activateInjectedProvider(providerName: 'MetaMask' | 'CoinBase') {
    const { ethereum } = window;

    if (!ethereum?.providers) {
        return undefined;
    }

    let provider;
    switch (providerName) {
        case 'CoinBase':
            provider = ethereum.providers.find(({ isCoinbaseWallet }) => isCoinbaseWallet);
            break;
        case 'MetaMask':
            provider = ethereum.providers.find(({ isMetaMask }) => isMetaMask);
            break;
    }

    if (provider) {
        ethereum.setSelectedProvider(provider);
    }
}

activateInjectedProvider('MetaMask');
activate(injected);

As @japhex mentioned it would be great if we could somehow pass the desired provider as a parameter to the injected connector.

This works perfectly fine on other wallets except from phantom wallet someone they're able to override that code and always popup all the time I've been trying to maybe look for the phantom wallet in window and disable it before connecting any of the other injected web3 provider but I can't seem to find a way

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests