Skip to content

Conversation

@joaquim-verges
Copy link
Member

@joaquim-verges joaquim-verges commented Nov 17, 2025


PR-Codex overview

This PR focuses on improving the handling of wallet capabilities and connection features in the thirdweb and wagmi libraries, along with updating dependencies for better functionality and compatibility.

Detailed summary

  • Enhanced handling of erc5792 capabilities in the getCapabilities function.
  • Improved auto-connect feature to the last connected chain in the wagmi adapter.
  • Updated viem version from 2.37.9 to 2.39.0 in multiple packages.
  • Updated wagmi version from 2.17.5 to 2.19.4.
  • Added new wallet options in the ConnectButton configuration.
  • Modified capability filtering logic to use chainIdFilter instead of chainId.
  • Refactored toGetCapabilitiesResult to improve capability return structure.
  • Adjusted storage handling for active chains in wallet connectors.

The following files were skipped due to too many changes: pnpm-lock.yaml

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

Summary by CodeRabbit

  • New Features
    • Automatic reconnection to your last-used blockchain networklg
    • Added support for MetaMask, Coinbase Wallet, and Rainbow in the demo connect UI
  • Improvements
    • Improved detection and reporting of wallet capabilities across chains
    • More reliable cross-chain wallet handling and active-chain persistence

@vercel
Copy link

vercel bot commented Nov 17, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
docs-v2 Ready Ready Preview Comment Nov 17, 2025 11:56pm
nebula Ready Ready Preview Comment Nov 17, 2025 11:56pm
thirdweb_playground Ready Ready Preview Comment Nov 17, 2025 11:56pm
thirdweb-www Ready Ready Preview Comment Nov 17, 2025 11:56pm
wallet-ui Ready Ready Preview Comment Nov 17, 2025 11:56pm

@changeset-bot
Copy link

changeset-bot bot commented Nov 17, 2025

🦋 Changeset detected

Latest commit: d90a81d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
thirdweb Patch
@thirdweb-dev/wagmi-adapter Patch
@thirdweb-dev/nebula Patch
wagmi-inapp Patch

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 17, 2025

Walkthrough

Adds two changesets; bumps viem and wagmi in the demo; expands demo ConnectButton wallets; refactors capability retrieval to support optional chain filtering; and refactors wagmi-adapter storage to track the active chain under a new key.

Changes

Cohort / File(s) Summary
Changeset entries
.changeset/few-hats-slide.md, .changeset/sixty-views-relate.md
Two new changeset files declaring patch releases: one for thirdweb (better handling of getCapabilities) and one for @thirdweb-dev/wagmi-adapter (better autoconnect-to-last-connected-chain).
Dependency updates
apps/wagmi-demo/package.json, packages/thirdweb/package.json
Bump viem (2.37.9 → 2.39.0) and wagmi (demo: 2.17.5 → 2.19.4).
Demo app—wallet list changes
apps/wagmi-demo/src/App.tsx
Adds createWallet import from thirdweb/wallets and expands ConnectButton wallets to include createWallet("io.metamask"), createWallet("com.coinbase.wallet"), and createWallet("me.rainbow").
EIP-5792 capability filtering
packages/thirdweb/src/adapters/eip1193/to-eip1193.ts, packages/thirdweb/src/wallets/eip5792/get-capabilities.ts, packages/thirdweb/src/wallets/injected/index.ts
Accepts optional chains array in requests; derives a chainId filter when provided and passes it through to getCapabilities/toGetCapabilitiesResult (renamed param chainIdFilter); returns filtered single-entry result when chainIdFilter is a number, otherwise full map.
Wagmi adapter active-chain storage refactor
packages/wagmi-adapter/src/connector.ts
Introduces activeChainIdKey ("thirdweb:active-chain"), reads/writes active chain via rawStorage (JSON-serialized Chain), updates connect/reconnect/getProvider/switchChain flows, and adds several narrowed type casts to any in returned payloads.

Sequence Diagram(s)

sequenceDiagram
    participant App as App/User
    participant Connector as Wagmi Connector
    participant Storage as rawStorage
    participant Account as Account

    rect rgb(235, 245, 230)
    Note over Connector,Storage: Reconnect / autoConnect (init)
    Connector->>Storage: read "thirdweb:active-chain"
    Storage-->>Connector: JSON string (Chain) or empty
    Connector->>Connector: parse → lastChain (Chain | undefined)
    Connector->>Account: autoConnect(chain = lastChain?.id || smartAccount?.id || 1)
    Account-->>Connector: accounts, chain
    end
Loading
sequenceDiagram
    participant DApp as DApp
    participant Provider as EIP-1193 Adapter
    participant Wallet as Wallet (erc5792)

    rect rgb(245, 245, 255)
    Note over DApp,Wallet: Capability request with optional chains
    DApp->>Provider: request wallet_getCapabilities (params: [account, chains?])
    Provider->>Provider: if chains is array → parse first chainId
    alt chainId parsed
        Provider->>Wallet: account.getCapabilities({ chainId: firstChainId })
        Wallet-->>Provider: capabilities for specific chain
    else no chainId
        Provider->>Wallet: account.getCapabilities({})
        Wallet-->>Provider: full capabilities map
    end
    Provider-->>DApp: normalized capabilities (filtered or full)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Review chain-filter propagation: to-eip1193.tsget-capabilities.tsinjected/index.ts to ensure correct parsing and result shape.
  • Inspect packages/wagmi-adapter/src/connector.ts storage changes: JSON (de)serialization, fallback logic, and where any casts were introduced.
  • Validate demo changes (apps/wagmi-demo/src/App.tsx) compile with updated dependencies and new wallet entries.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description includes a PR-Codex overview with detailed summary but lacks required template sections like issue tag, reviewer notes, and testing instructions. Add missing required sections: issue tag (TEAM-0000 format), notes for reviewer, and how to test (unit tests, playground, etc.) from the template.
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title is fully related to the main change in the changeset, clearly summarizing the primary objective of better EIP5792 handling improvements.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch Update_viem_to_2.39.0_and_wagmi_to_2.19.4

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • TEAM-0000: Entity not found: Issue - Could not find referenced Issue.

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added packages SDK Involves changes to the thirdweb SDK labels Nov 17, 2025
Copy link
Member Author


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • merge-queue - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

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 joaquim-verges changed the title Update viem to 2.39.0 and wagmi to 2.19.4 [SDK] Better eip5792 handling Nov 17, 2025
@joaquim-verges joaquim-verges marked this pull request as ready for review November 17, 2025 23:37
@joaquim-verges joaquim-verges requested review from a team as code owners November 17, 2025 23:37
@socket-security
Copy link

socket-security bot commented Nov 17, 2025

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updated@​wagmi/​cli@​2.7.1 ⏵ 2.8.09810078 +197 +2100

View full report

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (5)
packages/thirdweb/src/wallets/eip5792/get-capabilities.ts (1)

55-76: LGTM! Consider documenting the chainIdFilter behavior.

The refactoring to support chain-specific filtering is well-implemented. The parameter rename to chainIdFilter and the conditional return logic correctly filter capabilities by chain ID when provided.

Note that when chainIdFilter is specified but doesn't exist in the capabilities map, the result will be { [chainIdFilter]: undefined }. If this is the intended behavior, consider adding a comment or documenting it.

Consider adding JSDoc or a comment to clarify the behavior:

 export function toGetCapabilitiesResult(
   result: Record<string, WalletCapabilities>,
+  // When provided, returns capabilities for only the specified chainId
   chainIdFilter?: number,
 ): GetCapabilitiesResult {
packages/thirdweb/src/adapters/eip1193/to-eip1193.ts (1)

167-177: Consider validating the parsed chain ID.

The new chain-specific capability retrieval logic correctly handles hex and decimal chain IDs. However, if chains[0] contains an invalid value, Number(firstChainStr) could produce NaN, which would be truthy but invalid.

Consider adding validation:

 const chains = request.params[1];
 if (chains && Array.isArray(chains)) {
   const firstChainStr = chains[0];
   const firstChainId = isHex(firstChainStr)
     ? hexToNumber(firstChainStr)
     : Number(firstChainStr);
-  return account.getCapabilities(
-    firstChainId ? { chainId: firstChainId } : {},
-  );
+  return account.getCapabilities(
+    firstChainId && !Number.isNaN(firstChainId) 
+      ? { chainId: firstChainId } 
+      : {},
+  );
 }
 return account.getCapabilities({});
apps/wagmi-demo/src/App.tsx (1)

46-47: Consider removing debug console logs.

The console.log statements for capabilities are useful for debugging but may be leftover development code.

If these logs are for debugging purposes, consider removing them:

-  console.log("wagmi capabilities", capabilities.error, capabilities.data);
-  console.log("thirdweb capabilities", thirdwebCapabilities.error, thirdwebCapabilities.data);

Alternatively, if this logging is intentional for the demo, consider adding a comment to clarify.

packages/wagmi-adapter/src/connector.ts (2)

55-57: Align StorageItem with actual storage usage (optional)

StorageItem still models "thirdweb:lastChainId", but the implementation has moved to raw localStorage with activeChainIdKey = "thirdweb:active-chain" and no longer touches config.storage. To avoid confusion, consider either removing StorageItem or updating it to reflect keys you actually read/write via wagmi config.storage.

-type StorageItem = {
-  "thirdweb:lastChainId": number;
-};
+type StorageItem = Record<string, never>;

Also applies to: 61-61


153-153: accounts casts to any trade away type safety (optional)

Both early-return and post-connect paths cast accounts to any (Lines 153, 201-204). It’s pragmatic, but it bypasses the withCapabilities typing in Properties["connect"]. If you want to keep stronger typing for EIP‑5792 flows, consider tightening these casts to the actual union type or refactoring connect’s implementation to be generic‑aware; otherwise this is acceptable as a compromise.

Also applies to: 201-204

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between c76fe10 and 26d008f.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (9)
  • .changeset/few-hats-slide.md (1 hunks)
  • .changeset/sixty-views-relate.md (1 hunks)
  • apps/wagmi-demo/package.json (1 hunks)
  • apps/wagmi-demo/src/App.tsx (4 hunks)
  • packages/thirdweb/package.json (1 hunks)
  • packages/thirdweb/src/adapters/eip1193/to-eip1193.ts (1 hunks)
  • packages/thirdweb/src/wallets/eip5792/get-capabilities.ts (2 hunks)
  • packages/thirdweb/src/wallets/injected/index.ts (1 hunks)
  • packages/wagmi-adapter/src/connector.ts (7 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: joaquim-verges
Repo: thirdweb-dev/js PR: 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.
📚 Learning: 2025-08-28T20:50:33.170Z
Learnt from: joaquim-verges
Repo: thirdweb-dev/js PR: 7922
File: apps/playground-web/src/app/ai/ai-sdk/components/chat-container.tsx:167-181
Timestamp: 2025-08-28T20:50:33.170Z
Learning: The SignTransactionInput schema in thirdweb-dev/ai-sdk-provider uses snake_case field names (chain_id) rather than camelCase (chainId).

Applied to files:

  • packages/thirdweb/src/wallets/injected/index.ts
📚 Learning: 2025-06-03T23:44:40.243Z
Learnt from: joaquim-verges
Repo: thirdweb-dev/js PR: 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/wallets/injected/index.ts
  • packages/thirdweb/src/adapters/eip1193/to-eip1193.ts
  • apps/wagmi-demo/src/App.tsx
  • packages/wagmi-adapter/src/connector.ts
📚 Learning: 2025-06-06T23:46:08.795Z
Learnt from: MananTank
Repo: thirdweb-dev/js PR: 7298
File: apps/dashboard/src/app/nebula-app/move-funds/move-funds.tsx:424-424
Timestamp: 2025-06-06T23:46:08.795Z
Learning: The thirdweb project has an ESLint rule that restricts direct usage of `defineChain`. When it's necessary to use `defineChain` directly, it's acceptable to disable the rule with `// eslint-disable-next-line no-restricted-syntax`.

Applied to files:

  • packages/wagmi-adapter/src/connector.ts
🧬 Code graph analysis (3)
packages/thirdweb/src/wallets/injected/index.ts (1)
packages/thirdweb/src/wallets/eip5792/get-capabilities.ts (1)
  • toGetCapabilitiesResult (55-76)
apps/wagmi-demo/src/App.tsx (3)
packages/thirdweb/src/exports/react.native.ts (2)
  • useActiveAccount (78-78)
  • useCapabilities (87-87)
packages/thirdweb/src/exports/react.ts (2)
  • useActiveAccount (80-80)
  • useCapabilities (89-89)
apps/wagmi-demo/src/wagmi.ts (1)
  • wallet (19-24)
packages/wagmi-adapter/src/connector.ts (1)
apps/wagmi-demo/src/wagmi.ts (2)
  • wallet (19-24)
  • chain (26-26)
⏰ 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). (8)
  • GitHub Check: Size
  • GitHub Check: E2E Tests (pnpm, vite)
  • GitHub Check: E2E Tests (pnpm, webpack)
  • GitHub Check: Unit Tests
  • GitHub Check: E2E Tests (pnpm, esbuild)
  • GitHub Check: Build Packages
  • GitHub Check: Lint Packages
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (6)
packages/thirdweb/src/wallets/injected/index.ts (1)

376-394: LGTM!

The parameter rename from chainId to chainIdFilter improves clarity and aligns with the updated signature of toGetCapabilitiesResult. The logic remains unchanged.

apps/wagmi-demo/src/App.tsx (1)

73-78: LGTM! Enhanced wallet support.

The expanded wallet list now includes MetaMask, Coinbase Wallet, and Rainbow in addition to the in-app wallet, providing users with more connection options in the demo.

apps/wagmi-demo/package.json (1)

20-21: Dependency versions verified as available and compatible.

Both viem 2.39.0 and wagmi 2.19.4 exist on the npm registry. wagmi 2.19.4 requires viem 2.x, and 2.39.0 satisfies this constraint. No security advisories were found for either package.

packages/thirdweb/package.json (1)

41-41: viem 2.39.0 is available with no breaking changes or security advisories.

The release includes feature additions (sender and nonce properties to getTransaction) and bug fixes (timeout clearing in waitForUserOperationReceipt), along with chain configuration updates. viem@2.39.0 has been released and no security vulnerabilities were found for the package.

packages/wagmi-adapter/src/connector.ts (2)

3-8: Import additions look correct and align with new usage

The added Chain and ThirdwebClient type imports, plus reuse of defineChain/getAddress, are consistent with the rest of this file and the updated active-chain handling. No issues here.


61-61: Persisting full Chain JSON for activeChainIdKey looks consistent with new reconnect flow

Storing JSON.stringify(defineChain(chain.id)) under activeChainIdKey (Lines 271-274), combined with the JSON parse into Chain in connect (Lines 124-128), is a coherent approach for reusing the same chain object across reconnects and switches. Aside from the getProvider reader inconsistency noted above, this pattern looks solid.

Also applies to: 262-275


function App() {
const account = useAccount();
const twAccount = useActiveAccount();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Remove unused variable.

The twAccount variable is declared but never used in the component.

Apply this diff:

-  const twAccount = useActiveAccount();
-
📝 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.

Suggested change
const twAccount = useActiveAccount();
🤖 Prompt for AI Agents
In apps/wagmi-demo/src/App.tsx around line 21, the local constant `twAccount` is
declared via `useActiveAccount()` but never used; remove that unused variable
declaration (or if the hook is needed for side effects, call
`useActiveAccount()` without assigning it) so the code no longer contains an
unused binding.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 17, 2025

size-limit report 📦

Path Size
@thirdweb-dev/nexus (esm) 104.88 KB (0%)
@thirdweb-dev/nexus (cjs) 316.6 KB (0%)

@codecov
Copy link

codecov bot commented Nov 17, 2025

Codecov Report

❌ Patch coverage is 0% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.84%. Comparing base (f07b4b5) to head (d90a81d).
⚠️ Report is 13 commits behind head on main.

Files with missing lines Patch % Lines
...ckages/thirdweb/src/adapters/eip1193/to-eip1193.ts 0.00% 11 Missing ⚠️
...s/thirdweb/src/wallets/eip5792/get-capabilities.ts 0.00% 4 Missing ⚠️
packages/thirdweb/src/wallets/injected/index.ts 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8425   +/-   ##
=======================================
  Coverage   54.83%   54.84%           
=======================================
  Files         919      919           
  Lines       60844    60865   +21     
  Branches     4134     4143    +9     
=======================================
+ Hits        33364    33379   +15     
- Misses      27378    27384    +6     
  Partials      102      102           
Flag Coverage Δ
packages 54.84% <0.00%> (+<0.01%) ⬆️
Files with missing lines Coverage Δ
packages/thirdweb/src/wallets/injected/index.ts 31.18% <0.00%> (ø)
...s/thirdweb/src/wallets/eip5792/get-capabilities.ts 51.42% <0.00%> (-3.12%) ⬇️
...ckages/thirdweb/src/adapters/eip1193/to-eip1193.ts 50.63% <0.00%> (-3.43%) ⬇️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@joaquim-verges joaquim-verges merged commit 419fb31 into main Nov 17, 2025
20 of 24 checks passed
@joaquim-verges joaquim-verges deleted the Update_viem_to_2.39.0_and_wagmi_to_2.19.4 branch November 17, 2025 23:49
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
packages/wagmi-adapter/src/connector.ts (1)

3-8: Active-chain persistence format is inconsistent between switchChain writer and getProvider reader

The overall direction here (persisting the active chain under thirdweb:active-chain and using it to drive reconnect/auto-connect) makes sense, and the following look good:

  • Using defineChain and Chain type from thirdweb (Lines [3-8]) gives a consistent shape for stored chain data.
  • connect now:
    • Accepts a pre-created wallet via params.wallet (Line [121]).
    • Reads activeChainIdKey and parses it as a Chain JSON payload (Lines [124-129]).
    • Uses lastChainId in both reconnect and first-connect flows (Lines [129], [165-170]).
    • Returns normalized accounts via getAddress (Lines [153], [201-204]).
  • switchChain writes the active chain as JSON.stringify(defineChain(chain.id)) (Lines [271-274]), which matches the JSON-Chain format that connect expects.

However, getProvider still treats activeChainIdKey as a plain numeric id:

  • Line [220] reads lastChainIdStr and Line [221] does Number(lastChainIdStr), assuming the stored value is a stringified number.
  • With the new writer (JSON.stringify(defineChain(chain.id))), Number('{"id":1,...}') is NaN, so lastChainId in getProvider is always undefined for data written by the current code.
  • As a result, the last-chain fallback on Line [223] never actually uses the persisted chain in this path; it silently falls back to params?.chainId || args.smartAccount?.chain?.id || 1.

This is effectively the same mismatch that was flagged previously in an earlier review.

A minimal fix is to align getProvider with the JSON-Chain format used everywhere else and drop the unnecessary await:

-    getProvider: async (params) => {
-      const lastChainIdStr = await rawStorage?.getItem(activeChainIdKey);
-      const lastChainId = lastChainIdStr ? Number(lastChainIdStr) : undefined;
-      const chain = defineChain(
-        params?.chainId || args.smartAccount?.chain?.id || lastChainId || 1,
-      );
+    getProvider: async (params) => {
+      const lastChainIdStr = rawStorage?.getItem(activeChainIdKey);
+      let lastChainId: number | undefined;
+      if (lastChainIdStr) {
+        try {
+          const lastChain = JSON.parse(lastChainIdStr) as Chain;
+          if (lastChain && typeof lastChain.id === "number") {
+            lastChainId = lastChain.id;
+          }
+        } catch {
+          // ignore malformed stored value and fall back to defaults
+        }
+      }
+      const chain = defineChain(
+        params?.chainId || args.smartAccount?.chain?.id || lastChainId || 1,
+      );

This keeps behavior consistent between connect, getProvider, and switchChain, and ensures the “autoconnect to last connected chain” story actually works in both connect and provider paths.

Also applies to: 61-61, 121-129, 153-153, 201-204, 220-223, 271-274

🧹 Nitpick comments (3)
packages/thirdweb/src/wallets/injected/index.ts (1)

377-383: Chain filter wiring looks correct; add coverage for filtered path

Renaming to chainIdFilter on Line [377] and threading it into toGetCapabilitiesResult on Line [383] correctly hook this injected wallet path into the new chain-filtering logic.

Given this now changes behavior when options.chainId is provided (only returning that chain’s capabilities), consider adding a test that:

  • Calls account.getCapabilities({ chainId }) and
  • Asserts that only the requested chain appears in the result.

This will exercise the new filter plumbing and satisfy the patch coverage warnings.

packages/thirdweb/src/adapters/eip1193/to-eip1193.ts (1)

167-177: Harden wallet_getCapabilities params access and chain parsing

The new chains handling is a good addition, but there are a couple of robustness nits:

  • Line [167] assumes request.params is always defined; if a caller omits params, request.params[1] will throw before your “wallet does not support” error can be raised.
  • Lines [170-172] parse the first chain via Number(firstChainStr) without guarding for NaN. You partially mitigate this by checking firstChainId truthiness, but NaN is falsy and may be confusing to future readers.

You could make this safer and clearer as:

-          const chains = request.params[1];
+          const chains = request.params?.[1];

           if (chains && Array.isArray(chains)) {
             const firstChainStr = chains[0];
-            const firstChainId = isHex(firstChainStr)
-              ? hexToNumber(firstChainStr)
-              : Number(firstChainStr);
-            return account.getCapabilities(
-              firstChainId ? { chainId: firstChainId } : {},
-            );
+            const parsed =
+              typeof firstChainStr === "string"
+                ? isHex(firstChainStr)
+                  ? hexToNumber(firstChainStr)
+                  : Number(firstChainStr)
+                : undefined;
+            const firstChainId =
+              typeof parsed === "number" && !Number.isNaN(parsed)
+                ? parsed
+                : undefined;
+            return account.getCapabilities(
+              firstChainId !== undefined ? { chainId: firstChainId } : {},
+            );
           }

This avoids runtime errors on malformed/omitted params while keeping behavior the same for valid calls.

packages/thirdweb/src/wallets/eip5792/get-capabilities.ts (1)

57-57: Avoid returning undefined capability entries when filtering by chain

The new chainIdFilter parameter and conditional on Lines [72-74] correctly introduce per-chain filtering, but there’s a small edge case:

  • If chainIdFilter is provided but result doesn’t contain that chain, capabilities[chainIdFilter] is undefined, so you return { [chainIdFilter]: undefined } (despite the WalletCapabilitiesRecord type).

That’s not a hard bug, but it’s a bit surprising for consumers.

Consider instead:

-  return (
-    typeof chainIdFilter === "number"
-      ? { [chainIdFilter]: capabilities[chainIdFilter] }
-      : capabilities
-  ) as never;
+  if (typeof chainIdFilter === "number") {
+    if (capabilities[chainIdFilter]) {
+      return { [chainIdFilter]: capabilities[chainIdFilter] } as never;
+    }
+    // If the requested chain isn't present, return an empty record
+    return {} as never;
+  }
+  return capabilities as never;

And add a unit test that covers:

  • Filtering to an existing chain.
  • Filtering to a non-existing chain (expects {} or your preferred empty shape).

This makes the API behavior more predictable and addresses the uncovered lines.

Also applies to: 72-74

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 26d008f and d90a81d.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (9)
  • .changeset/few-hats-slide.md (1 hunks)
  • .changeset/sixty-views-relate.md (1 hunks)
  • apps/wagmi-demo/package.json (1 hunks)
  • apps/wagmi-demo/src/App.tsx (2 hunks)
  • packages/thirdweb/package.json (1 hunks)
  • packages/thirdweb/src/adapters/eip1193/to-eip1193.ts (1 hunks)
  • packages/thirdweb/src/wallets/eip5792/get-capabilities.ts (2 hunks)
  • packages/thirdweb/src/wallets/injected/index.ts (1 hunks)
  • packages/wagmi-adapter/src/connector.ts (7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/wagmi-demo/package.json
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: joaquim-verges
Repo: thirdweb-dev/js PR: 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.
📚 Learning: 2025-08-28T20:50:33.170Z
Learnt from: joaquim-verges
Repo: thirdweb-dev/js PR: 7922
File: apps/playground-web/src/app/ai/ai-sdk/components/chat-container.tsx:167-181
Timestamp: 2025-08-28T20:50:33.170Z
Learning: The SignTransactionInput schema in thirdweb-dev/ai-sdk-provider uses snake_case field names (chain_id) rather than camelCase (chainId).

Applied to files:

  • packages/thirdweb/src/wallets/injected/index.ts
  • packages/wagmi-adapter/src/connector.ts
📚 Learning: 2025-06-03T23:44:40.243Z
Learnt from: joaquim-verges
Repo: thirdweb-dev/js PR: 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/wallets/injected/index.ts
  • apps/wagmi-demo/src/App.tsx
  • packages/wagmi-adapter/src/connector.ts
  • packages/thirdweb/src/adapters/eip1193/to-eip1193.ts
📚 Learning: 2025-06-06T23:46:08.795Z
Learnt from: MananTank
Repo: thirdweb-dev/js PR: 7298
File: apps/dashboard/src/app/nebula-app/move-funds/move-funds.tsx:424-424
Timestamp: 2025-06-06T23:46:08.795Z
Learning: The thirdweb project has an ESLint rule that restricts direct usage of `defineChain`. When it's necessary to use `defineChain` directly, it's acceptable to disable the rule with `// eslint-disable-next-line no-restricted-syntax`.

Applied to files:

  • packages/wagmi-adapter/src/connector.ts
🧬 Code graph analysis (3)
packages/thirdweb/src/wallets/injected/index.ts (1)
packages/thirdweb/src/wallets/eip5792/get-capabilities.ts (1)
  • toGetCapabilitiesResult (55-76)
apps/wagmi-demo/src/App.tsx (1)
apps/wagmi-demo/src/wagmi.ts (1)
  • wallet (19-24)
packages/wagmi-adapter/src/connector.ts (1)
apps/wagmi-demo/src/wagmi.ts (2)
  • wallet (19-24)
  • chain (26-26)
🪛 GitHub Check: codecov/patch
packages/thirdweb/src/wallets/injected/index.ts

[warning] 377-377: packages/thirdweb/src/wallets/injected/index.ts#L377
Added line #L377 was not covered by tests


[warning] 383-383: packages/thirdweb/src/wallets/injected/index.ts#L383
Added line #L383 was not covered by tests

packages/thirdweb/src/adapters/eip1193/to-eip1193.ts

[warning] 167-177: packages/thirdweb/src/adapters/eip1193/to-eip1193.ts#L167-L177
Added lines #L167 - L177 were not covered by tests

packages/thirdweb/src/wallets/eip5792/get-capabilities.ts

[warning] 57-57: packages/thirdweb/src/wallets/eip5792/get-capabilities.ts#L57
Added line #L57 was not covered by tests


[warning] 72-74: packages/thirdweb/src/wallets/eip5792/get-capabilities.ts#L72-L74
Added lines #L72 - L74 were not covered by tests

⏰ 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). (4)
  • GitHub Check: E2E Tests (pnpm, webpack)
  • GitHub Check: Size
  • GitHub Check: Lint Packages
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
packages/thirdweb/package.json (1)

41-41: viem 2.39.0 update confirmed as appropriate.

The bump to 2.39.0 includes the EIP-5792 improvements from earlier 2.38.x releases (sendCalls support in 2.38.0 and Trust Wallet fallback in 2.38.1), aligning with the PR's "Better eip5792 handling" objective. 2.39.0 itself adds sender+nonce on getTransaction, a bugfix for the waitForUserOperationReceipt timeout, and new chain entries. The update is stable with no breaking changes.

.changeset/sixty-views-relate.md (1)

1-5: Changeset description matches implementation scope

Patch entry for @thirdweb-dev/wagmi-adapter and its message accurately describe the autoconnect-to-last-chain work in this PR. Nothing to change here.

.changeset/few-hats-slide.md (1)

1-5: Changeset aligns with EIP-5792 tweaks

The patch entry for thirdweb and its description (“Better handling of erc5792 getCapabilities”) accurately reflect the code changes to capability filtering. No changes needed.

apps/wagmi-demo/src/App.tsx (1)

3-3: Demo wallet expansion is consistent with connector API

Importing createWallet (Line [3]) and extending ConnectButton’s wallets prop (Lines [62-67]) to include MetaMask, Coinbase, and Rainbow lines up with the ConnectionOptions branch that accepts a wallet instance. The existing onConnect handler will correctly pass whichever wallet was selected into the in-app connector.

Looks good as-is.

Also applies to: 62-67

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

packages SDK Involves changes to the thirdweb SDK

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants