-
Notifications
You must be signed in to change notification settings - Fork 3
SDK upgrade with the ModularEtherspotWalletFactory contracts (on all chains) with latest IHook #65
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
- Updated factory, bootstrap and MOECDSA validator contract addresses. - Updated CHANGLOG.md with changes. - Bumped version from 5.1.2 to 6.0.0.
WalkthroughThis update introduces new contract addresses for Wallet Factory, Bootstrap, and Multiple Owner ECDSA Validator across all supported networks, and adds support for a new HookMultiPlexer contract. The SDK and examples are updated to utilize these addresses, and two new example scripts demonstrate module installation and initialization checks. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ExampleScript
participant ModularSdk
participant Network
User->>ExampleScript: Run install-hook.ts
ExampleScript->>ModularSdk: Initialize SDK with config
ModularSdk->>Network: Fetch contract addresses (including HookMultiPlexer)
ExampleScript->>ModularSdk: Prepare installModule calldata
ExampleScript->>ModularSdk: Add user operation to batch
ExampleScript->>ModularSdk: Estimate user operation
ExampleScript->>ModularSdk: Send user operation
ModularSdk->>Network: Submit transaction
Network-->>ModularSdk: Return transaction hash
ModularSdk-->>ExampleScript: Provide transaction hash
ExampleScript->>ModularSdk: Poll for receipt
ModularSdk->>Network: Query for receipt
Network-->>ModularSdk: Return receipt
ModularSdk-->>ExampleScript: Provide receipt
ExampleScript-->>User: Log transaction outcome
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
examples/modules/install-hook.tsOops! Something went wrong! :( ESLint: 8.28.0 ReferenceError: Cannot read config file: /.eslintrc.js examples/modules/is-module-initialised.tsOops! Something went wrong! :( ESLint: 8.28.0 ReferenceError: Cannot read config file: /.eslintrc.js examples/basics/transfer-erc20.tsOops! Something went wrong! :( ESLint: 8.28.0 ReferenceError: Cannot read config file: /.eslintrc.js
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
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.
LGTM
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: 2
🧹 Nitpick comments (3)
examples/basics/transfer-erc20.ts (1)
16-16
: Consider making the token address configurable while maintaining usability.The hardcoded token address makes the example work out-of-the-box, but reduces flexibility. Consider using a fallback pattern to maintain both usability and configurability.
Apply this diff to make it more flexible:
-const tokenAddress = '0xDeDf3B40f8c44b1fF195F37F35c6b8199C7ee443'; // token address +const tokenAddress = process.env.TOKEN_ADDRESS || '0xDeDf3B40f8c44b1fF195F37F35c6b8199C7ee443'; // token addressThis allows users to override the token address via environment variable while providing a working default.
examples/modules/install-hook.ts (2)
12-13
: Consider making configuration more flexible.The hard-coded chain ID and hook multiplexer address could benefit from environment variable configuration for better flexibility across different environments.
- const CHAIN_ID = '1'; - const HOOK_MULTIPLEXER_ADDRESS = '0xDcA918dd23456d321282DF9507F6C09A50522136'; + const CHAIN_ID = process.env.CHAIN_ID || '1'; + const HOOK_MULTIPLEXER_ADDRESS = process.env.HOOK_MULTIPLEXER_ADDRESS || '0xDcA918dd23456d321282DF9507F6C09A50522136';
60-60
: Fix timeout duration comment.The comment indicates "1 minute timeout" but the code sets 1200000ms which is 20 minutes.
- const timeout = Date.now() + 1200000; // 1 minute timeout + const timeout = Date.now() + 1200000; // 20 minutes timeoutOr if 1 minute was intended:
- const timeout = Date.now() + 1200000; // 1 minute timeout + const timeout = Date.now() + 60000; // 1 minute timeout
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
CHANGELOG.md
(1 hunks)examples/basics/custom-chain.ts
(3 hunks)examples/basics/get-address.ts
(0 hunks)examples/basics/transfer-erc20.ts
(1 hunks)examples/modules/install-hook.ts
(1 hunks)examples/modules/is-module-initialised.ts
(1 hunks)examples/modules/uninstall-module.ts
(2 hunks)package.json
(1 hunks)src/sdk/base/BaseAccountAPI.ts
(3 hunks)src/sdk/base/EtherspotWalletAPI.ts
(1 hunks)src/sdk/network/constants.ts
(37 hunks)src/sdk/network/interfaces.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- examples/basics/get-address.ts
🧰 Additional context used
🧬 Code Graph Analysis (4)
examples/basics/custom-chain.ts (2)
src/sdk/sdk.ts (1)
ModularSdk
(30-356)src/sdk/bundler/providers/EtherspotBundler.ts (1)
EtherspotBundler
(5-23)
src/sdk/base/BaseAccountAPI.ts (2)
src/sdk/network/constants.ts (1)
Networks
(91-577)src/sdk/common/constants.ts (1)
AddressZero
(16-16)
examples/modules/is-module-initialised.ts (1)
src/sdk/base/EtherspotWalletAPI.ts (1)
isModuleInstalled
(67-77)
src/sdk/network/constants.ts (1)
src/sdk/network/interfaces.ts (1)
NetworkConfig
(9-21)
🪛 Gitleaks (8.26.0)
src/sdk/network/constants.ts
116-116: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
482-482: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
579-579: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (16)
src/sdk/network/constants.ts (4)
1-4
: LGTM: Good formatting consistencyThe switch to single quotes and proper imports enhances code consistency.
45-48
: LGTM: Improved array formattingThe multi-line formatting of SupportedNetworks improves readability and maintainability.
100-105
: LGTM: Systematic contract address updatesThe consistent updates to walletFactory, bootstrap, multipleOwnerECDSAValidator, and the addition of hookMultiPlexer across all networks demonstrate a well-coordinated deployment strategy.
579-582
: LGTM: Default constants properly updatedThe default constants correctly reflect the new contract addresses and include the new hookMultiPlexer.
Note on static analysis false positives: The static analysis tool incorrectly flagged Ethereum contract addresses as API keys. These are legitimate smart contract addresses, not sensitive credentials.
🧰 Tools
🪛 Gitleaks (8.26.0)
579-579: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
package.json (1)
3-3
: Version inconsistency with PR objectivesThe PR objectives state: "The SDK release version was upgraded from 6.0.1 to 7.0.0", but the package.json shows version "6.1.0". Given the scope of changes (new contract addresses across all networks and new contract support), please clarify the correct target version.
src/sdk/network/interfaces.ts (1)
19-19
: LGTM: Proper interface extension for new contractThe addition of the hookMultiPlexer property correctly supports the new contract addresses introduced in constants.ts and maintains type safety.
examples/modules/uninstall-module.ts (2)
30-31
: LGTM: Consistent module address updateThe module address has been properly updated to align with the new contract deployments.
38-39
: LGTM: Matching address in uninstall callThe uninstallModule call correctly uses the same updated module address, ensuring consistency in the uninstall operation.
CHANGELOG.md (1)
3-10
: Well-documented breaking changes.The changelog entry properly documents all the contract address updates and their impact. The categorization as "Breaking Changes" is appropriate since these updates will affect precomputed account addresses.
examples/modules/is-module-initialised.ts (1)
8-27
: Implementation looks correct.The script properly demonstrates how to check if a module is installed. The SDK initialization, address retrieval, and module checking logic are all implemented correctly.
src/sdk/base/BaseAccountAPI.ts (3)
3-3
: Good addition of AddressZero import.The import is correctly added to support the new hookMultiplexerAddress initialization logic.
59-59
: Well-structured property addition.The hookMultiplexerAddress property is appropriately typed as optional, which aligns with its usage pattern and the fallback to AddressZero.
100-100
: Consistent initialization pattern.The hookMultiplexerAddress initialization follows the established pattern used by other address properties in the class. The fallback to AddressZero ensures the property always has a valid value even when the network configuration doesn't include the hookMultiPlexer address.
examples/basics/custom-chain.ts (2)
37-47
: Improved constructor readability.The refactored ModularSdk constructor using an options object enhances readability while preserving all parameters and their values.
15-17
:✅ Verification successful
Contract addresses updated correctly.
The new contract addresses align with the PR objectives:
- Wallet Factory:
0x38CC0EDdD3a944CA17981e0A19470d2298B8d43a
- Bootstrap:
0xCF2808eA7d131d96E5C73Eb0eCD8Dc84D33905C7
- Multiple Owner ECDSA Validator:
0x0eA25BF9F313344d422B513e1af679484338518E
Verify these addresses are consistently updated across the codebase:
🏁 Script executed:
#!/bin/bash # Description: Verify contract address consistency across the codebase # Expected: All occurrences should use the new addresses echo "=== Checking Wallet Factory Address ===" rg "0x38CC0EDdD3a944CA17981e0A19470d2298B8d43a" --type ts echo "=== Checking Bootstrap Address ===" rg "0xCF2808eA7d131d96E5C73Eb0eCD8Dc84D33905C7" --type ts echo "=== Checking Multiple Owner ECDSA Validator Address ===" rg "0x0eA25BF9F313344d422B513e1af679484338518E" --type ts echo "=== Checking for old addresses that should be updated ===" rg "0x2A40091f044e48DEB5C0FCbc442E443F3341B451|0x0D5154d7751b6e2fDaa06F0cC9B400549394C8AA|0x0740Ed7c11b9da33d9C80Bd76b826e4E90CC1906" --type tsLength of output: 12139
All contract addresses updated successfully
The new addresses are now consistently used throughout the codebase, and no references to the old addresses remain.
• src/sdk/network/constants.ts
• examples/basics/custom-chain.tsexamples/modules/install-hook.ts (1)
16-66
: Well-structured hook installation example.The script demonstrates a clear workflow for installing a hook multiplexer module with proper:
- SDK initialization
- Balance checking
- Module installation preparation
- Transaction estimation and execution
- Receipt polling with timeout
The implementation follows good practices and provides a valuable example for users.
|
||
dotenv.config(); | ||
|
||
// tsx examples/modules/is-module-installed.ts |
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.
Fix the filename and comment inconsistency.
There's a mismatch between the filename (is-module-initialised.ts
) and the comment (is-module-installed
). The script actually calls isModuleInstalled
, so the filename should reflect what it's actually doing.
Apply this diff to fix the comment:
-// tsx examples/modules/is-module-installed.ts
+// tsx examples/modules/is-module-initialised.ts
Or consider renaming the file to is-module-installed.ts
if that better reflects the intended functionality.
📝 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.
// tsx examples/modules/is-module-installed.ts | |
// tsx examples/modules/is-module-initialised.ts |
🤖 Prompt for AI Agents
In examples/modules/is-module-initialised.ts at line 7, the comment references a
different filename (is-module-installed.ts) which causes inconsistency. Fix this
by updating the comment to match the actual filename or rename the file to
is-module-installed.ts if that better reflects the script's functionality.
Ensure the comment and filename are consistent and accurately describe the
module's purpose.
async isModuleInitialised(moduleTypeId: MODULE_TYPE, module: string, initData = '0x'): Promise<boolean> { | ||
const accountAddress = await this.getAccountAddress(); | ||
if (!accountAddress) throw new Error('Account address not found'); | ||
const response = await this.publicClient.readContract({ | ||
address: accountAddress as Hex, | ||
abi: parseAbi(accountAbi), | ||
functionName: 'isModuleInstalled', | ||
args: [moduleTypeId, module, initData] | ||
}); | ||
return response as boolean; | ||
} |
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.
💡 Verification agent
🧩 Analysis chain
Clarify the purpose of isModuleInitialised
method.
This new method appears to be an exact duplicate of the existing isModuleInstalled
method. Both call the same contract function isModuleInstalled
, which creates confusion about their intended purposes.
Options to resolve:
- If this should check module initialization (different from installation), update to call the appropriate contract function:
async isModuleInitialised(moduleTypeId: MODULE_TYPE, module: string, initData = '0x'): Promise<boolean> {
const accountAddress = await this.getAccountAddress();
if (!accountAddress) throw new Error('Account address not found');
const response = await this.publicClient.readContract({
address: accountAddress as Hex,
abi: parseAbi(accountAbi),
- functionName: 'isModuleInstalled',
+ functionName: 'isModuleInitialised', // or the correct function name
args: [moduleTypeId, module, initData]
});
return response as boolean;
}
- If this is intended as an alias, document it clearly and consider using a simpler implementation:
+ /**
+ * Alias for isModuleInstalled for backward compatibility
+ */
async isModuleInitialised(moduleTypeId: MODULE_TYPE, module: string, initData = '0x'): Promise<boolean> {
- const accountAddress = await this.getAccountAddress();
- if (!accountAddress) throw new Error('Account address not found');
- const response = await this.publicClient.readContract({
- address: accountAddress as Hex,
- abi: parseAbi(accountAbi),
- functionName: 'isModuleInstalled',
- args: [moduleTypeId, module, initData]
- });
- return response as boolean;
+ return this.isModuleInstalled(moduleTypeId, module, initData);
}
Check if the contract has a separate isModuleInitialised
function:
🏁 Script executed:
#!/bin/bash
# Description: Check if the account ABI contains isModuleInitialised function
# Expected: Should show if this function exists in the contract
echo "=== Checking for isModuleInitialised in account ABI ==="
rg -A 5 -B 5 "isModuleInitialised" src/sdk/common/abis.ts
echo "=== Checking usage of this new method ==="
rg "isModuleInitialised" --type ts -A 3 -B 3
Length of output: 896
Clarify the purpose of isModuleInitialised
The account ABI (src/sdk/common/abis.ts
) does not define any isModuleInitialised
function, so this new method on lines 79–89 in src/sdk/base/EtherspotWalletAPI.ts
is an exact duplicate of isModuleInstalled
. You need to resolve the intent:
• If you meant to call a different on‐chain function, update the functionName
to the correct ABI entry (e.g. isModuleInitialised
or whatever the actual name is).
• If this is merely an alias for backward compatibility, simplify the implementation and document it accordingly:
+ /**
+ * Alias for isModuleInstalled (kept for backward compatibility).
+ */
async isModuleInitialised(
moduleTypeId: MODULE_TYPE,
module: string,
initData = '0x'
): Promise<boolean> {
- const accountAddress = await this.getAccountAddress();
- if (!accountAddress) throw new Error('Account address not found');
- const response = await this.publicClient.readContract({
- address: accountAddress as Hex,
- abi: parseAbi(accountAbi),
- functionName: 'isModuleInstalled',
- args: [moduleTypeId, module, initData]
- });
- return response as boolean;
+ return this.isModuleInstalled(moduleTypeId, module, initData);
}
Please choose one of these resolutions so the code’s intent matches its implementation.
📝 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.
async isModuleInitialised(moduleTypeId: MODULE_TYPE, module: string, initData = '0x'): Promise<boolean> { | |
const accountAddress = await this.getAccountAddress(); | |
if (!accountAddress) throw new Error('Account address not found'); | |
const response = await this.publicClient.readContract({ | |
address: accountAddress as Hex, | |
abi: parseAbi(accountAbi), | |
functionName: 'isModuleInstalled', | |
args: [moduleTypeId, module, initData] | |
}); | |
return response as boolean; | |
} | |
/** | |
* Alias for isModuleInstalled (kept for backward compatibility). | |
*/ | |
async isModuleInitialised( | |
moduleTypeId: MODULE_TYPE, | |
module: string, | |
initData = '0x' | |
): Promise<boolean> { | |
return this.isModuleInstalled(moduleTypeId, module, initData); | |
} |
🤖 Prompt for AI Agents
In src/sdk/base/EtherspotWalletAPI.ts between lines 79 and 89, the
isModuleInitialised method duplicates the isModuleInstalled method by calling
the same contract function, causing confusion. To fix this, first verify if the
contract has a distinct function for module initialization; if so, update the
functionName in the readContract call to that specific function. If no such
function exists and this method is intended as an alias, simplify the
implementation to call isModuleInstalled internally and add clear documentation
explaining it is an alias for clarity.
Description
SmartContract Change Details
SDK updates
Updated contract addresses for all chains
release version of sdk upgraded from
6.0.1
to6.1.0
contract Address updates
0x2A40091f044e48DEB5C0FCbc442E443F3341B451
to0x38CC0EDdD3a944CA17981e0A19470d2298B8d43a
0x0D5154d7751b6e2fDaa06F0cC9B400549394C8AA
to0xCF2808eA7d131d96E5C73Eb0eCD8Dc84D33905C7
0x0740Ed7c11b9da33d9C80Bd76b826e4E90CC1906
to0x0eA25BF9F313344d422B513e1af679484338518E
0xDcA918dd23456d321282DF9507F6C09A50522136
.Types of changes
Summary by CodeRabbit
New Features
Updates
Documentation
Examples
Other