feat: Swap commands #278
feat: Swap commands #278
Conversation
📝 WalkthroughWalkthroughThis change set introduces a new CLI deployment tool for swap contracts using TypeScript and the Commander library, replacing the previous Hardhat-based deployment and swap task scripts. Several configuration files are updated to explicitly set the EVM version to "paris." Redundant Hardhat task files and related imports are removed, and minor adjustments are made to imports and formatting. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI (deploy.ts)
participant FileSystem
participant ethers.js
participant EthereumNetwork
User->>CLI (deploy.ts): Run deploy command with options
CLI (deploy.ts)->>FileSystem: Load contract artifacts (ABI, bytecode)
CLI (deploy.ts)->>ethers.js: Connect to RPC, create signer
CLI (deploy.ts)->>EthereumNetwork: Deploy implementation contract
EthereumNetwork-->>CLI (deploy.ts): Implementation contract address
CLI (deploy.ts)->>FileSystem: Load proxy contract artifacts
CLI (deploy.ts)->>EthereumNetwork: Deploy proxy with implementation and init data
EthereumNetwork-->>CLI (deploy.ts): Proxy contract address
CLI (deploy.ts)->>User: Output deployment results (JSON)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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/call/foundry.toml (1)
6-6: Consolidateevm_versionsetting across example projectsDefining the same
evm_version = "paris"line in everyfoundry.tomlunderexamples/*risks configuration drift if you ever update the value. Consider placing the setting in a single parent‐levelfoundry.toml(or leveraging an include/extend mechanism) so all examples inherit it automatically.examples/hello/foundry.toml (1)
6-6: Explicit EVM version improves determinism; verify compatibility with newer opcodesLocking the build to
parissecures identical byte-code across environments, but it also excludes any post-Merge features (e.g.,PUSH0in Shanghai, blob transactions in Cancun). If dependencies or future upgrades lean on those opcodes, compilation will break.Either bump to the latest EVM version your deployment network supports or leave an inline comment clarifying this intentional freeze, so future maintainers don’t treat it as technical debt.
examples/swap/commands/index.ts (1)
1-8: Enhance CLI robustness and metadata.The CLI implementation is minimal but could benefit from additional robustness features:
- Shebang dependency: Using
npx tsxmay not be available in all environments- Missing CLI metadata: No version, description, or help information
- Error handling: No global error handling for command failures
#!/usr/bin/env npx tsx import { Command } from "commander"; import { deploy } from "./deploy"; -const program = new Command().addCommand(deploy); +const program = new Command() + .name("swap-cli") + .description("CLI tool for swap contract deployment and management") + .version("1.0.0") + .addCommand(deploy); +// Global error handler +program.exitOverride((err) => { + console.error(`Error: ${err.message}`); + process.exit(err.exitCode); +}); + program.parse();Consider adding a compiled JavaScript version for broader compatibility if tsx dependency becomes an issue.
examples/swap/commands/deploy.ts (2)
45-51: Enhance error handling and logging.Consider improving error handling with structured logging and graceful error propagation instead of abrupt process termination.
} catch (err) { - console.error( - "Deployment failed:", - err instanceof Error ? err.message : err - ); - process.exit(1); + const errorMessage = err instanceof Error ? err.message : String(err); + console.error(JSON.stringify({ + error: "Deployment failed", + message: errorMessage, + timestamp: new Date().toISOString() + })); + throw err; // Let the caller handle the exit }
59-59: Consider environment-based configuration for default values.Hardcoded testnet defaults make the tool less flexible across different deployment environments.
Consider using environment variables for defaults:
- "https://zetachain-athens-evm.blockpi.network/v1/rpc/public" + process.env.DEFAULT_RPC_URL || "https://zetachain-athens-evm.blockpi.network/v1/rpc/public"- "0x2ca7d64A7EFE2D62A725E2B35Cf7230D6677FfEe" + process.env.DEFAULT_UNISWAP_ROUTER || "0x2ca7d64A7EFE2D62A725E2B35Cf7230D6677FfEe"- "0x6c533f7fe93fae114d0954697069df33c9b74fd7" + process.env.DEFAULT_GATEWAY || "0x6c533f7fe93fae114d0954697069df33c9b74fd7"Also applies to: 66-66, 71-71
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
examples/call/foundry.toml(1 hunks)examples/hello/foundry.toml(1 hunks)examples/messaging/foundry.toml(1 hunks)examples/nft/foundry.toml(1 hunks)examples/swap/commands/common.ts(1 hunks)examples/swap/commands/deploy.ts(1 hunks)examples/swap/commands/index.ts(1 hunks)examples/swap/foundry.toml(1 hunks)examples/swap/hardhat.config.ts(0 hunks)examples/swap/package.json(1 hunks)examples/swap/tasks/companionSwap.ts(0 hunks)examples/swap/tasks/deploy.ts(0 hunks)examples/swap/tasks/deployCompanion.ts(0 hunks)examples/swap/tasks/evmSwap.ts(0 hunks)examples/swap/tasks/index.ts(0 hunks)examples/swap/tasks/zetachainSwap.ts(0 hunks)examples/swap/test/SwapTest.t.sol(1 hunks)examples/token/foundry.toml(1 hunks)
💤 Files with no reviewable changes (7)
- examples/swap/hardhat.config.ts
- examples/swap/tasks/zetachainSwap.ts
- examples/swap/tasks/deploy.ts
- examples/swap/tasks/index.ts
- examples/swap/tasks/evmSwap.ts
- examples/swap/tasks/deployCompanion.ts
- examples/swap/tasks/companionSwap.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: test (examples/swap)
- GitHub Check: test (examples/nft)
- GitHub Check: test (examples/token)
- GitHub Check: test (examples/hello)
- GitHub Check: test (examples/call)
- GitHub Check: test (examples/call)
- GitHub Check: test (examples/swap)
- GitHub Check: test (examples/hello)
- GitHub Check: test (examples/nft)
- GitHub Check: test (examples/token)
- GitHub Check: slither (examples/hello, hello.sarif)
- GitHub Check: slither (examples/call, call.sarif)
- GitHub Check: slither (examples/swap, swap.sarif)
- 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/nft/foundry.toml (1)
6-6: Explicitly pinning the EVM version improves build determinism and cross-project consistency.No issues detected; aligning this example with the rest of the repository will prevent subtle bytecode disparities across environments.
examples/token/foundry.toml (1)
6-6: Ensure Solidity compiler version supportsevm_version = "paris"
parisis recognised only by Solidity ≥ 0.8.21. If the Foundry tool-chain pulls an older compiler, every build will fail.Consider pinning an explicit compiler version (or confirm the global default is already ≥ 0.8.21):
[profile.default] … evm_version = "paris" +solc_version = "0.8.25" # any version ≥ 0.8.21examples/messaging/foundry.toml (1)
6-6: evm_version locked to Paris is safe; no post-Paris opcodes detectedA case-insensitive search across all
.sol,.yul, and.evmfiles forPUSH0andBLOBHASHreturned no matches. Keepingevm_version = "paris"ensures deterministic builds without missing any post-Paris opcodes. No changes required.
examples/swap/test/SwapTest.t.sol (1)
8-8: Import path verified and correct—no further changes needed.The
SwapTest.t.solimport now points to./SwapCompanion.sol, and the companion contract exists inexamples/swap/test/SwapCompanion.sol(with no leftover file in the oldcontractsfolder). Everything aligns with the PR’s directory restructuring.examples/swap/foundry.toml (1)
6-6: Standardized EVM version configuration.Setting
evm_version = "paris"explicitly ensures consistent compilation behavior across the project. This aligns with the standardization effort mentioned in the PR summary and is compatible with the Solidity version used (0.8.26).examples/swap/package.json (1)
63-63: Minor formatting improvement.Removal of the trailing newline improves file consistency and aligns with standard formatting practices.
examples/swap/commands/deploy.ts (1)
1-3: LGTM: Clean and appropriate imports.The imports are well-organized and suitable for the deployment functionality.
| export const loadContractArtifacts = ( | ||
| contractName: string, | ||
| sourceName?: string | ||
| ) => { | ||
| const sourcePath = sourceName || `${contractName}.sol`; | ||
| const artifactPath = path.join( | ||
| __dirname, | ||
| `../out/${sourcePath}/${contractName}.json` | ||
| ); | ||
|
|
||
| try { | ||
| const artifact = JSON.parse(fs.readFileSync(artifactPath, "utf8")); | ||
| return { | ||
| abi: artifact.abi, | ||
| bytecode: artifact.bytecode, | ||
| } as { abi: any; bytecode: string }; | ||
| } catch (error) { | ||
| const message = error instanceof Error ? error.message : String(error); | ||
| throw new Error( | ||
| `Unable to load contract artifacts for ${contractName}: ${message}` | ||
| ); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance type safety and add input validation.
The function implementation is functional but could benefit from several improvements:
- Type safety: The ABI type is declared as
any, reducing type safety - Input validation: No validation of
contractNameparameter could allow path traversal - Error specificity: Could provide more specific error types
+interface ContractArtifact {
+ abi: any[]; // Could be further typed if ABI structure is known
+ bytecode: string;
+}
+
export const loadContractArtifacts = (
contractName: string,
sourceName?: string
-) => {
+): ContractArtifact => {
+ // Validate contractName to prevent path traversal
+ if (!contractName || contractName.includes('..') || contractName.includes('/')) {
+ throw new Error(`Invalid contract name: ${contractName}`);
+ }
+
const sourcePath = sourceName || `${contractName}.sol`;
const artifactPath = path.join(
__dirname,
`../out/${sourcePath}/${contractName}.json`
);
try {
const artifact = JSON.parse(fs.readFileSync(artifactPath, "utf8"));
return {
abi: artifact.abi,
bytecode: artifact.bytecode,
- } as { abi: any; bytecode: string };
+ };
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
throw new Error(
`Unable to load contract artifacts for ${contractName}: ${message}`
);
}
};🤖 Prompt for AI Agents
In examples/swap/commands/common.ts around lines 7 to 29, improve the
loadContractArtifacts function by replacing the ABI type from 'any' to a more
specific type reflecting the expected ABI structure to enhance type safety. Add
input validation to the contractName parameter to prevent path traversal or
invalid characters by sanitizing or restricting allowed characters. Also, refine
error handling by distinguishing between file read errors and JSON parsing
errors, throwing more specific error types or messages accordingly.
| import { ethers } from "ethers"; | ||
| import { loadContractArtifacts } from "./common"; | ||
|
|
||
| const main = async (opts: any) => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace any type with proper interface.
The function parameter should use a strongly-typed interface instead of any to ensure type safety and better developer experience.
Define an interface for the options:
+interface DeployOptions {
+ rpc: string;
+ privateKey: string;
+ name: string;
+ gateway: string;
+ uniswapRouter: string;
+ gasLimit: number;
+}
+
-const main = async (opts: any) => {
+const main = async (opts: DeployOptions) => {📝 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 main = async (opts: any) => { | |
| interface DeployOptions { | |
| rpc: string; | |
| privateKey: string; | |
| name: string; | |
| gateway: string; | |
| uniswapRouter: string; | |
| gasLimit: number; | |
| } | |
| const main = async (opts: DeployOptions) => { | |
| // ...rest of function body... | |
| } |
🤖 Prompt for AI Agents
In examples/swap/commands/deploy.ts at line 5, replace the use of the `any` type
for the `opts` parameter with a properly defined interface that describes the
expected shape and properties of the options object. Define this interface above
the function and update the function signature to use it, ensuring type safety
and improved code clarity.
| import { loadContractArtifacts } from "./common"; | ||
|
|
||
| const main = async (opts: any) => { | ||
| const provider = new ethers.providers.JsonRpcProvider(opts.rpc); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update to ethers v6 API.
The code uses deprecated ethers v5 API. Migrate to ethers v6 for better performance and future support.
Apply these changes:
- const provider = new ethers.providers.JsonRpcProvider(opts.rpc);
+ const provider = new ethers.JsonRpcProvider(opts.rpc);- await implementation.deployed();
+ await implementation.waitForDeployment();- await proxy.deployed();
+ await proxy.waitForDeployment();Also applies to: 16-16, 34-34
🤖 Prompt for AI Agents
In examples/swap/commands/deploy.ts at lines 6, 16, and 34, the code uses the
deprecated ethers v5 API for creating a JsonRpcProvider. Update these lines to
use the ethers v6 API by importing the provider from 'ethers' correctly and
instantiating the JsonRpcProvider according to v6 syntax, ensuring compatibility
with the new version's constructor and method usage.
| const initData = new ethers.utils.Interface(abi).encodeFunctionData( | ||
| "initialize", | ||
| [opts.gateway, opts.uniswapRouter, opts.gasLimit, signer.address] | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate contract interface before encoding function data.
The code assumes the contract has an initialize function with specific parameters without validation.
Add validation:
+ // Validate that the contract has the initialize function
+ const contractInterface = new ethers.utils.Interface(abi);
+ if (!contractInterface.functions['initialize(address,address,uint256,address)']) {
+ throw new Error(`Contract ${opts.name} does not have the expected initialize function`);
+ }
+
- const initData = new ethers.utils.Interface(abi).encodeFunctionData(
+ const initData = contractInterface.encodeFunctionData(
"initialize",
[opts.gateway, opts.uniswapRouter, opts.gasLimit, signer.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.
| const initData = new ethers.utils.Interface(abi).encodeFunctionData( | |
| "initialize", | |
| [opts.gateway, opts.uniswapRouter, opts.gasLimit, signer.address] | |
| ); | |
| // Validate that the contract has the initialize function | |
| const contractInterface = new ethers.utils.Interface(abi); | |
| if (!contractInterface.functions['initialize(address,address,uint256,address)']) { | |
| throw new Error(`Contract ${opts.name} does not have the expected initialize function`); | |
| } | |
| const initData = contractInterface.encodeFunctionData( | |
| "initialize", | |
| [opts.gateway, opts.uniswapRouter, opts.gasLimit, signer.address] | |
| ); |
🤖 Prompt for AI Agents
In examples/swap/commands/deploy.ts around lines 18 to 21, the code encodes
function data for the "initialize" function without verifying that this function
exists in the contract ABI. To fix this, add a check to confirm the "initialize"
function is present in the Interface before calling encodeFunctionData. If the
function is missing, throw an error or handle it gracefully to prevent runtime
failures.
| "RPC URL (default: testnet)", | ||
| "https://zetachain-athens-evm.blockpi.network/v1/rpc/public" | ||
| ) | ||
| .requiredOption("-k, --private-key <key>", "Private key") |
There was a problem hiding this comment.
Security risk: Private key exposure in command line.
Passing private keys as command-line arguments exposes them in process lists and shell history, creating a significant security vulnerability.
Consider alternative approaches:
- .requiredOption("-k, --private-key <key>", "Private key")
+ .option("-k, --private-key <key>", "Private key (consider using environment variable PRIVATE_KEY instead)")And in the main function:
+ const privateKey = opts.privateKey || process.env.PRIVATE_KEY;
+ if (!privateKey) {
+ throw new Error("Private key must be provided via --private-key option or PRIVATE_KEY environment variable");
+ }
- const signer = new ethers.Wallet(opts.privateKey, provider);
+ const signer = new ethers.Wallet(privateKey, provider);📝 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.
| .requiredOption("-k, --private-key <key>", "Private key") | |
| .option("-k, --private-key <key>", "Private key (consider using environment variable PRIVATE_KEY instead)") |
🤖 Prompt for AI Agents
In examples/swap/commands/deploy.ts at line 61, avoid accepting the private key
as a command-line argument to prevent exposure in process lists and shell
history. Instead, modify the code to read the private key from a secure source
such as an environment variable or a protected file. Update the option parsing
to remove the private key argument and adjust the main function to retrieve the
key securely from the chosen source.
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 (2)
.github/workflows/localnet.yaml(1 hunks)examples/swap/scripts/localnet.sh(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: slither (examples/hello, hello.sarif)
- GitHub Check: slither (examples/swap, swap.sarif)
- GitHub Check: slither (examples/call, call.sarif)
🔇 Additional comments (1)
examples/swap/scripts/localnet.sh (1)
25-29: Verify path robustness & provide explicitcwd
npx ts-node commands/index.ts deployrelies on the caller being inexamples/swap.
Running the script from another directory (e.g., CI root) will fail.Consider:
-UNIVERSAL=$(npx ts-node commands/index.ts deploy \ +UNIVERSAL=$(npx ts-node "$(dirname "$0")/../commands/index.ts" deploy \or
cd "$(dirname "$0")/.."near the top of the script.
Related docs: zeta-chain/docs#688
Summary by CodeRabbit
New Features
Refactor
Chores
Removals