-
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,15 +27,20 @@ export const evmDeposit = async function ( | |
args: { | ||
amount: string; | ||
erc20: string; | ||
gatewayEvm: string; | ||
gatewayEvm?: string; | ||
receiver: string; | ||
revertOptions: revertOptions; | ||
txOptions: txOptions; | ||
} | ||
) { | ||
const signer = this.signer; | ||
const { utils } = ethers; | ||
const gateway = new ethers.Contract(args.gatewayEvm, GatewayABI.abi, signer); | ||
const gatewayEvmAddress = args.gatewayEvm || this.getGatewayAddress(); | ||
const gateway = new ethers.Contract( | ||
gatewayEvmAddress, | ||
GatewayABI.abi, | ||
signer | ||
); | ||
Comment on lines
+38
to
+43
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Inconsistent gateway address usage detected The code shows an inconsistency where
🔗 Analysis chainVerify consistent gateway address usage The gateway address resolution looks good, matching the pattern in evmCall.ts. However, we should verify that all gateway address usages within the function are updated to use the resolved address. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Search for any remaining direct uses of args.gatewayEvm
rg "args\.gatewayEvm" "packages/client/src/evmDeposit.ts"
Length of output: 320 Script: #!/bin/bash
# Get more context around the approve call to understand the flow
rg -A 5 -B 5 "approve\(args\.gatewayEvm" "packages/client/src/evmDeposit.ts"
Length of output: 466 |
||
|
||
const revertOptions = { | ||
abortAddress: "0x0000000000000000000000000000000000000000", // not used | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -34,7 +34,7 @@ export const zetachainWithdrawAndCall = async function ( | |||||||||||||||||||||||||
amount: string; | ||||||||||||||||||||||||||
callOptions: any; | ||||||||||||||||||||||||||
function?: string; | ||||||||||||||||||||||||||
gatewayZetaChain: string; | ||||||||||||||||||||||||||
gatewayZetaChain?: string; | ||||||||||||||||||||||||||
receiver: string; | ||||||||||||||||||||||||||
revertOptions: revertOptions; | ||||||||||||||||||||||||||
txOptions: txOptions; | ||||||||||||||||||||||||||
|
@@ -46,8 +46,10 @@ export const zetachainWithdrawAndCall = async function ( | |||||||||||||||||||||||||
const signer = this.signer; | ||||||||||||||||||||||||||
const { utils } = ethers; | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
const gatewayZetaChainAddress = | ||||||||||||||||||||||||||
args.gatewayZetaChain || this.getGatewayAddress(); | ||||||||||||||||||||||||||
const gateway = new ethers.Contract( | ||||||||||||||||||||||||||
args.gatewayZetaChain, | ||||||||||||||||||||||||||
gatewayZetaChainAddress, | ||||||||||||||||||||||||||
Comment on lines
+49
to
+52
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use resolved gateway address consistently throughout the function The function uses const approveGasAndWithdraw = await zrc20.approve(
- args.gatewayZetaChain,
+ gatewayZetaChainAddress,
value.add(gasFee),
args.txOptions
);
// ... and similar changes in other approve calls Also applies to: 100-120 Add error handling for gateway address resolution The fallback mechanism could fail if const gatewayZetaChainAddress =
args.gatewayZetaChain || this.getGatewayAddress();
+ if (!utils.isAddress(gatewayZetaChainAddress)) {
+ throw new Error('Invalid or missing gateway address');
+ }
const gateway = new ethers.Contract(
gatewayZetaChainAddress,
GatewayABI.abi,
signer
); 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||
GatewayABI.abi, | ||||||||||||||||||||||||||
signer | ||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||
|
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.
Async in Array.find across Lines 193-207 Might Not Behave as Expected
The code block uses an async callback inside Array.find, for example:
However, Array.find does not wait for async callbacks to complete before deciding whether to include the element. This could result in unintended behavior, as the promise returned by the async callback is not utilized in a typical synchronous iteration.
Please consider one of the following approaches instead:
Below is a potential fix example using a simple for-of loop: