-
Notifications
You must be signed in to change notification settings - Fork 557
[SDK] EIP-7702 Session Keys #7432
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
Conversation
🦋 Changeset detectedLatest commit: e392604 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 |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
""" WalkthroughThis update introduces session key support for ERC-7702-powered accounts in the TypeScript SDK. It adds session key creation functionality, associated type definitions, EIP-712 struct representations, and public exports. A test suite verifies session key creation with both full and granular permissions. The minimal account implementation address is also updated. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SDK (createSessionKey)
participant AdminAccount
participant Contract
User->>SDK (createSessionKey): Call with session key options
SDK->>AdminAccount: Prepare session spec, request EIP-712 signature
AdminAccount-->>SDK: Return signature
SDK->>Contract: Call createSessionWithSig with spec and signature
Contract-->>SDK: Emit SessionCreated event
SDK-->>User: Return transaction result
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (8)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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. |
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.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/thirdweb/src/extensions/erc7702/account/sessionkey.test.ts (2)
80-122
: Consider more realistic test data for granular permissions test.The second test uses
ZERO_ADDRESS
and zero values for limits, which may not effectively test the granular permissions functionality. Consider using more realistic values that would demonstrate meaningful constraints.callPolicies: [ { - target: ZERO_ADDRESS, - selector: "0x00000000", - maxValuePerUse: 0n, + target: "0xA0b86a33E6411E3037E4827c80F7b79289F7F1C1", // Example ERC20 contract + selector: "0xa9059cbb", // transfer(address,uint256) + maxValuePerUse: 1000000000000000000n, // 1 ETH max per use valueLimit: { - limitType: 0, - limit: 0n, - period: 0n, + limitType: 1, + limit: 5000000000000000000n, // 5 ETH total limit + period: 86400n, // 24 hours }, constraints: [], }, ],
86-86
: Avoid session key address conflicts between tests.Both tests use the same session key address (
TEST_ACCOUNT_A.address
). Consider using different addresses to avoid potential conflicts, especially if tests run in parallel or if the session keys persist between test runs.- sessionKeyAddress: TEST_ACCOUNT_A.address, + sessionKeyAddress: TEST_ACCOUNT_B.address,packages/thirdweb/src/extensions/erc7702/account/createSessionKey.ts (1)
114-128
: Fix function name inconsistency in documentation.The JSDoc comment references
isAddSessionKeySupported
but the actual function name isisCreateSessionKeySupported
.* @returns A boolean indicating if the `isAddSessionKeySupported` method is supported. +* @returns A boolean indicating if the `isCreateSessionKeySupported` method is supported.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/thirdweb/src/extensions/erc7702/account/createSessionKey.ts
(1 hunks)packages/thirdweb/src/extensions/erc7702/account/sessionkey.test.ts
(1 hunks)packages/thirdweb/src/extensions/erc7702/account/types.ts
(1 hunks)packages/thirdweb/src/wallets/in-app/core/eip7702/minimal-account.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.@(ts|tsx)`: Accept a typed 'props' object and export a named function (e.g., export function MyComponent()). Combine class names via 'cn', expose 'className' prop if useful. ...
**/*.@(ts|tsx)
: Accept a typed 'props' object and export a named function (e.g., export function MyComponent()).
Combine class names via 'cn', expose 'className' prop if useful.
Reuse core UI primitives; avoid re-implementing buttons, cards, modals.
Local state or effects live inside; data fetching happens in hooks.
Merge class names with 'cn' from '@/lib/utils' to keep conditional logic readable.
Stick to design-tokens: background ('bg-card'), borders ('border-border'), muted text ('text-muted-foreground') etc.
Use the 'container' class with a 'max-w-7xl' cap for page width consistency.
Spacing utilities ('px-', 'py-', 'gap-*') are preferred over custom margins.
Responsive helpers follow mobile-first ('max-sm', 'md', 'lg', 'xl').
Never hard-code colors – always go through Tailwind variables.
Tailwind CSS is the styling system – avoid inline styles or CSS modules.
Prefix files with 'import "server-only";' so they never end up in the client bundle (for server-only code).
packages/thirdweb/src/wallets/in-app/core/eip7702/minimal-account.ts
packages/thirdweb/src/extensions/erc7702/account/createSessionKey.ts
packages/thirdweb/src/extensions/erc7702/account/sessionkey.test.ts
packages/thirdweb/src/extensions/erc7702/account/types.ts
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Build Packages
- GitHub Check: Unit Tests
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
packages/thirdweb/src/extensions/erc7702/account/types.ts (1)
1-85
: LGTM! Well-structured type definitions.The TypeScript interfaces and EIP-712 struct definitions are well-organized and follow good practices. The separation between input types and EIP-712 structs is clean, and the type safety with
0x${string}
for hex addresses is appropriate.packages/thirdweb/src/wallets/in-app/core/eip7702/minimal-account.ts (1)
26-27
: Verify this breaking change to the implementation address.Updating the
MINIMAL_ACCOUNT_IMPLEMENTATION_ADDRESS
is a significant change that could affect existing deployments and account verification. Ensure this change is coordinated with:
- Contract deployments on target networks
- Existing user accounts that may reference the old implementation
- Any dependent services or applications
#!/bin/bash # Description: Search for any hardcoded references to the old implementation address # Expected: Find any remaining references that need updating # Search for the old address in the codebase rg -i "0xbaC7e770af15d130Cd72838ff386f14FBF3e9a3D" --type ts --type js --type json # Search for any other references to minimal account implementation addresses rg -i "MINIMAL_ACCOUNT_IMPLEMENTATION" --type ts --type js -A 2 -B 2packages/thirdweb/src/extensions/erc7702/account/sessionkey.test.ts (1)
37-41
: Fix syntax error: missing semicolon.There's a missing semicolon after the wallet connection assignment.
account = await wallet.connect({ chain: defineChain(chainId), client: TEST_CLIENT, strategy: "guest", - }) + });Likely an incorrect or invalid review comment.
packages/thirdweb/src/extensions/erc7702/account/createSessionKey.ts
Outdated
Show resolved
Hide resolved
size-limit report 📦
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7432 +/- ##
==========================================
- Coverage 52.08% 51.99% -0.10%
==========================================
Files 947 949 +2
Lines 63636 64081 +445
Branches 4216 4229 +13
==========================================
+ Hits 33146 33318 +172
- Misses 30384 30656 +272
- Partials 106 107 +1
🚀 New features to boost your workflow:
|
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.
looks good! just needs a lint
biome needs to go |
Closes TOOL-4444
PR-Codex overview
This PR introduces Session Keys to EIP-7702-powered In-App Wallets, enhancing transaction capabilities by allowing specific permissions and time-limited access for session keys.
Detailed summary
MINIMAL_ACCOUNT_IMPLEMENTATION_ADDRESS
.createSessionKey
extension and related types.LimitType
andCondition
enums.UsageLimitInput
,ConstraintInput
,CallSpecInput
, andTransferSpecInput
.UsageLimitRequest
,ConstraintRequest
,CallSpecRequest
,TransferSpecRequest
, andSessionSpecRequest
.createSessionKey
function with validation and structured request creation.Summary by CodeRabbit
New Features
Tests
Chores