Add dynamic delegation contract support for EIP-7702 #156
Conversation
Replaces the hardcoded MINIMAL_ACCOUNT_7702 address with dynamic retrieval of the delegation contract address from the bundler. Updates EcosystemWallet and related utility methods to use the fetched delegation contract, improving flexibility and compatibility with different delegation contract deployments.
WalkthroughRemoved the hardcoded MINIMAL_ACCOUNT_7702 constant and updated delegation detection to accept a delegation contract address. EcosystemWallet fetches and stores a delegation contract via a new bundler RPC, and that address is propagated through authorization, execution, and delegation checks. A DTO and bundler client method were added. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as Console/App
participant BC as BundlerClient
participant EW as EcosystemWallet
participant RPC as RPC_Node
CLI->>BC: TwGetDelegationContract(client, url, requestId)
BC-->>CLI: TwGetDelegationContractResponse(delegationContract)
CLI->>EW: new EcosystemWallet(..., executionMode, delegationContractAddress)
rect rgba(230,245,255,0.6)
note over EW: Delegation-aware code lookup
EW->>RPC: eth_getCode(address)
RPC-->>EW: bytecode
EW->>EW: IsDelegatedAccount(..., delegationContractAddress)
end
rect rgba(235,255,235,0.6)
note over EW: Authorization & execution with delegation contract
EW->>EW: SignAuthorization(target=delegationContractAddress)
EW->>BC: TwExecute(..., delegationContractAddress)
BC-->>EW: Execute result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Updated the InAppWallet constructor and related method to accept and pass delegationContractAddress, ensuring compatibility with changes in the base EcosystemWallet class.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Thirdweb/Thirdweb.Wallets/InAppWallet/EcosystemWallet/EcosystemWallet.cs (2)
1299-1306: Avoid double eth_getCode and handle null delegation contract via lazy fetch.Currently you:
- Call IsDelegatedAccount once here, and again inside the TwExecute call.
- Depend on DelegationContractAddress possibly being null.
Refactor to resolve the contract per chain once, reuse the result, and avoid the redundant RPC.
Apply this diff:
- var userContract = await ThirdwebContract.Create(this.Client, userWalletAddress, transaction.ChainId, Constants.MINIMAL_ACCOUNT_7702_ABI); - var needsDelegation = !await Utils.IsDelegatedAccount(this.Client, transaction.ChainId, userWalletAddress, this.DelegationContractAddress); - EIP7702Authorization? authorization = needsDelegation - ? await this.SignAuthorization(transaction.ChainId, this.DelegationContractAddress, willSelfExecute: this.ExecutionMode != ExecutionMode.EIP7702Sponsored) + var userContract = await ThirdwebContract.Create(this.Client, userWalletAddress, transaction.ChainId, Constants.MINIMAL_ACCOUNT_7702_ABI).ConfigureAwait(false); + var delegationContract = this.DelegationContractAddress ?? await this.GetDelegationContractAddress(transaction.ChainId).ConfigureAwait(false); + var needsDelegation = !await Utils.IsDelegatedAccount(this.Client, transaction.ChainId, userWalletAddress, delegationContract).ConfigureAwait(false); + EIP7702Authorization? authorization = needsDelegation + ? await this.SignAuthorization(transaction.ChainId, delegationContract, willSelfExecute: this.ExecutionMode != ExecutionMode.EIP7702Sponsored).ConfigureAwait(false) : null;
1482-1489: Ensure delegation verification uses chain-aware address.
Ensure7702should not rely on a possibly-null singleton address. Resolve per chain and reuse.Apply this diff:
- if (!await Utils.IsDelegatedAccount(this.Client, chainId, this.Address, this.DelegationContractAddress).ConfigureAwait(false)) + { + var delegationContract = this.DelegationContractAddress ?? await this.GetDelegationContractAddress(chainId).ConfigureAwait(false); + if (!await Utils.IsDelegatedAccount(this.Client, chainId, this.Address, delegationContract).ConfigureAwait(false)) { if (ensureDelegated) { throw new InvalidOperationException("This operation requires a delegated account. Please ensure you have transacted at least once with the account to set up delegation."); } } - } + }
🧹 Nitpick comments (5)
Thirdweb/Thirdweb.Wallets/SmartWallet/Thirdweb.AccountAbstraction/AATypes.cs (1)
243-247: DTO shape looks fine; suggest asserting presence and documenting units.
- Consider marking the property as required to fail fast if the bundler changes shape or omits the field.
- Minor: add an XML doc comment clarifying that the value is a 20-byte hex Ethereum address with 0x prefix.
Apply this diff to make the JSON contract explicit:
public class TwGetDelegationContractResponse { - [JsonProperty("delegationContract")] + [JsonProperty("delegationContract", Required = Required.Always)] public string DelegationContract { get; set; } }Thirdweb/Thirdweb.Wallets/InAppWallet/EcosystemWallet/EcosystemWallet.cs (4)
64-66: Make delegationContractAddress optional and normalize.Constructor forces a delegation contract up-front, but you may not know the target chain at creation. Accept null and normalize if provided.
Apply this diff:
- ExecutionMode executionMode, - string delegationContractAddress + ExecutionMode executionMode, + string delegationContractAddress ) @@ - this.DelegationContractAddress = delegationContractAddress; + this.DelegationContractAddress = string.IsNullOrEmpty(delegationContractAddress) ? null : delegationContractAddress.ToChecksumAddress();
200-202: Pass-through constructor arg after lazy retrieval change.Update to pass the local variable (currently null) instead of an early-fetched value.
Apply this diff:
- executionMode, - delegationContractResponse.DelegationContract + executionMode, + delegationContractAddress
223-224: Same as above: pass the local (null) and resolve lazily later.Apply this diff:
- executionMode, - delegationContractResponse.DelegationContract + executionMode, + delegationContractAddress
1339-1347: Reuse needsDelegation instead of re-checking chain state.You already computed
needsDelegation. Re-checkingIsDelegatedAccounthere adds an unnecessary extra RPC.Apply this diff:
- authorization: authorization != null && !await Utils.IsDelegatedAccount(this.Client, transaction.ChainId, userWalletAddress, this.DelegationContractAddress) ? authorization : null + authorization: needsDelegation ? authorization : null
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
Thirdweb/Thirdweb.Utils/Constants.cs(0 hunks)Thirdweb/Thirdweb.Utils/Utils.cs(1 hunks)Thirdweb/Thirdweb.Wallets/InAppWallet/EcosystemWallet/EcosystemWallet.cs(9 hunks)Thirdweb/Thirdweb.Wallets/SmartWallet/Thirdweb.AccountAbstraction/AATypes.cs(1 hunks)Thirdweb/Thirdweb.Wallets/SmartWallet/Thirdweb.AccountAbstraction/BundlerClient.cs(1 hunks)
💤 Files with no reviewable changes (1)
- Thirdweb/Thirdweb.Utils/Constants.cs
🧰 Additional context used
🧬 Code graph analysis (2)
Thirdweb/Thirdweb.Wallets/SmartWallet/Thirdweb.AccountAbstraction/BundlerClient.cs (2)
Thirdweb/Thirdweb.Wallets/SmartWallet/Thirdweb.AccountAbstraction/AATypes.cs (1)
TwGetDelegationContractResponse(243-247)Thirdweb/Thirdweb.Client/ThirdwebClient.cs (3)
ThirdwebClient(11-115)ThirdwebClient(34-84)ThirdwebClient(100-114)
Thirdweb/Thirdweb.Wallets/InAppWallet/EcosystemWallet/EcosystemWallet.cs (2)
Thirdweb/Thirdweb.Wallets/SmartWallet/Thirdweb.AccountAbstraction/BundlerClient.cs (1)
BundlerClient(6-147)Thirdweb/Thirdweb.Utils/Utils.cs (1)
Utils(27-1341)
🔇 Additional comments (1)
Thirdweb/Thirdweb.Wallets/SmartWallet/Thirdweb.AccountAbstraction/BundlerClient.cs (1)
9-13: Confirm Bundler Response Shape fortw_getDelegationContractPlease verify in the Thirdweb Bundler documentation or sample responses whether the
tw_getDelegationContractmethod can ever return:
- A null
result(in which case we should throw an error),- A plain JSON string (e.g.
"0x123..."),- Or an object with a
delegationContractfield (e.g.{ "delegationContract": "0x123..." }).If it does support both string and object shapes, apply the following patch to Thirdweb/Thirdweb.Wallets/SmartWallet/Thirdweb.AccountAbstraction/BundlerClient.cs (method
TwGetDelegationContract):--- a/Thirdweb/Thirdweb.Wallets/SmartWallet/Thirdweb.AccountAbstraction/BundlerClient.cs +++ b/Thirdweb/Thirdweb.Wallets/SmartWallet/Thirdweb.AccountAbstraction/BundlerClient.cs @@ public static async Task<TwGetDelegationContractResponse> TwGetDelegationContract(ThirdwebClient client, string url, int requestId) - var response = await BundlerRequest(client, url, requestId, "tw_getDelegationContract").ConfigureAwait(false); - return JsonConvert.DeserializeObject<TwGetDelegationContractResponse>(response.Result.ToString()); + var response = await BundlerRequest(client, url, requestId, "tw_getDelegationContract").ConfigureAwait(false); + if (response.Result == null) + { + throw new Exception("Bundler Request Failed. tw_getDelegationContract returned null result."); + } + // Handle both: result: "<addr>" or result: { delegationContract: "<addr>" } + if (response.Result is string addr) + { + return new TwGetDelegationContractResponse { DelegationContract = addr }; + } + return JsonConvert.DeserializeObject<TwGetDelegationContractResponse>(response.Result.ToString());• File:
Thirdweb/Thirdweb.Wallets/SmartWallet/Thirdweb.AccountAbstraction/BundlerClient.cs
• Method:TwGetDelegationContractOnce you’ve confirmed the exact JSON shape, please update or remove the alternate-handling logic as needed.
Thirdweb/Thirdweb.Wallets/InAppWallet/EcosystemWallet/EcosystemWallet.cs
Show resolved
Hide resolved
Thirdweb/Thirdweb.Wallets/InAppWallet/EcosystemWallet/EcosystemWallet.cs
Show resolved
Hide resolved
Thirdweb/Thirdweb.Wallets/InAppWallet/EcosystemWallet/EcosystemWallet.cs
Show resolved
Hide resolved
Replaces the commented-out call to Utils.IsDelegated with Utils.IsDeployed for checking the deployment status of the account. This improves clarity and accuracy in the code comments.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
Thirdweb.Console/Program.cs (1)
410-413: Rename variable/log to reflect IsDeployed semantics (not delegation).You switched to Utils.IsDeployed but kept variable/logging names as “isDelegated,” which is misleading. Recommend renaming to “isDeployed” for clarity in this sample.
Apply:
-// var isDelegated = await Utils.IsDeployed(client, chain, smartEoaAddress); -// Console.WriteLine($"Is delegated: {isDelegated}"); +// var isDeployed = await Utils.IsDeployed(client, chain, smartEoaAddress); +// Console.WriteLine($"Is deployed: {isDeployed}");Optional: If you actually want to assert delegation to a specific 7702 delegation contract (now dynamic), consider the explicit check:
-// var isDeployed = await Utils.IsDeployed(client, chain, smartEoaAddress); +// var isDelegated = await Utils.IsDelegatedAccount(client, chain, smartEoaAddress, smartEoa.DelegationContractAddress); +// Console.WriteLine($"Is delegated: {isDelegated}");Thirdweb/Thirdweb.Wallets/InAppWallet/InAppWallet.cs (2)
23-27: Make delegationContractAddress optional and document null/empty handling.Constructor signature change is internal, but making the new parameter optional reduces blast radius and clarifies intent. If base safely handles null, this keeps behavior stable for any un-updated callers.
- ExecutionMode executionMode, - string delegationContractAddress + ExecutionMode executionMode, + string delegationContractAddress = null ) - : base(null, null, client, embeddedWallet, httpClient, email, phoneNumber, authProvider, siweSigner, legacyEncryptionKey, walletSecret, executionMode, delegationContractAddress) + : base( + null, + null, + client, + embeddedWallet, + httpClient, + email, + phoneNumber, + authProvider, + siweSigner, + legacyEncryptionKey, + walletSecret, + executionMode, + delegationContractAddress + )If null/empty is not acceptable by the base, add a guard here and fail fast with a descriptive exception.
59-61: Qualify static Create call for clarity and to avoid accidental self-calls.The unqualified Create(...) relies on overload resolution to pick EcosystemWallet.Create (because of arg count). Qualifying improves readability and future-proofing if this class gains overloads.
- var ecoWallet = await Create(client, null, null, email, phoneNumber, authProvider, storageDirectoryPath, siweSigner, legacyEncryptionKey, walletSecret, twAuthTokenOverride, executionMode); + var ecoWallet = await EcosystemWallet.Create( + client, + null, // ecosystemId + null, // domain + email, + phoneNumber, + authProvider, + storageDirectoryPath, + siweSigner, + legacyEncryptionKey, + walletSecret, + twAuthTokenOverride, + executionMode + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
Thirdweb.Console/Program.cs(1 hunks)Thirdweb/Thirdweb.Wallets/InAppWallet/InAppWallet.cs(2 hunks)
⏰ 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). (1)
- GitHub Check: build-test-cov
🔇 Additional comments (2)
Thirdweb/Thirdweb.Wallets/InAppWallet/InAppWallet.cs (2)
72-74: Good propagation of DelegationContractAddress.Passing through ecoWallet.ExecutionMode and ecoWallet.DelegationContractAddress ensures the in-app wallet carries the dynamic delegation contract required for EIP-7702 flows.
23-27: All delegation changes verified and up to date.
- Only InAppWallet instantiation is in Thirdweb/Thirdweb.Wallets/InAppWallet/InAppWallet.cs (lines 61–74) and it now passes ecoWallet.DelegationContractAddress.
- Every Utils.IsDelegatedAccount call in Thirdweb/Thirdweb.Wallets/InAppWallet/EcosystemWallet/EcosystemWallet.cs (lines 1303, 1346, 1482) includes the new delegationContractAddress argument.
- The EcosystemWallet constructor (lines 63–81) correctly assigns this.DelegationContractAddress before use.
No stale usages or missing parameters remain.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
Thirdweb/Thirdweb.Utils/Constants.cs(1 hunks)
⏰ 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). (1)
- GitHub Check: build-test-cov
Replaces the hardcoded MINIMAL_ACCOUNT_7702 address with dynamic retrieval of the delegation contract address from the bundler. Updates EcosystemWallet and related utility methods to use the fetched delegation contract, improving flexibility and compatibility with different delegation contract deployments.
PR-Codex overview
This PR introduces a new class for handling delegation contract responses and modifies several methods to accommodate delegation contract addresses in various wallet operations.
Detailed summary
TwGetDelegationContractResponseclass to handle delegation contract responses.TwGetDelegationContractmethod to return the new response type.IsDelegatedAccountmethod to accept adelegationContractparameter.InAppWalletandEcosystemWalletto includedelegationContractAddress.IsDelegatedAccountto use the new delegation contract address parameter throughout the codebase.Summary by CodeRabbit
New Features
Refactor
Bug Fixes