[SDK] Allow custom chain for SIWE authentication #8365
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 848ae57 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 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 |
github-actions
bot
added
Playground
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. |
WalkthroughSIWE authentication for in-app and ecosystem wallets now accepts an optional chain parameter and uses it when present; otherwise it falls back to mainnet. A playground dependency was upgraded and a changeset was added for the patch release. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Connector as Connector (native / web)
participant SIWE as siweAuthenticate
participant ChainSel as Chain Selection
App->>Connector: authenticate(args { wallet, chain? })
Connector->>SIWE: siweAuthenticate({ wallet, client, ecosystem, chain? })
SIWE->>ChainSel: is chain provided?
alt chain provided
ChainSel-->>SIWE: use provided chain
else no chain
ChainSel-->>SIWE: use getCachedChain(1) (mainnet)
end
SIWE-->>Connector: auth token / cookie
Connector-->>App: authentication result
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Comment |
joaquim-verges
changed the title
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
size-limit report 📦
|
joaquim-verges
force-pushed
the
Allow_custom_chain_for_SIWE_authentication
branch
from
43025c7 to
848ae57
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/thirdweb/src/wallets/in-app/web/lib/web-connector.test.ts (1)
89-112: Consider adding test coverage for optional chain parameter.The PR description indicates that
chainis optional and defaults to mainnet when not provided. Consider adding a test case that omits thechainparameter to verify the fallback behavior:it("should handle wallet authentication without explicit chain (defaults to mainnet)", async () => { vi.mocked(siweAuthenticate).mockResolvedValueOnce(mockAuthToken); const mockWallet = createWalletAdapter({ adaptedAccount: TEST_ACCOUNT_A, chain: ethereum, client: TEST_CLIENT, onDisconnect: () => {}, switchChain: () => {}, }); await connector.connect({ strategy: "wallet", wallet: mockWallet, // chain intentionally omitted }); expect(siweAuthenticate).toHaveBeenCalledWith({ client: TEST_CLIENT, ecosystem: undefined, wallet: mockWallet, // Verify behavior when chain is undefined }); });
📜 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 (7)
.changeset/loose-moose-draw.md(1 hunks)apps/playground-web/package.json(1 hunks)packages/thirdweb/src/wallets/in-app/core/authentication/siwe.ts(2 hunks)packages/thirdweb/src/wallets/in-app/core/authentication/types.ts(1 hunks)packages/thirdweb/src/wallets/in-app/native/native-connector.ts(1 hunks)packages/thirdweb/src/wallets/in-app/web/lib/web-connector.test.ts(1 hunks)packages/thirdweb/src/wallets/in-app/web/lib/web-connector.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/loose-moose-draw.md
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/thirdweb/src/wallets/in-app/native/native-connector.ts
- apps/playground-web/package.json
- packages/thirdweb/src/wallets/in-app/core/authentication/siwe.ts
- packages/thirdweb/src/wallets/in-app/web/lib/web-connector.ts
- packages/thirdweb/src/wallets/in-app/core/authentication/types.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/wallets/in-app/web/lib/web-connector.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Place tests alongside code:foo.ts↔foo.test.ts
Use real function invocations with stub data in tests; avoid brittle mocks
Use Mock Service Worker (MSW) for fetch/HTTP call interception in tests
Keep tests deterministic and side-effect free
UseFORKED_ETHEREUM_CHAINfor mainnet interactions andANVIL_CHAINfor isolated tests
**/*.test.{ts,tsx}: Co‑locate tests asfoo.test.ts(x)next to the implementation
Use real function invocations with stub data; avoid brittle mocks
Use MSW to intercept HTTP calls for network interactions; mock only hard‑to‑reproduce scenarios
Keep tests deterministic and side‑effect free; use Vitest
Files:
packages/thirdweb/src/wallets/in-app/web/lib/web-connector.test.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/wallets/in-app/web/lib/web-connector.test.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/in-app/web/lib/web-connector.test.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/wallets/in-app/web/lib/web-connector.test.ts
🧠 Learnings (7)
📓 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.
Learnt from: CR
Repo: thirdweb-dev/js PR: 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
Learnt from: CR
Repo: thirdweb-dev/js PR: 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)
📚 Learning: 2025-07-18T19:19:55.613Z
Learnt from: CR
Repo: thirdweb-dev/js PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to test/src/test-wallets.ts : Predefined test accounts are in `test/src/test-wallets.ts`
Applied to files:
packages/thirdweb/src/wallets/in-app/web/lib/web-connector.test.ts
📚 Learning: 2025-07-18T19:19:55.613Z
Learnt from: CR
Repo: thirdweb-dev/js PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to **/*.test.{ts,tsx} : Use `FORKED_ETHEREUM_CHAIN` for mainnet interactions and `ANVIL_CHAIN` for isolated tests
Applied to files:
packages/thirdweb/src/wallets/in-app/web/lib/web-connector.test.ts
📚 Learning: 2025-07-18T19:19:55.613Z
Learnt from: CR
Repo: thirdweb-dev/js PR: 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:
packages/thirdweb/src/wallets/in-app/web/lib/web-connector.test.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/in-app/web/lib/web-connector.test.ts
📚 Learning: 2025-07-18T19:19:55.613Z
Learnt from: CR
Repo: thirdweb-dev/js PR: 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:
packages/thirdweb/src/wallets/in-app/web/lib/web-connector.test.ts
📚 Learning: 2025-07-18T19:19:55.613Z
Learnt from: CR
Repo: thirdweb-dev/js PR: 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/in-app/web/lib/web-connector.test.ts
🧬 Code graph analysis (1)
packages/thirdweb/src/wallets/in-app/web/lib/web-connector.test.ts (1)
packages/thirdweb/src/exports/chains.ts (1)
ethereum(33-33)
⏰ 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: E2E Tests (pnpm, webpack)
- GitHub Check: Size
- GitHub Check: Build Packages
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Unit Tests
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
packages/thirdweb/src/wallets/in-app/web/lib/web-connector.test.ts (1)
106-111: LGTM! Test correctly verifies chain propagation.The assertion now properly validates that the
chainparameter passed toconnector.connect()is propagated tosiweAuthenticate().
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (33.33%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #8365 +/- ##
=======================================
Coverage 54.64% 54.64%
=======================================
Files 919 919
Lines 60692 60693 +1
Branches 4115 4110 -5
=======================================
+ Hits 33164 33167 +3
+ Misses 27426 27424 -2
Partials 102 102
🚀 New features to boost your workflow:
|
PR-Codex overview
This PR focuses on enhancing the
siweAuthenticatefunction to respect the passedchainwhen performing SIWE (Sign-In with Ethereum) for in-app and ecosystem wallets. It also updates dependencies for better compatibility.Detailed summary
siweAuthenticateto accept an optionalchainparameter.chainor default to mainnet.chainparameter.@abstract-foundation/agw-reactand@abstract-foundation/agw-clientdependencies to version1.10.0.viemdependency version to2.33.3.Summary by CodeRabbit
Bug Fixes
Dependencies