-
Notifications
You must be signed in to change notification settings - Fork 35
feat: fees show command #420
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughIntroduces a new “fees” CLI with list and show subcommands, adds schemas/types to support fee data, and adjusts client contract typings/casts (IZRC20, UniswapV2Router02). Implements data fetching for withdraw gas fees with optional source conversion via Uniswap router, including JSON output and console formatting. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant CLI as fees show (Commander)
participant S as fetchShowFeesData
participant API as Chain/Token API
participant RPC as EVM Provider
participant ZRC as IZRC20 (target/gas)
participant UNI as UniswapV2Router02
U->>CLI: Run "fees show" with options
CLI->>S: Validate & call with target/source/rpc/router/api
S->>API: fetchAllChainData()
API-->>S: Chains, tokens, labels
S->>RPC: Create JsonRpcProvider(rpc)
S->>ZRC: Instantiate target & gas token contracts
alt gasLimit provided
S->>ZRC: withdrawGasFee(gasLimit?)
else no gasLimit
S->>ZRC: withdrawGasFee()
end
ZRC-->>S: fee, gas token address
opt source provided
alt source != gas token
S->>UNI: getAmountsIn(fee, [source, ..., gas])
UNI-->>S: amountsIn
else source == gas token
S-->>S: required = fee
end
end
S-->>CLI: ShowFeesData
alt --json
CLI-->>U: JSON ShowFeesData
else human-readable
CLI-->>U: Formatted console output
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/client/src/getFees.ts (1)
97-103
: Incorrect error context string in fetchCCMFees catchMessage references “CCTX By Inbound hash” but this block is fetching CCM fees. Update for clarity.
Apply this diff:
- context: "Something failed fetching CCTX By Inbound hash", + context: "Something went wrong fetching CCM fees",
🧹 Nitpick comments (5)
src/schemas/commands/fees.ts (1)
18-25
: Align feesShowOptionsSchema with show command options (avoid drift)This schema lacks router, sourceChain, and gasLimit, and doesn’t accept number inputs for chain IDs. Align it with the show command and reuse it there.
Apply this diff:
export const feesShowOptionsSchema = z.object({ api: z.string().default(DEFAULT_API_URL), json: z.boolean().default(false), rpc: z.string().default(DEFAULT_EVM_RPC_URL), - source: z.string().optional(), - target: z.string().optional(), - targetChain: z.string().optional(), + router: z.string().optional(), // validated downstream as an address + gasLimit: z.union([z.string(), z.number()]).optional(), + source: z.string().optional(), + sourceChain: z.union([z.string(), z.number()]).optional(), + target: z.string().optional(), + targetChain: z.union([z.string(), z.number()]).optional(), });Then import and use this schema in the show command to eliminate duplication.
packages/commands/src/query/fees/list.ts (1)
26-29
: Spinner handling is fine; consider a finally for DRYnessStopping/clearing the spinner in both try/catch works. You can move it to a finally block to avoid duplication.
Also applies to: 68-69
packages/commands/src/query/fees/show.ts (3)
144-156
: Avoid duplicating option schemas; reuse central schemaThis local zod schema duplicates/extends the one in src/schemas/commands/fees.ts. Prefer importing a unified schema (after adding router/sourceChain/gasLimit there) to prevent drift.
Example:
-const showFeesOptionsSchema = z.object({ ... }); +import { feesShowOptionsSchema as showFeesOptionsSchema } from "../../../../../src/schemas/commands/fees";
267-276
: Avoid duplicate API calls for chain metadatafetchAllChainData is called again only for labels. Pass tokens/labels from fetchShowFeesData or cache locally to reduce I/O.
371-373
: Router default: prefer auto-resolving over hardcodingHardcoding the router address can drift. Consider defaulting via protocol registry (e.g., getAddress("uniswapV2Router02", "")) with CLI override.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
packages/client/src/getFees.ts
(2 hunks)packages/client/src/getQuote.ts
(1 hunks)packages/commands/src/query/fees/index.ts
(1 hunks)packages/commands/src/query/fees/list.ts
(4 hunks)packages/commands/src/query/fees/show.ts
(1 hunks)src/schemas/commands/fees.ts
(1 hunks)types/contracts.types.ts
(2 hunks)types/fees.types.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: fadeev
PR: zeta-chain/toolkit#346
File: packages/commands/src/ton/depositAndCall.ts:0-0
Timestamp: 2025-06-13T15:33:54.781Z
Learning: fadeev prefers responses in English.
🧬 Code graph analysis (6)
packages/client/src/getQuote.ts (1)
types/contracts.types.ts (1)
UniswapV2Router02Contract
(23-23)
types/fees.types.ts (1)
types/shared.schema.ts (1)
evmAddressSchema
(9-11)
src/schemas/commands/fees.ts (1)
src/constants/api.ts (2)
DEFAULT_API_URL
(1-2)DEFAULT_EVM_RPC_URL
(3-4)
packages/commands/src/query/fees/show.ts (5)
types/fees.types.ts (1)
showFeesDataSchema
(27-30)packages/commands/src/query/chains/list.ts (1)
fetchAllChainData
(36-57)types/shared.schema.ts (1)
evmAddressSchema
(9-11)utils/validateAndParseSchema.ts (1)
validateAndParseSchema
(28-59)src/constants/api.ts (2)
DEFAULT_EVM_RPC_URL
(3-4)DEFAULT_API_URL
(1-2)
packages/client/src/getFees.ts (1)
typechain-types/@zetachain/protocol-contracts/contracts/zevm/interfaces/IZRC20.sol/IZRC20.ts (1)
IZRC20
(125-301)
packages/commands/src/query/fees/index.ts (2)
packages/commands/src/query/fees/list.ts (1)
listCommand
(79-95)packages/commands/src/query/fees/show.ts (1)
showCommand
(343-379)
🔇 Additional comments (8)
packages/client/src/getFees.ts (1)
21-26
: Type cast to IZRC20 looks goodCasting the contract instance to IZRC20 is correct and keeps typings aligned with the ABI.
packages/client/src/getQuote.ts (1)
35-40
: LGTM on router typingTwo-step cast to UniswapV2Router02Contract matches the new alias and keeps types precise.
types/contracts.types.ts (1)
1-1
: Clean alias for UniswapV2Router02Type-only import and alias export are correct and simplify usage across the codebase.
Also applies to: 23-23
packages/commands/src/query/fees/index.ts (1)
6-10
: Good composition of fees commandRoot “fees” command correctly aggregates list and show subcommands.
types/fees.types.ts (1)
27-30
: Schema shape looks solidTarget required, source optional with proper address validation. Matches the show output.
packages/commands/src/query/fees/show.ts (3)
33-39
: Contract typing/calls are correctIZRC20 cast and withdrawGasFee/withdrawGasFeeWithGasLimit usage are correct for ethers v6.
41-47
: Gas-limit branching is correctCleanly prefers withdrawGasFeeWithGasLimit when provided; falls back otherwise.
107-114
: Confirm WETH() returns WZETA on ZEVMAssumption: router.WETH() yields the wrapped ZETA address (WZETA). If not, the pathing would be wrong.
Would you like me to switch to resolving WZETA via protocol-contracts getAddress for clarity, or add a brief comment stating WETH() == WZETA on ZEVM?
Withdraw fee
Withdraw fee from a specific source chain
How much would it cost to make a call to Ethereum from Base in Base ETH:
With a gas limit
JSON output
Summary by CodeRabbit