fix: clear restored walletconnect connections on disconnect - #2688
Conversation
|
@o-mid is attempting to deploy a commit to the rainbowdotme Team on Vercel. A member of the Team first needs to authorize it. |
🦋 Changeset detectedLatest commit: 1b81bb6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1236816c4e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // the user closes it, we need to know the wallet isn't connecting anymore. | ||
| // So if it's connecting, we disconnect it. | ||
| if (isConnecting) disconnect(); | ||
| if (isConnecting) void disconnectAll(); |
There was a problem hiding this comment.
When the WalletButton QR flow is still pending and the user closes the connect modal, wagmi has isConnecting set but has not added anything to config.state.connections yet; useDisconnectAll therefore snapshots an empty list and this call becomes a no-op. The previous no-argument disconnect() still forced wagmi's status back to disconnected, which is exactly what the comment above this line says the close handler needs, so this can leave the custom WalletButton stuck in its loading/disabled connecting state until the pending connect promise settles. Preserve the old no-connection fallback when cancelling an in-flight connect.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ce83e754e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| for (const { connector } of connections) { | ||
| await disconnectAsync({ connector }); |
There was a problem hiding this comment.
When there are multiple restored connections and any connector's disconnectAsync rejects (for example a provider teardown error before wagmi deletes that connection), this loop exits before trying the remaining connectors; the current call sites all invoke void disconnectAll(), so the rejection is also unhandled and the user can remain connected through another restored connection after clicking Disconnect. Isolate/catch per-connector failures so one stale connection cannot prevent clearing the rest.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44be001ae0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } catch { | ||
| // One stale provider teardown must not block clearing the rest. |
There was a problem hiding this comment.
When one connector in the snapshot rejects from disconnectAsync({ connector }) (for example a restored WalletConnect provider hits a teardown error), wagmi leaves that connection in config.state.connections because the connection is removed only after the connector disconnect succeeds. This catch now swallows the rejection and continues, so after the remaining connectors are cleared the failed connection can become/stay current and the user is still connected after pressing Disconnect; the new regression test only checks that the remaining connectors were called, not that the failing connection was removed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b81bb68e8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| await disconnectAsync({ connector }); | ||
| } catch { | ||
| // wagmi only drops the connection after connector.disconnect succeeds. | ||
| removeConnection(config, connector); |
There was a problem hiding this comment.
When a connector's disconnectAsync({ connector }) rejects, this fallback only removes its entry from wagmi state. Wagmi's normal disconnect path also removes the connector's change/disconnect emitter listeners and re-adds the connect listener; skipping that cleanup leaves a connector that RainbowKit now treats as disconnected still wired as connected, so wallet-initiated reconnects are ignored and a later manual reconnect can attach duplicate handlers. The fresh evidence is this forced-removal path bypasses that listener cleanup entirely; mirror wagmi's disconnect cleanup before deleting the connection.
Useful? React with 👍 / 👎.
1 participant
Summary
walletConnectconnections.useDisconnectAll).Fixes #2401
Problem
RainbowKit registers multiple WalletConnect connectors that share
id: "walletConnect". After refresh, reconnect can restore more than one. Wagmi's defaultdisconnect()only removes the current connection and switches to the next, so the account UI remains connected.Reproduction
Root cause
Multi-wrapper WalletConnect connectors + wagmi disconnect-current semantics after reconnect.
Fix
Add
useDisconnectAlland use it from Account / Chain / Connect disconnect paths.Tests
disconnect()leaves restored WC connections connecteduseDisconnectAll()clears all in one clickpnpm test:unit run packages/rainbowkit/src/hooks/useDisconnectAll.test.tsxpnpm --filter @rainbow-me/rainbowkit typecheckCompatibility
Test plan