fix: revoke the entire session on connect-evm disconnect - #344
Open
adonesky1 wants to merge 2 commits into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Make
connect-evm'sdisconnect()revoke the entire session instead of only itseip155:*scopes, so scopes granted beyond the client's request don't get stranded in a session the wallet still reports as connected.Problem
disconnect()filters the session scopes toeip155:*and passes only those towallet_revokeSession. The wallet's revoke handler does a partial revocation when scopes are specified and keeps the permission alive while any accounts remain on other scopes.The wallet can grant more than the client requested: for EIP-1193 compatible connections it pre-selects all of the user's enabled networks on the connect prompt (see fix: pre-select all networks for eip1193-compatible requests), so a single approval can include Solana, Bitcoin, and Tron scopes alongside the requested EVM chains. No client on the page is responsible for revoking those extras:
connect-evmrevokes onlyeip155:*solana:*bip122:*/tron:*Net effect (reproduced while testing the extension PR above on app.uniswap.org): after the dapp's disconnect flow runs, the wallet still shows the site as connected, with Bitcoin and Tron listed as connected networks, and the user can only fully disconnect from inside the wallet UI.
Solution
Call
core.disconnect()with no scope filter. The transport sendswallet_revokeSessionwith empty scopes, which the wallet treats as "revoke the whole session".This matches the legacy EIP-1193 behavior this client emulates: wagmi's injected disconnect calls
wallet_revokePermissions, which revokes the origin's entire CAIP-25 permission, over-granted scopes included. That symmetry (broad grant, broad revoke) is why stranded scopes were never an issue before the migration to scoped revocation.Risk
A dapp that manages its EVM and Solana connections independently and expects
connect-evm.disconnect()to leave a concurrent Solana connection intact will now see the full session revoked (the Solana client is notified viawallet_sessionChanged, so its state stays consistent). This is the pre-migration behavior of the injected flow, and we're not aware of a dapp relying on the scoped behavior; flagging it in case reviewers know of one.