fix: use pairingTopic for permission revocation in removeSession()#27945
Conversation
|
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. |
Permissions are created with origin = pairingTopic (channelId) in _handleSessionProposal, but removeSession() was revoking using session.topic — a different value. The revocation was silently failing (caught by try/catch), leaving stale permission entries. Fixes: https://consensyssoftware.atlassian.net/browse/WAPI-1361
a6e6748 to
da94ec8
Compare
9627828 to
da94ec8
Compare
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection: Impact areas:
Why not broader tags: The change is isolated to WalletConnect V2 session removal logic. It doesn't affect swap/bridge flows, account management, confirmations (directly), or other wallet features. No UI changes, no performance impact. Confidence note: Moderate confidence because there are no dedicated WalletConnect E2E tests found in the test directories, so the selected tags represent the closest coverage for dApp connection/permission flows that WalletConnect V2 supports. Performance Test Selection: |
|
✅ E2E Fixture Validation — Schema is up to date |
|
Description
removeSession()callspermissionsController.revokeAllPermissions(session.topic), but permissions are stored undersession.pairingTopic(used aschannelId/originwhenrequestPermissionsis called in_handleSessionProposal— lines 468, 581).Since
topicandpairingTopicare always different values, the revocation was targeting a non-existent subject and silently failing (caught by the surrounding try/catch). Stale permission entries accumulated in thePermissionControllerfor every disconnected WC session.This changes the revocation key to
session.pairingTopicto match how permissions are created.Related issues
Fixes: https://consensyssoftware.atlassian.net/browse/WAPI-1361
Changelog
CHANGELOG entry: Fixed - Fixed removeSession() using session.topic instead of session.pairingTopic when revoking WalletConnect permissions, which caused stale permission entries to persist in the PermissionController (#27945)
Manual testing steps
PermissionController.state.subjects— the dapp's entry should be keyed bypairingTopicPermissionController.state.subjects— the entry should now be removedPre-merge author checklist
Note
Medium Risk
Changes how WalletConnect v2 session teardown revokes permissions, which impacts permission state cleanup and could affect disconnect behavior if the wrong identifier is used elsewhere. Scope is small and covered by an updated unit test.
Overview
Fixes
WC2Manager.removeSession()to revoke permissions usingsession.pairingTopic(and logs that identifier) instead ofsession.topic, preventing stalePermissionControllersubjects from accumulating after disconnects.Updates the
WalletConnectV2unit test to assertrevokeAllPermissionsis called with the pairing topic.Written by Cursor Bugbot for commit 8ba16c9. This will update automatically on new commits. Configure here.