Fix network switch bug for generic EIP-1193 providers

[everdimension]

A EIP-1193 spec does not describe an "isConnected()" method, so if this method is not found on the provider, the dapp should not assume it is not connected.

Instead, we can only perform the check if the method is found.

This fixed a bug when the UI goes into a infinite cycle of switching
the network back and forth.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
web3-react	✅ Ready (Inspect)	Visit Preview	Aug 10, 2022 at 10:52AM (UTC)

[zzmp]

The file you've updated is specific to MetaMask, not generic EIP-1193 providers.
Can you describe how to reproduce your issue? If you're able, can you include a test that would fail without your changes?

[everdimension]

Hi @zzmp! Sorry for a delayed response

The file you've updated is specific to MetaMask

Kinda not true since uniswap's interface uses it for all injected wallets, not necessarily metamask

Can you describe how to reproduce your issue?

To reproduce, you need to inject an ethereum provider without an "isConnected" method

Without these changes, the UI goes into an infinite cycle of switching the network back and forth

[everdimension]

Did you have time to take a look by any chance?

[everdimension]

Hi! Any updates?

[grabbou]

Hey!

While your PR makes sense, I just wanted to point out that this is not EIP-1193 provider, but MetaMask provider, so it is expected to work with MetaMask. If there's a dapp that mimics MetaMask by injecting window.ethereum and faking isMetaMask but not providing full API, it's on them to fix it.

The provider that Zach was referring to is an EIP1193 package that has EIP1193 compatible provider and I guess that one does not rely on out-of-spec methods.

Lastly, the code you're updating is confusing to me as I don't see:

if this method is not found on the provider, the dapp should not assume it is not connected.
being utilised in the code.

We call the rest of the code (initialisation) regardless of whether that method was present or not. The fact that we conditionally call startActivation action (but always run activation anyway) sounds like a bug to me. @zzmp do you have any context here?

[everdimension]

If there's a dapp that mimics MetaMask by injecting window.ethereum and faking isMetaMask but not providing full API, it's on them to fix it.

That would make sense, but I faced this by using a provider that does not mimic the .isMetaMask property. Meaning, the code in question is used to handle generic EIP-1193 providers.

In any case, you can reproduce it on your side if you wish to support EIP-1193 specification and not only metamask.

We call the rest of the code (initialisation) regardless of whether that method was present or not. The fact that we conditionally call startActivation action (but always run activation anyway) sounds like a bug to me

The problem with the original code is that it confuses missing isConnected() method with a method that exists, but returns false. This is a logical error mostly, and it leads to a dead end in your UI.