refactor: deploy command #285
Conversation
📝 WalkthroughWalkthroughAdds new deploy CLI commands for call, hello, and swap using ethers; introduces getAbi helpers and replaces artifact loaders; rewrites swap deploy to deterministic nonce-based implementation+proxy deployment with --tx-gas-limit; removes the Hardhat deploy task; wires commands into indices; bumps tsx and updates zetachain version in example packages. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Provider
participant Signer
participant ContractFactory
User->>CLI: run deploy --rpc --private-key --name --gateway
CLI->>Provider: new JsonRpcProvider(rpc)
CLI->>Signer: new Wallet(privateKey, Provider)
CLI->>CLI: getAbi(name) → artifact {abi, bytecode}
CLI->>ContractFactory: new ContractFactory(abi, bytecode, Signer)
ContractFactory->>Provider: deploy(gateway)
Provider-->>CLI: deployment tx
CLI->>CLI: predict address via getContractAddress({from, nonce})
CLI-->>User: print {contractAddress, deployer, network, transactionHash}
sequenceDiagram
participant User
participant CLI
participant Provider
participant Signer
participant ImplFactory
participant ProxyFactory
User->>CLI: run swap deploy --rpc --private-key --name --gateway [--tx-gas-limit]
CLI->>Provider: connect
CLI->>Signer: connect
CLI->>CLI: baseNonce = Signer.getTransactionCount("pending")
CLI->>CLI: predict implAddress = getContractAddress({from, nonce: baseNonce})
CLI->>ImplFactory: deploy(...){nonce: baseNonce}
ImplFactory-->>CLI: implTx.hash
CLI->>CLI: predict proxyAddress = getContractAddress({from, nonce: baseNonce+1})
CLI->>ProxyFactory: deploy(implAddress, initData){nonce: baseNonce+1, gasLimit}
ProxyFactory-->>CLI: proxyTx.hash
CLI-->>User: print {contractAddress: proxyAddress, implementationAddress: implAddress, transactionHash: proxyTx.hash, implementationTransactionHash: implTx.hash}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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/Issue comments)Type 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: 0
♻️ Duplicate comments (6)
examples/swap/commands/deploy.ts (2)
13-14: Confirm artifact shape includes bytecodegetAbi must return a bytecode string compatible with ethers.ContractFactory. If artifacts come from Hardhat, bytecode can live under evm.bytecode.object. Ensure getAbi normalizes this (see common.ts comment).
28-33: ERC1967Proxy artifact lookup may fail with current getAbiIf ERC1967Proxy’s artifact is nested under a dependency path, getAbi("ERC1967Proxy") will not find it. The robust lookup proposed in common.ts resolves this.
examples/call/commands/deploy.ts (4)
12-17: Same suggestions as in hello deploy
- Validate and normalize gateway address before deployment.
- Consider normalizing artifact.bytecode in the helper.
18-21: Same nit: prefer tx.from for predictionMirror the small resilience improvement suggested in the hello deploy.
42-46: Same nit: use option instead of requiredOption with defaultFor consistency and clearer help text.
1-8: Confirm ethers major version usageReplicating the ethers v5 API check across packages to avoid runtime surprises.
🧹 Nitpick comments (12)
examples/swap/package.json (1)
64-64: Nit: ensure swap package exposes a CLI script (optional)Given new commands exist under examples/swap/commands, consider adding a convenience script to run them via tsx for parity with other packages.
Apply this diff if you want a yarn script:
"scripts": { "test": "echo \"Error: no test specified\" && exit 1", "lint:fix": "npx eslint . --ext .js,.ts --fix", "lint": "npx eslint . --ext .js,.ts", - "deploy:localnet": "npx hardhat compile --force && npx hardhat deploy --network localhost --gateway 0x5FC8d32690cc91D4c39d9d3abcBD16989F875707" + "deploy:localnet": "npx hardhat compile --force && npx hardhat deploy --network localhost --gateway 0x5FC8d32690cc91D4c39d9d3abcBD16989F875707", + "cli": "tsx commands/index.ts" },examples/swap/commands/common.ts (1)
4-9: Optional: rename for clarity and add typesThe function returns more than an ABI. Consider renaming to getArtifact and adding a minimal return type to aid callers.
examples/swap/commands/deploy.ts (4)
3-3: Use local import pathThe file resides in the same folder as common.ts. Simplify the import path to avoid brittle ../../ segments.
Apply:
-import { getAbi } from "../../swap/commands/common"; +import { getAbi } from "./common";
39-47: Apply tx gas limit consistently to both deploymentsYou set gasLimit for the proxy deploy but not for the implementation. Apply the same override to avoid OOG on the first transaction and for consistency.
Apply:
- const implementation = await factory.deploy({ nonce: baseNonce }); + const implementation = await factory.deploy({ + nonce: baseNonce, + gasLimit: ethers.BigNumber.from(opts.txGasLimit || 4000000), + });
88-92: CLI option naming: potential confusion with --gas-limitYou now have --gas-limit (contract initialization parameter) and --tx-gas-limit (transaction gas). Consider renaming the latter to --deploy-gas-limit for clarity. No functional change required.
93-96: Early validate user-supplied addressesFail fast on invalid addresses to prevent opaque RPC errors later.
Apply:
.action((opts) => { opts.gasLimit = Number(opts.gasLimit); if (opts.txGasLimit) opts.txGasLimit = Number(opts.txGasLimit); + if (!ethers.utils.isAddress(opts.gateway)) { + throw new Error(`Invalid --gateway address: ${opts.gateway}`); + } + if (!ethers.utils.isAddress(opts.uniswapRouter)) { + throw new Error(`Invalid --uniswap-router address: ${opts.uniswapRouter}`); + } main(opts).catch((err) => { console.error("Unhandled error:", err); process.exit(1); }); });examples/hello/commands/index.ts (2)
1-1: Shebang should use env -S for multiple argumentsOn many systems, using multiple arguments in a shebang via env requires -S. This avoids brittle behavior across shells/OSes.
Apply:
-#!/usr/bin/env npx tsx +#!/usr/bin/env -S npx tsx
6-8: Guard CLI execution and export the program for composabilityAlign with the pattern used in examples/call/commands/index.ts to avoid auto-parsing when imported and to enable reuse in tests or other scripts.
-const program = new Command().addCommand(deploy); - -program.parse(); +const program = new Command().addCommand(deploy); + +if (require.main === module) program.parse(); + +export default program;examples/hello/commands/deploy.ts (4)
12-17: Validate gateway address before deployingNormalize and validate the gateway address to fail fast with a clear error if it’s invalid.
- const { abi, bytecode } = getAbi(opts.name); - const factory = new ethers.ContractFactory(abi, bytecode, signer); - - const contract = await factory.deploy(opts.gateway); + const { abi, bytecode } = getAbi(opts.name); + const factory = new ethers.ContractFactory(abi, bytecode, signer); + + // Validate/normalize address (throws if invalid) + const gateway = ethers.utils.getAddress(opts.gateway); + + const contract = await factory.deploy(gateway);
18-21: Use transaction sender when predicting contract addressUsing tx.from (fallback to signer.address) makes the prediction resilient to wrappers/middleware that may alter the from field.
- const predictedAddress = ethers.utils.getContractAddress({ - from: signer.address, - nonce: tx.nonce, - }); + const predictedAddress = ethers.utils.getContractAddress({ + from: tx.from ?? signer.address, + nonce: tx.nonce, + });
42-46: Do not mark an option with a default as requiredrequiredOption with a default is misleading in help text. Use option for the RPC URL since a sensible default is provided.
- .requiredOption( + .option( "-r, --rpc <url>", "RPC URL (default: testnet)", "https://zetachain-athens-evm.blockpi.network/v1/rpc/public" )
23-30: Optional: add a confirmations flag for users who want to waitSome users may prefer a wait for N confirmations when the RPC behaves correctly. Consider adding a --confirmations option (default 0) and, when >0, await tx.wait(n) while still printing the predicted address immediately.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
examples/call/yarn.lockis excluded by!**/yarn.lock,!**/*.lockexamples/messaging/yarn.lockis excluded by!**/yarn.lock,!**/*.lockexamples/swap/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (12)
examples/call/commands/deploy.ts(1 hunks)examples/call/commands/index.ts(1 hunks)examples/call/package.json(1 hunks)examples/hello/commands/common.ts(1 hunks)examples/hello/commands/deploy.ts(1 hunks)examples/hello/commands/index.ts(1 hunks)examples/hello/hardhat.config.ts(0 hunks)examples/hello/tasks/deploy.ts(0 hunks)examples/messaging/package.json(1 hunks)examples/swap/commands/common.ts(1 hunks)examples/swap/commands/deploy.ts(3 hunks)examples/swap/package.json(2 hunks)
💤 Files with no reviewable changes (2)
- examples/hello/tasks/deploy.ts
- examples/hello/hardhat.config.ts
🧰 Additional context used
🧬 Code Graph Analysis (4)
examples/hello/commands/index.ts (1)
examples/hello/commands/deploy.ts (1)
deploy(40-59)
examples/call/commands/deploy.ts (1)
examples/hello/commands/common.ts (1)
getAbi(4-10)
examples/hello/commands/deploy.ts (3)
examples/hello/commands/common.ts (1)
getAbi(4-10)examples/call/commands/deploy.ts (1)
deploy(40-59)examples/swap/commands/deploy.ts (1)
deploy(68-100)
examples/swap/commands/deploy.ts (1)
examples/swap/commands/common.ts (1)
getAbi(4-10)
⏰ 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). (11)
- 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/hello)
- GitHub Check: test (examples/nft)
- GitHub Check: test (examples/token)
- GitHub Check: test (examples/call)
- GitHub Check: test (examples/swap)
- GitHub Check: slither (examples/hello, hello.sarif)
- GitHub Check: slither (examples/call, call.sarif)
- GitHub Check: slither (examples/swap, swap.sarif)
🔇 Additional comments (10)
examples/swap/package.json (1)
50-50: Dev dependency bump to tsx v4.20.4 looks goodNo compatibility concerns from this change; aligns with other packages in the repo.
examples/messaging/package.json (1)
50-50: Dev dependency addition to tsx v4.20.4 approvedConsistent with repo-wide updates; no risk.
examples/call/package.json (1)
50-50: Dev dependency bump to tsx v4.20.4 approvedMatches other example packages. No further action required.
examples/swap/commands/common.ts (1)
4-9: Couldn’t locate any compiled JSON artifacts in theexamples/swapdirectory tree. Since the repo doesn’t include built outputs, please verify in your local build wherever contract JSONs actually land (e.g., Foundry’sout/<Contract>.sol/<Contract>.json, Hardhat’sartifacts/contracts/..., or a custom path). Ensure that getAbi searches all relevant directories and normalizes both theabiand bytecode fields across toolchains.examples/swap/commands/deploy.ts (2)
15-22: Deterministic address precomputation via explicit nonces: goodUsing pending nonce and explicit nonces ensures predictable implementation/proxy addresses and aligns with the PR objective of surfacing the address despite RPC quirks.
49-57: Output format aligns with goalsLogging predicted addresses and tx hashes as JSON meets the UX requirement to extract contractAddress even when RPC responses are unreliable.
examples/call/commands/index.ts (1)
6-12: LGTM: clean wiring of the new deploy subcommandThe new deploy command is registered cleanly and ordering with existing subcommands is consistent. The guarded parse keeps this module import-friendly.
examples/hello/commands/common.ts (1)
4-10: Normalize artifact shape and improve error messagesI wasn’t able to inspect the compiled JSON artifacts (they’re not checked into the repo), so please manually verify that your bytecode‐normalization covers all expected formats (plain string,
.bytecode.object,.evm.bytecode.object) and that missing/invalid artifacts fail fast with clear messaging. Consider adding a small unit test forgetAbiinexamples/hello/commands/common.tsto guard against future regressions.File: examples/hello/commands/common.ts
-export const getAbi = (name: string) => { - const abiPath = path.resolve( - __dirname, - path.join("..", "out", `${name}.sol`, `${name}.json`) - ); - return JSON.parse(fs.readFileSync(abiPath, "utf8")); -}; +export const getAbi = (name: string) => { + const abiPath = path.resolve( + __dirname, + path.join("..", "out", `${name}.sol`, `${name}.json`) + ); + if (!fs.existsSync(abiPath)) { + throw new Error(`Artifact not found for ${name}: ${abiPath}`); + } + let artifact: any; + try { + artifact = JSON.parse(fs.readFileSync(abiPath, "utf8")); + } catch (e) { + throw new Error(`Invalid JSON in artifact for ${name}: ${abiPath}`); + } + const bytecode = + typeof artifact.bytecode === "string" + ? artifact.bytecode + : artifact.bytecode?.object ?? + artifact.evm?.bytecode?.object ?? + undefined; + if (!artifact?.abi || !bytecode) { + throw new Error( + `Artifact for ${name} missing abi/bytecode (checked: bytecode, bytecode.object, evm.bytecode.object).` + ); + } + return { ...artifact, bytecode }; +};examples/hello/commands/deploy.ts (1)
1-8: Ethers version usage in deploy.ts matches v5 declaration
The CLI commands under examples/hello are pinned to ethers ^5.4.7 (v5) in examples/hello/package.json, so the v5 API calls in deploy.ts are correct. The frontend at examples/hello/frontend uses ethers ^6.13.2 in isolation. No changes required.examples/call/commands/deploy.ts (1)
3-3: Import path for getAbi verified
The fileexamples/call/commands/common.tsexists and exportsgetAbiat line 8. No further changes needed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
examples/hello/package.json (1)
6-11: Align scripts with new deploy CLI (Hardhat deploy task likely removed).Given the PR migrates to custom tsx commands and removes the Hardhat deploy task, the current "deploy:localnet" script will likely fail. Recommend adding a build step for Foundry artifacts and a deploy script that uses the new CLI.
Apply this diff to replace the legacy Hardhat deploy with the new flow:
"scripts": { "test": "echo \"Error: no test specified\" && exit 1", "lint:fix": "npx eslint . --ext .js,.ts --fix", "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" + "build": "forge build", + "deploy": "yarn build && npx tsx commands deploy --private-key $PRIVATE_KEY" },Verification checklist:
- Ensure Foundry is available in CI/dev environments so "forge build" succeeds.
- Confirm commands/ contains the new deploy CLI and that it reads artifacts from out/.
- Optionally keep a separate legacy script if you still need local Hardhat-based flows.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
examples/hello/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (1)
examples/hello/package.json(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). (13)
- GitHub Check: test (examples/nft)
- GitHub Check: test (examples/token)
- GitHub Check: test (examples/hello)
- GitHub Check: test (examples/swap)
- GitHub Check: test (examples/call)
- GitHub Check: test (examples/swap)
- GitHub Check: test (examples/token)
- GitHub Check: test (examples/nft)
- GitHub Check: test (examples/hello)
- GitHub Check: test (examples/call)
- 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/hello/package.json (1)
60-60: Refresh the Hello example lockfile
Runyarn installinexamples/helloto update itsyarn.lockafter bumping"zetachain": "^6.2.0". Commit the updated lockfile to lock in the new version.Note: All other examples (token, messaging, swap, nft, call) are self-contained projects with their own Zetachain versions; no cross-project dependency drift will occur.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/localnet.yaml (2)
40-41: Pin looks fine; confirm Foundry 1.2.3 supportsforge soldeerand add a brief toolchain assertion.You’re pinning Foundry to 1.2.3, which is good for reproducibility. Given the later step uses
forge soldeer update, please confirm this Foundry version includes Soldeer support. If not, bump to a version that does, or add a short assertion step to fail fast.If Soldeer is supported, consider adding a quick verification step right after installing Foundry:
- name: Verify Foundry toolchain run: | forge --version cast --version anvil --version
52-60: Strengthen the runtime check: verify all Foundry tools and fail fast.
anvil --versionis a good sanity check. Make the step more robust by failing fast on errors and logging all tool versions to aid debugging.Apply this diff to the step body:
- name: Run Test Script run: | - cd "${{ matrix.example-dir }}" - yarn - forge soldeer update - anvil --version - chmod +x ./scripts/localnet.sh - ./scripts/localnet.sh start + set -euxo pipefail + cd "${{ matrix.example-dir }}" + # Ensure Yarn is available if the runner image changes + corepack enable || true + # Capture tool versions in the job summary for easier debugging + forge --version | tee -a "$GITHUB_STEP_SUMMARY" + cast --version | tee -a "$GITHUB_STEP_SUMMARY" + anvil --version | tee -a "$GITHUB_STEP_SUMMARY" + yarn + forge soldeer update + chmod +x ./scripts/localnet.sh + ./scripts/localnet.sh startOptionally, cache Yarn dependencies to speed up the matrix:
- name: Setup Node.js uses: actions/setup-node@v4 with: node-version: "22" registry-url: "https://registry.npmjs.org" cache: yarn cache-dependency-path: "**/yarn.lock"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/localnet.yaml(2 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). (8)
- GitHub Check: test (examples/swap)
- GitHub Check: test (examples/nft)
- GitHub Check: test (examples/call)
- GitHub Check: test (examples/token)
- GitHub Check: test (examples/hello)
- GitHub Check: slither (examples/call, call.sarif)
- GitHub Check: slither (examples/hello, hello.sarif)
- GitHub Check: slither (examples/swap, swap.sarif)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/localnet.yaml (2)
40-41: Pinning Foundry is good; centralize the version to ease future bumps.Using a hardcoded Foundry version improves reproducibility. To avoid touching multiple places when upgrading, reference an env variable instead of an inline literal.
Apply this diff within this step:
with: - version: v1.2.3 + version: ${{ env.FOUNDRY_VERSION }}And add this near the top-level of the workflow (outside this hunk) to define the version once:
env: FOUNDRY_VERSION: v1.2.3
57-57: Good pre-flight; also validate Forge and consider a dedicated step for clearer logs.Running
anvil --versionearly is a helpful sanity check. Addforge --versionalongside it to verify both tools, or split these into a small “Check Foundry versions” step for visibility.Minimal inline addition:
- anvil --version + anvil --version + forge --versionAlternatively, a separate step (outside this hunk) for clearer CI output:
- name: Check Foundry versions run: | anvil --version forge --version
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/localnet.yaml(2 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). (8)
- GitHub Check: test (examples/nft)
- GitHub Check: test (examples/call)
- GitHub Check: test (examples/swap)
- GitHub Check: test (examples/hello)
- 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)
|
This PR also fixes CI by specifying Anvil version 1.2.3, because Localnet does not work with 1.3.0: zeta-chain/localnet#229 |
| from: signer.address, | ||
| nonce: tx.nonce, | ||
| }); | ||
|
|
There was a problem hiding this comment.
maybe to add check that contract.address == predictedAddress
Right now when deploying contracts on ZetaChain, the RPC doesn't send a correct response, so commands like
forge createthrow an error. Even thoughforge createactually deploys a contract it doesn't return contract address and it looks like it's broken, so we're switching to commands instead to:Related: zeta-chain/docs#703
Summary by CodeRabbit
New Features
Refactor
Chores