WalletConnect / walletconnect-monorepo

WalletConnect Monorepo
Apache License 2.0
1.46k stars 714 forks source link

Two issues: walletconnect and metamask mobile #4275

Open siosio34 opened 8 months ago

siosio34 commented 8 months ago

Describe the bug

Hello, I am using WalletConnect and Metamask Mobile and have the following two issues.

First, Switchchain does not work properly. When you send a Switchchain request and see the chain request window on MetaMask Mobile, if you reject it, the request will not be completed, and even if you accept it, the chain change within the mobile does not work properly, so you have to change the chain within MetaMask Mobile and then process the request.

Second, when sending a sign request while connected to a chain other than Ethereum (ex polygon), there was an issue where the chain was unnecessarily requested to be changed to Ethereum.

These two issues were so critical that we had no choice but to give up on introducing WalletConnect. Is there any progress on these two issues?

Links to related issues are below. https://github.com/MetaMask/metamask-mobile/issues/8380 https://github.com/MetaMask/metamask-mobile/issues/7206 https://github.com/MetaMask/metamask-mobile/issues/7023

SDK Version (if relevant)

To Reproduce Steps to reproduce the behavior:

  1. Go to '...'
  2. Click on '....'
  3. Scroll down to '....'
  4. See error

Expected behavior A clear and concise description of what you expected to happen.

Screenshots If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

Smartphone (please complete the following information):

Additional context Add any other context about the problem here.

glitch-txs commented 8 months ago

Hi @siosio34, thanks for the report, we'll try to contact MetaMask and suggest these fixes

siosio34 commented 8 months ago

@glitch-txs

Other than that, walletconnect provides a better user experience. Thank you to the walletconnect team for creating and maintaining this product.

glitch-txs commented 7 months ago

Hi @siosio34 just wanted to give a heads up we've been working together with MetaMask to address the most relevant issues and a new release with a UX improvement around chain switching and else are coming eventually.

siosio34 commented 7 months ago

@glitch-txs thank you I’m so excited for tonight that I can’t sleep 0_<

siosio34 commented 6 months ago

@glitch-txs hello. Has this release been released yet?

glitch-txs commented 6 months ago

Hey @siosio34 the new MetaMask version should be released early next month with these fixed 👍

siosio34 commented 6 months ago

I think what you said is included in https://github.com/MetaMask/metamask-mobile/releases/tag/v7.21.0. Is this correct?

@glitch-txs

glitch-txs commented 6 months ago

Mmm I think not, @ganchoradkov do you remember which one is the PR for the MetaMask update? But should be on the next release I think

0xShankar commented 1 month ago

The issue likely revolves around connection or integration problems between WalletConnect and MetaMask, such as:

WalletConnect not initializing properly with MetaMask. Session persistence issues: MetaMask disconnects after a session, or users experience connection drops. Multi-chain support issues: WalletConnect v2 supports multiple chains, but MetaMask

Proposed Solution: Ensure Proper Session Persistence:

Problem: Users often experience session disconnects after some idle time. Solution: Implement session persistence by storing the session key in local storage and reusing it when reconnecting. This avoids MetaMask having to re-establish a new session every time the page reloads.

const provider = new WalletConnectProvider({
  rpc: { 1: 'https://mainnet.infura.io/v3/INFURA_PROJECT_ID' }
});

// Check if there is an existing session
if (provider.connector.connected) {
  await provider.enable();
}
  1. Improve Error Handling and User Messaging:

Problem: When MetaMask fails to connect, users see a generic error message like "Connection failed." Solution: Modify the error-handling logic to display user-friendly messages specific to MetaMask errors. Here’s an example for handling MetaMask-specific errors:

provider.on("error", (error) => {
  if (error.message.includes("MetaMask")) {
    alert("MetaMask connection failed. Please try reconnecting.");
  } else {
    alert("Connection failed. Check your network or wallet.");
  }
});
  1. Multi-chain Support for MetaMask:

Problem: WalletConnect supports multi-chain connections, but MetaMask might not handle chain switching smoothly. Solution: Handle chain switching explicitly when using MetaMask with WalletConnect. If MetaMask fails to switch chains, display a prompt for the user to manually switch networks:

async function switchChain(chainId) {
  try {
    await provider.request({
      method: "wallet_switchEthereumChain",
      params: [{ chainId }],
    });
  } catch (error) {
    if (error.code === 4902) {
      // If the chain is not added, add it to MetaMask
      await provider.request({
        method: "wallet_addEthereumChain",
        params: [{ chainId }],
      });
    } else {
      alert("Please switch the network manually in MetaMask.");
    }
  }
}

4.Testing & Debugging MetaMask + WalletConnect Interactions:

Test the interactions by attempting the following: Switch chains on MetaMask and see if WalletConnect properly updates. Disconnect and reconnect sessions to check if sessions persist across reloads. Ensure proper error messages appear when connections fail.