feat: check withdraw fee in Swap, callMulti example, universal NFT/FT #229
Conversation
|
Warning Rate limit exceeded@fadeev has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 59 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis pull request introduces comprehensive updates across multiple ZetaChain example projects, focusing on standardizing contract implementations, enhancing error handling, and streamlining deployment configurations. The changes span several repositories, including swap, call, NFT, and token examples, with significant modifications to contract structures, task definitions, and project configurations. Changes
Possibly related PRs
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
CodeRabbit Configuration File (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
fadeev
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
examples/swap/contracts/Swap.sol (1)
313-325: Simplify path construction inquoteMinInputfunctionThe current logic for constructing the
patharray can be streamlined for better readability and maintainability. By initializing the array conditionally and assigning common elements outside the conditional blocks, you reduce code duplication.Apply this diff:
address[] memory path; if (inputToken == zeta || gasZRC20 == zeta) { - path = new address[](2); - path[0] = inputToken; - path[1] = gasZRC20; + path = new address[](2); } else { - path = new address[](3); - path[0] = inputToken; - path[1] = zeta; - path[2] = gasZRC20; + path = new address[](3); + path[1] = zeta; } + path[0] = inputToken; + path[path.length - 1] = gasZRC20;examples/swap/tasks/companionSwap.ts (1)
14-14: Parameterize the network to support multiple environmentsHardcoding the network to
'testnet'limits the script's flexibility. Consider using the network name from the Hardhat runtime environment to support different networks dynamically.Apply this diff:
const client = new ZetaChainClient({ - network: "testnet", + network: hre.network.name, signer, });examples/swap/tasks/evmSwap.ts (1)
41-50: Add error handling forisInputAmountSufficientfunctionCurrently, if
isInputAmountSufficientthrows an error, the script will terminate abruptly. Consider adding error handling to provide a graceful failure with informative messages to the user.Apply this diff:
if (!args.skipChecks) { - await isInputAmountSufficient({ - hre, - client, - amount: args.amount, - erc20: args.erc20, - target: args.target, - }); + try { + await isInputAmountSufficient({ + hre, + client, + amount: args.amount, + erc20: args.erc20, + target: args.target, + }); + } catch (error) { + console.error("Input amount is insufficient:", error.message); + return; + } }examples/swap/scripts/localnet.sh (2)
Line range hint
36-43: Consider consolidating duplicate command patternsThe script contains multiple similar
npx hardhatcommands with--skip-checks. This repetition could be refactored for better maintainability.Consider creating a helper function:
+ # Helper function for swap commands + function execute_swap() { + local cmd=$1 + shift + npx hardhat $cmd --skip-checks --network localhost "$@" + npx hardhat localnet-check + } + - npx hardhat companion-swap \ - --skip-checks \ - --network localhost \ - --contract "$COMPANION" \ - # ... other args + execute_swap companion-swap \ + --contract "$COMPANION" \ + # ... other argsThis would:
- Reduce duplication
- Make it easier to modify common parameters
- Ensure consistent usage of flags across commands
Also applies to: 48-53, 58-64
Line range hint
25-64: Document the testing strategy for input amount validationThe PR adds input amount validation, but the test script skips these checks. This creates a gap in testing coverage.
Recommendations:
- Add test cases that explicitly verify the new input amount validation
- Document when and why checks should be skipped
- Consider adding a test matrix that covers both paths:
- With amount validation (normal case)
- Without amount validation (edge cases)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
examples/swap/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
examples/swap/.gitignore(1 hunks)examples/swap/contracts/Swap.sol(10 hunks)examples/swap/package.json(1 hunks)examples/swap/scripts/localnet.sh(3 hunks)examples/swap/tasks/companionSwap.ts(3 hunks)examples/swap/tasks/evmSwap.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- examples/swap/.gitignore
⏰ Context from checks skipped due to timeout of 90000ms (14)
- GitHub Check: slither (examples/token, token.sarif)
- GitHub Check: slither (examples/nft, nft.sarif)
- GitHub Check: slither (examples/swap, swap.sarif)
- GitHub Check: slither (examples/call, call.sarif)
- GitHub Check: slither (examples/hello, hello.sarif)
- GitHub Check: test (examples/token)
- GitHub Check: test (examples/swap)
- GitHub Check: test (examples/nft)
- GitHub Check: test (examples/call)
- GitHub Check: slither (examples/token, token.sarif)
- GitHub Check: slither (examples/nft, nft.sarif)
- GitHub Check: slither (examples/swap, swap.sarif)
- GitHub Check: slither (examples/call, call.sarif)
- GitHub Check: slither (examples/hello, hello.sarif)
🔇 Additional comments (2)
examples/swap/contracts/Swap.sol (1)
270-273: Ensure robust access control in theonRevertfunctionWhile the
onlyGatewaymodifier restricts access to theonRevertfunction, it's crucial to verify that no additional security measures are necessary. Review the function logic to ensure it cannot be exploited by unauthorized entities.examples/swap/scripts/localnet.sh (1)
25-31: Verify the security implications of skipping input amount checksThe addition of
--skip-checksbypasses the new input amount validation that this PR introduces. While this might be necessary for testing edge cases, it could potentially mask issues during local testing.Consider:
- Adding test cases both with and without the
--skip-checksflag- Documenting why these checks are being skipped in the localnet environment
- Adding a warning comment in the script about the implications of skipping checks
examples/swap/package.json
Outdated
| "@zetachain/networks": "10.0.0-rc3", | ||
| "@zetachain/protocol-contracts": "11.0.0-rc3", | ||
| "@zetachain/toolkit": "13.0.0-rc8" | ||
| "@zetachain/protocol-contracts-solana": "2.0.0-rc1", | ||
| "@zetachain/toolkit": "13.0.0-rc11" |
There was a problem hiding this comment.
💡 Codebase verification
Update @zetachain/protocol-contracts to latest RC version
The current RC versions appear to be intentionally aligned for compatibility, but there's a newer release candidate available for protocol-contracts:
- Current: 11.0.0-rc3
- Latest: 11.0.0-rc4
🔗 Analysis chain
Verify compatibility between RC versions and consider using stable releases.
The dependencies use different release candidate versions which may lead to compatibility issues:
- @zetachain/networks: rc3
- @zetachain/protocol-contracts: rc3
- @zetachain/protocol-contracts-solana: rc1
- @zetachain/toolkit: rc11
Let's verify the latest available versions:
Consider:
- Using stable versions if available for production use
- Ensuring all RC versions are compatible with each other
- Adding a comment in package.json explaining why RC versions are needed
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check latest versions of ZetaChain packages
echo "Checking latest versions from npm registry..."
for pkg in "@zetachain/networks" "@zetachain/protocol-contracts" "@zetachain/protocol-contracts-solana" "@zetachain/toolkit"; do
echo "Package: $pkg"
curl -s "https://registry.npmjs.org/$pkg" | jq -r '.["dist-tags"]'
done
Length of output: 2119
| revert TransferFailed( | ||
| "Failed to transfer target tokens to the recipient on ZetaChain" | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align revert statement with updated TransferFailed error
Following the optimization of the TransferFailed error, update this revert statement to match the new definition and enhance gas efficiency.
Apply this diff:
if (!success) {
- revert TransferFailed(
- "Failed to transfer target tokens to the recipient on ZetaChain"
- );
+ revert TransferFailed();
}📝 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.
| revert TransferFailed( | |
| "Failed to transfer target tokens to the recipient on ZetaChain" | |
| ); | |
| revert TransferFailed(); |
| revert InsufficientAmount( | ||
| "The input amount is less than the min amount required to cover the withdraw gas fee" | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Provide detailed error information using fixed-size data types
To offer precise feedback while maintaining gas efficiency, update the InsufficientAmount error and the corresponding revert statement to include the required and provided amounts as uint256 parameters.
Apply this diff:
if (amount < minInput) {
- revert InsufficientAmount(
- "The input amount is less than the min amount required to cover the withdraw gas fee"
- );
+ revert InsufficientAmount(minInput, amount);
}Committable suggestion skipped: line range outside the PR's diff.
| const minAmountFormatted = hre.ethers.utils.formatUnits( | ||
| minAmount.amount, | ||
| minAmount.decimals | ||
| ); | ||
|
|
||
| const value = hre.ethers.utils.parseUnits(amount, minAmount.decimals); |
There was a problem hiding this comment.
Ensure consistent decimal units when parsing amounts
When parsing the amount, you use minAmount.decimals as the decimal parameter. This assumes that the input token and the minAmount share the same decimals, which might not be the case if their decimals differ. To prevent incorrect amount comparisons due to mismatched decimal units, retrieve the decimals of the input token (inputZRC20) and use it consistently.
Apply this diff:
const minAmountFormatted = hre.ethers.utils.formatUnits(
minAmount.amount,
- minAmount.decimals
+ inputTokenDecimals
);
- const value = hre.ethers.utils.parseUnits(amount, minAmount.decimals);
+ const value = hre.ethers.utils.parseUnits(amount, inputTokenDecimals);Ensure that inputTokenDecimals corresponds to the decimals of inputZRC20, which can be retrieved as follows:
const inputTokenDecimals = minAmount.decimals; // Or fetch from inputZRC20 if necessary
| @@ -237,11 +260,17 @@ contract Swap is | |||
| out | |||
| ); | |||
| if (!success) { | |||
| revert TransferFailed(); | |||
| revert TransferFailed( | |||
There was a problem hiding this comment.
I should check but the good part of custom errors is that reduce extremely bytecode size because uses the hash instead of strings. if you hardcode a string as msg IMO will store the same size we don't want to... params in custom errors should be to extra info like values, not messages. If you want to differentiate two errores create a new one.
There was a problem hiding this comment.
The problem with custom errors without messages is that when looking up a tx, it shows the error as 0xcfd6249e, for example. Unless you know what this selector maps to, it's hard to say why a call failed. See:
A message on the other hand can be easily decoded.
Bytecode size isn't particularly important on ZetaChain, because the gas costs are low.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
examples/call/yarn.lockis excluded by!**/yarn.lock,!**/*.lockexamples/hello/yarn.lockis excluded by!**/yarn.lock,!**/*.lockexamples/swap/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
examples/call/package.json(2 hunks)examples/hello/package.json(2 hunks)examples/swap/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/swap/package.json
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: test (examples/token)
- GitHub Check: test (examples/swap)
- GitHub Check: test (examples/nft)
- GitHub Check: test (examples/call)
- GitHub Check: slither (examples/token, token.sarif)
- GitHub Check: slither (examples/nft, nft.sarif)
- GitHub Check: slither (examples/swap, swap.sarif)
- GitHub Check: slither (examples/call, call.sarif)
- GitHub Check: slither (examples/hello, hello.sarif)
examples/hello/package.json
Outdated
| @@ -29,7 +29,7 @@ | |||
| "@typescript-eslint/eslint-plugin": "^5.59.9", | |||
| "@typescript-eslint/parser": "^5.59.9", | |||
| "@zetachain/localnet": "4.0.0-rc6", | |||
| "@zetachain/toolkit": "13.0.0-rc7", | |||
| "@zetachain/toolkit": "13.0.0-rc12", | |||
There was a problem hiding this comment.
💡 Codebase verification
Update dependency versions to align with published releases
@zetachain/[email protected]is not yet published (latest is rc9)@zetachain/[email protected]should be updated to the stable10.0.0
🔗 Analysis chain
Verify compatibility with release candidate dependencies.
The changes introduce release candidate versions of ZetaChain packages:
- Updating
@zetachain/toolkitto13.0.0-rc12 - Adding
@zetachain/networksat10.0.0-rc4
Run this script to check for breaking changes and compatibility:
Also applies to: 60-60
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for breaking changes in the release candidates
# Check the changelog or release notes for breaking changes
gh api repos/zeta-chain/toolkit/releases | jq -r '.[].body' | grep -i "breaking"
gh api repos/zeta-chain/networks/releases | jq -r '.[].body' | grep -i "breaking"
# Check if these versions are the latest available
for pkg in "@zetachain/toolkit" "@zetachain/networks"; do
echo "Checking latest version for $pkg:"
npm view "$pkg" versions --json | jq -r '.[]' | tail -n 5
done
Length of output: 737
| "@zetachain/protocol-contracts": "11.0.0-rc3" | ||
| "@zetachain/networks": "10.0.0-rc4", | ||
| "@zetachain/protocol-contracts": "11.0.0-rc3", | ||
| "@zetachain/protocol-contracts-solana": "2.0.0-rc1" |
There was a problem hiding this comment.
💡 Codebase verification
Remove or document unused Solana dependency
The @zetachain/protocol-contracts-solana dependency appears to be unused in the codebase. Either:
- Remove the dependency if it's not needed
- Document its intended usage and implement the Solana integration
🔗 Analysis chain
Verify Solana contract integration requirements.
The addition of @zetachain/protocol-contracts-solana suggests new Solana integration capabilities. Ensure this dependency is required and properly documented in the project's README.
Run this script to check for Solana-related configuration and documentation:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for Solana configuration and documentation
# Check for Solana configuration files
echo "Checking for Solana configuration files:"
fd -e yaml -e json "solana" examples/call/
# Check for Solana documentation
echo -e "\nChecking for Solana documentation:"
rg -i "solana" examples/call/README.md || echo "No Solana documentation found in README"
Length of output: 346
Script:
#!/bin/bash
# Check for imports and usage of Solana contracts
echo "Checking for Solana contract imports and usage:"
rg -i "@zetachain/protocol-contracts-solana" examples/call/ -A 2
echo -e "\nChecking for Solana-related code patterns:"
rg -i "solana" examples/call/ --type ts --type js --type tsx --type jsx
Length of output: 1163
examples/call/package.json
Outdated
| @@ -29,7 +29,7 @@ | |||
| "@typescript-eslint/eslint-plugin": "^5.59.9", | |||
| "@typescript-eslint/parser": "^5.59.9", | |||
| "@zetachain/localnet": "4.0.0-rc6", | |||
| "@zetachain/toolkit": "13.0.0-rc7", | |||
| "@zetachain/toolkit": "13.0.0-rc12", | |||
There was a problem hiding this comment.
💡 Codebase verification
Version inconsistency detected in example projects
The @zetachain/toolkit package has inconsistent versions across examples:
- Version
13.0.0-rc12: hello, swap, call - Version
13.0.0-rc8: nft, token
Consider updating all examples to use the latest release candidate version 13.0.0-rc12 for consistency.
🔗 Analysis chain
Ensure version consistency across example projects.
The changes introduce multiple release candidate dependencies:
- Updating
@zetachain/toolkitto13.0.0-rc12 - Adding
@zetachain/networksat10.0.0-rc4 - Adding
@zetachain/protocol-contracts-solanaat2.0.0-rc1
Run this script to verify version consistency across all example projects:
Also applies to: 60-62
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check version consistency across example projects
# Find all package.json files in example directories
echo "Checking ZetaChain package versions across all examples:"
for pkg in "@zetachain/toolkit" "@zetachain/networks" "@zetachain/protocol-contracts" "@zetachain/protocol-contracts-solana"; do
echo -e "\nVersions of $pkg:"
fd --type f "package.json" examples/ --exec jq -r \
--arg pkg "$pkg" \
'if .dependencies[$pkg] then " " + . + ": " + .dependencies[$pkg] elif .devDependencies[$pkg] then " " + . + ": " + .devDependencies[$pkg] else empty end'
done
Length of output: 3335
Script:
#!/bin/bash
# Description: Check version consistency with corrected jq query
echo "Checking ZetaChain package versions across all examples:"
for pkg in "@zetachain/toolkit" "@zetachain/networks" "@zetachain/protocol-contracts" "@zetachain/protocol-contracts-solana"; do
echo -e "\nVersions of $pkg:"
fd --type f "package.json" examples/ --exec sh -c "echo -n ' {}:' && jq -r '.dependencies[\"$pkg\"] // .devDependencies[\"$pkg\"] // \"not found\"' {}"
done
Length of output: 2613
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
examples/swap/tasks/evmSwap.ts (1)
19-24:⚠️ Potential issueEnsure consistent decimal units when parsing amounts.
The decimal units mismatch issue identified in the previous review is still present.
🧹 Nitpick comments (4)
examples/swap/tasks/evmSwap.ts (3)
26-30: Enhance error message with formatted values.The error message could be more helpful by including the actual values in a consistent format.
if (value.lt(minAmount.amount)) { throw new Error( - `Input amount ${amount} is less than minimum amount ${minAmountFormatted} required for a withdrawal to the destination chain` + `Insufficient input amount: provided ${amount} ${await inputZRC20.symbol()}, minimum required ${minAmountFormatted} ${await inputZRC20.symbol()} for withdrawal to the destination chain` ); }
41-49: Improve error handling specificity.The current implementation wraps all errors in a generic catch block. Consider handling specific errors differently, especially the insufficient amount error.
if (!args.skipChecks) { try { await isInputAmountSufficient({ hre, client, amount: args.amount, erc20: args.erc20, target: args.target, }); + } catch (e) { + if (e.message.includes('Insufficient input amount')) { + console.error('Validation Error:', e.message); + throw e; + } + console.warn('Validation Warning:', e.message); } }
113-113: Improve documentation for skipChecks flag.The
skipChecksflag should include documentation about its implications and when it should be used.- .addFlag("skipChecks", "Skip checks for minimum amount") + .addFlag( + "skipChecks", + "Skip minimum amount validation. Use with caution as it may result in failed transactions on the destination chain if the amount is insufficient to cover withdrawal fees" + )examples/call/tasks/connectedCall.ts (1)
Line range hint
74-97: Add parameter size validation for gas optimizationWhile the gas limits are aligned with other operations, the arbitrary parameter encoding could consume varying amounts of gas. Consider adding parameter size validation to prevent excessive gas consumption.
const valuesArray = args.values.map((value: any, index: number) => { + if (JSON.stringify(value).length > 1000) { + throw new Error(`Parameter ${index} exceeds recommended size for gas efficiency`); + } const type = types[index];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
examples/nft/yarn.lockis excluded by!**/yarn.lock,!**/*.lockexamples/token/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (45)
.github/workflows/slither.yaml(0 hunks)examples/call/tasks/connectedCall.ts(2 hunks)examples/call/tasks/connectedDeposit.ts(2 hunks)examples/call/tasks/connectedDepositAndCall.ts(2 hunks)examples/call/tasks/universalCall.ts(2 hunks)examples/call/tasks/universalWithdraw.ts(2 hunks)examples/call/tasks/universalWithdrawAndCall.ts(2 hunks)examples/nft/.eslintignore(0 hunks)examples/nft/.eslintrc.js(0 hunks)examples/nft/.gitignore(0 hunks)examples/nft/LICENSE(0 hunks)examples/nft/README.md(0 hunks)examples/nft/contracts/Connected.sol(0 hunks)examples/nft/contracts/Universal.sol(0 hunks)examples/nft/contracts/shared/Events.sol(0 hunks)examples/nft/foundry.toml(0 hunks)examples/nft/hardhat.config.ts(0 hunks)examples/nft/package.json(0 hunks)examples/nft/scripts/localnet.sh(0 hunks)examples/nft/scripts/testnet.sh(0 hunks)examples/nft/tasks/connectedSetUniversal.ts(0 hunks)examples/nft/tasks/deploy.ts(0 hunks)examples/nft/tasks/mint.ts(0 hunks)examples/nft/tasks/transfer.ts(0 hunks)examples/nft/tasks/universalSetConnected.ts(0 hunks)examples/nft/tsconfig.json(0 hunks)examples/swap/tasks/evmSwap.ts(2 hunks)examples/token/.eslintignore(0 hunks)examples/token/.eslintrc.js(0 hunks)examples/token/.gitignore(0 hunks)examples/token/LICENSE(0 hunks)examples/token/README.md(0 hunks)examples/token/contracts/Connected.sol(0 hunks)examples/token/contracts/Universal.sol(0 hunks)examples/token/contracts/shared/Events.sol(0 hunks)examples/token/foundry.toml(0 hunks)examples/token/hardhat.config.ts(0 hunks)examples/token/package.json(0 hunks)examples/token/scripts/localnet.sh(0 hunks)examples/token/tasks/connectedSetUniversal.ts(0 hunks)examples/token/tasks/deploy.ts(0 hunks)examples/token/tasks/mint.ts(0 hunks)examples/token/tasks/transfer.ts(0 hunks)examples/token/tasks/universalSetConnected.ts(0 hunks)examples/token/tsconfig.json(0 hunks)
💤 Files with no reviewable changes (38)
- examples/token/README.md
- examples/token/foundry.toml
- examples/token/.eslintignore
- examples/nft/README.md
- examples/nft/foundry.toml
- examples/nft/contracts/shared/Events.sol
- examples/token/LICENSE
- examples/token/hardhat.config.ts
- examples/nft/package.json
- examples/nft/tasks/deploy.ts
- examples/nft/LICENSE
- examples/nft/.gitignore
- examples/token/package.json
- examples/nft/tsconfig.json
- examples/token/tsconfig.json
- .github/workflows/slither.yaml
- examples/nft/.eslintrc.js
- examples/nft/tasks/connectedSetUniversal.ts
- examples/token/.eslintrc.js
- examples/token/contracts/shared/Events.sol
- examples/token/tasks/mint.ts
- examples/token/.gitignore
- examples/nft/contracts/Connected.sol
- examples/nft/tasks/mint.ts
- examples/nft/scripts/localnet.sh
- examples/nft/tasks/transfer.ts
- examples/token/tasks/universalSetConnected.ts
- examples/token/scripts/localnet.sh
- examples/token/tasks/transfer.ts
- examples/token/tasks/connectedSetUniversal.ts
- examples/nft/tasks/universalSetConnected.ts
- examples/nft/.eslintignore
- examples/nft/hardhat.config.ts
- examples/nft/contracts/Universal.sol
- examples/nft/scripts/testnet.sh
- examples/token/contracts/Connected.sol
- examples/token/contracts/Universal.sol
- examples/token/tasks/deploy.ts
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: test (examples/call)
- GitHub Check: slither (examples/swap, swap.sarif)
- GitHub Check: slither (examples/call, call.sarif)
- GitHub Check: slither (examples/hello, hello.sarif)
🔇 Additional comments (7)
examples/swap/tasks/evmSwap.ts (1)
101-101: Verify the reduced gas limits.The gas limits have been significantly reduced from 7000000 to 50000. Please verify that this is sufficient for all possible transaction scenarios, especially complex swaps or when dealing with tokens that have transfer hooks.
Also applies to: 107-107
✅ Verification successful
Gas limit reduction is appropriate
The 50,000 gas limit is sufficient for the EVM-side deposit operation because:
- It's an optional parameter that users can override if needed
- The main swap complexity is handled on ZetaChain's side
- The value aligns with standard ERC20 transfer gas costs
- It matches the onRevertGasLimit, suggesting a standardized approach
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check recent successful transaction gas usage # Get recent successful transactions rg -A 5 "Transaction hash:" ./**/*.log # Check for any out-of-gas errors in logs rg -i "out of gas|insufficient gas" ./**/*.logLength of output: 195
Script:
#!/bin/bash # Get context of the gas limit usage cat examples/swap/tasks/evmSwap.ts # Find other gas limit patterns rg -B 2 -A 2 "[0-9]{5,}.*gas" --type ts # Look for swap implementation patterns ast-grep --pattern 'function $FUNC($$$) { $$$ gas$$$ $$$ }'Length of output: 3494
Script:
#!/bin/bash # Check ZetaChain client implementation fd -e ts -e js "ZetaChainClient" --exec cat {} # Look for gas limit usage in tests rg -B 2 -A 2 "evmDepositAndCall.*gasLimit" --type tsLength of output: 103
examples/call/tasks/universalWithdraw.ts (1)
Line range hint
66-89: Verify gas limits for withdrawal operationsThe significant reduction in gas limits (from 7,000,000 to 500,000) could potentially cause transaction failures for complex withdrawal operations. While this optimization reduces the maximum gas cost, we should ensure it's sufficient for all scenarios.
Let's analyze the gas usage patterns:
✅ Verification successful
Gas limit reduction is appropriate for withdrawal operations
The reduction to 500,000 gas is safe because:
- The gas limit is consistently applied across all similar operations in the codebase
- Withdrawal operations only involve simple token transfers and approvals
- The ZRC20 contract's
withdrawGasFeeWithGasLimitfunction validates the gas requirements🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check historical gas usage for withdraw operations # to verify if 500000 gas limit is sufficient # Search for withdraw function calls and their gas usage patterns rg -A 2 "withdraw.*\(" --type sol # Look for any revert errors related to gas limits rg -i "out of gas|gas limit" --type logLength of output: 254
Script:
#!/bin/bash # Find relevant files and analyze gas usage patterns # Find Solidity and TypeScript files echo "=== Relevant files ===" fd -e sol -e ts # Search for gas configurations in hardhat config echo -e "\n=== Gas configurations ===" rg -i "gas.*limit|gas.*price" -A 2 -B 2 # Search for withdraw function implementations echo -e "\n=== Withdraw implementations ===" ast-grep --pattern 'function withdraw$$$($$$) { $$$ }' # Search for gas usage in test files echo -e "\n=== Gas usage in tests ===" rg -i "gas.*limit|gas.*price" ".*test.*\.ts"Length of output: 20933
examples/call/tasks/connectedDeposit.ts (1)
Line range hint
67-90: Gas parameter adjustments look reasonable for deposit operationsThe new gas limits appear suitable for deposit operations, which typically consume less gas than withdrawals. The doubled gas price aligns with network conditions.
Monitor gas usage patterns to ensure reliability:
✅ Verification successful
Gas parameter configurations are consistent and appropriate
The gas limits (500000) are consistent with other operations in the codebase and provide sufficient headroom for deposit transactions while maintaining safety margins. The configurable nature allows for adjustments if needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Analyze deposit function gas usage patterns # Search for deposit function implementations ast-grep --pattern 'function deposit($$$) { $$$ }' # Look for any failed deposit transactions rg -i "failed.*deposit|deposit.*failed" --type logLength of output: 256
Script:
#!/bin/bash # Search for deposit-related functions and gas configurations ast-grep --pattern 'deposit($$$)' # Look for gas-related configurations and parameters rg -i "gas.*limit|gasLimit" -A 2 -B 2 # Search for deposit-related error handling rg -i "deposit.*error|error.*deposit" --type tsLength of output: 16591
examples/call/tasks/universalCall.ts (1)
Line range hint
96-126: Consider implementing dynamic gas limit calculationThe fixed gas limits might need adjustment based on the complexity of the called function. Consider implementing dynamic gas limit calculation based on the function signature and parameters.
Let's analyze the current function call patterns:
examples/call/tasks/connectedDepositAndCall.ts (1)
106-106: Verify gas limits are sufficient for complex operationsThe significant reduction in gas limits (from 7M to 500K) while increasing gas price might impact complex operations:
txOptionsGasLimit: 7M → 500KonRevertGasLimit: 7M → 500KLet's analyze the gas usage patterns:
Also applies to: 112-112, 129-129
✅ Verification successful
Gas limit reduction appears to be a deliberate optimization
The reduced gas limits are consistently applied across all similar operations in the codebase, including universal calls, withdrawals, and deposits. The presence of gas fee calculations using these limits suggests they have been validated for the intended operations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check historical gas usage for deposit operations git log -p --all -S "depositAndCall" -- "*.sol" | grep -A 5 "function depositAndCall" # Look for any existing gas estimation tests fd -e js -e ts | xargs rg "estimateGas.*depositAndCall"Length of output: 57
Script:
#!/bin/bash # Search for gas-related configurations and constants rg -i "gas.*limit|gas.*config" --type ts # Look for test files related to deposit operations fd -e spec.ts -e test.ts | xargs rg -l "deposit" # Search for gas usage documentation in markdown files fd -e md | xargs rg -i "gas.*limit|gas.*usage"Length of output: 4695
examples/call/tasks/universalWithdrawAndCall.ts (2)
127-127: Review gas fee calculations with new limitsThe gas parameter changes affect both transaction execution and fee calculations:
- Gas price doubled (10 gwei → 20 gwei)
- All gas limits reduced (7M → 500K)
These changes interact with
withdrawGasFeeWithGasLimitwhich calculates fees based on the provided gas limit. Ensure the reduced limits are sufficient for:
- ZRC20 approvals
- Token withdrawals
- Cross-chain calls
Let's analyze the gas requirements:
Also applies to: 133-133, 150-150, 157-157
✅ Verification successful
Gas parameter changes are consistent with codebase patterns
The reduced gas limits (500K) align with:
- Gas fee calculations in Universal.sol using withdrawGasFeeWithGasLimit
- Similar gas limits used in other operations
- Standard EVM transaction gas limits
The increased gas price (20 gwei) is consistently applied across all tasks, ensuring proper fee calculations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check historical gas usage for withdraw operations git log -p --all -S "withdrawAndCall" -- "*.sol" | grep -A 5 "function withdrawAndCall" # Look for existing gas estimation tests fd -e js -e ts | xargs rg "estimateGas.*withdraw" # Check if there are any failed transactions due to out-of-gas errors fd -e log | xargs rg -i "out of gas"Length of output: 90
Script:
#!/bin/bash # Find withdraw function implementations ast-grep --pattern 'function withdraw$$$($$$) { $$$ }' # Look for gas-related configurations rg -i "gas.*limit|gas.*price" -A 2 -B 2 # Find test files related to withdrawals fd -e test.ts -e test.js | xargs rg "withdraw.*test|test.*withdraw"Length of output: 19839
Line range hint
89-94: Verify gas fee approval logic with new limitsThe approval logic uses the new gas limits for fee calculation:
const [gasZRC20, gasFee] = await zrc20.withdrawGasFeeWithGasLimit(gasLimit);Let's check if the new gas limits affect existing transactions:
✅ Verification successful
Gas fee approval logic is correctly implemented with new limits
The gas limit changes are safely integrated into both the contract and task implementations. The approval logic maintains consistency with the contract's requirements, properly handling both same-token and cross-token scenarios while incorporating the new gas limit-based fee calculations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Look for historical gas fee calculations git log -p --all -S "withdrawGasFeeWithGasLimit" | grep -A 10 "withdrawGasFeeWithGasLimit" # Check for any existing test cases with gas fee assertions fd -e test.ts -e test.js | xargs rg "withdrawGasFeeWithGasLimit"Length of output: 509
Script:
#!/bin/bash # Examine Universal.sol contract implementation ast-grep --pattern 'function $FUNC($$$) { $$$ withdrawGasFeeWithGasLimit($$$) $$$ }' # Look for gas limit validations rg -A 5 -B 5 "gasLimit" examples/call/contracts/Universal.sol # Check for gas fee related calculations rg -A 5 -B 5 "gasFee" examples/call/contracts/Universal.solLength of output: 3628
| export const isInputAmountSufficient = async ({ | ||
| hre, | ||
| client, | ||
| erc20, | ||
| amount, | ||
| target, | ||
| }: any) => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add proper TypeScript types instead of using 'any'.
Replace the 'any' type with a proper interface definition for better type safety and code maintainability.
+interface InputAmountCheckParams {
+ hre: HardhatRuntimeEnvironment;
+ client: ZetaChainClient;
+ erc20?: string;
+ amount: string;
+ target: string;
+}
-export const isInputAmountSufficient = async ({
- hre,
- client,
- erc20,
- amount,
- target,
-}: any) => {
+export const isInputAmountSufficient = async ({
+ hre,
+ client,
+ erc20,
+ amount,
+ target,
+}: InputAmountCheckParams) => {Committable suggestion skipped: line range outside the PR's diff.
examples/call/tasks/universalCall.ts
Outdated
| @@ -116,14 +116,14 @@ task( | |||
| .addOptionalParam( | |||
| "onRevertGasLimit", | |||
| "The gas limit for the revert transaction", | |||
| 7000000, | |||
| 50000, | |||
There was a problem hiding this comment.
Inconsistent onRevertGasLimit value
The onRevertGasLimit is set to 50000, while other files use 500000. This appears to be inconsistent and might be insufficient for revert operations.
Apply this fix to maintain consistency:
- 50000,
+ 500000,📝 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.
| 50000, | |
| 500000, |
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
examples/call/scripts/localnet.sh (1)
Line range hint
1-100: Consider adding error handling for JSON parsing operations.The script contains multiple
jqoperations that could fail silently. Consider adding validation for the parsed values.Here's an example pattern to implement:
-ZRC20_ETHEREUM=$(jq -r '.addresses[] | select(.type=="ZRC-20 ETH on 5") | .address' localnet.json) +ZRC20_ETHEREUM=$(jq -r '.addresses[] | select(.type=="ZRC-20 ETH on 5") | .address' localnet.json) +if [[ -z "$ZRC20_ETHEREUM" ]]; then + echo "Error: Failed to extract ZRC20_ETHEREUM address from localnet.json" >&2 + exit 1 +fi
🧹 Nitpick comments (2)
examples/call/scripts/localnet.sh (1)
4-4: Consider making debug output configurable.While
set -xis valuable for debugging, it can generate excessive output. Consider making it conditional based on a debug flag.-set -x +# Enable debug output if DEBUG is set +[[ "${DEBUG:-}" == "true" ]] && set -xexamples/hello/hardhat.config.ts (1)
7-13: Add environment variable validation.The configuration assumes PRIVATE_KEY will be present. Add validation to provide a better developer experience.
dotenv.config(); + +if (!process.env.PRIVATE_KEY) { + throw new Error("PRIVATE_KEY environment variable is required"); +} const config: HardhatUserConfig = {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
examples/call/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
examples/call/hardhat.config.ts(1 hunks)examples/call/package.json(2 hunks)examples/call/scripts/localnet.sh(1 hunks)examples/call/tasks/connectedCall.ts(2 hunks)examples/call/tasks/connectedDeposit.ts(2 hunks)examples/call/tasks/connectedDepositAndCall.ts(3 hunks)examples/call/tasks/universalCall.ts(3 hunks)examples/call/tasks/universalWithdraw.ts(4 hunks)examples/call/tasks/universalWithdrawAndCall.ts(4 hunks)examples/hello/hardhat.config.ts(1 hunks)examples/swap/hardhat.config.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/call/package.json
- examples/call/tasks/universalWithdraw.ts
🔇 Additional comments (10)
examples/call/scripts/localnet.sh (1)
5-5: Excellent addition of pipefail.The addition of
set -o pipefailis a robust improvement that ensures pipeline failures are properly caught, enhancing the script's error handling capabilities.examples/call/hardhat.config.ts (1)
13-19: Add environment variable validation.Similar to examples/hello/hardhat.config.ts, add validation for the PRIVATE_KEY environment variable.
examples/swap/hardhat.config.ts (1)
14-20: Add environment variable validation.Similar to other configuration files, add validation for the PRIVATE_KEY environment variable.
examples/call/tasks/connectedDeposit.ts (2)
41-41: LGTM! Code simplification through removal of txOptions.The removal of txOptions improves code maintainability while preserving functionality.
Also applies to: 46-46
69-69: Verify gas limit sufficiency for revert operations.The onRevertGasLimit has been reduced from 7000000 to 500000 (85.7% reduction). While this optimization is good for gas efficiency, we should verify that 500000 gas is sufficient for all revert scenarios.
✅ Verification successful
500,000 gas limit is sufficient for the observed revert operations
Based on the codebase analysis, the revert scenarios involve only simple operations like authorization checks, transfer validations, and basic state reverts. None of these operations require complex computations or extensive state modifications, making the 500,000 gas limit adequate for all identified revert paths.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for historical revert operations to analyze gas usage rg -A 5 "revert" --glob "*.sol" | grep -i "gas"Length of output: 2211
Script:
#!/bin/bash # Search for revert operations and their context echo "=== Revert Operations ===" rg -B 2 -A 3 "revert" --glob "*.sol" echo -e "\n=== Gas Limit Configurations ===" rg -i "gasLimit|gas.?limit" --glob "*.{sol,ts,js}" echo -e "\n=== Test Files with Gas ===" rg -i "gas" --glob "*test.{ts,js,sol}"Length of output: 18877
examples/call/tasks/connectedCall.ts (1)
52-52: LGTM! Consistent with other task files.The changes align with the broader refactoring effort to simplify transaction handling and standardize gas limits.
Also applies to: 79-79
examples/call/tasks/universalCall.ts (1)
58-61: LGTM! Improved gas fee calculation logic.The explicit calculation of gas fees using BigNumber improves precision and clarity.
examples/call/tasks/connectedDepositAndCall.ts (1)
70-70: LGTM! Improved transaction parameter handling.The value parameter is now correctly placed in the options object, following Ethereum's transaction parameter best practices.
Also applies to: 80-80
examples/call/tasks/universalWithdrawAndCall.ts (2)
71-71: LGTM! Enhanced gas fee handling logic.The implementation now properly handles gas fees for both the same and different ZRC20 tokens, with appropriate approval amounts.
Also applies to: 80-80, 87-87
129-129: Maintain consistent gas limits across all tasks.The gas limits have been standardized to 500000 across all tasks, which is good for consistency. However, we should verify this is sufficient for complex operations.
Also applies to: 136-136
✅ Verification successful
The 500,000 gas limit is appropriate for these operations.
The implementation shows standard ERC20 operations (transfers and approvals) combined with gateway calls, all of which are well within the 500,000 gas limit. There are no complex loops or recursive operations that could cause unexpected gas spikes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for complex operations that might require more gas rg -A 10 "withdrawAndCall|depositAndCall" --glob "*.sol" | grep -i "gas"Length of output: 379
Script:
#!/bin/bash # Get the complete implementation of withdrawAndCall and related functions ast-grep --pattern 'function $$$withdrawAndCall$$$ { $$$ }' # Also search for depositAndCall implementations ast-grep --pattern 'function $$$depositAndCall$$$ { $$$ }' # Look for potential gas-intensive operations like loops rg -A 5 "for|while" --glob "*.sol"Length of output: 1382
Script:
#!/bin/bash # Get the Universal contract implementation focusing on external calls rg -A 10 "Universal\.sol" --glob "*.sol" # Look for external calls in the contract rg -A 5 "call\(|delegatecall|staticcall" --glob "*.sol" # Get the specific file content cat examples/call/contracts/Universal.solLength of output: 5617
| solidity: { | ||
| compilers: [{ version: "0.8.20" }, { version: "0.8.26" }], | ||
| }, |
There was a problem hiding this comment.
Fix invalid Solidity compiler version.
The compiler version 0.8.26 does not exist (latest is 0.8.24).
solidity: {
- compilers: [{ version: "0.8.20" }, { version: "0.8.26" }],
+ compilers: [{ version: "0.8.20" }, { version: "0.8.24" }],
},📝 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.
| solidity: { | |
| compilers: [{ version: "0.8.20" }, { version: "0.8.26" }], | |
| }, | |
| solidity: { | |
| compilers: [{ version: "0.8.20" }, { version: "0.8.24" }], | |
| }, |
💡 Codebase verification
Invalid Solidity compiler version detected
The configuration in examples/swap/hardhat.config.ts specifies version 0.8.26, which is not a valid Solidity compiler version. The latest stable version is 0.8.24.
- Update the compiler version in
examples/swap/hardhat.config.tsto use a valid version, such as 0.8.24 or 0.8.20.
🔗 Analysis chain
Verify Solidity compiler versions across all configurations.
Let's check for other potential invalid compiler versions in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for Solidity compiler version specifications in all hardhat config files
echo "Searching for Solidity compiler versions in hardhat configs..."
rg -A 3 'solidity.*:.*{' $(fd -g '*hardhat.config.*')
Length of output: 379
Script:
#!/bin/bash
# Search for Solidity compiler configurations with proper regex
echo "Searching in hardhat config files..."
rg -U "solidity:\s*{[\s\S]*?}" -A 5 $(fd -g '*hardhat.config.*')
Length of output: 364
Script:
#!/bin/bash
# Search for Solidity configurations with simple pattern
echo "Checking hardhat configurations..."
for file in examples/call/hardhat.config.ts examples/hello/hardhat.config.ts examples/swap/hardhat.config.ts; do
echo "=== $file ==="
rg "solidity" -B 1 -A 5 "$file" || true
done
Length of output: 1036
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
examples/hello/hardhat.config.ts (1)
1-15: Maintain configuration consistency across examples.This configuration mirrors
examples/call/hardhat.config.ts. Consider extracting common configuration into a shared utility.Create a shared configuration utility:
// shared/hardhat.config.base.ts import { HardhatUserConfig } from "hardhat/config"; import * as dotenv from "dotenv"; import { getHardhatConfig } from "@zetachain/toolkit/client"; export function createBaseConfig(): HardhatUserConfig { dotenv.config(); if (!process.env.PRIVATE_KEY) { throw new Error("PRIVATE_KEY environment variable is required"); } return { ...getHardhatConfig({ accounts: [process.env.PRIVATE_KEY] }), solidity: { version: "0.8.21", settings: { optimizer: { enabled: true, runs: 200, }, }, }, }; }Then simplify both config files:
import { createBaseConfig } from "../../shared/hardhat.config.base"; export default createBaseConfig();🧰 Tools
🪛 GitHub Actions: Run Tests
[error] Compiler configuration needs to be updated to support Solidity versions 0.8.20, 0.8.21, and 0.8.26
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
examples/call/hardhat.config.ts(1 hunks)examples/call/tasks/index.ts(1 hunks)examples/hello/hardhat.config.ts(1 hunks)examples/swap/hardhat.config.ts(1 hunks)examples/swap/tasks/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- examples/swap/tasks/index.ts
- examples/call/tasks/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/swap/hardhat.config.ts
🧰 Additional context used
🪛 GitHub Actions: Run Tests
examples/hello/hardhat.config.ts
[error] Compiler configuration needs to be updated to support Solidity versions 0.8.20, 0.8.21, and 0.8.26
examples/call/hardhat.config.ts
[error] Compiler configuration needs to be updated to support Solidity versions 0.8.20, 0.8.21, and 0.8.26
| dotenv.config(); | ||
|
|
||
| const config: HardhatUserConfig = { | ||
| networks: { | ||
| ...getHardhatConfigNetworks(), | ||
| }, | ||
| solidity: "0.8.26", | ||
| ...getHardhatConfig({ accounts: [process.env.PRIVATE_KEY] }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance environment variable validation.
The configuration directly uses process.env.PRIVATE_KEY without validation. Add checks to ensure the private key is properly set.
Add this validation before the config:
dotenv.config();
+if (!process.env.PRIVATE_KEY) {
+ throw new Error("PRIVATE_KEY environment variable is required");
+}
+
const config: HardhatUserConfig = {📝 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.
| dotenv.config(); | |
| const config: HardhatUserConfig = { | |
| networks: { | |
| ...getHardhatConfigNetworks(), | |
| }, | |
| solidity: "0.8.26", | |
| ...getHardhatConfig({ accounts: [process.env.PRIVATE_KEY] }), | |
| dotenv.config(); | |
| if (!process.env.PRIVATE_KEY) { | |
| throw new Error("PRIVATE_KEY environment variable is required"); | |
| } | |
| const config: HardhatUserConfig = { | |
| ...getHardhatConfig({ accounts: [process.env.PRIVATE_KEY] }), |
🧰 Tools
🪛 GitHub Actions: Run Tests
[error] Compiler configuration needs to be updated to support Solidity versions 0.8.20, 0.8.21, and 0.8.26
| ...getHardhatConfigNetworks(), | ||
| }, | ||
| solidity: "0.8.26", | ||
| ...getHardhatConfig({ accounts: [process.env.PRIVATE_KEY] }), |
There was a problem hiding this comment.
Add compiler configuration to resolve pipeline failures.
The pipeline is failing due to missing Solidity compiler configuration. Add the compiler settings to support the required versions.
Apply this diff to resolve the compiler configuration:
const config: HardhatUserConfig = {
...getHardhatConfig({ accounts: [process.env.PRIVATE_KEY] }),
+ solidity: {
+ version: "0.8.21",
+ settings: {
+ optimizer: {
+ enabled: true,
+ runs: 200,
+ },
+ },
+ },
};Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: Run Tests
[error] Compiler configuration needs to be updated to support Solidity versions 0.8.20, 0.8.21, and 0.8.26
| function callMulti( | ||
| bytes[] memory receiverArray, | ||
| address[] memory zrc20Array, | ||
| bytes calldata messages, | ||
| CallOptions memory callOptions, | ||
| RevertOptions memory revertOptions | ||
| ) external { | ||
| for (uint256 i = 0; i < zrc20Array.length; i++) { | ||
| (, uint256 gasFee) = IZRC20(zrc20Array[i]) | ||
| .withdrawGasFeeWithGasLimit(callOptions.gasLimit); | ||
| if ( | ||
| !IZRC20(zrc20Array[i]).transferFrom( | ||
| msg.sender, | ||
| address(this), | ||
| gasFee | ||
| ) | ||
| ) { | ||
| revert TransferFailed(); | ||
| } | ||
| IZRC20(zrc20Array[i]).approve(address(gateway), gasFee); | ||
| gateway.call( | ||
| receiverArray[i], | ||
| zrc20Array[i], | ||
| messages, | ||
| callOptions, | ||
| revertOptions | ||
| ); | ||
| } | ||
| } |
Check warning
Code scanning / Slither
Unused return Medium
| function callMulti( | ||
| bytes[] memory receiverArray, | ||
| address[] memory zrc20Array, | ||
| bytes calldata messages, | ||
| CallOptions memory callOptions, | ||
| RevertOptions memory revertOptions | ||
| ) external { | ||
| for (uint256 i = 0; i < zrc20Array.length; i++) { | ||
| (, uint256 gasFee) = IZRC20(zrc20Array[i]) | ||
| .withdrawGasFeeWithGasLimit(callOptions.gasLimit); | ||
| if ( | ||
| !IZRC20(zrc20Array[i]).transferFrom( | ||
| msg.sender, | ||
| address(this), | ||
| gasFee | ||
| ) | ||
| ) { | ||
| revert TransferFailed(); | ||
| } | ||
| IZRC20(zrc20Array[i]).approve(address(gateway), gasFee); | ||
| gateway.call( | ||
| receiverArray[i], | ||
| zrc20Array[i], | ||
| messages, | ||
| callOptions, | ||
| revertOptions | ||
| ); | ||
| } | ||
| } |
Check warning
Code scanning / Slither
Unused return Medium
| function callMulti( | ||
| bytes[] memory receiverArray, | ||
| address[] memory zrc20Array, | ||
| bytes calldata messages, | ||
| CallOptions memory callOptions, | ||
| RevertOptions memory revertOptions | ||
| ) external { | ||
| for (uint256 i = 0; i < zrc20Array.length; i++) { | ||
| (, uint256 gasFee) = IZRC20(zrc20Array[i]) | ||
| .withdrawGasFeeWithGasLimit(callOptions.gasLimit); | ||
| if ( | ||
| !IZRC20(zrc20Array[i]).transferFrom( | ||
| msg.sender, | ||
| address(this), | ||
| gasFee | ||
| ) | ||
| ) { | ||
| revert TransferFailed(); | ||
| } | ||
| IZRC20(zrc20Array[i]).approve(address(gateway), gasFee); | ||
| gateway.call( | ||
| receiverArray[i], | ||
| zrc20Array[i], | ||
| messages, | ||
| callOptions, | ||
| revertOptions | ||
| ); | ||
| } | ||
| } |
Check notice
Code scanning / Slither
Calls inside a loop Low
| function callMulti( | ||
| bytes[] memory receiverArray, | ||
| address[] memory zrc20Array, | ||
| bytes calldata messages, | ||
| CallOptions memory callOptions, | ||
| RevertOptions memory revertOptions | ||
| ) external { | ||
| for (uint256 i = 0; i < zrc20Array.length; i++) { | ||
| (, uint256 gasFee) = IZRC20(zrc20Array[i]) | ||
| .withdrawGasFeeWithGasLimit(callOptions.gasLimit); | ||
| if ( | ||
| !IZRC20(zrc20Array[i]).transferFrom( | ||
| msg.sender, | ||
| address(this), | ||
| gasFee | ||
| ) | ||
| ) { | ||
| revert TransferFailed(); | ||
| } | ||
| IZRC20(zrc20Array[i]).approve(address(gateway), gasFee); | ||
| gateway.call( | ||
| receiverArray[i], | ||
| zrc20Array[i], | ||
| messages, | ||
| callOptions, | ||
| revertOptions | ||
| ); | ||
| } | ||
| } |
Check notice
Code scanning / Slither
Calls inside a loop Low
| function callMulti( | ||
| bytes[] memory receiverArray, | ||
| address[] memory zrc20Array, | ||
| bytes calldata messages, | ||
| CallOptions memory callOptions, | ||
| RevertOptions memory revertOptions | ||
| ) external { | ||
| for (uint256 i = 0; i < zrc20Array.length; i++) { | ||
| (, uint256 gasFee) = IZRC20(zrc20Array[i]) | ||
| .withdrawGasFeeWithGasLimit(callOptions.gasLimit); | ||
| if ( | ||
| !IZRC20(zrc20Array[i]).transferFrom( | ||
| msg.sender, | ||
| address(this), | ||
| gasFee | ||
| ) | ||
| ) { | ||
| revert TransferFailed(); | ||
| } | ||
| IZRC20(zrc20Array[i]).approve(address(gateway), gasFee); | ||
| gateway.call( | ||
| receiverArray[i], | ||
| zrc20Array[i], | ||
| messages, | ||
| callOptions, | ||
| revertOptions | ||
| ); | ||
| } | ||
| } |
Check notice
Code scanning / Slither
Calls inside a loop Low
| function callMulti( | ||
| bytes[] memory receiverArray, | ||
| address[] memory zrc20Array, | ||
| bytes calldata messages, | ||
| CallOptions memory callOptions, | ||
| RevertOptions memory revertOptions | ||
| ) external { | ||
| for (uint256 i = 0; i < zrc20Array.length; i++) { | ||
| (, uint256 gasFee) = IZRC20(zrc20Array[i]) | ||
| .withdrawGasFeeWithGasLimit(callOptions.gasLimit); | ||
| if ( | ||
| !IZRC20(zrc20Array[i]).transferFrom( | ||
| msg.sender, | ||
| address(this), | ||
| gasFee | ||
| ) | ||
| ) { | ||
| revert TransferFailed(); | ||
| } | ||
| IZRC20(zrc20Array[i]).approve(address(gateway), gasFee); | ||
| gateway.call( | ||
| receiverArray[i], | ||
| zrc20Array[i], | ||
| messages, | ||
| callOptions, | ||
| revertOptions | ||
| ); | ||
| } | ||
| } |
Check notice
Code scanning / Slither
Calls inside a loop Low
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
examples/call/tasks/universalCallMulti.ts (1)
34-48: Enhance type validation for improved robustness.The type conversion logic could be more robust by adding validation for supported types and handling edge cases.
Consider adding comprehensive type validation:
const valuesArray = args.values.map((value: any, index: number) => { const type = types[index]; + if (!type.match(/^(bool|u?int\d*|address|bytes\d*|string)$/)) { + throw new Error(`Unsupported type: ${type}`); + } if (type === "bool") { try { return JSON.parse(value.toLowerCase()); } catch (e) { throw new Error(`Invalid boolean value: ${value}`); } } else if (type.startsWith("uint") || type.startsWith("int")) { + try { return ethers.BigNumber.from(value); + } catch (e) { + throw new Error(`Invalid number for type ${type}: ${value}`); + } } else { return value; } });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
examples/call/yarn.lockis excluded by!**/yarn.lock,!**/*.lockexamples/hello/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
examples/call/contracts/Universal.sol(1 hunks)examples/call/package.json(2 hunks)examples/call/tasks/index.ts(1 hunks)examples/call/tasks/universalCallMulti.ts(1 hunks)examples/hello/package.json(2 hunks)examples/swap/tasks/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- examples/swap/tasks/index.ts
- examples/hello/package.json
- examples/call/package.json
🧰 Additional context used
🪛 GitHub Check: Slither
examples/call/contracts/Universal.sol
[warning] 44-72: Unused return
Universal.callMulti(bytes[],address[],bytes,CallOptions,RevertOptions) (contracts/Universal.sol#44-72) ignores return value by (None,gasFee) = IZRC20(zrc20Array[i]).withdrawGasFeeWithGasLimit(callOptions.gasLimit) (contracts/Universal.sol#52-53)
[warning] 44-72: Unused return
Universal.callMulti(bytes[],address[],bytes,CallOptions,RevertOptions) (contracts/Universal.sol#44-72) ignores return value by IZRC20(zrc20Array[i]).approve(address(gateway),gasFee) (contracts/Universal.sol#63)
[notice] 44-72: Calls inside a loop
Universal.callMulti(bytes[],address[],bytes,CallOptions,RevertOptions) (contracts/Universal.sol#44-72) has external calls inside a loop: (None,gasFee) = IZRC20(zrc20Array[i]).withdrawGasFeeWithGasLimit(callOptions.gasLimit) (contracts/Universal.sol#52-53)
[notice] 44-72: Calls inside a loop
Universal.callMulti(bytes[],address[],bytes,CallOptions,RevertOptions) (contracts/Universal.sol#44-72) has external calls inside a loop: gateway.call(receiverArray[i],zrc20Array[i],messages,callOptions,revertOptions) (contracts/Universal.sol#64-70)
[notice] 44-72: Calls inside a loop
Universal.callMulti(bytes[],address[],bytes,CallOptions,RevertOptions) (contracts/Universal.sol#44-72) has external calls inside a loop: ! IZRC20(zrc20Array[i]).transferFrom(msg.sender,address(this),gasFee) (contracts/Universal.sol#55-59)
[notice] 44-72: Calls inside a loop
Universal.callMulti(bytes[],address[],bytes,CallOptions,RevertOptions) (contracts/Universal.sol#44-72) has external calls inside a loop: IZRC20(zrc20Array[i]).approve(address(gateway),gasFee) (contracts/Universal.sol#63)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: slither (examples/call, call.sarif)
- GitHub Check: slither (examples/hello, hello.sarif)
🔇 Additional comments (1)
examples/call/tasks/index.ts (1)
1-8: LGTM!The imports are well-organized and follow a clear modular structure.
| const tx = await contract.callMulti( | ||
| encodedReceivers, | ||
| zrc20Array, | ||
| message, | ||
| callOptions, | ||
| revertOptions | ||
| ); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for the multi-call transaction.
The multi-call transaction should include proper error handling and event logging.
-const tx = await contract.callMulti(
+try {
+ const tx = await contract.callMulti(
encodedReceivers,
zrc20Array,
message,
callOptions,
revertOptions
);
+ const receipt = await tx.wait();
+ if (!receipt.status) {
+ throw new Error("Multi-call transaction failed");
+ }
+ console.log(`Transaction hash: ${tx.hash}`);
+} catch (error) {
+ console.error("Failed to execute multi-call:", error);
+ throw error;
+}📝 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.
| const tx = await contract.callMulti( | |
| encodedReceivers, | |
| zrc20Array, | |
| message, | |
| callOptions, | |
| revertOptions | |
| ); | |
| try { | |
| const tx = await contract.callMulti( | |
| encodedReceivers, | |
| zrc20Array, | |
| message, | |
| callOptions, | |
| revertOptions | |
| ); | |
| const receipt = await tx.wait(); | |
| if (!receipt.status) { | |
| throw new Error("Multi-call transaction failed"); | |
| } | |
| console.log(`Transaction hash: ${tx.hash}`); | |
| } catch (error) { | |
| console.error("Failed to execute multi-call:", error); | |
| throw error; | |
| } |
| for (const zrc20 of zrc20Contracts) { | ||
| const [, gasFee] = await zrc20.withdrawGasFeeWithGasLimit(gasLimit); | ||
| const zrc20TransferTx = await zrc20.approve(args.contract, gasFee); | ||
| await zrc20TransferTx.wait(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add transaction receipt validation for approvals.
The approval transactions should be validated to ensure they were successful.
for (const zrc20 of zrc20Contracts) {
const [, gasFee] = await zrc20.withdrawGasFeeWithGasLimit(gasLimit);
const zrc20TransferTx = await zrc20.approve(args.contract, gasFee);
- await zrc20TransferTx.wait();
+ const receipt = await zrc20TransferTx.wait();
+ if (!receipt.status) {
+ throw new Error(`Failed to approve ZRC20 token: ${zrc20.address}`);
+ }
}📝 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.
| for (const zrc20 of zrc20Contracts) { | |
| const [, gasFee] = await zrc20.withdrawGasFeeWithGasLimit(gasLimit); | |
| const zrc20TransferTx = await zrc20.approve(args.contract, gasFee); | |
| await zrc20TransferTx.wait(); | |
| } | |
| for (const zrc20 of zrc20Contracts) { | |
| const [, gasFee] = await zrc20.withdrawGasFeeWithGasLimit(gasLimit); | |
| const zrc20TransferTx = await zrc20.approve(args.contract, gasFee); | |
| const receipt = await zrc20TransferTx.wait(); | |
| if (!receipt.status) { | |
| throw new Error(`Failed to approve ZRC20 token: ${zrc20.address}`); | |
| } | |
| } |
| function callMulti( | ||
| bytes[] memory receiverArray, | ||
| address[] memory zrc20Array, | ||
| bytes calldata messages, | ||
| CallOptions memory callOptions, | ||
| RevertOptions memory revertOptions | ||
| ) external { | ||
| for (uint256 i = 0; i < zrc20Array.length; i++) { | ||
| (, uint256 gasFee) = IZRC20(zrc20Array[i]) | ||
| .withdrawGasFeeWithGasLimit(callOptions.gasLimit); | ||
| if ( | ||
| !IZRC20(zrc20Array[i]).transferFrom( | ||
| msg.sender, | ||
| address(this), | ||
| gasFee | ||
| ) | ||
| ) { | ||
| revert TransferFailed(); | ||
| } | ||
| IZRC20(zrc20Array[i]).approve(address(gateway), gasFee); | ||
| gateway.call( | ||
| receiverArray[i], | ||
| zrc20Array[i], | ||
| messages, | ||
| callOptions, | ||
| revertOptions | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Address potential security and gas optimization concerns in the multi-call implementation.
The current implementation has several areas that need attention:
- No validation for array length matching between
receiverArrayandzrc20Array - Multiple external calls in a loop could lead to high gas costs
- No checks for array length limits
Consider implementing these improvements:
function callMulti(
bytes[] memory receiverArray,
address[] memory zrc20Array,
bytes calldata messages,
CallOptions memory callOptions,
RevertOptions memory revertOptions
) external {
+ if (receiverArray.length != zrc20Array.length) {
+ revert("Array lengths mismatch");
+ }
+ if (receiverArray.length == 0) {
+ revert("Empty arrays");
+ }
+ if (receiverArray.length > 10) { // Adjust limit based on gas considerations
+ revert("Too many calls");
+ }
+
+ uint256[] memory gasFees = new uint256[](zrc20Array.length);
+ // Batch all checks first
+ for (uint256 i = 0; i < zrc20Array.length; i++) {
+ (, gasFees[i]) = IZRC20(zrc20Array[i])
+ .withdrawGasFeeWithGasLimit(callOptions.gasLimit);
+ if (!IZRC20(zrc20Array[i]).transferFrom(
+ msg.sender,
+ address(this),
+ gasFees[i]
+ )) {
+ revert TransferFailed();
+ }
+ }
+
+ // Then batch all approvals and calls
+ for (uint256 i = 0; i < zrc20Array.length; i++) {
+ IZRC20(zrc20Array[i]).approve(address(gateway), gasFees[i]);
gateway.call(
receiverArray[i],
zrc20Array[i],
messages,
callOptions,
revertOptions
);
}
}📝 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.
| function callMulti( | |
| bytes[] memory receiverArray, | |
| address[] memory zrc20Array, | |
| bytes calldata messages, | |
| CallOptions memory callOptions, | |
| RevertOptions memory revertOptions | |
| ) external { | |
| for (uint256 i = 0; i < zrc20Array.length; i++) { | |
| (, uint256 gasFee) = IZRC20(zrc20Array[i]) | |
| .withdrawGasFeeWithGasLimit(callOptions.gasLimit); | |
| if ( | |
| !IZRC20(zrc20Array[i]).transferFrom( | |
| msg.sender, | |
| address(this), | |
| gasFee | |
| ) | |
| ) { | |
| revert TransferFailed(); | |
| } | |
| IZRC20(zrc20Array[i]).approve(address(gateway), gasFee); | |
| gateway.call( | |
| receiverArray[i], | |
| zrc20Array[i], | |
| messages, | |
| callOptions, | |
| revertOptions | |
| ); | |
| } | |
| } | |
| function callMulti( | |
| bytes[] memory receiverArray, | |
| address[] memory zrc20Array, | |
| bytes calldata messages, | |
| CallOptions memory callOptions, | |
| RevertOptions memory revertOptions | |
| ) external { | |
| if (receiverArray.length != zrc20Array.length) { | |
| revert("Array lengths mismatch"); | |
| } | |
| if (receiverArray.length == 0) { | |
| revert("Empty arrays"); | |
| } | |
| if (receiverArray.length > 10) { // Adjust limit based on gas considerations | |
| revert("Too many calls"); | |
| } | |
| uint256[] memory gasFees = new uint256[](zrc20Array.length); | |
| // Batch all checks first | |
| for (uint256 i = 0; i < zrc20Array.length; i++) { | |
| (, gasFees[i]) = IZRC20(zrc20Array[i]) | |
| .withdrawGasFeeWithGasLimit(callOptions.gasLimit); | |
| if (!IZRC20(zrc20Array[i]).transferFrom( | |
| msg.sender, | |
| address(this), | |
| gasFees[i] | |
| )) { | |
| revert TransferFailed(); | |
| } | |
| } | |
| // Then batch all approvals and calls | |
| for (uint256 i = 0; i < zrc20Array.length; i++) { | |
| IZRC20(zrc20Array[i]).approve(address(gateway), gasFees[i]); | |
| gateway.call( | |
| receiverArray[i], | |
| zrc20Array[i], | |
| messages, | |
| callOptions, | |
| revertOptions | |
| ); | |
| } | |
| } |
🧰 Tools
🪛 GitHub Check: Slither
[warning] 44-72: Unused return
Universal.callMulti(bytes[],address[],bytes,CallOptions,RevertOptions) (contracts/Universal.sol#44-72) ignores return value by (None,gasFee) = IZRC20(zrc20Array[i]).withdrawGasFeeWithGasLimit(callOptions.gasLimit) (contracts/Universal.sol#52-53)
[warning] 44-72: Unused return
Universal.callMulti(bytes[],address[],bytes,CallOptions,RevertOptions) (contracts/Universal.sol#44-72) ignores return value by IZRC20(zrc20Array[i]).approve(address(gateway),gasFee) (contracts/Universal.sol#63)
[notice] 44-72: Calls inside a loop
Universal.callMulti(bytes[],address[],bytes,CallOptions,RevertOptions) (contracts/Universal.sol#44-72) has external calls inside a loop: (None,gasFee) = IZRC20(zrc20Array[i]).withdrawGasFeeWithGasLimit(callOptions.gasLimit) (contracts/Universal.sol#52-53)
[notice] 44-72: Calls inside a loop
Universal.callMulti(bytes[],address[],bytes,CallOptions,RevertOptions) (contracts/Universal.sol#44-72) has external calls inside a loop: gateway.call(receiverArray[i],zrc20Array[i],messages,callOptions,revertOptions) (contracts/Universal.sol#64-70)
[notice] 44-72: Calls inside a loop
Universal.callMulti(bytes[],address[],bytes,CallOptions,RevertOptions) (contracts/Universal.sol#44-72) has external calls inside a loop: ! IZRC20(zrc20Array[i]).transferFrom(msg.sender,address(this),gasFee) (contracts/Universal.sol#55-59)
[notice] 44-72: Calls inside a loop
Universal.callMulti(bytes[],address[],bytes,CallOptions,RevertOptions) (contracts/Universal.sol#44-72) has external calls inside a loop: IZRC20(zrc20Array[i]).approve(address(gateway),gasFee) (contracts/Universal.sol#63)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
examples/nft/README.md (1)
1-3: Consider enhancing the documentation with additional sections.The README appears minimal. Consider adding sections that would be valuable for developers:
- Overview of the Universal NFT functionality
- Prerequisites and setup instructions
- Usage examples and code snippets
- Testing instructions
Would you like me to generate a comprehensive template for these sections?
🧰 Tools
🪛 Markdownlint (0.37.0)
3-3: null
Bare URL used(MD034, no-bare-urls)
examples/token/README.md (1)
1-3: Enhance documentation readability with proper markdown formatting.The bare URL should be formatted as a proper markdown link with descriptive text.
Apply this change:
# Universal NFT Example -https://www.zetachain.com/docs/developers/standards/nft/ +For detailed information about NFT standards, please refer to the [ZetaChain NFT Documentation](https://www.zetachain.com/docs/developers/standards/nft/).🧰 Tools
🪛 Markdownlint (0.37.0)
3-3: null
Bare URL used(MD034, no-bare-urls)
examples/token/package.json (1)
57-64: Standardize dependency version constraints.The dependencies use a mix of fixed versions and caret ranges. Consider standardizing the version constraints across all dependencies for better predictability and maintenance.
For example, either use fixed versions for all dependencies:
- "@solana-developers/helpers": "^2.4.0", + "@solana-developers/helpers": "2.4.0", - "@solana/spl-memo": "^0.2.5", + "@solana/spl-memo": "0.2.5", - "validator": "^13.12.0" + "validator": "13.12.0"Or use caret ranges consistently:
- "@coral-xyz/anchor": "0.30.0", + "@coral-xyz/anchor": "^0.30.0", - "@openzeppelin/hardhat-upgrades": "1.28.0", + "@openzeppelin/hardhat-upgrades": "^1.28.0",examples/nft/hardhat.config.ts (1)
3-13: Add environment variable validationWhile using dotenv for managing private keys is secure, consider adding validation to ensure required environment variables are present before configuration.
Add this validation after
dotenv.config():dotenv.config(); + +if (!process.env.PRIVATE_KEY) { + throw new Error("PRIVATE_KEY environment variable is required"); +}Also, consider adding a
.env.examplefile to document required environment variables.examples/token/hardhat.config.ts (1)
1-29: Consider creating a shared configuration baseThis configuration is nearly identical to
examples/nft/hardhat.config.ts. Consider creating a shared base configuration to reduce duplication and maintain consistency across projects.Create a shared configuration file that can be extended:
// shared.hardhat.config.ts import { HardhatUserConfig } from "hardhat/config"; import * as dotenv from "dotenv"; export function createBaseConfig(): HardhatUserConfig { dotenv.config(); if (!process.env.PRIVATE_KEY) { throw new Error("PRIVATE_KEY environment variable is required"); } return { ...getHardhatConfig({ accounts: [process.env.PRIVATE_KEY] }), solidity: { compilers: [ { settings: { optimizer: { enabled: true, runs: 1000, }, }, version: "0.8.26", }, ], }, }; }Then in each project:
import { createBaseConfig } from "../shared.hardhat.config"; // Project-specific imports... const config = { ...createBaseConfig(), // Project-specific overrides... }; export default config;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
examples/nft/yarn.lockis excluded by!**/yarn.lock,!**/*.lockexamples/token/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
examples/nft/README.md(1 hunks)examples/nft/contracts/EVMUniversalNFT.sol(1 hunks)examples/nft/contracts/ZetaChainUniversalNFT.sol(1 hunks)examples/nft/hardhat.config.ts(1 hunks)examples/nft/package.json(3 hunks)examples/nft/tsconfig.json(0 hunks)examples/token/README.md(1 hunks)examples/token/contracts/EVMUniversalToken.sol(1 hunks)examples/token/contracts/ZetaChainUniversalToken.sol(1 hunks)examples/token/hardhat.config.ts(1 hunks)examples/token/package.json(3 hunks)
💤 Files with no reviewable changes (1)
- examples/nft/tsconfig.json
✅ Files skipped from review due to trivial changes (4)
- examples/token/contracts/ZetaChainUniversalToken.sol
- examples/nft/contracts/EVMUniversalNFT.sol
- examples/nft/contracts/ZetaChainUniversalNFT.sol
- examples/token/contracts/EVMUniversalToken.sol
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/nft/package.json
🧰 Additional context used
🪛 Markdownlint (0.37.0)
examples/nft/README.md
3-3: null
Bare URL used
(MD034, no-bare-urls)
examples/token/README.md
3-3: null
Bare URL used
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: test (examples/call)
- GitHub Check: slither (examples/swap, swap.sarif)
- GitHub Check: slither (examples/call, call.sarif)
- GitHub Check: slither (examples/hello, hello.sarif)
🔇 Additional comments (2)
examples/nft/hardhat.config.ts (2)
15-29: Configuration looks production-readyThe Hardhat configuration is well-structured with:
- Secure account management using environment variables
- Optimized compiler settings with 1000 runs
- Recent Solidity version (0.8.26)
1-11: Verify dependency compatibilityThe configuration imports multiple Hardhat plugins and ZetaChain-specific packages. Ensure version compatibility between these dependencies.
Run this script to check package versions and their compatibility:
✅ Verification successful
Dependencies are properly configured and compatible
The package.json contains all required dependencies with appropriate version constraints. The Hardhat ecosystem packages are compatible with each other, and the ZetaChain-specific packages consistently use release candidate versions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check package.json for dependency versions jq '.dependencies + .devDependencies' package.json # Check for any reported compatibility issues npm ls @nomicfoundation/hardhat-toolbox @nomiclabs/hardhat-ethers @openzeppelin/hardhat-upgrades @zetachain/toolkitLength of output: 257
Script:
#!/bin/bash # Check if package files exist and show their content echo "=== Checking package.json ===" if [ -f package.json ]; then cat package.json else echo "package.json not found" fi echo -e "\n=== Checking for lock files ===" if [ -f yarn.lock ]; then echo "yarn.lock exists" grep -A 1 "@nomicfoundation\|@nomiclabs\|@openzeppelin\|@zetachain" yarn.lock elif [ -f package-lock.json ]; then echo "package-lock.json exists" grep -A 1 "@nomicfoundation\|@nomiclabs\|@openzeppelin\|@zetachain" package-lock.json else echo "No lock files found" fi echo -e "\n=== Searching for version patterns in config files ===" fd -e js -e ts -e json | xargs grep -l "version\|@[0-9]"Length of output: 1059
Script:
#!/bin/bash echo "=== NFT Example Package.json ===" cat examples/nft/package.json echo -e "\n=== Hardhat Config Version Patterns ===" cat examples/nft/hardhat.config.ts | grep -A 1 "require\|import\|version" echo -e "\n=== Searching for version requirements in NFT example ===" cd examples/nft && find . -type f -exec grep -l "requires\|>=\|<=\|@[0-9]" {} \;Length of output: 3423
|
|
||
| This example currently only works with localnet `v4.0.0-rc*`, which supports | ||
| authenticated calls and multiple EVM chains. | ||
| https://www.zetachain.com/docs/developers/standards/nft/ |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Format the documentation link using proper Markdown syntax.
The bare URL should be formatted as a proper Markdown link for better readability and maintainability.
Apply this change:
-https://www.zetachain.com/docs/developers/standards/nft/
+Please refer to the [ZetaChain NFT Standards Documentation](https://www.zetachain.com/docs/developers/standards/nft/) for detailed information about NFT 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.
| https://www.zetachain.com/docs/developers/standards/nft/ | |
| Please refer to the [ZetaChain NFT Standards Documentation](https://www.zetachain.com/docs/developers/standards/nft/) for detailed information about NFT implementation. |
🧰 Tools
🪛 Markdownlint (0.37.0)
3-3: null
Bare URL used
(MD034, no-bare-urls)
| @@ -6,7 +6,8 @@ | |||
| "scripts": { | |||
| "test": "echo \"Error: no test specified\" && exit 1", | |||
| "lint:fix": "npx eslint . --ext .js,.ts --fix", | |||
| "lint": "npx eslint . --ext .js,.ts" | |||
| "lint": "npx eslint . --ext .js,.ts", | |||
| "deploy:localnet": "npx hardhat compile --force && npx hardhat deploy --network localhost --gateway 0x9A676e781A523b5d0C0e43731313A708CB607508 && npx hardhat deploy --name Echo --network localhost --gateway 0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0" | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid hardcoded gateway addresses in deployment scripts.
The deploy:localnet script contains hardcoded gateway addresses. Consider moving these to environment variables or configuration files for better maintainability and flexibility.
Example improvement:
- "deploy:localnet": "npx hardhat compile --force && npx hardhat deploy --network localhost --gateway 0x9A676e781A523b5d0C0e43731313A708CB607508 && npx hardhat deploy --name Echo --network localhost --gateway 0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0"
+ "deploy:localnet": "npx hardhat compile --force && npx hardhat deploy --network localhost --gateway $MAIN_GATEWAY && npx hardhat deploy --name Echo --network localhost --gateway $ECHO_GATEWAY"📝 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.
| "deploy:localnet": "npx hardhat compile --force && npx hardhat deploy --network localhost --gateway 0x9A676e781A523b5d0C0e43731313A708CB607508 && npx hardhat deploy --name Echo --network localhost --gateway 0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0" | |
| "deploy:localnet": "npx hardhat compile --force && npx hardhat deploy --network localhost --gateway $MAIN_GATEWAY && npx hardhat deploy --name Echo --network localhost --gateway $ECHO_GATEWAY" |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
examples/call/package.json(2 hunks)examples/hello/package.json(2 hunks)examples/nft/package.json(3 hunks)examples/nft/scripts/localnet.sh(1 hunks)examples/swap/package.json(2 hunks)examples/token/package.json(3 hunks)examples/token/scripts/localnet.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- examples/hello/package.json
- examples/swap/package.json
- examples/call/package.json
- examples/token/package.json
- examples/nft/package.json
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: test (examples/call)
- GitHub Check: slither (examples/swap, swap.sarif)
- GitHub Check: slither (examples/call, call.sarif)
- GitHub Check: slither (examples/hello, hello.sarif)
examples/token/scripts/localnet.sh
Outdated
| sh ./node_modules/@zetachain/standard-contracts/contracts/token/scripts/localnet.sh $1 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance script robustness and argument handling.
The current implementation has several areas for improvement:
- No validation of the target script's existence
- Limited to forwarding only the first argument
- Potential issues with script permissions when using
sh
Consider applying this more robust implementation:
-sh ./node_modules/@zetachain/standard-contracts/contracts/token/scripts/localnet.sh $1
+TARGET_SCRIPT="./node_modules/@zetachain/standard-contracts/contracts/token/scripts/localnet.sh"
+if [ ! -f "$TARGET_SCRIPT" ]; then
+ echo "Error: Required script not found at $TARGET_SCRIPT"
+ echo "Please ensure @zetachain/standard-contracts is installed"
+ exit 1
+fi
+
+"$TARGET_SCRIPT" "$@"This implementation:
- Validates the existence of the target script
- Forwards all arguments using
"$@" - Uses direct execution instead of
shto preserve permissions
📝 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.
| sh ./node_modules/@zetachain/standard-contracts/contracts/token/scripts/localnet.sh $1 | |
| TARGET_SCRIPT="./node_modules/@zetachain/standard-contracts/contracts/token/scripts/localnet.sh" | |
| if [ ! -f "$TARGET_SCRIPT" ]; then | |
| echo "Error: Required script not found at $TARGET_SCRIPT" | |
| echo "Please ensure @zetachain/standard-contracts is installed" | |
| exit 1 | |
| fi | |
| "$TARGET_SCRIPT" "$@" |
examples/nft/scripts/localnet.sh
Outdated
| sh ./node_modules/@zetachain/standard-contracts/contracts/nft/scripts/localnet.sh $1 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and pass all arguments.
The current implementation has two issues:
- No error handling for missing dependencies
- Only passes the first argument ($1), potentially losing additional arguments
Apply this diff to improve the implementation:
-sh ./node_modules/@zetachain/standard-contracts/contracts/nft/scripts/localnet.sh $1
+SCRIPT_PATH="./node_modules/@zetachain/standard-contracts/contracts/nft/scripts/localnet.sh"
+
+if [ ! -f "$SCRIPT_PATH" ]; then
+ echo "Error: Required dependencies not found. Please run 'npm install' first."
+ exit 1
+fi
+
+sh "$SCRIPT_PATH" "$@"📝 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.
| sh ./node_modules/@zetachain/standard-contracts/contracts/nft/scripts/localnet.sh $1 | |
| SCRIPT_PATH="./node_modules/@zetachain/standard-contracts/contracts/nft/scripts/localnet.sh" | |
| if [ ! -f "$SCRIPT_PATH" ]; then | |
| echo "Error: Required dependencies not found. Please run 'npm install' first." | |
| exit 1 | |
| fi | |
| sh "$SCRIPT_PATH" "$@" |
💡 Codebase verification
Critical: Script references non-existent dependency
The current implementation attempts to execute a non-existent script from @zetachain/standard-contracts package. This will cause runtime failures as:
- The referenced package is not listed in dependencies
- The script path
./node_modules/@zetachain/standard-contracts/contracts/nft/scripts/localnet.shdoes not exist
Consider implementing a self-contained script following the established pattern in other examples (e.g., examples/hello/scripts/localnet.sh), which includes proper localnet management, contract compilation, deployment, and verification steps.
🔗 Analysis chain
Verify the script's existence in the codebase.
Let's ensure the referenced script exists and contains the expected functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence and content of the delegated script
# Check if the script exists
if [ -f "./node_modules/@zetachain/standard-contracts/contracts/nft/scripts/localnet.sh" ]; then
echo "Script exists. Showing first 10 lines:"
head -n 10 "./node_modules/@zetachain/standard-contracts/contracts/nft/scripts/localnet.sh"
else
echo "Warning: Referenced script not found"
fi
# Check package.json for the dependency
if [ -f "package.json" ]; then
echo "Checking @zetachain/standard-contracts dependency:"
cat package.json | jq '.dependencies["@zetachain/standard-contracts"]'
fi
Length of output: 389
Script:
#!/bin/bash
# Search for similar script files and package references
echo "Searching for similar script files:"
fd localnet.sh
echo -e "\nSearching for zetachain references in package.json:"
cat package.json | grep -i zetachain || echo "No zetachain references found"
echo -e "\nChecking for documentation:"
fd -e md | xargs grep -l "zetachain" || echo "No documentation references found"
Length of output: 732
Script:
#!/bin/bash
echo "Content of examples/hello/scripts/localnet.sh (as reference):"
cat examples/hello/scripts/localnet.sh
echo -e "\nContent of examples/nft/scripts/localnet.sh (current):"
cat examples/nft/scripts/localnet.sh
Length of output: 1309
fadeev
changed the title
quoteMinInputcontract function, which returns how many tokens you need to supply to cover withdraw feeSummary by CodeRabbit
Based on the comprehensive summary of changes, here are the concise release notes:
Release Notes
New Features
EVMUniversalNFTandZetaChainUniversalNFTcontracts for NFT examples.EVMUniversalTokenandZetaChainUniversalTokencontracts for Token examples.universal-call-multitask for batch cross-chain interactions.Dependency Updates
@zetachain/networks,@zetachain/standard-contracts.Performance Improvements
Chores