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

[bug] After connecting a wallet using the WalletConnect QR code, the wallet disconnection is not functioning properly. #1877

Open
1 task done
heebaebang opened this issue Mar 20, 2024 · 13 comments · May be fixed by #1871
Open
1 task done

Comments

@heebaebang
Copy link

heebaebang commented Mar 20, 2024

Is there an existing issue for this?

  • I have searched the existing issues

RainbowKit Version

2.0.2

wagmi Version

2.5.11

Current Behavior

After connecting the wallet via QR code, if the page is refreshed, the wallet connection is maintained. However, when attempting to disconnect the wallet at this point, the wallet disconnection does not work for some reason.

Introduction.RainbowKit.mp4
  • 11 seconds: Wallet is connected
  • 16 seconds: Page is refreshed
  • 20 seconds: Request to disconnect wallet (no response)
  • 24 seconds: Another request to disconnect wallet (no response)
  • 28 seconds: Another request to disconnect wallet (wallet finally disconnects)

Expected Behavior

The wallet disconnection is expected on the first request to disconnect the wallet.

Steps To Reproduce

https://www.rainbowkit.com/docs/introduction

  1. Connect wallet using QR code through WalletConnect or MetaMask.
  2. Refresh the page.
  3. Request to disconnect the wallet.
  4. The wallet is not disconnected.
  5. After requesting to disconnect multiple times, it finally disconnects.

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

No response

Anything else?

When observing the connection state using the useConnections hook from wagmi, an interesting situation can be identified.

Initially, after connecting the wallet, there is only one connection in the connections array. However, after refreshing the page and inspecting it again, the connections array now contains two connections.

@magiziz magiziz linked a pull request Mar 20, 2024 that will close this issue
@magiziz
Copy link
Contributor

magiziz commented Mar 20, 2024

@heebaebang This is a normal in WalletConnect. They don't trigger an instant disconnect like you would in an injected wallet since they're using socket server to trigger the disconnect which might take sometime. However we do work on getting this somehow improved by introducing optimistic disconnect in this PR: #1871

@heebaebang
Copy link
Author

@magiziz I'm not sure if the issue can be resolved with an optimistic response. This is because the problem is not that the disconnection is slow, but that the disconnection has to be requested multiple times in order to disconnect.

@magiziz
Copy link
Contributor

magiziz commented Mar 21, 2024

@heebaebang Yeah that's the problem, since you're already requesting WalletConnect to be disconnected. By the time you've done that, it'll take a bit time to have an effect.

This is something that's out of our control, but we're trying to have something to indicate that a user is about to be disconnected which is why optimistic disconnect feature is good for UX purposes.

@heebaebang
Copy link
Author

@magiziz I still think that an optimistic response is merely a temporary solution for good UX, but I'm not confident it's a fundamental solution to this issue. In the browser UI, an optimistic response may make the wallet appear disconnected, but in reality it will likely still show as connected. In my opinion, the fundamental solutions to this issue are:

  1. Fix the issue of multiple connections being created.
  2. Or support disconnecting all connections in case multiple connections occur.

@magiziz
Copy link
Contributor

magiziz commented Mar 23, 2024

@heebaebang When we do optimistic disconnect your wallet won't be marked as "Connected" at least on the RainbowKit side. As mentioned this is something that's out of our control since WalletConnect has their own disconnect behaviour which takes time to disconnect from their socket server.

We also don't initiate multiple connections with WalletConnect, only one at a time. The only time you'll have multiple connections is with injected providers (browser extension wallets) if you've connected multiple of them in the dApp.

Also this is again as i mentioned something that's out of our control and optimistic disconnect would be ideal in this case to improve overall UX experience.

@heebaebang
Copy link
Author

@magiziz Thank you for the kind explanation.

I'm wondering if you have actually tried running the Reproduce steps that I wrote. After connecting the wallet and refreshing the page, multiple connections are created. This is also happening on the https://www.rainbowkit.com/ site at the time I'm writing this.

The key point is the phenomenon that occurs after refreshing the page!

Initially, I suspected there might be an issue with wagmi. So through the wagmi development documentation, I also tested other wallets that use wagmi. The wallets I tested are as follows:

When tested in the same way as the reproduction method I wrote above, unlike rainbowkit, there was no issue at all.

After reproducing the issue, checking the browser developer tools' localStorage should help with understanding

Based on Chrome
Application - Storage - Local Storage - https://www.rainbowkit.com/ - wagmi.store

If you look at the state.connections.value property in the wagmi.store value, you can see that the value is an array.

Right after connecting the wallet, state.connections.value value is one, but after refreshing the page, you can see that it changes to multiple values, and you can confirm that disconnecting the wallet does not work as expected. You may realize that you need to request wallet disconnection as many times as state.connections.value.length.

Right after connecting the wallet, state.connections.value value
image

After refreshing the page, state.connections.value value
image

This is a phenomenon that does not occur with other wallet platforms using wagmi.

The core of this issue is that rainbowkit, unlike other wallet platforms, causes multiple connections. Should I suspect wagmi?

@magiziz
Copy link
Contributor

magiziz commented Mar 24, 2024

@heebaebang Just tried with ConnectKit and Dynamic and they both take time to disconnect the wallet. We use one WalletConnect instance for all those wallets that are not injected and we generate a QR code, once you're connected and disconnect right after it would take time to get the WalletConnect provider and disconnect it. Please look at the walletConnect wallet disconnect function here.

@magiziz
Copy link
Contributor

magiziz commented Mar 24, 2024

Also wagmi supports multiple connections right out of the box, but not WalletConnect sessions. Let's say you had metamask and rainbow connected in your dApp before and refresh the page you will get multiple connections at once (metamask and rainbow will be connected). You can disable that feature it by adding reconnectOnMount={false} to the WagmiProvider

@heebaebang
Copy link
Author

heebaebang commented Mar 24, 2024

@magiziz It is not the issue being raised that the process of disconnecting the wallet takes time. The problem is that after refreshing the page, multiple connections are being created in rainbowkit.

Also, reconnectOnMount={false} is not the solution I'm looking for. In the spec I am implementing, the wallet connection should be maintained even after a page refresh. In fact, ConnectKit and Dynamic work without any issues, maintaining the wallet connection.

Can you record a reproduction video showing the state.connections.value under the wagmi.store key in localStorage, just like the video I've attached?(WalletConnect with MetaMask Wallet)

2024-03-25.8.27.41.mov

@magiziz
Copy link
Contributor

magiziz commented Mar 25, 2024

@heebaebang I am able to reproduce this too. Is your concern that you get multiple wallet connections at once ? If that's the case then it's not a RainbowKit issue since Wagmi initiates multiple wallet connections at once by default. If you'd like it to be disabled you have to do reconnecOnMount={false} otherwise it won't take an affect.

If your issue is regarding multiple connections being created at once then that's Wagmi releated, and means you'd have to raise an issue on their repo here.

ConnectKit + other libraries works exactly the same. Please see the screenshots attached below (even wagmi has this feature enabled in their repo as a boilerplate).

Screenshot 2024-03-25 at 11 14 52

Screenshot 2024-03-25 at 11 13 51

@heebaebang
Copy link
Author

@magiziz I'm not questioning the issue of wallets being multiply connected. I just want to understand the phenomenon of wallets unintentionally being multiply connected with Rainbowkit.

I connected my wallet through WalletConnect, and initially there was only one connection. However, upon refreshing the page, multiple connections were created, and this only happens with Rainbowkit.

We had a long discussion, but unfortunately it was not productive. I will have another developer look into this or investigate the cause myself when I have time. Thank you.

@heebaebang
Copy link
Author

A simple solution for developers struggling with the same issue.

import { useConnections, useDisconnect } from "wagmi";

const useDisconnectAll = () => {
  const connections = useConnections();
  const { disconnect } = useDisconnect();
  const disconnectsAll = () => {
    connections.forEach(({ connector }) => {
      disconnect({ connector });
    });
  };
  return { disconnectsAll }
}

This is a way to disconnect all wallet connections in cases where unintended multiple connections occur in a dapp that should only allow a single connection. If your dapp supports multiple wallet platforms, the connection object should contain information about which wallet it is, so you could add a logic to handle that.

@magiziz
Copy link
Contributor

magiziz commented Mar 26, 2024

@heebaebang Thanks for the context. I'll keep this issue open for now since it's related to WalletConnect taking a bit time for it to be disconnected, and we'll raise a PR to make the UX much simpler.

Also to answer the first part of your message, you have multiple connections during on load because you have connected multiple wallet extensions before in the dApp so the session is still "alive". E.g if i've connected metamask, rainbow and WalletConnect before then refresh the page, the session will still be "alive" until you disconnect the current one, but the two other will still have "active" sessions.

Also i like the idea of your approach, but i would suggest to not do that since it might cause unexpected behaviours with the injected connectors not displaying correctly afterwards. Here is an example issue in wagmi.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants