-
Notifications
You must be signed in to change notification settings - Fork 5
InAppWallet EIP7702 Session Key Integration #150
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
WalkthroughThe changes activate and implement EIP-7702 session key functionality in the wallet code. This includes enabling previously commented-out logic in the console program, updating the minimal account contract address constant and ABI, and fully implementing the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Program
participant EcosystemWallet
participant EthereumNetwork
User->>Program: Start EIP-7702 demo
Program->>EcosystemWallet: Initialize with Guest Auth & EIP-7702 Sponsored mode
Program->>EcosystemWallet: Check if connected
alt Not connected
Program->>EcosystemWallet: Guest login with session ID override
end
Program->>EcosystemWallet: Get wallet address
Program->>EthereumNetwork: Send zero-wei transfer to vitalik.eth (triggers upgrade)
Program->>EcosystemWallet: Check if account is delegated
Program->>EcosystemWallet: CreateSessionKey(chainId, signerAddress, duration, permissions)
EcosystemWallet->>EcosystemWallet: Validate execution mode
EcosystemWallet->>EcosystemWallet: Generate session key signature (EIP-712)
EcosystemWallet->>EthereumNetwork: Simulate createSessionWithSig call
EcosystemWallet->>EthereumNetwork: Execute createSessionWithSig transaction
EcosystemWallet->>Program: Return transaction receipt
Program->>User: Print transaction hash
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (4)
✨ 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 (
|
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: 0
🧹 Nitpick comments (2)
Thirdweb/Thirdweb.Wallets/InAppWallet/EcosystemWallet/EcosystemWallet.cs (1)
453-470
: Well-implemented session key creation with proper validation and simulation.The
CreateSessionKey
method implementation is solid with good practices:✅ Strengths:
- Proper execution mode validation (EIP7702/EIP7702Sponsored only)
- EIP-712 signature generation for security
- Transaction simulation before execution for safety
- Appropriate error handling with descriptive exception message
Minor suggestions for improvement:
Consider adding parameter validation for the
SessionSpec
:public async Task<ThirdwebTransactionReceipt> CreateSessionKey(BigInteger chainId, SessionSpec sessionKeyParams) { + if (sessionKeyParams == null) + { + throw new ArgumentNullException(nameof(sessionKeyParams)); + } + if (chainId <= 0) + { + throw new ArgumentException("Chain ID must be greater than 0.", nameof(chainId)); + } + if (this.ExecutionMode is not ExecutionMode.EIP7702 and not ExecutionMode.EIP7702Sponsored) { throw new InvalidOperationException("CreateSessionKey is only supported for EIP7702 and EIP7702Sponsored execution modes."); }Also consider removing the console logging on line 467 in production code or making it configurable:
- Console.WriteLine($"Simulated session key creation transaction: {result}"); + // Consider using a proper logging framework instead of Console.WriteLineThirdweb.Console/Program.cs (1)
363-405
: Good demonstration of EIP-7702 session key functionality with proper workflow.The activated console code demonstrates the full EIP-7702 workflow effectively:
✅ Strengths:
- Proper chain setup (Sepolia testnet)
- Correct execution mode (EIP7702Sponsored)
- Logical workflow: connect → transfer (to trigger upgrade) → verify delegation → create session key
- Good use of ENS resolution for addresses
- Proper session key configuration with expiration and policies
Consider these improvements:
- Add error handling around OAuth login:
if (!await smartEoa.IsConnected()) { - _ = await smartEoa.LoginWithOauth( - isMobile: false, - (url) => - { - var psi = new ProcessStartInfo { FileName = url, UseShellExecute = true }; - _ = Process.Start(psi); - } - ); + try + { + _ = await smartEoa.LoginWithOauth( + isMobile: false, + (url) => + { + var psi = new ProcessStartInfo { FileName = url, UseShellExecute = true }; + _ = Process.Start(psi); + } + ); + } + catch (Exception ex) + { + Console.WriteLine($"OAuth login failed: {ex.Message}"); + return; + } }
- Consider making the session key UID more unique:
- Uid = "my-session-key-uid".HashMessage().HexToBytes() + Uid = $"session-{DateTime.UtcNow:yyyyMMddHHmmss}-{Guid.NewGuid()}".HashMessage().HexToBytes()
- Add validation for ENS resolution:
+var vitalikAddress = await Utils.GetAddressFromENS(client, "vitalik.eth"); +if (string.IsNullOrEmpty(vitalikAddress)) +{ + Console.WriteLine("Failed to resolve vitalik.eth ENS name"); + return; +} + -var receipt = await smartEoa.Transfer(chainId: chain, toAddress: await Utils.GetAddressFromENS(client, "vitalik.eth"), weiAmount: 0); +var receipt = await smartEoa.Transfer(chainId: chain, toAddress: vitalikAddress, weiAmount: 0);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Thirdweb.Console/Program.cs
(1 hunks)Thirdweb/Thirdweb.Utils/Constants.cs
(1 hunks)Thirdweb/Thirdweb.Wallets/InAppWallet/EcosystemWallet/EcosystemWallet.cs
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-test-cov
🔇 Additional comments (1)
Thirdweb/Thirdweb.Utils/Constants.cs (1)
38-38
: Verify updated minimal account contract address.Please confirm that the new address matches the deployed EIP-7702 minimal account contract on your target network and is correctly checksummed (EIP-55). Update any related documentation or tests to reference this value.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #150 +/- ##
==========================================
- Coverage 60.94% 60.84% -0.11%
==========================================
Files 41 41
Lines 6563 6563
Branches 832 832
==========================================
- Hits 4000 3993 -7
- Misses 2376 2382 +6
- Partials 187 188 +1 ☔ View full report in Codecov by Sentry. 🚀 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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Thirdweb.Console/Program.cs
(2 hunks)Thirdweb/Thirdweb.Wallets/InAppWallet/EcosystemWallet/EcosystemWallet.cs
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Thirdweb.Console/Program.cs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-test-cov
🔇 Additional comments (3)
Thirdweb/Thirdweb.Wallets/InAppWallet/EcosystemWallet/EcosystemWallet.cs (3)
453-461
: Method signature looks comprehensive and well-structured.The asynchronous method signature properly defines all necessary parameters with sensible defaults. The return type
Task<ThirdwebTransactionReceipt>
appropriately indicates the async nature and transaction result.
463-466
: Good execution mode validation with clear error messaging.The validation correctly restricts session key creation to EIP-7702 compatible execution modes and provides a clear error message explaining the requirement.
478-483
: Verify contract method existence and EIP-712 domain parameters.The implementation logic is well-structured, but the contract method call and signature parameters need verification to ensure compatibility.
Run the following script to verify the contract ABI contains the expected method:
#!/bin/bash # Description: Verify that MINIMAL_ACCOUNT_7702_ABI contains the createSessionWithSig method # Search for the createSessionWithSig method in the ABI constant rg -A 10 -B 5 "createSessionWithSig" --type csAlso verify that the EIP-712 domain parameters ("MinimalAccount", "1") match the actual contract implementation. The domain name and version must be consistent with the deployed contract to ensure signature validation succeeds.
PR-Codex overview
This PR focuses on enhancing the
CreateSessionKey
functionality for EIP-7702 execution modes, updating connection methods, and refining constants related to minimal accounts.Detailed summary
Thirdweb.Console/Program.cs
to clarify chain compatibility.authProvider
fromGoogle
toGuest
inInAppWallet.Create
.CreateSessionKey
method signature and implementation inEcosystemWallet.cs
.CreateSessionKey
.MINIMAL_ACCOUNT_7702
constant inConstants.cs
.Summary by CodeRabbit
New Features
Bug Fixes
Improvements