[SDK] Upgrade WalletConnect and improve chain switching reliability #8046
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 986bf85 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThe PR updates WalletConnect dependencies, hardens WalletConnect session/chain-switch logic, adds a post-switch verification in a transaction hook, fixes SIWE to always use mainnet and updates callers/tests accordingly, and adds release metadata. Minor comments were added in wallet creation logic. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App
participant Connector as In-App Connector (web/native)
participant SIWE as siweAuthenticate
participant Wallet
App->>Connector: connect({ auth: "wallet", chain: ... })
Connector->>SIWE: siweAuthenticate({ wallet, client, ecosystem })
note right of SIWE: SIWE now forces mainnet (chain id 1)
SIWE->>Wallet: connect({ chain: mainnet })
SIWE-->>Connector: token/cookie
Connector-->>App: auth result
sequenceDiagram
autonumber
participant DApp
participant WC as WalletConnect Controller
participant Provider as WC Provider
participant Wallet
DApp->>WC: connect()
WC->>Provider: connect({ optionalNamespaces })
Provider-->>WC: session established
WC->>WC: firstAccountOn(session, "eip155:1")
WC-->>DApp: address
DApp->>WC: switchChain(targetChain)
WC->>WC: ensureTargetChain(provider, targetChain, walletInfo)
alt Chain already enabled
WC->>Provider: setDefaultChain(targetChain)
else Attempt switch
WC->>Wallet: wallet_switchEthereumChain(targetChain)
alt User rejected (4001)
WC-->>DApp: error
else Not enabled
WC->>WC: anyRoutableChain(session)
alt Found routable chain
WC->>Wallet: wallet_addEthereumChain(via routable)
alt User rejected (4001)
WC-->>DApp: error
else
WC->>Wallet: wallet_switchEthereumChain(targetChain)
WC->>Provider: setDefaultChain(targetChain)
WC->>WC: verify chain enabled in session
alt Not enabled
WC-->>DApp: error
end
end
else None found
WC-->>DApp: error
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Comment |
joaquim-verges
changed the title
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
joaquim-verges
force-pushed
the
Upgrade_WalletConnect_and_improve_chain_switching_reliability
branch
from
24e4712 to
986bf85
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
size-limit report 📦
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8046 +/- ##
==========================================
- Coverage 56.64% 56.55% -0.09%
==========================================
Files 904 904
Lines 58755 58842 +87
Branches 4171 4166 -5
==========================================
Hits 33280 33280
- Misses 25369 25456 +87
Partials 106 106
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/thirdweb/src/wallets/create-wallet.ts (1)
486-493: Make switchChain fail-fast and verify post-switch before mutating local state.
Silent catch undermines reliability; also setchain = conly after confirming the active chain changed.Apply:
switchChain: async (c) => { - // TODO: this should actually throw an error if the chain switch fails - // but our useSwitchActiveWalletChain hook currently doesn't handle this try { - await handleSwitchChain(c); - chain = c; + await handleSwitchChain(c); + // verify that the provider actually switched before updating local cache + if (wallet.getChain?.()?.id !== c.id) { + throw new Error(`Failed to switch to chain ${c.id}`); + } + chain = c; } catch (e) { console.error("Error switching chain", e); + throw e; } },
🧹 Nitpick comments (5)
packages/thirdweb/src/react/core/hooks/transaction/useSendTransaction.ts (1)
153-161: Differentiate user rejection vs. switch failure for better UX.
Wrap the thrown error to preserve user-rejected semantics (e.g., map toUserRejectedRequestError) while keeping a distinct message for “could not switch”..changeset/hot-bikes-relax.md (1)
1-5: Consider mentioning SIWE mainnet enforcement in the note.
Helps consumers understand login behavior changes alongside WC reliability.packages/thirdweb/src/wallets/in-app/core/authentication/siwe.ts (1)
34-35: Nit: be explicit about SIWE chain semantics.
Add a short TSDoc note thatchainIdin the generated payload is forced to1, independent of the wallet’s current chain, to avoid ambiguity.packages/thirdweb/src/wallets/wallet-connect/controller.ts (2)
199-281: Ensure wallet_addEthereumChain payload always has complete metadata.
Ifchainlacksname/nativeCurrency/blockExplorers(e.g., from a minimalgetCachedChain),wallet_addEthereumChaincan fail. Consider enriching with known metadata or deriving RPC viagetRpcUrlForChain.Option: thread
clientintoensureTargetChainand set:
rpcUrls: [getRpcUrlForChain({ chain, client })]- Fill
chainName/nativeCurrency/blockExplorerUrlsfrom a chain registry fallback if missing.
554-577: Type the request helper generically.
Some requests returnvoid/boolean/non-hex. Make this generic to avoid misleading types.-async function requestAndOpenWallet(args: { +async function requestAndOpenWallet<T = unknown>(args: { provider: WCProvider; payload: RequestArguments; chain?: string; walletInfo: WalletInfo; sessionRequestHandler?: (uri: string) => void | Promise<void>; -}) { +}): Promise<T> { const { provider, payload, chain, walletInfo, sessionRequestHandler } = args; - const resultPromise: Promise<`0x${string}`> = provider.request( - payload, - chain, - ); + const resultPromise = provider.request<T>(payload, chain); … return resultPromise; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.changeset/hot-bikes-relax.md(1 hunks)packages/thirdweb/package.json(1 hunks)packages/thirdweb/src/react/core/hooks/transaction/useSendTransaction.ts(1 hunks)packages/thirdweb/src/wallets/create-wallet.ts(1 hunks)packages/thirdweb/src/wallets/in-app/core/authentication/siwe.ts(3 hunks)packages/thirdweb/src/wallets/in-app/native/native-connector.ts(0 hunks)packages/thirdweb/src/wallets/in-app/web/lib/web-connector.test.ts(0 hunks)packages/thirdweb/src/wallets/in-app/web/lib/web-connector.ts(0 hunks)packages/thirdweb/src/wallets/wallet-connect/controller.ts(5 hunks)
💤 Files with no reviewable changes (3)
- packages/thirdweb/src/wallets/in-app/native/native-connector.ts
- packages/thirdweb/src/wallets/in-app/web/lib/web-connector.ts
- packages/thirdweb/src/wallets/in-app/web/lib/web-connector.test.ts
🧰 Additional context used
📓 Path-based instructions (6)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
.changeset/*.md: Each change inpackages/*must include a changeset for the appropriate package
Version bump rules: patch for non‑API changes; minor for new/modified public API
Files:
.changeset/hot-bikes-relax.md
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Track bundle budgets via
package.json#size-limit
Files:
packages/thirdweb/package.json
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
**/*.{ts,tsx}: Use explicit function declarations and explicit return types in TypeScript
Limit each file to one stateless, single‑responsibility function
Re‑use shared types from@/typeswhere applicable
Prefertypealiases overinterfaceexcept for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Prefer composition over inheritance; use utility types (Partial, Pick, etc.)
Lazy‑import optional features and avoid top‑level side‑effects to reduce bundle size
Files:
packages/thirdweb/src/react/core/hooks/transaction/useSendTransaction.tspackages/thirdweb/src/wallets/create-wallet.tspackages/thirdweb/src/wallets/in-app/core/authentication/siwe.tspackages/thirdweb/src/wallets/wallet-connect/controller.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
packages/thirdweb/src/react/core/hooks/transaction/useSendTransaction.tspackages/thirdweb/src/wallets/create-wallet.tspackages/thirdweb/src/wallets/in-app/core/authentication/siwe.tspackages/thirdweb/src/wallets/wallet-connect/controller.ts
packages/thirdweb/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/thirdweb/**/*.{ts,tsx}: Every public symbol must have comprehensive TSDoc with at least one compiling@exampleand a custom tag (@beta,@internal,@experimental, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
Lazy‑load heavy dependencies inside async paths (e.g.,const { jsPDF } = await import("jspdf"))
Files:
packages/thirdweb/src/react/core/hooks/transaction/useSendTransaction.tspackages/thirdweb/src/wallets/create-wallet.tspackages/thirdweb/src/wallets/in-app/core/authentication/siwe.tspackages/thirdweb/src/wallets/wallet-connect/controller.ts
packages/thirdweb/src/wallets/**
📄 CodeRabbit inference engine (CLAUDE.md)
packages/thirdweb/src/wallets/**: UnifiedWalletandAccountinterfaces in wallet architecture
Support for in-app wallets (social/email login)
Smart wallets with account abstraction
EIP-1193, EIP-5792, EIP-7702 standard support in wallet modules
Files:
packages/thirdweb/src/wallets/create-wallet.tspackages/thirdweb/src/wallets/in-app/core/authentication/siwe.tspackages/thirdweb/src/wallets/wallet-connect/controller.ts
🧠 Learnings (5)
📚 Learning: 2025-08-29T15:37:38.513Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: AGENTS.md:0-0
Timestamp: 2025-08-29T15:37:38.513Z
Learning: Applies to .changeset/*.md : Version bump rules: patch for non‑API changes; minor for new/modified public API
Applied to files:
.changeset/hot-bikes-relax.md
📚 Learning: 2025-07-18T19:19:55.613Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : EIP-1193, EIP-5792, EIP-7702 standard support in wallet modules
Applied to files:
.changeset/hot-bikes-relax.mdpackages/thirdweb/src/wallets/create-wallet.tspackages/thirdweb/src/wallets/wallet-connect/controller.ts
📚 Learning: 2025-07-18T19:19:55.613Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : Support for in-app wallets (social/email login)
Applied to files:
.changeset/hot-bikes-relax.md
📚 Learning: 2025-06-03T23:44:40.243Z
Learnt from: joaquim-verges
PR: thirdweb-dev/js#7268
File: packages/thirdweb/src/wallets/in-app/core/wallet/in-app-core.ts:210-216
Timestamp: 2025-06-03T23:44:40.243Z
Learning: EIP7702 wallets do not need special handling for switching chains, unlike EIP4337 wallets which require reconnection when switching chains. In the switchChain method condition, EIP7702 should be intentionally excluded from the reconnection logic.
Applied to files:
packages/thirdweb/src/react/core/hooks/transaction/useSendTransaction.tspackages/thirdweb/src/wallets/create-wallet.tspackages/thirdweb/src/wallets/wallet-connect/controller.ts
📚 Learning: 2025-07-18T19:19:55.613Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : Unified `Wallet` and `Account` interfaces in wallet architecture
Applied to files:
packages/thirdweb/src/wallets/wallet-connect/controller.ts
🧬 Code graph analysis (2)
packages/thirdweb/src/wallets/in-app/core/authentication/siwe.ts (1)
packages/thirdweb/src/chains/utils.ts (1)
getCachedChain(79-89)
packages/thirdweb/src/wallets/wallet-connect/controller.ts (2)
packages/thirdweb/src/exports/wallets.ts (1)
WalletInfo(130-130)packages/thirdweb/src/exports/utils.ts (1)
numberToHex(82-82)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Size
- GitHub Check: Unit Tests
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (5)
packages/thirdweb/package.json (1)
28-29: WalletConnect bump looks localized — verify bundle budgets & single-version resolution
- Pins are only in packages/thirdweb/package.json (lines 28–29): "@walletconnect/sign-client": "2.21.8", "@walletconnect/universal-provider": "2.21.8".
- packages/thirdweb has size-limit configured and CI runs size-limit; run the build/size check to confirm budgets still pass.
- pnpm-lock.yaml in this repo snapshot contains no @WalletConnect entries — cannot confirm a single resolved version across the workspace. After installing deps run: pnpm install; rg -nE '^@walletconnect/' pnpm-lock.yaml; pnpm -w why @walletconnect/sign-client and dedupe/alignment if multiple versions appear.
packages/thirdweb/src/react/core/hooks/transaction/useSendTransaction.ts (1)
158-161: Good: explicit post-switch verification prevents “fake success”.
This will surface failed chain switches early to callers.packages/thirdweb/src/wallets/in-app/core/authentication/siwe.ts (1)
20-25: Clarify “always use mainnet” behavior and align implementation.
Current code uses mainnet only when no account exists; if already connected on a non‑mainnet chain, it won’t switch. If the intent is “payload always uses chainId 1” (not forcing a switch), document it; if the intent is “ensure wallet is on 1”, add an explicit switch.Would you like to enforce an actual switch to 1 when
wallet.getChain()?.id !== 1, or keep it payload-only?packages/thirdweb/src/wallets/wallet-connect/controller.ts (2)
135-152: Namespaces migration looks correct for UP v2.21.x.
Includes required methods/events and rpcMap; matches provider expectations.
708-714: Good call: always include mainnet in requested chains.
Improves compatibility across wallets and aligns with SIWE/mainnet address discovery.
| const account = firstAccountOn(provider.session, `eip155:1`); // grab the address from mainnet | ||
| const address = account; | ||
| if (!address) { | ||
| throw new Error("No accounts found on provider."); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fallback when no mainnet account exists in session.
Some wallets/sessions may not expose an account on eip155:1. Fall back to the first available account instead of failing hard.
- const account = firstAccountOn(provider.session, `eip155:1`); // grab the address from mainnet
- const address = account;
- if (!address) {
- throw new Error("No accounts found on provider.");
- }
+ const address =
+ firstAccountOn(provider.session, `eip155:1`) ??
+ provider.session?.namespaces?.[NAMESPACE]?.accounts?.[0]?.split(":")[2] ??
+ null;
+ if (!address) {
+ throw new Error("No accounts found in WalletConnect session.");
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const account = firstAccountOn(provider.session, `eip155:1`); // grab the address from mainnet | |
| const address = account; | |
| if (!address) { | |
| throw new Error("No accounts found on provider."); | |
| } | |
| const address = | |
| firstAccountOn(provider.session, `eip155:1`) ?? | |
| provider.session?.namespaces?.[NAMESPACE]?.accounts?.[0]?.split(":")[2] ?? | |
| null; | |
| if (!address) { | |
| throw new Error("No accounts found in WalletConnect session."); | |
| } |
joaquim-verges
deleted the
Upgrade_WalletConnect_and_improve_chain_switching_reliability
branch
Fixes BLD-307, BLD-306
PR-Codex overview
This PR focuses on improving the reliability of chain switching in the
WalletConnectintegration for thethirdweblibrary, enhancing error handling, and updating package dependencies.Detailed summary
@walletconnect/sign-clientand@walletconnect/universal-providerto version2.21.8.native-connector.tsandweb-connector.ts.ensureTargetChainfor better chain management incontroller.ts.siweAuthenticateto always use the mainnet for compatibility.Summary by CodeRabbit