fix: revoke permissions for each session in WC2Manager.removeAll()#27944
Conversation
…alled anywhere...)
removeSession() revokes permissions via revokeAllPermissions() after cleanup, but removeAll() did not — leaving stale permission entries in the PermissionController for every removed session.
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
…ke-permissions # Conflicts: # app/core/WalletConnect/WalletConnectV2.test.ts
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection: Key observations:
The selected tags cover the dApp permission and session management flows most likely to be affected by this change. Performance Test Selection: |
|
✅ E2E Fixture Validation — Schema is up to date |
|



Description
removeSession()revokes permissions viapermissionsController.revokeAllPermissions()after cleanup, butremoveAll()did not — leaving stale permission entries in thePermissionControllerfor every removed session.This adds the missing
revokeAllPermissions(session.pairingTopic)call inremoveAll().Note on permission key inconsistency
This PR uses
session.pairingTopicas the revocation key because that's the key permissions are created under (origin: channelIdwherechannelId = pairingTopic— see_handleSessionProposal).However,
removeSession()usessession.topicinstead (line 377), which is a different value. That call is likely a no-op (silently caught by the surrounding try/catch). This is a pre-existing bug inremoveSession— it should probably also usesession.pairingTopic. Left as-is to keep this PR scoped.Related issues
Follow-up to #27932
Manual testing steps
removeAll()(e.g., via wallet reset flow)PermissionController.state.subjectsPre-merge author checklist
Note
Low Risk
Low risk: small, well-scoped change that only adds missing permission cleanup during
WC2Manager.removeAll(), with a unit test covering the new behavior.Overview
Fixes
WC2Manager.removeAll()to also revoke WalletConnect v2 permissions for each active session (viaPermissionController.revokeAllPermissions(session.pairingTopic)) before disconnecting, preventing stale permission entries after bulk session removal.Adds a focused unit test asserting permission revocation is triggered during
removeAll(), and logs (without failing) if revocation throws.Written by Cursor Bugbot for commit e8b281e. This will update automatically on new commits. Configure here.