fix: sync approved CAIP networks before connection status transition#5639
fix: sync approved CAIP networks before connection status transition#5639
Conversation
Move `getApprovedCaipNetworksData` and `setApprovedCaipNetworksData` to run before `syncAccount()` in `syncWalletConnectAccount()`, ensuring `approvedCaipNetworkIds` and `supportsAllNetworks` are available in `useAppKitNetwork` at the moment the connection status transitions to 'connected'. Also scope the approved network sync to the `if (sessionAddress)` guard so namespaces without session data no longer overwrite with defaults, and add approved network sync in the `accountChanged` handler as a safety net for the wagmi reconnect path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
9 Skipped Deployments
|
Visual Regression Test Results ✅ PassedChromatic Build: https://www.chromatic.com/build?appId=6493191bf4b10fed8ca7353f&number=867 👉 Please review the visual changes in Chromatic and accept or reject them. |
There was a problem hiding this comment.
Pull request overview
This PR adjusts WalletConnect reconnection/account-sync flow so approved CAIP networks (approvedCaipNetworkIds, supportsAllNetworks) are synchronized earlier, ensuring consumers (e.g., useAppKitNetwork) have correct network-approval state before the connection status transitions to connected.
Changes:
- Sync approved CAIP network data before
syncAccount()insyncWalletConnectAccount()to make approval state available earlier in the connection lifecycle. - Avoid syncing approved-network defaults for namespaces without WalletConnect session account data (
if (sessionAddress)guard). - Add an approved-network sync in the
accountChangedhandler for WalletConnect as a reconnect safety net.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (connector.type === UtilConstantsUtil.CONNECTOR_TYPE_WALLET_CONNECT) { | ||
| const approvedData = this.getApprovedCaipNetworksData() | ||
| ChainController.setApprovedCaipNetworksData(chainNamespace, { | ||
| approvedCaipNetworkIds: approvedData.approvedCaipNetworkIds, | ||
| supportsAllNetworks: approvedData.supportsAllNetworks | ||
| }) | ||
| } |
There was a problem hiding this comment.
getApprovedCaipNetworksData() derives WalletConnect/non-WalletConnect behavior from ProviderController.getProviderId(ChainController.state.activeChain). In this accountChanged handler we may be syncing for a non-active namespace (and potentially in multi-wallet scenarios where the active chain uses a different connector), which can cause WalletConnect sessions to be treated as non-WalletConnect and overwrite approved networks with { supportsAllNetworks: true, approvedCaipNetworkIds: [] }. Consider changing getApprovedCaipNetworksData to accept a chainNamespace (or provider type) and use ProviderController.getProviderId(chainNamespace) (or skip the providerId check since this block is already gated on WalletConnect), then pass the current chainNamespace here.
| const data = this.getApprovedCaipNetworksData() | ||
| ChainController.setApprovedCaipNetworksData(chainNamespace, { | ||
| approvedCaipNetworkIds: data.approvedCaipNetworkIds, | ||
| supportsAllNetworks: data.supportsAllNetworks |
There was a problem hiding this comment.
getApprovedCaipNetworksData() checks the provider type of ChainController.state.activeChain, so calling it while iterating namespaces can yield default (non-WalletConnect) values if the active namespace’s providerId hasn’t been set yet (or if the active namespace is connected via a different connector in multi-wallet mode). This can store incorrect approved network data for the current chainNamespace. Prefer computing approved network data directly from the WalletConnect session here, or update getApprovedCaipNetworksData to take chainNamespace/providerType and use that instead of activeChain.
| const data = this.getApprovedCaipNetworksData() | |
| ChainController.setApprovedCaipNetworksData(chainNamespace, { | |
| approvedCaipNetworkIds: data.approvedCaipNetworkIds, | |
| supportsAllNetworks: data.supportsAllNetworks | |
| const approvedCaipNetworkIds = Array.from( | |
| new Set( | |
| namespaceAccounts.map(account => { | |
| const { chainId: approvedChainId } = ParseUtil.parseCaipAddress(account as CaipAddress) | |
| return `${chainNamespace}:${approvedChainId}` as CaipNetworkId | |
| }) | |
| ) | |
| ) | |
| const namespaceCaipNetworkIds = (this.getCaipNetworks() || []) | |
| .filter(network => network.chainNamespace === chainNamespace) | |
| .map(network => network.id) | |
| const supportsAllNetworks = | |
| namespaceCaipNetworkIds.length > 0 && | |
| namespaceCaipNetworkIds.every(networkId => approvedCaipNetworkIds.includes(networkId)) | |
| ChainController.setApprovedCaipNetworksData(chainNamespace, { | |
| approvedCaipNetworkIds, | |
| supportsAllNetworks |
📦 Bundle Size Check✅ All bundles are within size limits 📊 View detailed bundle sizes> @reown/appkit-monorepo@1.7.1 size /home/runner/work/appkit/appkit > size-limit |
|
═══════════════════════════════════════════ 🔬 SDET Report — appkit 📝 SUMMARY 🐛 BUG-PRONE PATTERNS 🔴 [race-condition] 🟠 [null-safety] 🟠 [state-mutation] 🟠 [type-coercion] 🧪 TEST SCENARIOS 🔴 [UNIT] syncWalletConnectAccount syncs approved networks before syncAccount 🔴 [INTEGRATION] WalletConnect connection has correct approved networks immediately on connected status 🟠 [UNIT] approved networks sync skipped when no sessionAddress 🟠 [UNIT] non-WalletConnect connectors unaffected by approved networks sync
🔧 TECHNICAL EDGE CASES • [null reference] • [error handling] • [boundary] 🎭 MOCKING NOTES ✅ TEST CHECKLIST WalletConnect QA Agent |
enesozturk
left a comment
enesozturk
left a comment
There was a problem hiding this comment.
Do this requires a unit test?
Summary
getApprovedCaipNetworksData+setApprovedCaipNetworksDatato run beforesyncAccount()insyncWalletConnectAccount(), soapprovedCaipNetworkIdsandsupportsAllNetworksare available inuseAppKitNetworkat the moment the connection status transitions toconnectedif (sessionAddress)guard — namespaces without session data no longer overwrite with defaultsaccountChangedhandler as a safety net for the wagmi reconnect pathTest plan
approvedCaipNetworkIdsandsupportsAllNetworksare correct immediately on connected status🤖 Generated with Claude Code