-
Notifications
You must be signed in to change notification settings - Fork 802
fix: derive capabilities from session namespaces when chainIds is omitted #6955
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
base: v2.0
Are you sure you want to change the base?
fix: derive capabilities from session namespaces when chainIds is omitted #6955
Conversation
|
All contributors have signed the CTA ✍️ ✅ |
|
I have read the CTA Document and I hereby sign the CTA |
…262) ### Description Send wallet capabilities within the session data to avoid an extra `wallet_getCapabilities` request that would otherwise require an extra user interaction. By including capabilities directly in the session, WalletConnect can respond to `wallet_getCapabilities` without prompting the user to switch to their wallet. This makes capability extraction seamless and improves the overall UX. ### WalletConnect capabilities support WalletConnect currently supports two approaches for including capabilities in session data: 1. Recommended: in `sessionProperties` or `scopedProperties`. [Docs](https://docs.walletconnect.network/wallet-sdk/react-native/eip5792#wallet-response) 2. Legacy: in the `capabilities` prop inside `sessionProperties`. [Source](https://github.com/WalletConnect/walletconnect-monorepo/blob/1e6d7793d0a30d2bf684cd3811ba120b4cdc0498/providers/universal-provider/src/providers/eip155.ts#L219-L223) We do support both approaches for maximal compatibility. ### Notes * When using the recommended approach, WalletConnect does not return capabilities if the optional `chainIds` array is omitted in `wallet_getCapabilities` request. * We've proposed a PR to address this in the WalletConnect repo: WalletConnect/walletconnect-monorepo#6955 ### Test plan * Updated unit test * Tested manually using https://bakoushin.github.io/wallet-connect-tester/ https://github.com/user-attachments/assets/bd3a6d15-4dd1-4dff-a295-a49828500d45 ### Related issues - Part of ENG-644 ### Backwards compatibility Y ### Network scalability If a new NetworkId and/or Network are added in the future, the changes in this PR will: - [x] Continue to work without code changes, OR trigger a compilation error (guaranteeing we find it when a new network is added)
…262) ### Description Send wallet capabilities within the session data to avoid an extra `wallet_getCapabilities` request that would otherwise require an extra user interaction. By including capabilities directly in the session, WalletConnect can respond to `wallet_getCapabilities` without prompting the user to switch to their wallet. This makes capability extraction seamless and improves the overall UX. ### WalletConnect capabilities support WalletConnect currently supports two approaches for including capabilities in session data: 1. Recommended: in `sessionProperties` or `scopedProperties`. [Docs](https://docs.walletconnect.network/wallet-sdk/react-native/eip5792#wallet-response) 2. Legacy: in the `capabilities` prop inside `sessionProperties`. [Source](https://github.com/WalletConnect/walletconnect-monorepo/blob/1e6d7793d0a30d2bf684cd3811ba120b4cdc0498/providers/universal-provider/src/providers/eip155.ts#L219-L223) We do support both approaches for maximal compatibility. ### Notes * When using the recommended approach, WalletConnect does not return capabilities if the optional `chainIds` array is omitted in `wallet_getCapabilities` request. * We've proposed a PR to address this in the WalletConnect repo: WalletConnect/walletconnect-monorepo#6955 ### Test plan * Updated unit test * Tested manually using https://bakoushin.github.io/wallet-connect-tester/ https://github.com/user-attachments/assets/bd3a6d15-4dd1-4dff-a295-a49828500d45 ### Related issues - Part of ENG-644 ### Backwards compatibility Y ### Network scalability If a new NetworkId and/or Network are added in the future, the changes in this PR will: - [x] Continue to work without code changes, OR trigger a compilation error (guaranteeing we find it when a new network is added)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the fix @bakoushin! I think the correct fix is a bit more involved though - I've added suggestions to improve it - for example to get the chains from the approved accounts rather than the chains since they are optionally present in the session. Further more I'm wondering about the case where both address specific and chain specific capabilities exist e.g.
"eip155:137": {
"eip155:137:0x0910e12C68d02B561a34569E1367c9AAb42bd811": {
atomic: {
status: "supported",
},
},
paymasterService: {
supported: true,
},
},
and the result should be
"0x89": {
atomic: {
status: "supported",
},
// this capability applies to all addresses on that chain
paymasterService: {
supported: true,
},
},
this case fails because we need to loop here to get all provider capabilities
https://github.com/WalletConnect/walletconnect-monorepo/pull/6955/files#diff-c0f6b48d5568497e5d855c04009289856302dc5a75aeebb143adb37986e12c59L65-L68
| const namespaceChainIds = | ||
| namespaces[EIP155_PREFIX]?.chains | ||
| ?.map((chain) => decimalToHex(chain.split(":")[1])) | ||
| .filter(Boolean) ?? []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const namespaceChainIds = | |
| namespaces[EIP155_PREFIX]?.chains | |
| ?.map((chain) => decimalToHex(chain.split(":")[1])) | |
| .filter(Boolean) ?? []; | |
| const namespaceChainIds = | |
| namespaces[EIP155_PREFIX]?.accounts | |
| .filter((account) => account.includes(`:${address}`)) | |
| ?.map((account) => decimalToHex(parseChainId(account).reference)) | |
| .filter(Boolean) ?? []; |
Check the accounts because we don't want to get all chains, just the chains this address was approved with
tip: you can import parseChainId from "@walletconnect/utils"
| "0x1": { | ||
| atomic: { | ||
| status: "unsupported", | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "0x1": { | |
| atomic: { | |
| status: "unsupported", | |
| }, | |
| }, |
with the change to loop through the accounts instead of chains, this 0x1 capabilty no longer applies
| const session = { | ||
| namespaces: { | ||
| eip155: { | ||
| chains: ["eip155:1", "eip155:137", "eip155:84532"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| chains: ["eip155:1", "eip155:137", "eip155:84532"], | |
| accounts: [ | |
| "eip155:137:0x0910e12C68d02B561a34569E1367c9AAb42bd811", | |
| "eip155:84532:0x0910e12C68d02B561a34569E1367c9AAb42bd810", | |
| ], |
|
@ganchoradkov Thanks for the detailed feedback! I guess it is now addressed:
|
Description
Updates
wallet_getCapabilitieshandling to fall back on the session’s namespace chain IDs when no explicitchainIdsare provided in the request.Per EIP-5792, the
chainIdsparameter is optional. If it is omitted, the wallet is expected to return capabilities for all supported chains.With this fix, the wallet correctly enumerates capabilities across all chains defined in the session namespaces, ensuring compliance with the spec and avoiding incomplete responses when chainIds is absent.
Type of change
How has this been tested?
Examples
scopedPropertiesas a part of session approval:wallet_getCapabilitieswithout providing an optional array of chain ids:chainIdsare providedChecklist
Note
extractCapabilitiesFromSessionnow falls back to EIP-155 session namespace chains whenchainIdsis not provided, with a new test covering this behavior.extractCapabilitiesFromSessioninproviders/universal-provider/src/utils/caip25.tsto:namespaces.eip155.chainsand convert to hex chain IDs.chainIdsis empty.providers/universal-provider/test/utils.spec.tsvalidating fallback to session namespace chains.Written by Cursor Bugbot for commit 573f2fa. This will update automatically on new commits. Configure here.