-
Notifications
You must be signed in to change notification settings - Fork 24
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
Refactor: Gateway functions to get Gateway address dynamically #205
Conversation
📝 WalkthroughWalkthroughThis pull request introduces modifications to the ZetaChain toolkit, focusing on enhancing the flexibility of gateway address handling across multiple functions. The changes primarily involve making gateway addresses optional in various client and task functions, introducing a dynamic method to retrieve gateway addresses using Changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
@fadeev pls review |
Looks good so far 👍 |
@coderabbitai review |
✅ Actions performedReview triggered.
|
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: 4
🔭 Outside diff range comments (7)
packages/tasks/src/evmDepositAndCall.ts (1)
Line range hint
14-33
: Add validation for optional gateway parameterWhile the gateway parameter has been made optional in the task definition, there's no visible validation or fallback logic in the function implementation. This could lead to runtime errors if the gateway address is not provided.
Consider adding validation like this:
const tx = await client.evmDepositAndCall({ amount: args.amount, erc20: args.erc20, - gatewayEvm: args.gatewayEvm, + gatewayEvm: args.gatewayEvm ?? await client.getGatewayAddress(), receiver: args.receiver, revertOptions: {packages/tasks/src/zetachainCall.ts (1)
Line range hint
20-37
: Add validation for optional gateway parameterSimilar to evmDepositAndCall, the gateway parameter needs validation logic to handle cases where it's not provided.
Consider adding validation:
const response = await client.zetachainCall({ callOptions, function: args.function, - gatewayZetaChain: args.gatewayZetaChain, + gatewayZetaChain: args.gatewayZetaChain ?? await client.getGatewayAddress(), receiver: args.receiver, revertOptions: {packages/tasks/src/zetachainWithdrawAndCall.ts (1)
Line range hint
20-37
: Add validation for optional gateway parameter and enhance error handlingThe withdraw and call operation is more complex and needs careful handling of the optional gateway parameter.
Consider these improvements:
const response = await client.zetachainWithdrawAndCall({ amount: args.amount, callOptions, function: args.function, - gatewayZetaChain: args.gatewayZetaChain, + gatewayZetaChain: args.gatewayZetaChain ?? await client.getGatewayAddress(), receiver: args.receiver, revertOptions: {Also, consider adding specific error handling for gateway-related failures:
} catch (e) { - console.error("Transaction error:", e); + if (e.message?.includes('gateway')) { + console.error("Gateway error: Please ensure gateway address is valid or provided in config"); + } else { + console.error("Transaction error:", e); + } }packages/client/src/evmDeposit.ts (1)
Line range hint
71-71
: Critical: Fix ERC20 approval to use resolved gateway addressThe ERC20 approval is still using
args.gatewayEvm
directly instead of the resolvedgatewayEvmAddress
. This could cause the approval to fail when the gateway address is provided through the fallback mechanism.- await erc20Contract.connect(signer).approve(args.gatewayEvm, value); + await erc20Contract.connect(signer).approve(gatewayEvmAddress, value);packages/client/src/zetachainWithdraw.ts (1)
Line range hint
82-83
: Critical: Fix multiple gateway address usages in token approvalsThere are multiple instances where the original
args.gatewayZetaChain
is used instead of the resolvedgatewayZetaChainAddress
. This could cause approvals to fail when the gateway address is provided through the fallback mechanism.- args.gatewayZetaChain, + gatewayZetaChainAddress,This fix needs to be applied to all three approve calls:
- Line 82: approveGasAndWithdraw
- Line 93: gasZRC20Contract approval
- Line 98: zrc20 approval
Also applies to: 93-94, 98-99
packages/client/src/zetachainCall.ts (1)
Line range hint
89-93
: Critical: Update approve() to use gatewayZetaChainAddressThe approve() call still uses args.gatewayZetaChain directly, which could be undefined. This needs to be updated to use the gatewayZetaChainAddress variable.
Apply this fix:
const approve = await gasZRC20Contract.approve( - args.gatewayZetaChain, + gatewayZetaChainAddress, gasFee, args.txOptions );packages/client/src/evmDepositAndCall.ts (1)
Line range hint
95-95
: Critical: Update approve() to use gatewayEvmAddressSimilar to zetachainCall.ts, the approve() call uses args.gatewayEvm directly, which could be undefined.
Apply this fix:
- await erc20Contract.connect(signer).approve(args.gatewayEvm, value); + await erc20Contract.connect(signer).approve(gatewayEvmAddress, value);
🧹 Nitpick comments (3)
packages/tasks/src/zetachainWithdrawAndCall.ts (1)
Line range hint
1-93
: Consider adding documentation for gateway configurationSince the gateway parameters are now optional across multiple files, it would be helpful to add documentation about how the gateway addresses are resolved when not provided.
Consider adding a comment block at the top of each file explaining:
- When gateway parameters are required vs optional
- How default gateway addresses are resolved
- Any environment-specific considerations (e.g., localhost requirements mentioned in PR objectives)
packages/client/src/evmCall.ts (1)
Line range hint
8-19
: Update JSDoc to reflect optional parameterThe
@param
description forargs.gatewayEvm
should indicate that it's now optional and mention the fallback behavior usinggetGatewayAddress()
.- * @param {string} args.gatewayEvm - The address of the EVM gateway contract. + * @param {string} [args.gatewayEvm] - The address of the EVM gateway contract. If not provided, falls back to the address from getGatewayAddress().packages/client/src/zetachainWithdrawAndCall.ts (1)
37-37
: Update JSDoc to reflect optional parameterThe
gatewayZetaChain
parameter is now optional, but the JSDoc comment still shows it as required. Please update the documentation to reflect this change.- * @param {string} args.gatewayZetaChain - The address of the ZetaChain gateway contract. + * @param {string} [args.gatewayZetaChain] - The address of the ZetaChain gateway contract. If not provided, defaults to the address from configuration.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
typechain-types/factories/contracts/SwapHelperLib__factory.ts
is excluded by!typechain-types/**
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (14)
package.json
(1 hunks)packages/client/src/client.ts
(5 hunks)packages/client/src/evmCall.ts
(2 hunks)packages/client/src/evmDeposit.ts
(1 hunks)packages/client/src/evmDepositAndCall.ts
(2 hunks)packages/client/src/zetachainCall.ts
(2 hunks)packages/client/src/zetachainWithdraw.ts
(2 hunks)packages/client/src/zetachainWithdrawAndCall.ts
(2 hunks)packages/tasks/src/evmCall.ts
(2 hunks)packages/tasks/src/evmDeposit.ts
(2 hunks)packages/tasks/src/evmDepositAndCall.ts
(2 hunks)packages/tasks/src/zetachainCall.ts
(2 hunks)packages/tasks/src/zetachainWithdraw.ts
(2 hunks)packages/tasks/src/zetachainWithdrawAndCall.ts
(2 hunks)
🔇 Additional comments (20)
packages/client/src/client.ts (5)
5-6
: Imports for Mainnet and Testnet Addresses Look Good
Bringing in distinct address configurations for mainnet and testnet from "@zetachain/protocol-contracts" helps you avoid scattering address data across the codebase. This modular approach is more maintainable.
36-36
: Optional Contracts Parameter
Introducing the optional "contracts" property in the base interface effectively supports both local and mainnet/testnet configurations. Good addition for future expansions.
74-86
: Clearly-Defined Interfaces for Localnet and Mainnet/Testnet
Defining separate interfaces "MainnetTestnetAddress" and "LocalnetAddress" simplifies type handling across different network environments.
95-95
: Storing Contracts in a Private Property
Having a private property "contracts" confines the address management logic within the class, preventing accidental external modification.
157-165
: Localnet Contracts Enforcement vs. Default Fallback
- Throwing an error when localnet or localhost is specified without providing contracts is a strong safeguard.
- Falling back to testnet or mainnet addresses otherwise is a good design for user convenience.
No issues found here.
packages/tasks/src/evmCall.ts (2)
9-10
: Dynamically Using Network Name
Using "hre.network.name" to configure the network dynamically, rather than hardcoding "testnet", is more flexible and allows the script to work across multiple environments seamlessly.
36-36
: Optional GatewayEvm Parameter
Removing the default address and making it optional clarifies execution requirements, forcing users to supply an explicit gateway address when needed. This is consistent with your new approach in other tasks.
packages/tasks/src/evmDeposit.ts (2)
9-10
: ** Leveraging hre.network.name Improves Extensibility**
Same reasoning as evmCall: automatically picking the correct environment is a cleaner approach.
38-38
: Optional GatewayEvm Parameter
In line with the new pattern, making "gatewayEvm" optional encourages explicit user configuration or a fallback logic handled in the ZetaChainClient.
packages/tasks/src/zetachainWithdraw.ts (2)
12-13
: Dynamic Network Selection
Again, picking up "hre.network.name" to specify the network is consistent with your revised approach across tasks, ensuring maintainability and reusability.
41-41
: Optional GatewayZetaChain Parameter
Allowing an optional "gatewayZetaChain" parameter makes sense given that you’re retrieving a default address inside the client when it’s not provided. This approach improves overall adaptability.
packages/tasks/src/evmDepositAndCall.ts (1)
12-13
: LGTM: Network configuration change looks good
The dynamic network configuration using hre.network.name
aligns with the PR objectives and is consistent with changes across other files.
packages/tasks/src/zetachainCall.ts (1)
17-18
: LGTM: Network configuration change is consistent
The dynamic network configuration using hre.network.name
is properly implemented and consistent with other files.
packages/tasks/src/zetachainWithdrawAndCall.ts (1)
17-18
: LGTM: Network configuration change is consistent
The dynamic network configuration using hre.network.name
is properly implemented and consistent with other files.
packages/client/src/evmCall.ts (1)
37-42
: Verify gateway address resolution
The fallback logic looks good, but we should verify that getGatewayAddress()
never returns undefined or invalid addresses.
✅ Verification successful
Gateway address resolution is properly validated
The implementation of getGatewayAddress()
has proper validation:
- For localnet/localhost, it validates the gateway address exists in the contracts configuration and throws an error if not found
- For other networks (mainnet/testnet), it uses predefined addresses from mainnet/testnet configurations
- The gateway address is required during client initialization, either through:
- Explicit contracts configuration parameter
- Built-in mainnet/testnet addresses
- For localnet, contracts configuration is mandatory and throws if not provided
The fallback logic in the original code is safe as getGatewayAddress()
will always return a valid address or throw an error if one cannot be found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for getGatewayAddress implementation and its error handling
ast-grep --pattern 'getGatewayAddress() {
$$$
}'
# Search for tests validating gateway address resolution
rg -l "getGatewayAddress.*test" -g "*.test.ts"
Length of output: 101
Script:
#!/bin/bash
# Search for getGatewayAddress method with more flexible pattern
ast-grep --pattern 'getGatewayAddress'
# Search for gateway address related tests with broader pattern
rg "getGatewayAddress" -g "*.test.ts" -A 5
# Search for gateway address validation or error handling
rg "gatewayEvmAddress|gatewayEvm" -g "*.ts" -A 3
Length of output: 6282
Script:
#!/bin/bash
# Search for the base class implementation to find getGatewayAddress
rg "class.*extends" -g "*.ts" -A 10
# Search for gateway address initialization or configuration
rg "constructor.*gateway" -g "*.ts" -A 10
# Look for any constant or configuration files that might contain gateway addresses
fd -e json -e ts -x rg -l "gateway.*address"
Length of output: 18851
Script:
#!/bin/bash
# Search for getGatewayAddress implementation in client.ts
rg "getGatewayAddress" ./packages/client/src/client.ts -A 5
# Look for gateway address initialization in client constructor
rg "constructor" ./packages/client/src/client.ts -A 10
# Search for any gateway address validation
rg "gatewayEvmAddress|getGatewayAddress" ./packages/client/src/client.ts -B 5 -A 5
Length of output: 1222
Script:
#!/bin/bash
# Search for the complete implementation of getGatewayAddress
rg "getGatewayAddress" ./packages/client/src/client.ts -B 5 -A 15
# Look for contract addresses configuration
rg "contracts.*=" ./packages/client/src/client.ts -A 5
# Search for LocalnetAddress type definition
rg "LocalnetAddress" -g "*.ts" -A 5
Length of output: 2416
packages/client/src/zetachainWithdraw.ts (1)
41-47
: Verify consistent gateway address resolution
The gateway address resolution follows the same pattern as other files. Let's verify the consistency of gateway address handling across all files.
✅ Verification successful
Based on the search results, I can now generate the final response:
Gateway address resolution is consistent across the codebase
The codebase consistently follows the pattern of using an optional gateway address parameter with a fallback to getGatewayAddress()
. This is implemented uniformly across all relevant files:
zetachainWithdraw.ts
,zetachainWithdrawAndCall.ts
,zetachainCall.ts
: Usesargs.gatewayZetaChain || this.getGatewayAddress()
evmDeposit.ts
,evmDepositAndCall.ts
,evmCall.ts
: Usesargs.gatewayEvm || this.getGatewayAddress()
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the pattern of gateway address resolution
rg "gatewayZetaChainAddress.*=.*getGatewayAddress" -A 5
# Search for any remaining direct uses of gateway parameters
rg "args\.gateway(Evm|ZetaChain)" -g "*.ts"
Length of output: 2731
packages/client/src/zetachainCall.ts (1)
35-35
: LGTM: Gateway parameter made optional with proper fallback
The implementation correctly makes the gateway parameter optional and provides a fallback using getGatewayAddress(), which aligns with the PR objectives.
Also applies to: 46-47, 49-49
packages/client/src/evmDepositAndCall.ts (1)
32-32
: LGTM: Consistent implementation of optional gateway parameter
The implementation follows the same pattern as zetachainCall.ts, making the gateway parameter optional with proper fallback to getGatewayAddress().
Also applies to: 42-47
package.json (1)
103-104
: Verify compatibility between RC versions
The update to @zetachain/networks v10.0.0-rc1 while using @zetachain/protocol-contracts 11.0.0-rc3 needs verification to ensure compatibility.
✅ Verification successful
No compatibility issues found between RC versions
Based on the analysis of the codebase:
- The package uses
@zetachain/networks
v10.0.0-rc1 and@zetachain/protocol-contracts
11.0.0-rc3 as direct dependencies - The imports and usage patterns show standard integration with these packages through their stable interfaces (ABIs, types, and network configurations)
- Both packages are release candidates from consecutive major versions (10 and 11) which typically maintain compatibility during the RC phase
- The extensive usage across the codebase (client, tasks, templates) shows these versions are being actively used together
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any breaking changes or compatibility issues in the changelog/release notes
# Check for version compatibility mentions in README files
fd -t f "README.md" -X grep -l "compatibility"
# Look for version constraints or compatibility matrices in documentation
fd -t f -e md -e txt | xargs grep -l "version.*compatibility"
# Search for potential version-related issues in recent commits
git log --since="1 month ago" --grep="breaking\|compatibility\|version"
Length of output: 108
Script:
#!/bin/bash
# Let's check the dependencies in package.json and look for version patterns
cat package.json
# Check if there are any specific version requirements or constraints mentioned
fd -t f -e json -e yaml -e yml | xargs grep -l "@zetachain"
# Look for import statements or usage of these packages
rg "@zetachain/(networks|protocol-contracts)" -A 2
Length of output: 21067
packages/client/src/zetachainWithdrawAndCall.ts (1)
Line range hint 37-52
: Verify impact on existing integrations
The gateway parameter is now optional, which could affect existing integrations that rely on this function. Please ensure that:
- All existing calls to this function are reviewed
- Integration tests cover both cases (with and without gateway parameter)
- Documentation is updated to reflect this change
✅ Verification successful
Optional gateway parameter change is safe and properly handled
The verification shows that the change is safe and well-integrated:
-
The function is only used in two main contexts:
- As part of the client API (packages/client/src/client.ts)
- In a Hardhat task (packages/tasks/src/zetachainWithdrawAndCall.ts)
-
The task implementation already handles the optional gateway parameter correctly, using it as an optional parameter in its interface.
-
The client implementation maintains backward compatibility by falling back to
this.getGatewayAddress()
when the gateway parameter is not provided.
No breaking changes or integration issues were found. The optional parameter pattern is consistently implemented across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for direct calls to zetachainWithdrawAndCall
rg -A 5 "zetachainWithdrawAndCall" --type typescript
# Look for potential test files
fd -e test.ts -e spec.ts | xargs rg "zetachainWithdrawAndCall"
Length of output: 150
Script:
#!/bin/bash
# Search for direct calls to zetachainWithdrawAndCall using ripgrep
rg -A 5 "zetachainWithdrawAndCall"
# Look for test files and their content
find . -type f \( -name "*test*.ts" -o -name "*spec*.ts" \) -exec grep -l "zetachainWithdrawAndCall" {} \;
# Search for function definition and its usage pattern
ast-grep --pattern 'function zetachainWithdrawAndCall'
Length of output: 3618
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 (1)
packages/client/src/evmDeposit.ts (1)
30-30
: Update JSDoc to reflect optional gateway parameterThe
gatewayEvm
parameter is now optional in the function signature, but the JSDoc comment still describes it as required. Update the documentation to indicate that this parameter is optional and explain the fallback behavior when it's not provided.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/client/src/evmDeposit.ts
(2 hunks)packages/client/src/evmDepositAndCall.ts
(3 hunks)packages/client/src/zetachainCall.ts
(3 hunks)packages/client/src/zetachainWithdraw.ts
(4 hunks)packages/client/src/zetachainWithdrawAndCall.ts
(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/client/src/zetachainCall.ts
- packages/client/src/zetachainWithdraw.ts
- packages/client/src/zetachainWithdrawAndCall.ts
- packages/client/src/evmDepositAndCall.ts
🔇 Additional comments (3)
packages/client/src/evmDeposit.ts (3)
38-43
: LGTM: Gateway address resolution looks good
The implementation correctly handles the optional gateway parameter by falling back to this.getGatewayAddress()
when not provided. The gateway contract instantiation properly uses the resolved address.
69-69
: Fixed: Consistent usage of resolved gateway address
The ERC20 approve call now correctly uses the resolved gatewayEvmAddress
instead of args.gatewayEvm
, addressing the inconsistency noted in previous reviews.
38-38
: Verify getGatewayAddress implementation
Let's verify the implementation of getGatewayAddress()
and ensure there are no remaining direct uses of args.gatewayEvm
.
✅ Verification successful
Let me gather more information about the implementation of getGatewayAddress since it wasn't found in the first search.
Let me check one more thing to understand the network and contracts configuration.
Implementation of getGatewayAddress is properly implemented and used consistently
The verification shows that:
getGatewayAddress()
is properly implemented in the client class, handling both localnet and non-localnet cases- The pattern
args.gatewayEvm || this.getGatewayAddress()
is consistently used across all relevant files (evmDeposit, evmCall, evmDepositAndCall) - The implementation follows a proper fallback pattern where custom gateway addresses can be provided, defaulting to the client's gateway address if not specified
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for getGatewayAddress implementation
ast-grep --pattern 'getGatewayAddress() {
$$$
}'
# Check for any remaining direct uses of args.gatewayEvm
rg 'args\.gatewayEvm' --type ts
Length of output: 1037
Script:
#!/bin/bash
# Search for class that might contain getGatewayAddress
ast-grep --pattern 'class $_ {
$$$
getGatewayAddress$$$
$$$
}'
# Also search for any potential interface definitions
ast-grep --pattern 'interface $_ {
$$$
getGatewayAddress$$$
$$$
}'
# Search for any file containing getGatewayAddress with context
rg "getGatewayAddress" -A 5 --type ts
Length of output: 2995
Script:
#!/bin/bash
# Check for network and contracts configuration
rg "LocalnetAddress" -A 5 --type ts
# Look for contract addresses configuration
fd "addresses.ts|addresses.json" --type f --exec cat {}
Length of output: 1413
@lukema95 ready to merge once the conflicts are resolved. |
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
♻️ Duplicate comments (1)
packages/client/src/client.ts (1)
193-208
:⚠️ Potential issueFix async usage in Array.find
The async callback in Array.find won't work as expected since Array.find doesn't await the promises.
This is a duplicate of a previous review comment. Please implement one of the suggested fixes:
- Get chainId first, then use it in a synchronous find
- Use a for-of loop with await
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (2)
package.json
(1 hunks)packages/client/src/client.ts
(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🔇 Additional comments (2)
packages/client/src/client.ts (2)
5-6
: LGTM: Well-structured type definitions and imports
The new interfaces and imports provide a clear and type-safe way to handle different network configurations. The separation between MainnetTestnetAddress
and LocalnetAddress
interfaces appropriately models the different requirements for each environment.
Also applies to: 37-37, 75-87
158-166
: LGTM: Clear initialization logic with proper error handling
The constructor properly handles contract initialization with appropriate error handling for localnet environments. The fallback to testnet/mainnet addresses is well-implemented.
public getGatewayAddress(): string { | ||
if (this.network === "localnet" || this.network === "localhost") { | ||
const gateway = (this.contracts as LocalnetAddress[]).find( | ||
(item) => item.type === "gatewayZEVM" | ||
); | ||
|
||
if (!gateway) { | ||
throw new Error("Gateway address not found in localnet configuration"); | ||
} | ||
|
||
return gateway.address; | ||
} else { | ||
let gateway; | ||
if (this.wallet) { | ||
try { | ||
gateway = (this.contracts as MainnetTestnetAddress[]).find( | ||
async (item) => | ||
(await this.wallet!.getChainId()) === item.chain_id && | ||
item.type === "gateway" | ||
); | ||
} catch (error) { | ||
throw new Error("Failed to get gateway address: " + error); | ||
} | ||
} else { | ||
try { | ||
gateway = (this.contracts as MainnetTestnetAddress[]).find( | ||
async (item) => | ||
(await this.signer!.getChainId()) === item.chain_id && | ||
item.type === "gateway" | ||
); | ||
} catch (error) { | ||
throw new Error("Failed to get gateway address: " + error); | ||
} | ||
} | ||
|
||
if (!gateway) { | ||
throw new Error(`Gateway address not found in signer or wallet`); | ||
} | ||
|
||
return gateway.address; | ||
} | ||
} |
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.
🛠️ Refactor suggestion
Refactor to reduce duplication and improve error handling
The current implementation has duplicate try-catch blocks and could be simplified.
Consider this refactored version:
public getGatewayAddress(): string {
if (this.network === "localnet" || this.network === "localhost") {
const gateway = (this.contracts as LocalnetAddress[]).find(
(item) => item.type === "gatewayZEVM"
);
if (!gateway) {
throw new Error("Gateway address not found in localnet configuration");
}
return gateway.address;
}
- let gateway;
- if (this.wallet) {
- try {
- gateway = (this.contracts as MainnetTestnetAddress[]).find(
- async (item) =>
- (await this.wallet!.getChainId()) === item.chain_id &&
- item.type === "gateway"
- );
- } catch (error) {
- throw new Error("Failed to get gateway address: " + error);
- }
- } else {
- try {
- gateway = (this.contracts as MainnetTestnetAddress[]).find(
- async (item) =>
- (await this.signer!.getChainId()) === item.chain_id &&
- item.type === "gateway"
- );
- } catch (error) {
- throw new Error("Failed to get gateway address: " + error);
- }
- }
+ try {
+ const provider = this.wallet || this.signer;
+ if (!provider) {
+ throw new Error("No wallet or signer available");
+ }
+
+ const chainId = await provider.getChainId();
+ const gateway = (this.contracts as MainnetTestnetAddress[]).find(
+ (item) => item.chain_id === chainId && item.type === "gateway"
+ );
+
+ if (!gateway) {
+ throw new Error(`Gateway address not found for chain ID ${chainId}`);
+ }
+
+ return gateway.address;
+ } catch (error) {
+ throw new Error(`Failed to get gateway address: ${error}`);
+ }
- if (!gateway) {
- throw new Error(`Gateway address not found in signer or wallet`);
- }
-
- return gateway.address;
}
This refactored version:
- Eliminates duplicate code by using a common provider
- Fixes the async/await issue in Array.find
- Provides more specific error messages
- Has better null checking
📝 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.
public getGatewayAddress(): string { | |
if (this.network === "localnet" || this.network === "localhost") { | |
const gateway = (this.contracts as LocalnetAddress[]).find( | |
(item) => item.type === "gatewayZEVM" | |
); | |
if (!gateway) { | |
throw new Error("Gateway address not found in localnet configuration"); | |
} | |
return gateway.address; | |
} else { | |
let gateway; | |
if (this.wallet) { | |
try { | |
gateway = (this.contracts as MainnetTestnetAddress[]).find( | |
async (item) => | |
(await this.wallet!.getChainId()) === item.chain_id && | |
item.type === "gateway" | |
); | |
} catch (error) { | |
throw new Error("Failed to get gateway address: " + error); | |
} | |
} else { | |
try { | |
gateway = (this.contracts as MainnetTestnetAddress[]).find( | |
async (item) => | |
(await this.signer!.getChainId()) === item.chain_id && | |
item.type === "gateway" | |
); | |
} catch (error) { | |
throw new Error("Failed to get gateway address: " + error); | |
} | |
} | |
if (!gateway) { | |
throw new Error(`Gateway address not found in signer or wallet`); | |
} | |
return gateway.address; | |
} | |
} | |
public getGatewayAddress(): string { | |
if (this.network === "localnet" || this.network === "localhost") { | |
const gateway = (this.contracts as LocalnetAddress[]).find( | |
(item) => item.type === "gatewayZEVM" | |
); | |
if (!gateway) { | |
throw new Error("Gateway address not found in localnet configuration"); | |
} | |
return gateway.address; | |
} | |
try { | |
const provider = this.wallet || this.signer; | |
if (!provider) { | |
throw new Error("No wallet or signer available"); | |
} | |
const chainId = await provider.getChainId(); | |
const gateway = (this.contracts as MainnetTestnetAddress[]).find( | |
(item) => item.chain_id === chainId && item.type === "gateway" | |
); | |
if (!gateway) { | |
throw new Error(`Gateway address not found for chain ID ${chainId}`); | |
} | |
return gateway.address; | |
} catch (error) { | |
throw new Error(`Failed to get gateway address: ${error}`); | |
} | |
} |
done |
@lukema95 I've made a pre-release with these changes to make it easier to test changes to the UniversalKit: https://github.com/zeta-chain/toolkit/releases/tag/v13.0.0-rc9 |
issue: #201
Update:
Summary by CodeRabbit
New Features
gatewayEvm
andgatewayZetaChain
parameters optional.ZetaChainClient
.Bug Fixes
Chores
Documentation