-
Notifications
You must be signed in to change notification settings - Fork 3
Custom_Chain #63
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
Custom_Chain #63
Conversation
WalkthroughThis update introduces support for custom blockchain networks by allowing users to specify custom chain configurations throughout the SDK. New optional parameters for custom chain, bootstrap, and validator addresses are added to configuration interfaces and constructors. A usage example is provided, and internal logic is adjusted to prioritize user-supplied chain data over defaults. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ExampleScript
participant ModularSdk
participant WalletService
participant KeyWalletProvider
participant CustomChain
User->>ExampleScript: Run custom-chain.ts
ExampleScript->>ModularSdk: Initialize with custom chain config
ModularSdk->>WalletService: Pass custom chain in options
WalletService->>KeyWalletProvider: Instantiate with custom chain
KeyWalletProvider->>CustomChain: Use chain parameters for client
ExampleScript->>ModularSdk: Perform wallet and transaction operations
ModularSdk->>CustomChain: Interact using custom configuration
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/basics/custom-chain.tsOops! Something went wrong! :( ESLint: 8.28.0 Error [ERR_REQUIRE_ESM]: require() of ES Module /.eslintrc.js from /node_modules/.pnpm/@eslint[email protected]/node_modules/@eslint/eslintrc/dist/eslintrc.cjs not supported.
src/sdk/base/BaseAccountAPI.tsOops! Something went wrong! :( ESLint: 8.28.0 Error [ERR_REQUIRE_ESM]: require() of ES Module /.eslintrc.js from /node_modules/.pnpm/@eslint[email protected]/node_modules/@eslint/eslintrc/dist/eslintrc.cjs not supported.
src/sdk/base/EtherspotWalletAPI.tsOops! Something went wrong! :( ESLint: 8.28.0 Error [ERR_REQUIRE_ESM]: require() of ES Module /.eslintrc.js from /node_modules/.pnpm/@eslint[email protected]/node_modules/@eslint/eslintrc/dist/eslintrc.cjs not supported.
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
✨ 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 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
🧹 Nitpick comments (5)
CHANGELOG.md (1)
2-4
: Consider categorizing this as "Feature" instead of "Fix"The addition of custom chain support is a new capability rather than a bug fix. For clarity and consistency, consider changing the category from "Fix" to "Feature" or "Enhancement".
-### Fix +### Feature - Added ability to use custom chainsrc/sdk/common/utils/viem-utils.ts (1)
29-35
: Good implementation of public client creation for custom chainsThe new utility function properly creates a public client directly from a Chain object, enabling support for custom chains beyond the predefined Networks.
However, consider adding some error handling for when a null or invalid chain is provided:
export const getPublicClientByChain = ({ chain, transport }: { chain: Chain, transport: Transport }) => { + if (!chain) { + throw new Error('Chain is required for creating a public client'); + } const publicClient = createPublicClient({ chain: chain, transport: transport }); return publicClient; }src/sdk/sdk.ts (1)
77-94
: Improve error message clarity for custom chain requirement.The conditional logic for handling custom chains is well-implemented, but the error message could be more informative.
- throw new Exception('chain needs to be set when chainId is not in default Networks'); + throw new Exception('Custom chain object must be provided when using a chainId not found in default Networks. Use the defineChain function from viem to create a custom chain.');examples/basics/custom-chain.ts (2)
9-17
: Consider using environment variables for configuration.Hardcoded values for addresses and URLs would be better placed in environment variables for flexibility and security, similar to how you're handling the private key.
- const recipient = '0x80a1874E1046B1cc5deFdf4D3153838B72fF94Ac'; // recipient wallet address - const value = '0.0000001'; // transfer value - const bundlerApiKey = 'etherspot_public_key'; - const bundlerUrl = 'https://testnet-rpc.etherspot.io/v2/84532'; // bundler url - const chainId = 84532; // chain id - const entryPointAddress = '0x0000000071727De22E5E9d8BAf0edAc6f37da032'; // entry point address - const walletFactoryAddress = '0x2A40091f044e48DEB5C0FCbc442E443F3341B451'; // wallet factory address - const bootstrapAddress = '0x0D5154d7751b6e2fDaa06F0cC9B400549394C8AA'; // bootstrap address - const multiECDSAValidatorAddress = '0x0740Ed7c11b9da33d9C80Bd76b826e4E90CC1906'; // multi owner ECDSA validator factory address + const recipient = process.env.RECIPIENT_ADDRESS || '0x80a1874E1046B1cc5deFdf4D3153838B72fF94Ac'; // recipient wallet address + const value = process.env.TRANSFER_VALUE || '0.0000001'; // transfer value + const bundlerApiKey = process.env.BUNDLER_API_KEY || 'etherspot_public_key'; + const bundlerUrl = process.env.BUNDLER_URL || 'https://testnet-rpc.etherspot.io/v2/84532'; // bundler url + const chainId = Number(process.env.CHAIN_ID || 84532); // chain id + const entryPointAddress = process.env.ENTRY_POINT_ADDRESS || '0x0000000071727De22E5E9d8BAf0edAc6f37da032'; // entry point address + const walletFactoryAddress = process.env.WALLET_FACTORY_ADDRESS || '0x2A40091f044e48DEB5C0FCbc442E443F3341B451'; // wallet factory address + const bootstrapAddress = process.env.BOOTSTRAP_ADDRESS || '0x0D5154d7751b6e2fDaa06F0cC9B400549394C8AA'; // bootstrap address + const multiECDSAValidatorAddress = process.env.VALIDATOR_ADDRESS || '0x0740Ed7c11b9da33d9C80Bd76b826e4E90CC1906'; // multi owner ECDSA validator factory address
79-79
: Correct the timeout comment.The timeout is set to 1,200,000 ms which is 20 minutes, not 1 minute as the comment suggests.
- const timeout = Date.now() + 1200000; // 1 minute timeout + const timeout = Date.now() + 1200000; // 20 minutes timeout
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
CHANGELOG.md
(1 hunks)examples/basics/custom-chain.ts
(1 hunks)package.json
(1 hunks)src/sdk/base/BaseAccountAPI.ts
(3 hunks)src/sdk/base/EtherspotWalletAPI.ts
(1 hunks)src/sdk/common/utils/viem-utils.ts
(1 hunks)src/sdk/interfaces.ts
(2 hunks)src/sdk/sdk.ts
(3 hunks)src/sdk/wallet/interfaces.ts
(1 hunks)src/sdk/wallet/providers/key.wallet-provider.ts
(2 hunks)src/sdk/wallet/wallet.service.ts
(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/sdk/wallet/providers/key.wallet-provider.ts (1)
src/sdk/network/constants.ts (1)
Networks
(90-540)
src/sdk/base/EtherspotWalletAPI.ts (1)
src/sdk/network/constants.ts (2)
Networks
(90-540)DEFAULT_BOOTSTRAP_ADDRESS
(543-543)
src/sdk/wallet/wallet.service.ts (1)
src/sdk/wallet/providers/key.wallet-provider.ts (1)
KeyWalletProvider
(6-110)
src/sdk/base/BaseAccountAPI.ts (1)
src/sdk/network/constants.ts (2)
Networks
(90-540)DEFAULT_MULTIPLE_OWNER_ECDSA_VALIDATOR_ADDRESS
(544-544)
🔇 Additional comments (21)
package.json (1)
3-3
: Version bump looks goodThe version increment from 5.1.1 to 5.1.2 is appropriate for this feature addition that maintains backward compatibility.
src/sdk/wallet/interfaces.ts (2)
1-1
: Proper import for Chain typeThe import for the Chain type from viem is correctly added to support the interface extension.
10-10
: Well-designed interface extensionThe optional chain property is correctly added to the WalletOptions interface, maintaining backward compatibility while enabling the new custom chain functionality.
src/sdk/common/utils/viem-utils.ts (1)
29-35
:✅ Verification successful
Verify custom chain usage across the SDK
This utility function is part of a broader feature for custom chains. Ensure that all related components (SDK constructors, wallet providers, etc.) consistently use this function when dealing with custom chains.
🏁 Script executed:
#!/bin/bash # Search for usages of getPublicClientByChain across the codebase echo "Checking for usages of getPublicClientByChain:" rg "getPublicClientByChain" --type ts # Check if custom chain property is consistently passed through in key files echo "Checking if custom chain is properly propagated in SDK constructor:" rg -A 10 -B 5 "export class ModularSdk" --type ts echo "Checking wallet provider implementation for custom chain support:" rg -A 10 "WalletProvider.*chain" --type ts echo "Checking for custom chain examples:" fd "custom-chain" --type tsLength of output: 2760
🏁 Script executed:
#!/bin/bash echo "Searching for direct createPublicClient usage excluding viem-utils.ts:" rg -n "createPublicClient" --type ts | grep -v "viem-utils.ts" || echo "No direct createPublicClient calls found outside viem-utils.ts" echo echo "Inspecting sdk.ts usage snippet around getPublicClientByChain:" rg -n -A5 -B5 "getPublicClientByChain" src/sdk/sdk.ts echo echo "Inspecting imports in wallet.service.ts:" sed -n '1,30p' src/sdk/wallet/wallet.service.ts echo echo "Locating KeyWalletProvider implementation:" rg -l "class KeyWalletProvider" --type ts echo echo "Snippet around KeyWalletProvider definition:" rg -n -A5 -B5 "class KeyWalletProvider" --type tsLength of output: 3746
Custom chain support verified—no changes needed
I checked that:
src/sdk/sdk.ts
usesgetPublicClientByChain
when falling back to a custom chain.- Wallet providers (e.g.
KeyWalletProvider
) accept the optionalchainObject
and pass it into Viem’screateWalletClient
, so custom chains are handled there as well.- No direct calls to Viem’s
createPublicClient
remain in the SDK (only in example scripts).Everything is already wired to support custom chains correctly. You can disregard the earlier verification request.
src/sdk/interfaces.ts (3)
1-1
: Import ofChain
from 'viem' is correctly added.The import statement appropriately includes the
Chain
type from the 'viem' package, which is needed for the new optional property.
15-15
: Good addition of optionalchain
property.This new optional property allows users to specify a custom blockchain network, which aligns with the PR objective to support custom chains.
23-24
: Appropriate addition of address customization options.These optional properties enable users to specify custom addresses for bootstrap and validator components, directly supporting the PR's objective to allow custom factory addresses.
src/sdk/base/BaseAccountAPI.ts (3)
71-71
: Correctly extractschain
property from options.The extraction of the
chain
property fromoptionsLike
is implemented properly, making it available for use within the constructor.
82-82
: Properly passeschain
to WalletService constructor.This change ensures the custom chain configuration is propagated to the wallet service, enabling consistent chain context throughout the SDK.
98-98
: Well-implemented validator address resolution.The code now implements a clear fallback mechanism for validator address resolution:
- First tries to use the address from the predefined Networks configuration
- Then falls back to the user-provided custom address
- Finally uses the default address as a last resort
This approach maintains backward compatibility while enabling customization.
src/sdk/wallet/providers/key.wallet-provider.ts (3)
1-1
: Appropriate import of Chain type.The Chain type is correctly imported from 'viem' to support the new parameter.
13-13
: Constructor parameter properly updated.The constructor signature is correctly enhanced to accept an optional
chain
parameter without breaking existing functionality.
16-16
: Effective chain resolution strategy implemented.The implementation tries to use the chain from the Networks object first, then falls back to the custom chain if provided. This maintains backward compatibility while enabling custom chain support.
src/sdk/base/EtherspotWalletAPI.ts (1)
57-57
: Enhanced bootstrap address resolution implemented.The code now implements a clear fallback mechanism for the bootstrap address:
- First tries to use the address from the predefined Networks configuration
- Then falls back to the user-provided custom address
- Finally uses the default address as a last resort
This change is consistent with the other modifications in this PR and properly supports custom chain configurations.
src/sdk/wallet/wallet.service.ts (2)
4-4
: LGTM! Clean implementation of custom chain support.The addition of Chain import and chainObject property aligns perfectly with the PR objective to support custom chains.
Also applies to: 18-18, 31-31
83-83
: Correctly passes chainObject to KeyWalletProvider.The implementation properly propagates the custom chain configuration to the wallet provider, ensuring consistent chain context throughout the SDK.
Also applies to: 91-91
src/sdk/sdk.ts (2)
6-7
: LGTM! Necessary imports added.Added import for getPublicClientByChain to support custom chain configuration.
56-56
: LGTM! Chain parameter added to options.Correctly extracting the chain parameter from options to support custom chains.
examples/basics/custom-chain.ts (3)
38-38
: Verify private key existence before SDK initialization.You should validate that the private key is present in the environment variables to prevent runtime errors.
+ if (!process.env.WALLET_PRIVATE_KEY) { + throw new Error('WALLET_PRIVATE_KEY environment variable is required'); + } const modularSdk = new ModularSdk( process.env.WALLET_PRIVATE_KEY as string,
22-35
: LGTM! Well-defined custom chain.The custom chain definition is clear and comprehensive, including all required properties.
40-48
: LGTM! Properly configured SDK with custom chain.The SDK initialization correctly demonstrates the use of the new custom chain feature along with all necessary addresses.
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.
minor changes for naming conventions and changes to use addresses provided by user.
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 💯
Description
Types of changes
What types of changes does your code introduce?
Further comments (optional)
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores