wevm / references

Collection of Chains & Connectors for wagmi
MIT License
97 stars 204 forks source link

bug: core package `getNetwork` and `switchNetwork` not working with WalletConnect #393

Closed zdenham closed 1 year ago

zdenham commented 1 year ago

Is there an existing issue for this?

Package Version

"@wagmi/core" "1.3.0"

Current Behavior

const chains = [goerli]
    const { provider: wagmiProvider } = configureChains(chains, [
      infuraProvider({
        apiKey: 'redacted'
      }),
      w3mProvider({ projectId: this.walletConnectProjectId })
    ])

    const wagmiClient = createClient({
      autoConnect: true,
      connectors: w3mConnectors({
        projectId: this.walletConnectProjectId,
        version: 2,
        chains
      }),
      provider: wagmiProvider,
      chains
    })

Expected Behavior

Steps To Reproduce

See above

Link to Minimal Reproducible Example (StackBlitz, CodeSandbox, GitHub repo etc.)

No response

Anything else?

We are using the w3mConnectors with infura fallback provider for reads

siosio34 commented 1 year ago

This error seems to happen to me too. Chain switching works well in the desktop-Metamask browser, but chain switching does not work in mobile when using Wallet Connect v2 and Metamask mobile wagmi.

glitch-txs commented 1 year ago

WalletConnect v2 is multichain, you connect to multiple chains at once so you don't actually need to accept a "switch request" because you're already connected. By switching a chain what you are actually doing is changing the default chain, which is the one that's going to be used when triggering a transaction. This is handled on the dapp side, once the wallet accepts a session request then it doesn't need to do more on its side.

Hi-Nemo commented 1 year ago

@zdenham In new version it is createConfig and not createClient. While creating client you are using connectors key. It shoulbe an Array format like below

const wagmiClient = createConfig({
      autoConnect: true,
      connectors: [ w3mConnectors({
        projectId: this.walletConnectProjectId,
        version: 2,
        chains
      })],
      publicClient,
      webSocketPublicClient,
    })
0xvidejk commented 1 year ago

Then how to handle this situation? for example: selected network is binance, user clicks to send transaction in dapp => metamask is opened (selected network is ethereum) => no pop-up to confirm transaction. if the user selects right network manually everything works well

glitch-txs commented 1 year ago

Then I guess this is an issue on MetaMask wallet side, it should popup the tx request in the default chain automatically

zdenham commented 1 year ago

WalletConnect v2 is multichain, you connect to multiple chains at once so you don't actually need to accept a "switch request" because you're already connected. By switching a chain what you are actually doing is changing the default chain, which is the one that's going to be used when triggering a transaction. This is handled on the dapp side, once the wallet accepts a session request then it doesn't need to do more on its side.

The issue is that when you initiate a transaction on the wrong chain, nothing happens. No error is thrown, no prompt to sign the txn on the user's behalf, just radio silence.

If WalletConnect does not intend to support switchChain capability, then I believe wagmi (or the wallet connect connector) should:

a. Return the current network the user is connected with via wallet connect on getNetwork, not the default network b. Throw with a reasonable error message if a transaction is initiated while user is connected to the wrong chain c. Throw if switchNetwork is called while the user is connected via wallet connect (because the function doesn't work)

glitch-txs commented 1 year ago

Switching chain is supported but has a different behavior and it SHOULD popup the tx. I assume you're testing with MetaMask? I saw this issue somewhere else, in said case it needs to be fixed in the MM side.

zdenham commented 1 year ago

I see, maybe this is the issue?

https://github.com/MetaMask/metamask-mobile/issues/6655

glitch-txs commented 1 year ago

Could be, did you try updating deps?

zdenham commented 1 year ago

Same behavior after upgrading to the following:

  "@wagmi/core": "^0.10.16",
  "@web3modal/ethereum": "^2.6.1",
  "@web3modal/html": "^2.6.1",
"@walletconnect/ethereum-provider" "2.8.4"
zdenham commented 1 year ago

@zdenham In new version it is createConfig and not createClient. While creating client you are using connectors key. It shoulbe an Array format like below

const wagmiClient = createConfig({
      autoConnect: true,
      connectors: [ w3mConnectors({
        projectId: this.walletConnectProjectId,
        version: 2,
        chains
      })],
      publicClient,
      webSocketPublicClient,
    })

@Hi-Nemo when was this API introduced? createConfig is not exported on latest version of core package for me

tmm commented 1 year ago

@zdenham check what version you are using. it's definitely exported on the latest wagmi.

Hi-Nemo commented 1 year ago

@zdenham I am using "@wagmi/core": "^1.2.0", You will get it here

import { configureChains, createConfig, mainnet, ...... } from '@wagmi/core'

zdenham commented 1 year ago

Ah my bad, TIL using yarn upgrade only seems to update you to the latest minor version, will see if this resolves my issues...

luisdeka commented 1 year ago

Same error, switchNetwork didnt popup on walllet app connected, i have temporaly fixed it popping it out of the JS flow with a timeout, then switchNetwork popups on wallet app, the problem is after user accept change network, the promise didnt resolve...

I will wait the fix for remove all my temporal shit code for fix the problem.

0xvidejk commented 1 year ago

Same error, switchNetwork didnt popup on walllet app connected, i have temporaly fixed it popping it out of the JS flow with a timeout, then switchNetwork popups on wallet app, the problem is after user accept change network, the promise didnt resolver...

I will wait the fix for remove all my temporal shit code for fix the problem.

Could you please share the code?

zdenham commented 1 year ago

Migrating to viem is not really an option for our project at this point, so may be stuck with this issue if it is in fact fixed after 1.0.

tmm commented 1 year ago

@zdenham we've been adding fixes to pre-1.0 to the legacy-v0 tag so expect to see something there.

siosio34 commented 1 year ago

@zdenham Have you ever solved this error?

Did the code below help?

const wagmiClient = createConfig({
      autoConnect: true,
      connectors: [ w3mConnectors({
        projectId: this.walletConnectProjectId,
        version: 2,
        chains
      })],
      publicClient,
      webSocketPublicClient,
    })
zdenham commented 1 year ago

Did the code below help?

This code is from wagmi ^1.0 which has viem peer dependency. For our project is not feasible to switch to in the near term

jingxuan98 commented 1 year ago

still having this issue any fix?

When calling the switchNetwork() in wagmi, the walletClient, signer and network on my dapp switched to the correct network but not my metamask mobile (no-popup and stay at the previous network page), then when try to proceed with a contract interaction theres no pop-up as well, but everything works fine after switching to the correct network manually in metamask mobile

zdenham commented 1 year ago

I have a feeling the issue is either w/ wallet connect connector or MM mobile itself. But no resolution so far for me

luisdeka commented 1 year ago

Same error, switchNetwork didnt popup on walllet app connected, i have temporaly fixed it popping it out of the JS flow with a timeout, then switchNetwork popups on wallet app, the problem is after user accept change network, the promise didnt resolver... I will wait the fix for remove all my temporal shit code for fix the problem.

Could you please share the code?

Something like that, using wagmi + viem

// Bad temporal fix because when doing this the promise never resolves after user switchNetwork (Bug cascade), so you have to watch during X period of time to see if user has changed the network, 3500ms for secure switch, with low ms sometimes it works and sometimes it doesn't
setTimeout(()=> switchNetwork({ chainId }).then(checkIfChangedAndContinueProcress), 3500)

I think the better approach for now is to give a nice button to the user for retry the "switchNetwork" manually

siosio34 commented 1 year ago

@luisdeka If I apply this code, can I change the chain within WalletConnect?

luisdeka commented 1 year ago

@luisdeka If I apply this code, can I change the chain within WalletConnect?

for what i have test, yes

siosio34 commented 1 year ago

thanks @luisdeka I'm really sorry but can you show me the whole code from connection to network change?

I don't know if I understood what you said correctly, but from what I understand, it is as follows.

// This solves the problem that the network switching popup does not appear in the app.
setTimeout(() => connect({ chainId })), 1500)

// In this way, after performing the switchNetwork process, the next process did not proceed.
setTimeout(() => switchNetwork({ chainId }).then(checkIfChangedAndContinueProcress), 3500)

Is my understanding of the above code correct?

luisdeka commented 1 year ago

thanks @luisdeka I'm really sorry but can you show me the whole code from connection to network change?

I don't know if I understood what you said correctly, but from what I understand, it is as follows.

// This solves the problem that the network switching popup does not appear in the app.
setTimeout(() => connect({ chainId })), 1500)

// In this way, after performing the switchNetwork process, the next process did not proceed.
setTimeout(() => switchNetwork({ chainId }).then(checkIfChangedAndContinueProcress), 3500)

Is my understanding of the above code correct?

I have not tested the first one, the second is exactly that.

lunarinlunarinlunarin commented 1 year ago

is there an update on this? same issue for me

imaksp commented 1 year ago

is there an update on this? same issue for me

see this issue, it will be fixed in MM 7.4 version. https://github.com/MetaMask/metamask-mobile/issues/6655#issuecomment-1658587787