MetaMask / metamask-mobile

Mobile web browser providing access to websites that use the Ethereum blockchain
https://metamask.io
Other
2.17k stars 1.12k forks source link

MetaMask redirect's to the Dapp before sending the chainChanged event #6706

Closed nhatnguyen95 closed 12 months ago

nhatnguyen95 commented 1 year ago

Describe the bug I am using @walletconnect/modal-react-native": "1.0.0-rc.2, when I try to connect to MetaMask and the chainId different with the chainId of activating network. chainChanged event didn't fire after returning to my app on Android (it works on iOS) so I can't know user has chosen the right network or not on Android.

To Reproduce

  1. Open Metamask and active Polygon network
  2. Connect with chainId of ETH(1) using Metamask over Wallet connect V2 modal
  3. Click on Connect button and return to Dapp
  4. chainChanged event doesn't notify on Android (It notifies on iOS)

Expected behavior chainChanged should notify to the DApp on Android

Smartphone:

I also created a discussion to ask Wallet connect team here: https://github.com/orgs/WalletConnect/discussions/2773

imaksp commented 1 year ago

I think It is because chain cannot be changed to dapp chain (if different) in metmask using RPC call, because currently wallet_switchEthereumChain is not working with WC v2. Are sure that chain is changed to chain which required by app inside metamask app? & if chain cannot be changed then event is out of question.

check #6655 for issue details.

nhatnguyen95 commented 1 year ago

@imaksp Here is my code:

useEffect(() => {
const registerEvents = () => {
provider?.on('chainChanged', (chainId: string) => {
setChainIdConnected(parseInt(chainId));
});
provider?.on('connect', ({chainId}) => {
setChainIdConnected(parseInt(chainId));
});
};
registerEvents();
}, [provider]);

I just call to connect function with chainId and wait for these events when return to app, on iOS it will notify chainChanged event if chainId to connect different with the chainId selecting in Metamask, but not for Android. I 've worked with Wallet connect team and they have responded to me: MetaMask redirect's to the dapp before sending the chainChanged event.

imaksp commented 1 year ago

Ok for getting current chain initially you need to send eth_chainId (getChai) method & if it is returning wrong/previous chain id then it means wallet app chain is different & not changed. & until 6655 is fixed, this issue will stay.

nhatnguyen95 commented 1 year ago

@imaksp it returned the chainId I called in connect function, (my expectation is current active chainId in Metamask). But after connected to Metamask successfully, I open Metamask and return to app again, "chainChanged" event is notified. It seems some Metamask functions is not called because it is suspended when open Dapp's deep link, then it will continue to execute functions when Metamask resume

imaksp commented 1 year ago

Ok if eth_chainId method is returning the expected id, you don't need to depend on chain changed event, just call eth_chainId when you need to match network.

nhatnguyen95 commented 1 year ago

@imaksp no it returned wrong chainId, I expect it is chainId of active network in Metamask but now it isn't . So this is still an issue on Android

imaksp commented 1 year ago

Ok, can you check again. with latest (2.8.4) walletconnect version, it fixed switch chain issue. not checked eth_chainId & chainChanged event yet.

nhatnguyenbp commented 1 year ago

@imaksp @anaamolnar this is still an issue in @walletconnect/modal-react-native:: "1.0.0-rc.3" with the latest 2.8.4 wallet connect core and only happened on Android

Thormeard commented 1 year ago

Still an issue in @walletconnect/modal-react-native:: "1.0.0-rc.10".

imaksp commented 1 year ago

has anyone checked this with MM 7.4 version, it has some fixes related to switch chain.

Thormeard commented 1 year ago

@imaksp Yes I tried yesterday with latest (7.4) MetaMask app and @walletconnect/modal-react-native:: "1.0.0-rc.10". on IOS

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity in the last 90 days. It will be closed in 7 days. Thank you for your contributions.

github-actions[bot] commented 12 months ago

This issue was closed because there has been no follow activity in 7 days. If you feel this was closed in error please provide evidence on the current production app in a new issue or comment in the existing issue to a maintainer. Thank you for your contributions.