-
Notifications
You must be signed in to change notification settings - Fork 34
feat: uniswap v3 deployment and liquidity commands #295
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
📝 WalkthroughWalkthroughThis update introduces a comprehensive CLI suite for managing Uniswap V3 pools on a blockchain network. It adds commands for deploying Uniswap contracts, creating and initializing pools, showing pool and liquidity position details, performing swaps, and managing liquidity (add/remove/show). Supporting constants, type definitions, and dependencies are included. GitHub Actions workflows are updated to use Node.js v22. Additionally, asynchronous improvements were made to pool token detail fetching in the client and utility modules. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Blockchain
participant UniswapContracts
User->>CLI: Run pools deploy/create/show/swap/liquidity command
CLI->>Blockchain: Connect via RPC using provided credentials
alt deploy
CLI->>UniswapContracts: Deploy Factory, Swap Router, Position Manager
UniswapContracts-->>CLI: Return contract addresses and tx hashes
else create
CLI->>UniswapContracts: Check pool existence and create if needed
UniswapContracts-->>CLI: Return pool address
CLI->>UniswapContracts: Initialize pool with sqrtPriceX96
else show
CLI->>UniswapContracts: Query pool info (by address or tokens+fee)
UniswapContracts-->>CLI: Return pool state (tokens, fee, price, liquidity, tick)
else swap
CLI->>UniswapContracts: Calculate target price and determine swap direction
CLI->>UniswapContracts: Approve tokens if needed
CLI->>UniswapContracts: Execute swap transaction
else add liquidity
CLI->>UniswapContracts: Approve tokens
CLI->>UniswapContracts: Mint liquidity position NFT
UniswapContracts-->>CLI: Return tx receipt and position NFT ID
else remove liquidity
CLI->>UniswapContracts: Select position NFT if needed
CLI->>UniswapContracts: Decrease liquidity and collect tokens
CLI->>UniswapContracts: Optionally burn position NFT
else show liquidity
CLI->>UniswapContracts: Query owned liquidity position NFTs
UniswapContracts-->>CLI: Return position details
end
CLI-->>User: Output results (addresses, prices, confirmations, errors)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (1)
✨ 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. 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
Documentation and Community
|
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: 9
🧹 Nitpick comments (16)
packages/commands/src/pools/constants.ts (1)
1-2
: Consider environment configuration for RPC URL.Hardcoding the RPC URL might cause issues across different environments. Consider making it configurable through environment variables.
-export const DEFAULT_RPC = - "https://zetachain-athens.g.allthatnode.com/archive/evm"; +export const DEFAULT_RPC = + process.env.ZETACHAIN_RPC_URL || "https://zetachain-athens.g.allthatnode.com/archive/evm";packages/commands/src/pools/liquidity/index.ts (1)
1-7
: LGTM - Command structure follows project conventions.The command structure looks good and follows the same pattern as other commands in the project.
Consider planning for additional liquidity management commands.
While the current implementation only supports adding liquidity, consider planning for future commands like removing liquidity or adjusting positions.
packages/commands/src/pools/deploy.ts (4)
18-33
: Gas estimation function could be improvedThe function logs estimated gas even when returning it, and the gas estimation error handling could be more specific by capturing and reporting different error types more distinctly.
Consider refining the error handling to provide more specific feedback on gas estimation failures:
const estimateGas = async ( contractFactory: ContractFactory, args: unknown[] = [] ): Promise<bigint | null> => { try { const deployment = await contractFactory.getDeployTransaction(...args); const gasEstimate = await contractFactory.runner?.provider?.estimateGas( deployment ); - console.log("Estimated gas:", gasEstimate?.toString()); return gasEstimate ?? null; } catch (error) { - console.error("Gas estimation failed:", error); + console.error("Gas estimation failed:", error instanceof Error + ? `${error.name}: ${error.message}` + : String(error)); return null; } };
61-65
: Gas limit doubling is arbitrary and could be excessiveDoubling the gas estimate is a common practice, but it can lead to overspending on gas. A more sophisticated approach would be to add a percentage buffer.
// Estimate gas for factory deployment const factoryGasEstimate = await estimateGas(uniswapV3Factory); if (factoryGasEstimate) { - deployOpts.gasLimit = Number(factoryGasEstimate * 2n); + // Add 30% buffer instead of doubling + deployOpts.gasLimit = Number(factoryGasEstimate + (factoryGasEstimate * 30n / 100n)); }
168-179
: Improve error handling specificityThe current error handling casts all errors to
DeploymentError
, which might not capture all possible error scenarios. A more robust approach would include specific handling for different error types.} catch (error) { - const deploymentError = error as DeploymentError; - console.error("\nDeployment failed with error:"); - console.error("Error message:", deploymentError.message); - if (deploymentError.receipt) { - console.error("Transaction receipt:", deploymentError.receipt); - } - if (deploymentError.transaction) { - console.error("Transaction details:", deploymentError.transaction); - } + console.error("\nDeployment failed with error:"); + if (error instanceof Error) { + console.error("Error message:", error.message); + // Check if it's a DeploymentError with type assertion + const deploymentError = error as DeploymentError; + if (deploymentError.receipt) { + console.error("Transaction receipt:", deploymentError.receipt); + } + if (deploymentError.transaction) { + console.error("Transaction details:", deploymentError.transaction); + } + } else { + console.error("Unknown error:", String(error)); + } process.exit(1); }
182-187
: Add confirmation prompt for deployment transactionsFor an operation as significant as contract deployment, adding an interactive confirmation prompt would be beneficial to prevent accidental deployments.
import { Command } from "commander"; import { ContractFactory, ethers, JsonRpcProvider, Wallet } from "ethers"; +import inquirer from "inquirer"; // ... export const deployCommand = new Command("deploy") .description("Deploy Uniswap V3 contracts") .requiredOption("--private-key <privateKey>", "Private key for deployment") .option("--rpc <rpc>", "RPC URL for the network", DEFAULT_RPC) .option("--wzeta <wzeta>", "WZETA token address", DEFAULT_WZETA) + .option("--confirm", "Skip confirmation prompt") .action(async (options) => { + if (!options.confirm) { + const { proceed } = await inquirer.prompt([{ + type: 'confirm', + name: 'proceed', + message: 'This will deploy Uniswap V3 contracts. Continue?', + default: false + }]); + if (!proceed) { + console.log("Deployment cancelled"); + return; + } + } + await main(options); + });packages/commands/src/pools/create.ts (2)
71-82
: Error handling could be more specificSimilar to the deploy command, error handling could be improved with more specific error type checking.
} catch (error) { - const poolError = error as PoolCreationError; - console.error("\nPool creation failed with error:"); - console.error("Error message:", poolError.message); - if (poolError.receipt) { - console.error("Transaction receipt:", poolError.receipt); - } - if (poolError.transaction) { - console.error("Transaction details:", poolError.transaction); - } + console.error("\nPool creation failed with error:"); + if (error instanceof Error) { + console.error("Error message:", error.message); + // Check for specific error properties + const poolError = error as PoolCreationError; + if (poolError.receipt) { + console.error("Transaction receipt:", poolError.receipt); + } + if (poolError.transaction) { + console.error("Transaction details:", poolError.transaction); + } + } else { + console.error("Unknown error:", String(error)); + } process.exit(1); }
85-106
: Add confirmation prompt before transaction executionFor an operation like pool creation that consumes gas, adding an interactive confirmation would be beneficial.
import { Command } from "commander"; import { Contract, ethers, JsonRpcProvider, Wallet } from "ethers"; +import inquirer from "inquirer"; // ... export const createCommand = new Command("create") .description("Create a new Uniswap V3 pool") .requiredOption( "--private-key <privateKey>", "Private key for signing transactions" ) .option("--rpc <rpc>", "RPC URL for the network", DEFAULT_RPC) .option( "--factory <factory>", "Uniswap V3 Factory contract address", DEFAULT_FACTORY ) .requiredOption( "--tokens <tokens...>", "Token addresses for the pool (exactly 2 required)" ) .option( "--fee <fee>", "Fee tier for the pool (3000 = 0.3%)", DEFAULT_FEE.toString() ) + .option("--confirm", "Skip confirmation prompt") - .action(main); + .action(async (options) => { + if (!options.confirm) { + const { proceed } = await inquirer.prompt([{ + type: 'confirm', + name: 'proceed', + message: 'This will create a new Uniswap V3 pool. Continue?', + default: false + }]); + if (!proceed) { + console.log("Pool creation cancelled"); + return; + } + } + await main(options); + });packages/commands/src/pools/liquidity/add.ts (2)
37-53
: Consolidate token contract instancesMultiple contract instances are created for the same tokens with different interfaces. This can be consolidated to improve code organization and reduce duplication.
Create a helper function for token contracts:
+// Helper to create token contract with needed interfaces +const createTokenContract = (tokenAddress: string, provider: JsonRpcProvider, signer?: Wallet) => { + const abi = [ + "function decimals() view returns (uint8)", + "function symbol() view returns (string)", + "function balanceOf(address) view returns (uint256)", + "function approve(address spender, uint256 amount) returns (bool)" + ]; + + return new Contract(tokenAddress, abi, signer || provider); +}; // Initialize token contracts to get decimals and symbols -const token0Contract = new Contract( - token0, - [ - "function decimals() view returns (uint8)", - "function symbol() view returns (string)", - ], - provider -); -const token1Contract = new Contract( - token1, - [ - "function decimals() view returns (uint8)", - "function symbol() view returns (string)", - ], - provider -); +const token0Contract = createTokenContract(token0, provider); +const token1Contract = createTokenContract(token1, provider); // ... // Check token balances -const token0ContractForBalance = new Contract( - token0, - ["function balanceOf(address) view returns (uint256)"], - provider -); -const token1ContractForBalance = new Contract( - token1, - ["function balanceOf(address) view returns (uint256)"], - provider -); +// Reuse token contracts for balance checks // ... // Initialize token contracts for approval -const token0ContractForApproval = new Contract( - token0, - ["function approve(address spender, uint256 amount) returns (bool)"], - signer -); -const token1ContractForApproval = new Contract( - token1, - ["function approve(address spender, uint256 amount) returns (bool)"], - signer -); +const token0ContractWithSigner = createTokenContract(token0, provider, signer); +const token1ContractWithSigner = createTokenContract(token1, provider, signer);Also applies to: 91-100, 159-168
257-278
: Current default tick range might not be appropriate for most usersThe command uses a very wide tick range by default (-887220 to 887220), which means liquidity is provided across the entire price range. This is inefficient for most use cases.
export const addCommand = new Command("add") .description("Add liquidity to a Uniswap V3 pool") .option("--rpc <rpc>", "RPC URL for the network", DEFAULT_RPC) .requiredOption( "--tokens <tokens...>", "Token addresses for the pool (exactly 2 required)" ) .requiredOption( "--amounts <amounts...>", "Amounts of tokens to add (in human-readable format, e.g. 0.1 5)" ) .option( "--recipient <recipient>", "Address that will receive the liquidity position NFT (defaults to signer's address)" ) .requiredOption( "--private-key <privateKey>", "Private key of the account that will send the transaction" ) - .option("--tick-lower <tickLower>", "Lower tick of the position", "-887220") - .option("--tick-upper <tickUpper>", "Upper tick of the position", "887220") + .option( + "--tick-lower <tickLower>", + "Lower tick of the position (default: 20% below current price)", + ) + .option( + "--tick-upper <tickUpper>", + "Upper tick of the position (default: 20% above current price)", + ) + .option( + "--full-range", + "Use full range for the position (-887220 to 887220)", + false + ) .action(main);And then in the main function, fetch the current price and set sensible defaults if not using full range:
// Set default tick range if not provided -const tickLower = validatedOptions.tickLower ?? -887220; -const tickUpper = validatedOptions.tickUpper ?? 887220; +let tickLower = validatedOptions.tickLower ? parseInt(validatedOptions.tickLower) : undefined; +let tickUpper = validatedOptions.tickUpper ? parseInt(validatedOptions.tickUpper) : undefined; + +// If full range option is selected, use min/max ticks +if (validatedOptions.fullRange) { + tickLower = -887220; + tickUpper = 887220; +} +// If no ticks provided and not full range, calculate sensible defaults based on current price +else if (tickLower === undefined || tickUpper === undefined) { + // Get pool contract to check current price + const poolContract = new Contract(poolAddress, [ + "function slot0() view returns (uint160 sqrtPriceX96, int24 tick, uint16 observationIndex, uint16 observationCardinality, uint16 observationCardinalityNext, uint8 feeProtocol, bool unlocked)" + ], provider); + + const { tick } = await poolContract.slot0(); + + // Default to ±20% range around current price + const tickSpacing = 60; // For 0.3% fee tier + tickLower = Math.floor((tick - (tick * 0.2)) / tickSpacing) * tickSpacing; + tickUpper = Math.ceil((tick + (tick * 0.2)) / tickSpacing) * tickSpacing; +}packages/commands/src/pools/show.ts (2)
67-76
: Enhance pool information displayThe current information display is basic. Additional useful information could be shown, such as token symbols and both price directions.
console.log("\nPool Information:"); console.log("Pool Address:", poolAddress); -console.log("Token 0:", token0); -console.log("Token 1:", token1); +// Get token symbols for better readability +const token0Contract = new Contract(token0, ["function symbol() view returns (string)"], provider); +const token1Contract = new Contract(token1, ["function symbol() view returns (string)"], provider); +const [symbol0, symbol1] = await Promise.all([token0Contract.symbol(), token1Contract.symbol()]); + +console.log(`Token 0 (${symbol0}):`, token0); +console.log(`Token 1 (${symbol1}):`, token1); console.log("Fee Tier:", `${Number(fee) / 10000}%`); console.log("Tick Spacing:", tickSpacing.toString()); -console.log("Current Price:", price.toFixed(6)); +console.log(`Current Price (${symbol1}/${symbol0}):`, price.toFixed(6)); +console.log(`Current Price (${symbol0}/${symbol1}):`, (1 / price).toFixed(6)); console.log("Liquidity:", liquidity.toString()); console.log("Current Tick:", slot0.tick.toString()); +// Add more pool information if available +console.log("Total Value Locked (TVL):", "Coming soon...");
87-109
: Add option to display verbose token informationThe show command could be enhanced with an option to display more detailed token information.
export const showCommand = new Command("show") .description("Show information about a Uniswap V3 pool") .option("--rpc <rpc>", "RPC URL for the network", DEFAULT_RPC) .addOption( new Option("--pool <pool>", "Pool contract address").conflicts("tokens") ) .addOption( new Option( "--tokens <tokens...>", "Token addresses for the pool (exactly 2 required)" ).conflicts("pool") ) .option( "--factory <factory>", "Uniswap V3 Factory contract address", DEFAULT_FACTORY ) .option( "--fee <fee>", "Fee tier for the pool (3000 = 0.3%)", DEFAULT_FEE.toString() ) + .option( + "--verbose", + "Display additional information about tokens and pool", + false + ) + .option( + "--format <format>", + "Output format (text, json)", + "text" + ) .action(main);This would require updating the main function to handle verbose mode and different output formats, but it would make the command more flexible for integrations and advanced users.
types/pools.ts (4)
67-73
: Align createPool schema with address validation
Apply the same address checks and privateKey validation here to keep schemas DRY and safe:-export const createPoolOptionsSchema = z.object({ - factory: z.string().default(DEFAULT_FACTORY), - fee: z.number().default(DEFAULT_FEE), - privateKey: z.string(), - rpc: z.string().default(DEFAULT_RPC), - tokens: z.array(z.string()).min(2).max(2), -}); +export const createPoolOptionsSchema = z.object({ + factory: evmAddressSchema.default(DEFAULT_FACTORY), + fee: z.number().default(DEFAULT_FEE), + privateKey: z + .string() + .refine((val) => /^0x[0-9a-fA-F]{64}$/.test(val), "Must be a valid private key"), + rpc: z.string().default(DEFAULT_RPC), + tokens: z.array(evmAddressSchema).length(2), +});
75-79
: Validate deployment options more strictly
FordeployOptionsSchema
, ensure both the wrapped ZETA token andprivateKey
are correctly formatted:-export const deployOptionsSchema = z.object({ - privateKey: z.string(), - rpc: z.string().default(DEFAULT_RPC), - wzeta: z.string().default(DEFAULT_WZETA), -}); +export const deployOptionsSchema = z.object({ + privateKey: z + .string() + .refine((val) => /^0x[0-9a-fA-F]{64}$/.test(val), "Must be a valid private key"), + rpc: z.string().default(DEFAULT_RPC), + wzeta: evmAddressSchema.default(DEFAULT_WZETA), +});
11-23
: Consider using Ethers’ BigNumberish or BigNumber types
TheMintParams
interface here uses nativebigint
for amounts, whereas the rest of your code likely works withethers.BigNumber
orBigNumberish
. For consistency and to avoid conversion boilerplate, consider switching toBigNumberish
(string | number | BigNumber
).
25-33
: Deduplicate error interfaces
PoolCreationError
andDeploymentError
share the same optional fields. You could define a singleTxError
interface and extend it for both contexts, reducing duplication.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (10)
package.json
(2 hunks)packages/commands/src/index.ts
(1 hunks)packages/commands/src/pools/constants.ts
(1 hunks)packages/commands/src/pools/create.ts
(1 hunks)packages/commands/src/pools/deploy.ts
(1 hunks)packages/commands/src/pools/index.ts
(1 hunks)packages/commands/src/pools/liquidity/add.ts
(1 hunks)packages/commands/src/pools/liquidity/index.ts
(1 hunks)packages/commands/src/pools/show.ts
(1 hunks)types/pools.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
packages/commands/src/pools/liquidity/index.ts (1)
packages/commands/src/pools/liquidity/add.ts (1)
addCommand
(257-278)
packages/commands/src/index.ts (3)
packages/commands/src/solanaEncode.ts (1)
solanaEncodeCommand
(15-25)packages/commands/src/pools/index.ts (1)
poolsCommand
(8-13)packages/commands/src/accounts/index.ts (1)
accountsCommand
(8-13)
packages/commands/src/pools/index.ts (4)
packages/commands/src/pools/deploy.ts (1)
deployCommand
(182-187)packages/commands/src/pools/create.ts (1)
createCommand
(85-106)packages/commands/src/pools/show.ts (1)
showCommand
(87-109)packages/commands/src/pools/liquidity/index.ts (1)
liquidityCommand
(5-7)
packages/commands/src/pools/show.ts (2)
types/pools.ts (3)
ShowPoolOptions
(81-81)showPoolOptionsSchema
(49-55)Slot0Result
(35-43)packages/commands/src/pools/constants.ts (3)
DEFAULT_RPC
(1-2)DEFAULT_FACTORY
(3-3)DEFAULT_FEE
(5-5)
types/pools.ts (1)
packages/commands/src/pools/constants.ts (4)
DEFAULT_FACTORY
(3-3)DEFAULT_FEE
(5-5)DEFAULT_RPC
(1-2)DEFAULT_WZETA
(4-4)
🔇 Additional comments (3)
package.json (1)
114-117
: Appropriate Uniswap V3 dependencies added.The addition of Uniswap V3 core and periphery packages will provide the necessary contract interfaces and utilities for interacting with Uniswap V3 pools.
packages/commands/src/pools/constants.ts (1)
3-7
:❓ Verification inconclusive
Verify contract addresses are correct for the target network.
The hardcoded contract addresses should be verified to ensure they are the correct and up-to-date addresses for the Uniswap V3 contracts on ZetaChain.
🏁 Script executed:
#!/bin/bash # Check if the addresses are valid contract addresses on the ZetaChain network echo "Verifying contract addresses..." ADDRESSES=("0x7E032E349853178C233a2560d9Ea434ac82228e0" "0x5F0b1a82749cb4E2278EC87F8BF6B618dC71a8bf" "0xFc5D90f650cf46Cecf96C66a4993f97D2a49f93B") RPC_URL="https://zetachain-athens.g.allthatnode.com/archive/evm" for ADDR in "${ADDRESSES[@]}"; do echo "Checking address: $ADDR" # Get the bytecode to verify this is a contract CODE=$(curl -s -X POST $RPC_URL -H "Content-Type: application/json" \ --data "{\"jsonrpc\":\"2.0\",\"method\":\"eth_getCode\",\"params\":[\"$ADDR\", \"latest\"],\"id\":1}" | \ jq -r '.result') if [ "$CODE" = "0x" ] || [ "$CODE" = "null" ]; then echo "WARNING: $ADDR does not appear to be a contract on the network" else echo "Confirmed $ADDR is a contract" fi doneLength of output: 98995
Action Required: Verify Remaining Contract Addresses
The script confirmed that DEFAULT_FACTORY (0x7E032E349853178C233a2560d9Ea434ac82228e0) is deployed, but it did not return results for the other two. Please re‑verify or manually confirm on ZetaChain that the following addresses have on‑chain bytecode and match the intended Uniswap V3 deployments:
- DEFAULT_WZETA: 0x5F0b1a82749cb4E2278EC87F8BF6B618dC71a8bf
- DEFAULT_POSITION_MANAGER: 0xFc5D90f650cf46Cecf96C66a4993f97D2a49f93B
You can rerun
eth_getCode
RPC calls or check a block explorer to ensure they’re correct and up‑to‑date.packages/commands/src/pools/index.ts (1)
1-13
: LGTM - Well-structured command hierarchy.The pools command structure is well-organized, with clear separation of concerns between deploying contracts, creating pools, showing pool information, and managing liquidity.
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: 4
🧹 Nitpick comments (1)
packages/commands/src/zetachain/pools/create.ts (1)
54-70
: Check if pool already exists before attempting creation.The factory will revert if the pool already exists, but checking beforehand provides better user experience.
Add this check before line 54:
// Check if pool already exists const existingPool = await uniswapV3FactoryInstance.getPool( validatedOptions.tokens[0], validatedOptions.tokens[1], fee ) as string; if (existingPool !== ethers.ZeroAddress) { throw new Error(`Pool already exists at address: ${existingPool}`); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (10)
package.json
(3 hunks)packages/commands/src/zetachain/index.ts
(2 hunks)packages/commands/src/zetachain/pools/create.ts
(1 hunks)packages/commands/src/zetachain/pools/deploy.ts
(1 hunks)packages/commands/src/zetachain/pools/index.ts
(1 hunks)packages/commands/src/zetachain/pools/liquidity/add.ts
(1 hunks)packages/commands/src/zetachain/pools/liquidity/index.ts
(1 hunks)packages/commands/src/zetachain/pools/show.ts
(1 hunks)src/constants/pools.ts
(1 hunks)types/pools.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- packages/commands/src/zetachain/pools/liquidity/index.ts
- packages/commands/src/zetachain/pools/index.ts
- src/constants/pools.ts
- types/pools.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: s2imonovic
PR: zeta-chain/toolkit#370
File: contracts/UniswapV3SetupLib.sol:124-128
Timestamp: 2025-06-26T14:06:16.147Z
Learning: In the ZetaChain toolkit codebase, accurate documentation is important - comments should correctly reflect what the code actually does (e.g., fixing comments that incorrectly reference Uniswap V2 when the code deploys V3 contracts).
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.
Learnt from: fadeev
PR: zeta-chain/toolkit#346
File: packages/commands/src/ton/deposit.ts:0-0
Timestamp: 2025-06-13T15:32:09.225Z
Learning: User fadeev prefers that responses be in English; avoid replying in Russian in future interactions.
packages/commands/src/zetachain/pools/deploy.ts (3)
Learnt from: hernan-clich
PR: zeta-chain/toolkit#329
File: utils/evm.command.helpers.ts:21-38
Timestamp: 2025-05-16T20:59:05.735Z
Learning: In the ZetaChain toolkit, base schemas like `baseEvmDepositOptionsSchema` are intentionally kept flexible, with mutual exclusivity refinements like name/privateKey validation happening in the specific command implementations that extend the base schema, rather than in the base schema itself.
Learnt from: s2imonovic
PR: zeta-chain/toolkit#370
File: contracts/UniswapV3SetupLib.sol:124-128
Timestamp: 2025-06-26T14:06:16.147Z
Learning: In the ZetaChain toolkit codebase, accurate documentation is important - comments should correctly reflect what the code actually does (e.g., fixing comments that incorrectly reference Uniswap V2 when the code deploys V3 contracts).
Learnt from: hernan-clich
PR: zeta-chain/toolkit#238
File: packages/tasks/src/verify.ts:0-0
Timestamp: 2025-03-18T14:06:01.531Z
Learning: The verify.ts file in the zeta-chain/toolkit repository already implements proper validation for empty contract lists using a condition that checks names.length.
🧬 Code Graph Analysis (1)
packages/commands/src/zetachain/index.ts (1)
packages/commands/src/zetachain/pools/index.ts (1)
poolsCommand
(8-14)
⏰ 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). (2)
- GitHub Check: build
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
packages/commands/src/zetachain/index.ts (1)
4-4
: LGTM!The integration of
poolsCommand
into the zetachain command structure is clean and follows the existing pattern.Also applies to: 14-14
packages/commands/src/zetachain/pools/deploy.ts (1)
53-167
: Well-structured deployment sequence.The deployment order (Factory → Router → Position Manager) correctly respects contract dependencies, and the error handling provides useful debugging information.
packages/commands/src/zetachain/pools/show.ts (1)
17-89
: Clean implementation with efficient data fetching.The pool information display logic is well-structured with proper validation, efficient parallel data fetching, and correct price calculations.
const params: MintParams = { | ||
amount0Desired: amount0, | ||
amount0Min: 0n, | ||
amount1Desired: amount1, | ||
amount1Min: 0n, | ||
deadline: Math.floor(Date.now() / 1000) + 60 * 20, | ||
fee: DEFAULT_FEE, | ||
recipient, | ||
tickLower, | ||
tickUpper, | ||
token0, | ||
token1, | ||
}; |
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.
Add slippage protection to prevent sandwich attacks.
Setting amount0Min
and amount1Min
to 0 provides no slippage protection, which could result in users receiving significantly less liquidity than expected due to price movements or sandwich attacks.
Consider implementing slippage protection:
+ // Calculate minimum amounts with slippage tolerance (e.g., 1%)
+ const slippageTolerance = 0.01; // 1%
+ const amount0Min = amount0 * BigInt(Math.floor((1 - slippageTolerance) * 10000)) / 10000n;
+ const amount1Min = amount1 * BigInt(Math.floor((1 - slippageTolerance) * 10000)) / 10000n;
+
const params: MintParams = {
amount0Desired: amount0,
- amount0Min: 0n,
+ amount0Min,
amount1Desired: amount1,
- amount1Min: 0n,
+ amount1Min,
deadline: Math.floor(Date.now() / 1000) + 60 * 20,
Alternatively, add a --slippage
option to the command to let users specify their tolerance.
📝 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 params: MintParams = { | |
amount0Desired: amount0, | |
amount0Min: 0n, | |
amount1Desired: amount1, | |
amount1Min: 0n, | |
deadline: Math.floor(Date.now() / 1000) + 60 * 20, | |
fee: DEFAULT_FEE, | |
recipient, | |
tickLower, | |
tickUpper, | |
token0, | |
token1, | |
}; | |
// Calculate minimum amounts with slippage tolerance (e.g., 1%) | |
const slippageTolerance = 0.01; // 1% | |
const amount0Min = amount0 * BigInt(Math.floor((1 - slippageTolerance) * 10000)) / 10000n; | |
const amount1Min = amount1 * BigInt(Math.floor((1 - slippageTolerance) * 10000)) / 10000n; | |
const params: MintParams = { | |
amount0Desired: amount0, | |
amount0Min, | |
amount1Desired: amount1, | |
amount1Min, | |
deadline: Math.floor(Date.now() / 1000) + 60 * 20, | |
fee: DEFAULT_FEE, | |
recipient, | |
tickLower, | |
tickUpper, | |
token0, | |
token1, | |
}; |
🤖 Prompt for AI Agents
In packages/commands/src/zetachain/pools/liquidity/add.ts around lines 190 to
202, the parameters amount0Min and amount1Min are set to 0n, providing no
slippage protection. To fix this, calculate minimum amounts based on a slippage
tolerance percentage (e.g., 1% or a user-specified --slippage option) by
reducing amount0 and amount1 accordingly, and assign these calculated values to
amount0Min and amount1Min to prevent users from receiving significantly less
liquidity due to price changes or sandwich attacks.
const tickLower = validatedOptions.tickLower ?? -887220; | ||
const tickUpper = validatedOptions.tickUpper ?? 887220; |
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.
💡 Verification agent
🧩 Analysis chain
Verify the default tick range values for Uniswap V3.
The default tick range [-887220, 887220] appears to represent the full range. Please verify these are the correct maximum tick values for Uniswap V3.
🌐 Web query:
What are the maximum and minimum tick values in Uniswap V3?
💡 Result:
In Uniswap V3, the ticks represent discrete price intervals and have defined minimum and maximum values.
- The minimum tick value is: -887272
- The maximum tick value is: 887272
These tick values correspond to the price ranges allowed in the Uniswap V3 protocol, determining the granularity in price movement within the pool.
Update default tick range to correct Uniswap V3 limits
The current defaults (-887220
/887220
) don’t cover the full Uniswap V3 range. According to the protocol spec, the minimum and maximum tick values are -887272 and 887272, respectively.
• File: packages/commands/src/zetachain/pools/liquidity/add.ts
Lines ~125–126
Suggested change:
- const tickLower = validatedOptions.tickLower ?? -887220;
- const tickUpper = validatedOptions.tickUpper ?? 887220;
+ const tickLower = validatedOptions.tickLower ?? -887272;
+ const tickUpper = validatedOptions.tickUpper ?? 887272;
🤖 Prompt for AI Agents
In packages/commands/src/zetachain/pools/liquidity/add.ts around lines 125 to
126, the default tick range values are set to -887220 and 887220, which do not
cover the full Uniswap V3 tick range. Update the default values to -887272 for
tickLower and 887272 for tickUpper to match the correct protocol limits.
deployOpts.gasLimit = Number(factoryGasEstimate * 2n); | ||
} |
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.
🛠️ Refactor suggestion
Consider reducing the gas multiplier to avoid wasting funds.
Doubling the gas estimate (2x) is excessive and will result in users paying more than necessary. Most production deployments use a 20-50% buffer.
- deployOpts.gasLimit = Number(factoryGasEstimate * 2n);
+ deployOpts.gasLimit = Number(factoryGasEstimate * 12n / 10n); // 20% buffer
Apply the same change to lines 95 and 134 for consistency.
Also applies to: 94-96, 133-135
🤖 Prompt for AI Agents
In packages/commands/src/zetachain/pools/deploy.ts around lines 64-65, 94-96,
and 133-135, the gas limit is set by doubling the gas estimate, which is
excessive. Change the multiplier from 2x to a smaller buffer, such as 1.2x to
1.5x, to reduce unnecessary gas costs. Update all three locations consistently
to apply this reduced multiplier.
// Calculate sqrtPriceX96 from USD prices | ||
const initialPrice = price1 / price0; | ||
const sqrtPrice = Math.sqrt(initialPrice); | ||
const sqrtPriceX96 = BigInt(Math.floor(sqrtPrice * 2 ** 96)); | ||
|
||
const initTx = (await pool.initialize( | ||
sqrtPriceX96 | ||
)) as ethers.TransactionResponse; |
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.
🛠️ Refactor suggestion
Add validation for sqrtPriceX96 bounds.
Uniswap V3 has minimum and maximum bounds for sqrtPriceX96. The calculated value should be validated.
const sqrtPriceX96 = BigInt(Math.floor(sqrtPrice * 2 ** 96));
+
+ // Uniswap V3 bounds
+ const MIN_SQRT_PRICE = 4295128739n;
+ const MAX_SQRT_PRICE = 1461446703485210103287273052203988822378723970342n;
+
+ if (sqrtPriceX96 < MIN_SQRT_PRICE || sqrtPriceX96 > MAX_SQRT_PRICE) {
+ throw new Error(`Calculated price is outside valid Uniswap V3 bounds`);
+ }
const initTx = (await pool.initialize(
📝 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.
// Calculate sqrtPriceX96 from USD prices | |
const initialPrice = price1 / price0; | |
const sqrtPrice = Math.sqrt(initialPrice); | |
const sqrtPriceX96 = BigInt(Math.floor(sqrtPrice * 2 ** 96)); | |
const initTx = (await pool.initialize( | |
sqrtPriceX96 | |
)) as ethers.TransactionResponse; | |
// Calculate sqrtPriceX96 from USD prices | |
const initialPrice = price1 / price0; | |
const sqrtPrice = Math.sqrt(initialPrice); | |
const sqrtPriceX96 = BigInt(Math.floor(sqrtPrice * 2 ** 96)); | |
// Uniswap V3 bounds | |
const MIN_SQRT_PRICE = 4295128739n; | |
const MAX_SQRT_PRICE = 1461446703485210103287273052203988822373052203988822378723970342n; | |
if (sqrtPriceX96 < MIN_SQRT_PRICE || sqrtPriceX96 > MAX_SQRT_PRICE) { | |
throw new Error(`Calculated price is outside valid Uniswap V3 bounds`); | |
} | |
const initTx = (await pool.initialize( | |
sqrtPriceX96 | |
)) as ethers.TransactionResponse; |
🤖 Prompt for AI Agents
In packages/commands/src/zetachain/pools/create.ts around lines 75 to 82, the
calculated sqrtPriceX96 value is used without validating if it falls within
Uniswap V3's allowed minimum and maximum bounds. Add checks to ensure
sqrtPriceX96 is not less than the minimum allowed value and not greater than the
maximum allowed value before passing it to pool.initialize. If it is out of
bounds, handle the error appropriately, such as throwing an exception or
adjusting the value to the nearest bound.
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: 1
♻️ Duplicate comments (6)
packages/commands/src/zetachain/pools/create.ts (1)
112-124
: Add validation for sqrtPriceX96 bounds.Uniswap V3 has minimum and maximum bounds for sqrtPriceX96. The calculated value should be validated to prevent initialization failures.
const sqrtPriceX96 = buildSqrtPriceX96( usdA, usdB, Number(dec0), Number(dec1), isCliOrderToken0 ); + // Uniswap V3 bounds + const MIN_SQRT_PRICE = 4295128739n; + const MAX_SQRT_PRICE = 1461446703485210103287273052203988822378723970342n; + + if (sqrtPriceX96 < MIN_SQRT_PRICE || sqrtPriceX96 > MAX_SQRT_PRICE) { + throw new Error(`Calculated sqrtPriceX96 ${sqrtPriceX96} is outside valid Uniswap V3 bounds [${MIN_SQRT_PRICE}, ${MAX_SQRT_PRICE}]`); + } if (sqrtPriceX96 === 0n) {packages/commands/src/zetachain/pools/liquidity/add.ts (2)
125-126
: Update default tick range to correct Uniswap V3 limitsThe current defaults (
-887220
/887220
) don't cover the full Uniswap V3 range. According to the protocol spec, the minimum and maximum tick values are -887272 and 887272, respectively.- const tickLower = validatedOptions.tickLower ?? -887220; - const tickUpper = validatedOptions.tickUpper ?? 887220; + const tickLower = validatedOptions.tickLower ?? -887272; + const tickUpper = validatedOptions.tickUpper ?? 887272;
194-206
: Add slippage protection to prevent sandwich attacks.Setting
amount0Min
andamount1Min
to 0 provides no slippage protection, which could result in users receiving significantly less liquidity than expected due to price movements or sandwich attacks.Consider implementing slippage protection:
+ // Calculate minimum amounts with slippage tolerance (e.g., 1%) + const slippageTolerance = 0.01; // 1% + const amount0Min = amount0 * BigInt(Math.floor((1 - slippageTolerance) * 10000)) / 10000n; + const amount1Min = amount1 * BigInt(Math.floor((1 - slippageTolerance) * 10000)) / 10000n; + const params: MintParams = { amount0Desired: amount0, - amount0Min: 0n, + amount0Min, amount1Desired: amount1, - amount1Min: 0n, + amount1Min, deadline: Math.floor(Date.now() / 1000) + 60 * 20,Alternatively, add a
--slippage
option to the command to let users specify their tolerance.types/pools.ts (3)
45-48
: Use the correct ethers address validation functionIn ethers v5+/v6 the
isAddress
helper lives underethers.utils
, not directly on theethers
namespace. The currentrefine
call will fail at compile/runtime.-export const evmAddressSchema = z - .string() - .refine((val) => ethers.isAddress(val), "Must be a valid EVM address"); +export const evmAddressSchema = z + .string() + .refine((val) => ethers.utils.isAddress(val), { + message: "Must be a valid EVM address", + });
60-74
: Use strict schemas for token addresses and amounts
addLiquidityOptionsSchema
currently accepts any two strings fortokens
andamounts
, and a free-form private key. Tighten it to catch invalid inputs early.export const addLiquidityOptionsSchema = z.object({ - amounts: z.array(z.string()).min(2).max(2), - privateKey: z.string(), + amounts: z + .array(z.string().regex(/^\d+(\.\d+)?$/, "Amounts must be numeric strings")) + .length(2), + privateKey: z + .string() + .regex(/^(0x)?[0-9a-fA-F]{64}$/, "Must be a valid private key"), recipient: z.string().optional(), rpc: z.string().default(DEFAULT_RPC), tickLower: z .string() .transform((val) => Number(val)) .optional(), tickUpper: z .string() .transform((val) => Number(val)) .optional(), - tokens: z.array(z.string()).min(2).max(2), + tokens: z.array(evmAddressSchema).length(2), });
49-58
: Enforce valid address pairs and tighten schemaThe
showPoolOptionsSchema
allows any two strings astokens
and doesn't validate addresses. You should reuseevmAddressSchema
to validate EVM addresses and ensure the factory is a valid address.export const showPoolOptionsSchema = z.object({ - factory: z.string().default(DEFAULT_FACTORY), + factory: evmAddressSchema.default(DEFAULT_FACTORY), fee: z .string() .transform((val) => Number(val)) .default(DEFAULT_FEE.toString()), - pool: z.string().optional(), + pool: evmAddressSchema.optional(), rpc: z.string().default(DEFAULT_RPC), - tokens: z.array(z.string()).optional(), + tokens: z.array(evmAddressSchema).length(2).optional(), });Consider also adding a refinement to ensure either
pool
ortokens
is provided for better UX.
🧹 Nitpick comments (6)
packages/commands/src/zetachain/pools/liquidity/show.ts (1)
4-4
: Remove unused import.The
ethers
import is not used in this file and should be removed to clean up the code.-import { Contract, ethers, JsonRpcProvider, Wallet } from "ethers"; +import { Contract, JsonRpcProvider, Wallet } from "ethers";packages/commands/src/zetachain/pools/liquidity/remove.ts (1)
4-4
: Remove unused import.The
ethers
import is not used in this file and should be removed.-import { Contract, ethers, JsonRpcProvider, Wallet } from "ethers"; +import { Contract, JsonRpcProvider, Wallet } from "ethers";packages/commands/src/zetachain/pools/swap.ts (2)
18-18
: Remove unused variable.The
TWO_192
constant is declared but never used in the code.-const TWO_192 = 1n << 192n;
21-30
: Convert to arrow functions for consistency.The ESLint configuration expects arrow functions instead of function declarations. Convert these helper functions to const arrow functions.
-function sqrtBig(n: bigint): bigint { +const sqrtBig = (n: bigint): bigint => { if (n < 2n) return n; let x = n, y = (x + 1n) >> 1n; while (y < x) { x = y; y = (x + n / x) >> 1n; } return x; -} +}; -function calcSqrtPriceX96( +const calcSqrtPriceX96 = ( usd0: number, usd1: number, dec0: number, dec1: number, cliIsTok0: boolean -): bigint { +): bigint => { const FIX = 1e18; // 18-dp fixed USD const p0 = BigInt(Math.round((cliIsTok0 ? usd0 : usd1) * FIX)); // token0 USD*1e18 const p1 = BigInt(Math.round((cliIsTok0 ? usd1 : usd0) * FIX)); // token1 USD*1e18 // token1/token0 ratio in base-units, scaled by 2^192 for Q64.96 const num = p1 * 10n ** BigInt(dec0); // p1 × 10^dec0 const den = p0 * 10n ** BigInt(dec1); // p0 × 10^dec1 if (num === 0n || den === 0n) throw new Error("bad price inputs"); const ratioX192 = (num << 192n) / den; // (token1/token0) × 2^192 return sqrtBig(ratioX192); -} +};Also applies to: 33-51
packages/commands/src/zetachain/pools/create.ts (1)
23-33
: Convert to arrow functions for consistency.Convert function declarations to arrow functions to match the project's ESLint configuration.
-function sqrtBig(n: bigint): bigint { +const sqrtBig = (n: bigint): bigint => { if (n < 0n) throw new Error("sqrt of negative"); if (n < 2n) return n; let x0 = n >> 1n; let x1 = (x0 + n / x0) >> 1n; while (x1 < x0) { x0 = x1; x1 = (x0 + n / x0) >> 1n; } return x0; -} +}; -function buildSqrtPriceX96( +const buildSqrtPriceX96 = ( usd0: number, usd1: number, dec0: number, dec1: number, isCliOrderToken0: boolean -): bigint { +): bigint => { // map USD prices into factory-sorted order const priceToken0Usd = isCliOrderToken0 ? usd0 : usd1; const priceToken1Usd = isCliOrderToken0 ? usd1 : usd0; // integer USD (6 decimals) to avoid FP const p0 = BigInt(Math.round(priceToken0Usd * Number(SCALE))); const p1 = BigInt(Math.round(priceToken1Usd * Number(SCALE))); // ratio (token1 / token0) in base units const ratio = (p0 * 10n ** BigInt(dec1)) / (p1 * 10n ** BigInt(dec0)); console.log("Ratio:", ratio.toString()); // sqrtPriceX96 = floor( sqrt(ratio) * 2^96 ) return sqrtBig(ratio * TWO_192); -} +};Also applies to: 36-57
types/pools.ts (1)
95-99
: Validate wzeta as an EVM addressThe
wzeta
field should validate as a proper EVM address for consistency and early error detection.export const deployOptionsSchema = z.object({ privateKey: z.string(), rpc: z.string().default(DEFAULT_RPC), - wzeta: z.string().default(DEFAULT_WZETA), + wzeta: evmAddressSchema.default(DEFAULT_WZETA), });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (9)
package.json
(3 hunks)packages/commands/src/zetachain/pools/create.ts
(1 hunks)packages/commands/src/zetachain/pools/index.ts
(1 hunks)packages/commands/src/zetachain/pools/liquidity/add.ts
(1 hunks)packages/commands/src/zetachain/pools/liquidity/index.ts
(1 hunks)packages/commands/src/zetachain/pools/liquidity/remove.ts
(1 hunks)packages/commands/src/zetachain/pools/liquidity/show.ts
(1 hunks)packages/commands/src/zetachain/pools/swap.ts
(1 hunks)types/pools.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/commands/src/zetachain/pools/liquidity/index.ts
- package.json
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: s2imonovic
PR: zeta-chain/toolkit#370
File: contracts/UniswapV3SetupLib.sol:124-128
Timestamp: 2025-06-26T14:06:16.147Z
Learning: In the ZetaChain toolkit codebase, accurate documentation is important - comments should correctly reflect what the code actually does (e.g., fixing comments that incorrectly reference Uniswap V2 when the code deploys V3 contracts).
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.
Learnt from: fadeev
PR: zeta-chain/toolkit#346
File: packages/commands/src/ton/deposit.ts:0-0
Timestamp: 2025-06-13T15:32:09.225Z
Learning: User fadeev prefers that responses be in English; avoid replying in Russian in future interactions.
packages/commands/src/zetachain/pools/create.ts (3)
Learnt from: hernan-clich
PR: zeta-chain/toolkit#290
File: utils/balances.ts:618-654
Timestamp: 2025-05-08T16:34:45.587Z
Learning: When working with Ethereum gas calculations, magic numbers like gas limits and gas prices should be stored in descriptive named constants rather than using raw numeric values directly in the code.
Learnt from: s2imonovic
PR: zeta-chain/toolkit#370
File: contracts/UniswapV3SetupLib.sol:124-128
Timestamp: 2025-06-26T14:06:16.147Z
Learning: In the ZetaChain toolkit codebase, accurate documentation is important - comments should correctly reflect what the code actually does (e.g., fixing comments that incorrectly reference Uniswap V2 when the code deploys V3 contracts).
Learnt from: hernan-clich
PR: zeta-chain/toolkit#329
File: utils/evm.command.helpers.ts:21-38
Timestamp: 2025-05-16T20:59:05.735Z
Learning: In the ZetaChain toolkit, base schemas like `baseEvmDepositOptionsSchema` are intentionally kept flexible, with mutual exclusivity refinements like name/privateKey validation happening in the specific command implementations that extend the base schema, rather than in the base schema itself.
types/pools.ts (11)
Learnt from: hernan-clich
PR: zeta-chain/toolkit#329
File: utils/evm.command.helpers.ts:21-38
Timestamp: 2025-05-16T20:59:05.735Z
Learning: In the ZetaChain toolkit, base schemas like `baseEvmDepositOptionsSchema` are intentionally kept flexible, with mutual exclusivity refinements like name/privateKey validation happening in the specific command implementations that extend the base schema, rather than in the base schema itself.
Learnt from: hernan-clich
PR: zeta-chain/toolkit#291
File: packages/commands/src/accounts/list.ts:0-0
Timestamp: 2025-04-17T15:12:06.167Z
Learning: In the zetachain/toolkit repository, JSON parsing should be done using the `parseJson` utility function with an appropriate schema for validation, rather than direct calls to `JSON.parse`.
Learnt from: hernan-clich
PR: zeta-chain/toolkit#332
File: packages/commands/src/bitcoin/inscription/call.ts:66-67
Timestamp: 2025-05-27T16:09:13.029Z
Learning: The ZetaChain toolkit uses comprehensive zod schema validation for command inputs, including hexStringSchema for hex string validation with regex /^(0x)?[0-9a-fA-F]*$/, so manual validation in the command logic is typically unnecessary as validation occurs during option parsing.
Learnt from: s2imonovic
PR: zeta-chain/toolkit#370
File: contracts/UniswapV3SetupLib.sol:124-128
Timestamp: 2025-06-26T14:06:16.147Z
Learning: In the ZetaChain toolkit codebase, accurate documentation is important - comments should correctly reflect what the code actually does (e.g., fixing comments that incorrectly reference Uniswap V2 when the code deploys V3 contracts).
Learnt from: hernan-clich
PR: zeta-chain/toolkit#329
File: utils/evm.command.helpers.ts:21-38
Timestamp: 2025-05-16T20:59:05.735Z
Learning: In the ZetaChain toolkit, mutual exclusivity between 'name' and 'privateKey' is implemented using the 'namePkRefineRule' in the individual command schemas that extend the base schema, rather than in the base schema itself. This design pattern allows the base schema to remain flexible while specific commands can apply constraints as needed.
Learnt from: hernan-clich
PR: zeta-chain/toolkit#238
File: packages/tasks/src/verify.ts:0-0
Timestamp: 2025-03-18T14:06:01.531Z
Learning: The verify.ts file in the zeta-chain/toolkit repository already implements proper validation for empty contract lists using a condition that checks names.length.
Learnt from: hernan-clich
PR: zeta-chain/toolkit#290
File: utils/balances.ts:618-654
Timestamp: 2025-05-08T16:34:45.587Z
Learning: When working with Ethereum gas calculations, magic numbers like gas limits and gas prices should be stored in descriptive named constants rather than using raw numeric values directly in the code.
Learnt from: hernan-clich
PR: zeta-chain/toolkit#332
File: utils/bitcoin.command.helpers.ts:27-27
Timestamp: 2025-05-28T00:01:54.010Z
Learning: In the ZetaChain toolkit Bitcoin commands, the TransactionInfo interface uses `amount: "0"` for call-only operations that don't involve actual Bitcoin transfers, rather than making the amount field optional. This maintains interface consistency.
Learnt from: fadeev
PR: zeta-chain/toolkit#365
File: src/chains/zetachain/withdrawAndCall.ts:122-126
Timestamp: 2025-06-27T08:04:59.106Z
Learning: In smart contracts with overloaded functions (multiple functions with the same name but different parameters), using the full ABI signature string in ethers.js is the correct and necessary approach to specify exactly which function variant to call. This is not brittle code but rather the standard way to disambiguate between overloaded contract functions.
Learnt from: hernan-clich
PR: zeta-chain/toolkit#291
File: packages/commands/src/accounts/show.ts:0-0
Timestamp: 2025-04-17T15:59:56.736Z
Learning: In the ZetaChain toolkit codebase, `fsUtils.ts` helpers like `safeReadFile` and `safeExists` should be used for file operations instead of direct `fs` module functions, as they provide centralized error handling.
Learnt from: hernan-clich
PR: zeta-chain/toolkit#291
File: packages/commands/src/accounts/list.ts:0-0
Timestamp: 2025-04-17T15:59:39.612Z
Learning: In the zetachain/toolkit repository, file system operations should use the safe helper functions from fsUtils.ts (like safeReadFile, safeReadDir, safeExists) instead of direct Node.js fs methods to ensure consistent error handling.
packages/commands/src/zetachain/pools/swap.ts (1)
Learnt from: s2imonovic
PR: zeta-chain/toolkit#370
File: contracts/UniswapV3SetupLib.sol:124-128
Timestamp: 2025-06-26T14:06:16.147Z
Learning: In the ZetaChain toolkit codebase, accurate documentation is important - comments should correctly reflect what the code actually does (e.g., fixing comments that incorrectly reference Uniswap V2 when the code deploys V3 contracts).
packages/commands/src/zetachain/pools/liquidity/add.ts (3)
Learnt from: hernan-clich
PR: zeta-chain/toolkit#332
File: utils/bitcoin.command.helpers.ts:27-27
Timestamp: 2025-05-28T00:01:54.010Z
Learning: In the ZetaChain toolkit Bitcoin commands, the TransactionInfo interface uses `amount: "0"` for call-only operations that don't involve actual Bitcoin transfers, rather than making the amount field optional. This maintains interface consistency.
Learnt from: s2imonovic
PR: zeta-chain/toolkit#370
File: contracts/UniswapV3SetupLib.sol:124-128
Timestamp: 2025-06-26T14:06:16.147Z
Learning: In the ZetaChain toolkit codebase, accurate documentation is important - comments should correctly reflect what the code actually does (e.g., fixing comments that incorrectly reference Uniswap V2 when the code deploys V3 contracts).
Learnt from: hernan-clich
PR: zeta-chain/toolkit#238
File: packages/tasks/src/verify.ts:0-0
Timestamp: 2025-03-18T14:06:01.531Z
Learning: The verify.ts file in the zeta-chain/toolkit repository already implements proper validation for empty contract lists using a condition that checks names.length.
🧬 Code Graph Analysis (3)
packages/commands/src/zetachain/pools/index.ts (5)
packages/commands/src/zetachain/pools/deploy.ts (1)
deployCommand
(182-187)packages/commands/src/zetachain/pools/create.ts (1)
createCommand
(147-158)packages/commands/src/zetachain/pools/swap.ts (1)
swapCommand
(141-151)packages/commands/src/zetachain/pools/show.ts (1)
showCommand
(120-142)packages/commands/src/zetachain/pools/liquidity/index.ts (1)
liquidityCommand
(7-11)
types/pools.ts (1)
src/constants/pools.ts (4)
DEFAULT_FACTORY
(3-3)DEFAULT_FEE
(5-5)DEFAULT_RPC
(1-2)DEFAULT_WZETA
(4-4)
packages/commands/src/zetachain/pools/swap.ts (2)
src/constants/pools.ts (2)
DEFAULT_RPC
(1-2)DEFAULT_FACTORY
(3-3)packages/tasks/src/account.ts (1)
main
(121-169)
🪛 ESLint
packages/commands/src/zetachain/pools/liquidity/show.ts
[error] 4-4: 'ethers' is defined but never used.
(@typescript-eslint/no-unused-vars)
packages/commands/src/zetachain/pools/liquidity/remove.ts
[error] 4-4: 'ethers' is defined but never used.
(@typescript-eslint/no-unused-vars)
packages/commands/src/zetachain/pools/create.ts
[error] 23-33: Expected a function expression.
(func-style)
[error] 36-57: Expected a function expression.
(func-style)
packages/commands/src/zetachain/pools/swap.ts
[error] 18-18: 'TWO_192' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 21-30: Expected a function expression.
(func-style)
[error] 33-51: Expected a function expression.
(func-style)
🪛 GitHub Check: build
packages/commands/src/zetachain/pools/create.ts
[failure] 104-104:
Unsafe array destructuring of a tuple element with an any
value
[failure] 103-103:
Unsafe argument of type any
assigned to a parameter of type string | Addressable
[failure] 97-97:
Unsafe member access .hash on an any
value
[failure] 92-92:
Unsafe assignment of an any
value
[failure] 91-91:
Unsafe member access .wait on an any
value
[failure] 91-91:
Unsafe call of an any
typed value
[failure] 86-86:
Unsafe assignment of an any
value
[failure] 78-78:
Unsafe assignment of an any
value
[warning] 36-36:
Use const or class constructors instead of named functions
[failure] 36-36:
Expected a function expression
[warning] 23-23:
Use const or class constructors instead of named functions
[failure] 23-23:
Expected a function expression
packages/commands/src/zetachain/pools/swap.ts
[warning] 33-33:
Use const or class constructors instead of named functions
[warning] 21-21:
Use const or class constructors instead of named functions
🪛 GitHub Actions: Lint
packages/commands/src/zetachain/pools/create.ts
[error] 23-23: ESLint: Expected a function expression (func-style)
⏰ 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). (2)
- GitHub Check: build
- GitHub Check: Analyze (javascript)
🔇 Additional comments (5)
packages/commands/src/zetachain/pools/index.ts (1)
1-17
: LGTM! Well-organized command structure.The command aggregation pattern is clean and follows good CLI design practices by grouping related pool operations under a single namespace with a meaningful alias.
packages/commands/src/zetachain/pools/liquidity/show.ts (1)
17-92
: LGTM! Robust implementation with good error handling.The command logic is well-structured with:
- Proper input validation using zod schema
- Fail-safe token symbol resolution that falls back to addresses
- Clear console output formatting
- Appropriate error handling and exit codes
The iteration through position NFTs and data extraction is correctly implemented.
packages/commands/src/zetachain/pools/liquidity/remove.ts (1)
18-115
: LGTM! Correct Uniswap V3 liquidity removal flow.The implementation properly follows the Uniswap V3 liquidity removal sequence:
- Interactive position selection when not specified
- Position validation (checking for zero liquidity)
- User confirmation before proceeding
decreaseLiquidity
to remove all liquiditycollect
to retrieve tokens and fees- Optional
burn
to destroy the empty NFTThe use of
MaxUint128
for collection amounts and proper deadline handling are correct practices.packages/commands/src/zetachain/pools/swap.ts (1)
67-138
: LGTM! Solid swap implementation with proper price targeting.The swap logic correctly:
- Validates pool existence and fetches token decimals
- Calculates target sqrtPriceX96 using proper fixed-point arithmetic
- Determines swap direction by comparing current vs target price
- Handles ERC-20 approval when needed
- Uses appropriate swap parameters with price limits
The price calculation math and Uniswap V3 integration are implemented correctly.
packages/commands/src/zetachain/pools/create.ts (1)
60-144
: LGTM! Solid pool creation and initialization logic.The implementation correctly:
- Validates input prices for positivity
- Checks for existing pools before creation
- Handles both pool creation and initialization steps
- Computes sqrtPriceX96 accounting for token order and decimals
- Provides comprehensive error handling and logging
The overall flow follows Uniswap V3 patterns properly, though the bounds validation mentioned above should be added.
.option("--tick-lower <tickLower>", "Lower tick of the position", "-887220") | ||
.option("--tick-upper <tickUpper>", "Upper tick of the position", "887220") |
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.
🛠️ Refactor suggestion
Update command option defaults to match corrected tick range
The default values in the command options should be updated to match the full Uniswap V3 tick range.
- .option("--tick-lower <tickLower>", "Lower tick of the position", "-887220")
- .option("--tick-upper <tickUpper>", "Upper tick of the position", "887220")
+ .option("--tick-lower <tickLower>", "Lower tick of the position", "-887272")
+ .option("--tick-upper <tickUpper>", "Upper tick of the position", "887272")
📝 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.
.option("--tick-lower <tickLower>", "Lower tick of the position", "-887220") | |
.option("--tick-upper <tickUpper>", "Upper tick of the position", "887220") | |
.option("--tick-lower <tickLower>", "Lower tick of the position", "-887272") | |
.option("--tick-upper <tickUpper>", "Upper tick of the position", "887272") |
🤖 Prompt for AI Agents
In packages/commands/src/zetachain/pools/liquidity/add.ts at lines 294 to 295,
update the default values for the --tick-lower and --tick-upper command options
to reflect the full Uniswap V3 tick range. Replace the current defaults
"-887220" and "887220" with the correct minimum and maximum tick values defined
by Uniswap V3 to ensure accurate default behavior.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
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: 1
♻️ Duplicate comments (1)
packages/commands/src/zetachain/pools/create.ts (1)
36-55
: Add sqrtPriceX96 bounds validation and convert to function expression.This function needs two improvements:
Missing bounds validation (as previously noted): The calculated sqrtPriceX96 should be validated against Uniswap V3's minimum and maximum bounds to prevent runtime errors.
Function style: Convert to function expression for consistency.
Apply this diff to address both issues:
-function buildSqrtPriceX96( +const buildSqrtPriceX96 = ( usd0: number, usd1: number, dec0: number, dec1: number, cliToken0: boolean -): bigint { +): bigint => { // USD prices mapped to factory order const pTok0 = BigInt(Math.round((cliToken0 ? usd0 : usd1) * 1e18)); const pTok1 = BigInt(Math.round((cliToken0 ? usd1 : usd0) * 1e18)); // token1/token0 ratio in base-units, scaled by 2¹⁹² const num = pTok1 * 10n ** BigInt(dec0); // p₁ × 10^dec₀ const den = pTok0 * 10n ** BigInt(dec1); // p₀ × 10^dec₁ const ratioX192 = (num << 192n) / den; // shift before divide if (ratioX192 === 0n) throw new Error("ratio underflow – raise precision"); /* integer √ → Q64.96 */ - return sqrtBig(ratioX192); -} + const sqrtPriceX96 = sqrtBig(ratioX192); + + // Uniswap V3 bounds validation + const MIN_SQRT_PRICE = 4295128739n; + const MAX_SQRT_PRICE = 1461446703485210103287273052203988822378723970342n; + + if (sqrtPriceX96 < MIN_SQRT_PRICE || sqrtPriceX96 > MAX_SQRT_PRICE) { + throw new Error(`Calculated price is outside valid Uniswap V3 bounds`); + } + + return sqrtPriceX96; +};
🧹 Nitpick comments (2)
packages/commands/src/zetachain/pools/create.ts (2)
24-34
: Convert function declarations to function expressions.The static analysis tools prefer function expressions over function declarations for consistency.
Apply this diff to convert to function expressions:
-function sqrtBig(n: bigint): bigint { +const sqrtBig = (n: bigint): bigint => { // integer √ if (n < 2n) return n; let x = n, y = (x + 1n) >> 1n; while (y < x) { x = y; y = (x + n / x) >> 1n; } return x; -} +};
58-141
: Address TypeScript typing issues but logic is sound.The main function implements the pool creation flow correctly. However, there are TypeScript typing issues flagged by static analysis due to ethers.js contract interactions returning
any
types.The business logic is well-structured with proper error handling. Consider adding explicit typing for contract method calls to improve type safety:
// For contract method calls, you might want to cast or use proper typing const poolAddr: string = await factory.getPool( o.tokens[0], o.tokens[1], o.fee ?? DEFAULT_FEE ) as string;However, this is a minor improvement as the current implementation works correctly.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/build.yaml
(1 hunks).github/workflows/lint.yaml
(1 hunks).github/workflows/publish-npm.yaml
(1 hunks).github/workflows/test.yaml
(1 hunks)package.json
(3 hunks)packages/commands/src/zetachain/pools/create.ts
(1 hunks)packages/commands/src/zetachain/pools/index.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .github/workflows/test.yaml
- .github/workflows/build.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/publish-npm.yaml
- .github/workflows/lint.yaml
- package.json
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/commands/src/zetachain/pools/index.ts (5)
packages/commands/src/zetachain/pools/deploy.ts (1)
deployCommand
(182-187)packages/commands/src/zetachain/pools/create.ts (1)
createCommand
(144-159)packages/commands/src/zetachain/pools/swap.ts (1)
swapCommand
(141-151)packages/commands/src/zetachain/pools/show.ts (1)
showCommand
(120-142)packages/commands/src/zetachain/pools/liquidity/index.ts (1)
liquidityCommand
(7-11)
🪛 GitHub Check: build
packages/commands/src/zetachain/pools/create.ts
[failure] 87-87:
Unsafe assignment of an any
value
[failure] 86-86:
Unsafe member access .wait on an any
value
[failure] 86-86:
Unsafe call of an any
typed value
[failure] 81-81:
Unsafe assignment of an any
value
[failure] 73-73:
Unsafe assignment of an any
value
[warning] 36-36:
Use const or class constructors instead of named functions
[failure] 36-36:
Expected a function expression
[warning] 24-24:
Use const or class constructors instead of named functions
[failure] 24-24:
Expected a function expression
[failure] 23-23:
'TWO_192' is assigned a value but never used
[failure] 22-22:
'SCALE' is assigned a value but never used
[failure] 4-4:
Run autofix to sort these imports!
🪛 ESLint
packages/commands/src/zetachain/pools/create.ts
[error] 4-19: Run autofix to sort these imports!
(simple-import-sort/imports)
[error] 22-22: 'SCALE' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 23-23: 'TWO_192' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 24-34: Expected a function expression.
(func-style)
[error] 36-55: Expected a function expression.
(func-style)
🪛 GitHub Actions: Lint
packages/commands/src/zetachain/pools/create.ts
[error] 4-4: ESLint: Run autofix to sort these imports! (simple-import-sort/imports)
⏰ 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). (2)
- GitHub Check: build
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
packages/commands/src/zetachain/pools/index.ts (2)
1-7
: LGTM! Clean import organization.The imports are well-structured and all imported commands are properly utilized in the command setup.
9-19
: Well-structured command group setup.The pools command is properly configured with appropriate metadata and all subcommands are correctly integrated. The testing disclaimer in the description is a good practice for development tooling.
packages/commands/src/zetachain/pools/create.ts (1)
143-159
: Excellent CLI command configuration.The command setup is well-structured with clear required and optional parameters, sensible defaults, and helpful descriptions. This provides a good user experience for the CLI tool.
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
♻️ Duplicate comments (1)
packages/commands/src/zetachain/pools/liquidity/add.ts (1)
190-204
: Add slippage protection to prevent sandwich attacks.Setting
amount0Min
andamount1Min
to 0 provides no slippage protection, which could result in users receiving significantly less liquidity than expected due to price movements or sandwich attacks.Consider implementing slippage protection:
+ // Calculate minimum amounts with slippage tolerance (e.g., 1%) + const slippageTolerance = 0.01; // 1% + const amount0Min = amount0 * BigInt(Math.floor((1 - slippageTolerance) * 10000)) / 10000n; + const amount1Min = amount1 * BigInt(Math.floor((1 - slippageTolerance) * 10000)) / 10000n; + const params: MintParams = { amount0Desired: amount0, - amount0Min: 0n, + amount0Min, amount1Desired: amount1, - amount1Min: 0n, + amount1Min, deadline: Math.floor(Date.now() / 1000) + 60 * 20,Alternatively, add a
--slippage
option to the command to let users specify their tolerance.
🧹 Nitpick comments (3)
packages/commands/src/zetachain/pools/liquidity/add.ts (3)
8-12
: Remove unused import to clean up the code.The
DEFAULT_FEE
import is not used anywhere in this file, as the fee is obtained from command line options instead.import { DEFAULT_FACTORY, - DEFAULT_FEE, DEFAULT_POSITION_MANAGER, DEFAULT_RPC, } from "../../../../../../src/constants/pools";
56-64
: Consider improving type safety for Promise.all result.While the type assertion works, consider using proper typing for better type safety:
- const [decimals0, decimals1, symbol0, symbol1] = (await Promise.all([ + const [decimals0, decimals1, symbol0, symbol1] = await Promise.all([ token0Contract.decimals(), token1Contract.decimals(), token0Contract.symbol(), token1Contract.symbol(), - ])) as [number, number, string, string]; + ]);The contract methods already return properly typed values, making the type assertion unnecessary.
197-197
: Use property shorthand for cleaner code.ESLint correctly identifies that property shorthand should be used here.
- fee: fee, + fee,
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/commands/src/zetachain/pools/liquidity/add.ts
(1 hunks)packages/commands/src/zetachain/pools/show.ts
(1 hunks)src/constants/pools.ts
(1 hunks)types/pools.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/constants/pools.ts
- packages/commands/src/zetachain/pools/show.ts
- types/pools.ts
🧰 Additional context used
🪛 ESLint
packages/commands/src/zetachain/pools/liquidity/add.ts
[error] 9-9: 'DEFAULT_FEE' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 197-197: Expected property shorthand.
(object-shorthand)
⏰ 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). (2)
- GitHub Check: build
- GitHub Check: Analyze (javascript)
🔇 Additional comments (9)
packages/commands/src/zetachain/pools/liquidity/add.ts (9)
19-36
: Well-structured validation and initialization.The input validation using Zod schema and the check for exactly 2 token addresses is properly implemented. The provider and signer initialization follows ethers.js best practices.
66-82
: Proper pool existence validation with clear error messaging.The pool validation logic correctly checks for the zero address and provides a user-friendly error message with readable fee percentage conversion.
84-116
: Comprehensive balance validation with informative error messages.The balance checking logic properly prevents insufficient fund transactions and provides clear feedback showing both required and available amounts.
118-150
: Excellent user experience with transaction preview and confirmation.The implementation provides comprehensive transaction details and proper user confirmation flow, ensuring users understand what they're signing before proceeding.
152-188
: Proper token approval implementation with nonce management.The approval process correctly handles sequential nonce management and waits for both approvals to complete before proceeding with the mint transaction.
224-252
: Robust transaction receipt parsing with proper error handling.The implementation correctly filters logs by contract address and handles parsing errors gracefully to extract the position NFT token ID.
254-261
: Comprehensive success logging with all relevant transaction details.The success output provides complete information including transaction hash, NFT ID, and amounts, which is essential for users to track their liquidity positions.
262-270
: Proper error handling with user-friendly messages and exit codes.The error handling correctly catches all exceptions, provides clear error messages, and exits with appropriate error codes for CLI usage.
272-294
: Well-structured command definition with practical defaults.The command definition properly uses Commander.js with appropriate required and optional parameters. The concentrated tick range defaults (276000, 277000) are more practical for most users than full range positions, as they provide better capital efficiency.
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: 3
♻️ Duplicate comments (5)
packages/commands/src/zetachain/pools/liquidity/add.ts (2)
234-235
: Critical: Implement slippage protection for liquidity addition.Setting
amount0Min
andamount1Min
to0n
provides no slippage protection, making users vulnerable to sandwich attacks and significant value loss due to price movements between transaction submission and execution.This mirrors the previous review comment about adding slippage protection. Consider implementing a configurable slippage tolerance:
+ // Calculate minimum amounts with slippage tolerance (e.g., 0.5%) + const slippageTolerance = validatedOptions.slippage ?? 0.005; // 0.5% default + const amount0Min = finalAmount0 * BigInt(Math.floor((1 - slippageTolerance) * 10000)) / 10000n; + const amount1Min = finalAmount1 * BigInt(Math.floor((1 - slippageTolerance) * 10000)) / 10000n; + const params: MintParams = { // ... other params - amount0Min: 0n, - amount1Min: 0n, + amount0Min, + amount1Min, // ... rest of params };Also add a
--slippage
option to the command definition.
282-283
: Update tick range command options to use correct defaults.The command definition should include proper default values for tick ranges, or better yet, make them optional with sensible defaults calculated at runtime.
Based on the previous review feedback, consider this approach:
.option("--rpc <url>", "JSON-RPC endpoint", DEFAULT_RPC) - .option("--tick-lower <tick>", "Lower tick") - .option("--tick-upper <tick>", "Upper tick") + .option("--tick-lower <tick>", "Lower tick", "-887272") + .option("--tick-upper <tick>", "Upper tick", "887272") .option("--fee <fee>", "Fee tier", "3000")Alternatively, modify the validation logic to use these as defaults when not provided:
- if (!validatedOptions.tickLower || !validatedOptions.tickUpper) { - throw new Error("tickLower and tickUpper must be provided"); - } + const tickLower = validatedOptions.tickLower ?? -887272; + const tickUpper = validatedOptions.tickUpper ?? 887272;packages/commands/src/zetachain/pools/create.ts (3)
4-8
: Fix lint failure – reorder imports as per ‘simple-import-sort’ rulesThe linter still reports unsorted imports. This has already been raised in the previous review but was not addressed. Please run the project’s autofix or manually group the
commander / ethers
imports first, then external JSON artifacts, and finally local paths to silence the CI failure.
22-24
: Remove unused constants to unblock CI
SCALE
andTWO_192
are never referenced after their declaration, and the linter fails (@typescript-eslint/no-unused-vars
). Simply drop them unless you plan to use them in a subsequent patch.-const SCALE = 1_000_000_000_000_000_000n; // 1e18 -const TWO_192 = 1n << 192n;
36-55
: Still missing mandatory Uniswap V3 bound check forsqrtPriceX96
Previous review requested validation that
sqrtPriceX96
falls between the protocol min/max (4295128739 ≤ value ≤ 1461446703485210103287273052203988822378723970342
). This guard is crucial; an out-of-range value will makeinitialize
revert and leave the pool unusable. Please add the check right before returning frombuildSqrtPriceX96
.
🧹 Nitpick comments (9)
packages/commands/src/zetachain/pools/liquidity/add.ts (7)
1-17
: Address import sorting issue flagged by ESLint.The static analysis tool indicates that imports should be sorted. Consider running the autofix to resolve this formatting issue.
37-39
: Useconst
instead oflet
for immutable variables.The variables
inputTokenA
andinputTokenB
are never reassigned after initialization. Following best practices, they should be declared asconst
.- let [inputTokenA, inputTokenB] = validatedOptions.tokens.map((addr) => + const [inputTokenA, inputTokenB] = validatedOptions.tokens.map((addr) => ethers.getAddress(addr) );
121-122
: Useconst
for immutable token variables.The variables
finalToken0
andfinalToken1
are assigned once and never modified, so they should be declared asconst
.- let finalToken0 = poolToken0; - let finalToken1 = poolToken1; + const finalToken0 = poolToken0; + const finalToken1 = poolToken1;
254-266
: Improve NFT token ID extraction robustness.The current implementation for extracting the NFT token ID from Transfer events could be more robust. Consider adding better error handling and validation.
const transferLog = receipt.logs.find((l: Log) => { try { const parsed = iface.parseLog({ data: l.data, topics: l.topics }); - return parsed?.name === "Transfer"; + return parsed?.name === "Transfer" && parsed.args.length >= 3; } catch { return false; } }); - const tokenId = transferLog - ? iface.parseLog(transferLog)?.args[2] ?? "<unknown>" - : "<unknown>"; + let tokenId = "<unknown>"; + if (transferLog) { + try { + const parsed = iface.parseLog(transferLog); + if (parsed?.args && parsed.args.length >= 3) { + tokenId = parsed.args[2].toString(); + } + } catch (err) { + console.warn("Failed to parse Transfer event for token ID:", err); + } + }
200-202
: Fix object key ordering in inquirer prompt.The static analysis tool flags incorrect key ordering in the inquirer prompt object. While this doesn't affect functionality, it's good to maintain consistency.
const { confirm } = await inquirer.prompt([ - { type: "confirm", name: "confirm", message: "Proceed?" }, + { message: "Proceed?", name: "confirm", type: "confirm" }, ]);
226-238
: Fix object key ordering in MintParams.The static analysis tool flags multiple key ordering issues in the
params
object. Consider fixing these for consistency.const params: MintParams = { + amount0Desired: finalAmount0, + amount0Min: 0n, + amount1Desired: finalAmount1, + amount1Min: 0n, + deadline: Math.floor(Date.now() / 1000) + 60 * 20, + fee, + recipient, token0: finalToken0, token1: finalToken1, - fee, tickLower, tickUpper, - amount0Desired: finalAmount0, - amount1Desired: finalAmount1, - amount0Min: 0n, - amount1Min: 0n, - recipient, - deadline: Math.floor(Date.now() / 1000) + 60 * 20, };
270-274
: Consider more graceful error handling.The current error handling always exits the process with code 1. For a CLI command, this is appropriate, but consider whether more specific error codes or cleanup might be beneficial.
} catch (err) { console.error("Failed to add liquidity:", (err as Error).message); + // Consider logging additional debug info in verbose mode + if (process.env.DEBUG) { + console.error("Stack trace:", (err as Error).stack); + } process.exit(1); }packages/commands/src/zetachain/pools/create.ts (2)
24-34
: Conform tofunc-style
– convert helpers to const arrow functionsESLint is flagging both
sqrtBig
andbuildSqrtPriceX96
for violating thefunc-style
rule (expecting function expressions). Switching toconst fn = () => {}
clears the violation and keeps the helper declarations consistent with the rest of the codebase.-function sqrtBig(n: bigint): bigint { +const sqrtBig = (n: bigint): bigint => { … -} +};
116-123
:slot0()
probe can wrongly label partially initialised poolsA pool can be created but not yet initialised, causing
slot0()
to revert – your catch blanket setsneedInit = true
, which is fine. However, ifslot0()
succeeds yet returns a deprecated state (very old Uniswap bug) you only test forsqrtPriceX96 === 0
. Consider guarding againstsqrtPriceX96 < MIN_SQRT_PRICE
as well.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/commands/src/zetachain/pools/create.ts
(1 hunks)packages/commands/src/zetachain/pools/liquidity/add.ts
(1 hunks)src/constants/pools.ts
(1 hunks)types/pools.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/constants/pools.ts
- types/pools.ts
🧰 Additional context used
🪛 GitHub Check: build
packages/commands/src/zetachain/pools/create.ts
[failure] 87-87:
Unsafe assignment of an any
value
[failure] 86-86:
Unsafe member access .wait on an any
value
[failure] 86-86:
Unsafe call of an any
typed value
[failure] 81-81:
Unsafe assignment of an any
value
[failure] 73-73:
Unsafe assignment of an any
value
[warning] 36-36:
Use const or class constructors instead of named functions
[failure] 36-36:
Expected a function expression
[warning] 24-24:
Use const or class constructors instead of named functions
[failure] 24-24:
Expected a function expression
[failure] 23-23:
'TWO_192' is assigned a value but never used
[failure] 22-22:
'SCALE' is assigned a value but never used
[failure] 4-4:
Run autofix to sort these imports!
🪛 ESLint
packages/commands/src/zetachain/pools/create.ts
[error] 4-19: Run autofix to sort these imports!
(simple-import-sort/imports)
[error] 22-22: 'SCALE' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 23-23: 'TWO_192' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 24-34: Expected a function expression.
(func-style)
[error] 36-55: Expected a function expression.
(func-style)
packages/commands/src/zetachain/pools/liquidity/add.ts
[error] 1-17: Run autofix to sort these imports!
(simple-import-sort/imports)
[error] 37-37: 'inputTokenA' is never reassigned. Use 'const' instead.
(prefer-const)
[error] 37-37: 'inputTokenB' is never reassigned. Use 'const' instead.
(prefer-const)
[error] 42-42: Irregular whitespace not allowed.
(no-irregular-whitespace)
[error] 121-121: 'finalToken0' is never reassigned. Use 'const' instead.
(prefer-const)
[error] 122-122: 'finalToken1' is never reassigned. Use 'const' instead.
(prefer-const)
[error] 201-201: Expected object keys to be in ascending order. 'name' should be before 'type'.
(sort-keys-fix/sort-keys-fix)
[error] 201-201: Expected object keys to be in ascending order. 'message' should be before 'name'.
(sort-keys-fix/sort-keys-fix)
[error] 229-229: Expected object keys to be in ascending order. 'fee' should be before 'token1'.
(sort-keys-fix/sort-keys-fix)
[error] 232-232: Expected object keys to be in ascending order. 'amount0Desired' should be before 'tickUpper'.
(sort-keys-fix/sort-keys-fix)
[error] 234-234: Expected object keys to be in ascending order. 'amount0Min' should be before 'amount1Desired'.
(sort-keys-fix/sort-keys-fix)
[error] 237-237: Expected object keys to be in ascending order. 'deadline' should be before 'recipient'.
(sort-keys-fix/sort-keys-fix)
🪛 GitHub Actions: Lint
packages/commands/src/zetachain/pools/create.ts
[error] 4-4: ESLint: Run autofix to sort these imports! (simple-import-sort/imports)
⏰ 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). (2)
- GitHub Check: build
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
packages/commands/src/zetachain/pools/create.ts (1)
60-63
: Price array order is counter-intuitive – verify it’s intentional
const [usdB, usdA] = o.prices.map(Number);
reverses the user-supplied order without explanation. If this is not deliberate, it can silently mis-price the pool; if it is deliberate, consider an inline comment to avoid future confusion.
let poolAddr = await factory.getPool( | ||
o.tokens[0], | ||
o.tokens[1], | ||
o.fee ?? DEFAULT_FEE | ||
); | ||
|
||
if (poolAddr === ethers.ZeroAddress) { | ||
console.log("Creating pool …"); | ||
const tx = await factory.createPool( | ||
o.tokens[0], | ||
o.tokens[1], | ||
o.fee ?? DEFAULT_FEE | ||
); | ||
await tx.wait(); | ||
poolAddr = await factory.getPool( |
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.
🛠️ Refactor suggestion
Type safety & transaction status check
factory.createPool
and tx.wait()
are currently typed as any
, which hides errors (static analysis flags unsafe calls).
- Import and use the generated typings (
UniswapV3Factory__factory
) so thatgetPool
returnsPromise<string>
andcreatePool
returnsContractTransaction
. - After
await tx.wait()
inspectreceipt.status
and throw on failure to prevent falsely reporting success.
- await tx.wait();
+ const receipt = await tx.wait();
+ if (receipt?.status !== 1) {
+ throw new Error(`createPool failed (tx: ${tx.hash})`);
+ }
📝 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.
let poolAddr = await factory.getPool( | |
o.tokens[0], | |
o.tokens[1], | |
o.fee ?? DEFAULT_FEE | |
); | |
if (poolAddr === ethers.ZeroAddress) { | |
console.log("Creating pool …"); | |
const tx = await factory.createPool( | |
o.tokens[0], | |
o.tokens[1], | |
o.fee ?? DEFAULT_FEE | |
); | |
await tx.wait(); | |
poolAddr = await factory.getPool( | |
let poolAddr = await factory.getPool( | |
o.tokens[0], | |
o.tokens[1], | |
o.fee ?? DEFAULT_FEE | |
); | |
if (poolAddr === ethers.ZeroAddress) { | |
console.log("Creating pool …"); | |
const tx = await factory.createPool( | |
o.tokens[0], | |
o.tokens[1], | |
o.fee ?? DEFAULT_FEE | |
); | |
- await tx.wait(); | |
+ const receipt = await tx.wait(); | |
+ if (receipt?.status !== 1) { | |
+ throw new Error(`createPool failed (tx: ${tx.hash})`); | |
+ } | |
poolAddr = await factory.getPool( | |
o.tokens[0], | |
o.tokens[1], | |
o.fee ?? DEFAULT_FEE | |
); | |
} |
🧰 Tools
🪛 GitHub Check: build
[failure] 87-87:
Unsafe assignment of an any
value
[failure] 86-86:
Unsafe member access .wait on an any
value
[failure] 86-86:
Unsafe call of an any
typed value
[failure] 81-81:
Unsafe assignment of an any
value
[failure] 73-73:
Unsafe assignment of an any
value
🤖 Prompt for AI Agents
In packages/commands/src/zetachain/pools/create.ts around lines 73 to 87, the
calls to factory.createPool and tx.wait() are typed as any, which disables
static type checking and error detection. To fix this, import the generated
typings UniswapV3Factory__factory and use them to instantiate the factory so
getPool returns Promise<string> and createPool returns ContractTransaction.
After awaiting tx.wait(), check the receipt.status field and throw an error if
the transaction failed to ensure the code does not incorrectly assume success.
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: 1
♻️ Duplicate comments (9)
packages/commands/src/zetachain/pools/create.ts (6)
4-19
: Fix import sorting according to project linting rules.The imports need to be reordered according to the project's ESLint configuration.
-import * as UniswapV3Factory from "@uniswap/v3-core/artifacts/contracts/UniswapV3Factory.sol/UniswapV3Factory.json"; -import * as UniswapV3Pool from "@uniswap/v3-core/artifacts/contracts/UniswapV3Pool.sol/UniswapV3Pool.json"; -import { Command } from "commander"; -import { Contract, ethers, JsonRpcProvider, Wallet } from "ethers"; +import { Command } from "commander"; +import { Contract, ethers, JsonRpcProvider, Wallet } from "ethers"; + +import * as UniswapV3Factory from "@uniswap/v3-core/artifacts/contracts/UniswapV3Factory.sol/UniswapV3Factory.json"; +import * as UniswapV3Pool from "@uniswap/v3-core/artifacts/contracts/UniswapV3Pool.sol/UniswapV3Pool.json"; import { DEFAULT_FACTORY, DEFAULT_FEE, DEFAULT_RPC, } from "../../../../../src/constants/pools"; import { IERC20Metadata__factory } from "../../../../../typechain-types";
22-23
: Remove unused constants.The
SCALE
andTWO_192
constants are defined but never used in the code.-const SCALE = 1_000_000_000_000_000_000n; // 1e18 -const TWO_192 = 1n << 192n;
106-114
: Add validation for sqrtPriceX96 bounds.The calculated sqrtPriceX96 should be validated against Uniswap V3's minimum and maximum bounds.
const sqrtPriceX96 = buildSqrtPriceX96( usdA, usdB, Number(dec0), Number(dec1), cliToken0 ); + + // Uniswap V3 bounds + const MIN_SQRT_PRICE = 4295128739n; + const MAX_SQRT_PRICE = 1461446703485210103287273052203988822378723970342n; + + if (sqrtPriceX96 < MIN_SQRT_PRICE || sqrtPriceX96 > MAX_SQRT_PRICE) { + throw new Error(`Calculated price is outside valid Uniswap V3 bounds`); + }
67-92
: Improve type safety for factory operations.The factory contract operations are using unsafe 'any' types. Consider using typed contract factories.
Import and use the typed factory:
import { UniswapV3Factory__factory } from "@uniswap/v3-core/typechain-types";Then instantiate with proper typing:
- const factory = new Contract( - o.factory ?? DEFAULT_FACTORY, - UniswapV3Factory.abi, - signer - ); + const factory = UniswapV3Factory__factory.connect( + o.factory ?? DEFAULT_FACTORY, + signer + );
79-92
: Check transaction receipt status after pool creation.The transaction receipt status should be checked to ensure the pool creation succeeded.
- await tx.wait(); + const receipt = await tx.wait(); + if (receipt?.status !== 1) { + throw new Error(`createPool failed (tx: ${tx.hash})`); + }
124-131
: Check transaction receipt status after pool initialization.The transaction receipt status should be checked to ensure the initialization succeeded.
- await tx.wait(); + const receipt = await tx.wait(); + if (receipt?.status !== 1) { + throw new Error(`Pool initialization failed (tx: ${tx.hash})`); + }packages/commands/src/zetachain/pools/liquidity/add.ts (3)
282-283
: Update default tick range values to match Uniswap V3 limits.The command should use the correct Uniswap V3 tick range limits as defaults.
- .option("--tick-lower <tick>", "Lower tick") - .option("--tick-upper <tick>", "Upper tick") + .option("--tick-lower <tick>", "Lower tick", "-887272") + .option("--tick-upper <tick>", "Upper tick", "887272")
226-238
: Add slippage protection to prevent sandwich attacks.Setting
amount0Min
andamount1Min
to 0 provides no slippage protection.+ // Calculate minimum amounts with slippage tolerance (e.g., 1%) + const slippageTolerance = 0.01; // 1% + const amount0Min = finalAmount0 * BigInt(Math.floor((1 - slippageTolerance) * 10000)) / 10000n; + const amount1Min = finalAmount1 * BigInt(Math.floor((1 - slippageTolerance) * 10000)) / 10000n; + const params: MintParams = { amount0Desired: finalAmount0, - amount0Min: 0n, + amount0Min, amount1Desired: finalAmount1, - amount1Min: 0n, + amount1Min, deadline: Math.floor(Date.now() / 1000) + 60 * 20,Alternatively, add a
--slippage
option to let users specify their tolerance.
163-175
: Add validation for tick range bounds.The code should validate that ticks are within Uniswap V3's valid range.
const tickLower = Math.floor(rawLower / tickSpacing) * tickSpacing; const tickUpper = Math.floor(rawUpper / tickSpacing) * tickSpacing; + // Validate tick bounds (Uniswap V3 limits: -887272 to 887272) + const MIN_TICK = -887272; + const MAX_TICK = 887272; + + if (tickLower < MIN_TICK || tickLower > MAX_TICK) { + throw new Error(`tickLower must be between ${MIN_TICK} and ${MAX_TICK}`); + } + if (tickUpper < MIN_TICK || tickUpper > MAX_TICK) { + throw new Error(`tickUpper must be between ${MIN_TICK} and ${MAX_TICK}`); + } + if (tickLower >= tickUpper) { throw new Error("tickLower must be smaller than tickUpper"); }
🧹 Nitpick comments (1)
utils/uniswap.ts (1)
259-270
: Consider improving error typing for better error handling.The catch block uses a generic error type. Consider using a more specific type or error handling pattern.
const fetchDetails = async (address: string) => { try { const contract = new ethers.Contract(address, IERC20, provider); const [symbol, decimals] = await Promise.all([ contract.symbol() as Promise<string>, contract.decimals() as Promise<number>, ]); return { decimals: Number(decimals), symbol }; } catch (error) { - console.warn(`Failed to fetch details for token ${address}:`, error); + console.warn(`Failed to fetch details for token ${address}:`, error instanceof Error ? error.message : String(error)); return { decimals: 18, symbol: "UNKNOWN" }; } };
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/client/src/getPools.ts
(1 hunks)packages/commands/src/zetachain/pools/create.ts
(1 hunks)packages/commands/src/zetachain/pools/liquidity/add.ts
(1 hunks)packages/commands/src/zetachain/pools/show.ts
(1 hunks)packages/commands/src/zetachain/pools/swap.ts
(1 hunks)utils/uniswap.ts
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/commands/src/zetachain/pools/swap.ts
- packages/commands/src/zetachain/pools/show.ts
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: s2imonovic
PR: zeta-chain/toolkit#370
File: contracts/UniswapV3SetupLib.sol:124-128
Timestamp: 2025-06-26T14:06:16.147Z
Learning: In the ZetaChain toolkit codebase, accurate documentation is important - comments should correctly reflect what the code actually does (e.g., fixing comments that incorrectly reference Uniswap V2 when the code deploys V3 contracts).
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.
Learnt from: fadeev
PR: zeta-chain/toolkit#346
File: packages/commands/src/ton/deposit.ts:0-0
Timestamp: 2025-06-13T15:32:09.225Z
Learning: User fadeev prefers that responses be in English; avoid replying in Russian in future interactions.
📚 Learning: in the zetachain toolkit codebase, accurate documentation is important - comments should correctly r...
Learnt from: s2imonovic
PR: zeta-chain/toolkit#370
File: contracts/UniswapV3SetupLib.sol:124-128
Timestamp: 2025-06-26T14:06:16.147Z
Learning: In the ZetaChain toolkit codebase, accurate documentation is important - comments should correctly reflect what the code actually does (e.g., fixing comments that incorrectly reference Uniswap V2 when the code deploys V3 contracts).
Applied to files:
utils/uniswap.ts
packages/commands/src/zetachain/pools/create.ts
packages/commands/src/zetachain/pools/liquidity/add.ts
📚 Learning: when working with ethereum gas calculations, magic numbers like gas limits and gas prices should be ...
Learnt from: hernan-clich
PR: zeta-chain/toolkit#290
File: utils/balances.ts:618-654
Timestamp: 2025-05-08T16:34:45.587Z
Learning: When working with Ethereum gas calculations, magic numbers like gas limits and gas prices should be stored in descriptive named constants rather than using raw numeric values directly in the code.
Applied to files:
packages/commands/src/zetachain/pools/create.ts
📚 Learning: in the zetachain toolkit, base schemas like `baseevmdepositoptionsschema` are intentionally kept fle...
Learnt from: hernan-clich
PR: zeta-chain/toolkit#329
File: utils/evm.command.helpers.ts:21-38
Timestamp: 2025-05-16T20:59:05.735Z
Learning: In the ZetaChain toolkit, base schemas like `baseEvmDepositOptionsSchema` are intentionally kept flexible, with mutual exclusivity refinements like name/privateKey validation happening in the specific command implementations that extend the base schema, rather than in the base schema itself.
Applied to files:
packages/commands/src/zetachain/pools/create.ts
📚 Learning: the zetachain toolkit project uses an eslint plugin (simple-import-sort) that automatically handles ...
Learnt from: hernan-clich
PR: zeta-chain/toolkit#365
File: src/chains/ton/depositAndCall.ts:0-0
Timestamp: 2025-07-02T17:31:41.316Z
Learning: The ZetaChain toolkit project uses an ESLint plugin (simple-import-sort) that automatically handles import sorting, so manual suggestions for import order fixes are not needed as the tooling will auto-fix them.
Applied to files:
packages/commands/src/zetachain/pools/create.ts
📚 Learning: the verify.ts file in the zeta-chain/toolkit repository already implements proper validation for emp...
Learnt from: hernan-clich
PR: zeta-chain/toolkit#238
File: packages/tasks/src/verify.ts:0-0
Timestamp: 2025-03-18T14:06:01.531Z
Learning: The verify.ts file in the zeta-chain/toolkit repository already implements proper validation for empty contract lists using a condition that checks names.length.
Applied to files:
packages/commands/src/zetachain/pools/create.ts
packages/commands/src/zetachain/pools/liquidity/add.ts
📚 Learning: in the zetachain toolkit codebase, `fsutils.ts` helpers like `safereadfile` and `safeexists` should ...
Learnt from: hernan-clich
PR: zeta-chain/toolkit#291
File: packages/commands/src/accounts/show.ts:0-0
Timestamp: 2025-04-17T15:59:56.736Z
Learning: In the ZetaChain toolkit codebase, `fsUtils.ts` helpers like `safeReadFile` and `safeExists` should be used for file operations instead of direct `fs` module functions, as they provide centralized error handling.
Applied to files:
packages/commands/src/zetachain/pools/create.ts
📚 Learning: in the zetachain/toolkit repository, file system operations should use the safe helper functions fro...
Learnt from: hernan-clich
PR: zeta-chain/toolkit#291
File: packages/commands/src/accounts/list.ts:0-0
Timestamp: 2025-04-17T15:59:39.612Z
Learning: In the zetachain/toolkit repository, file system operations should use the safe helper functions from fsUtils.ts (like safeReadFile, safeReadDir, safeExists) instead of direct Node.js fs methods to ensure consistent error handling.
Applied to files:
packages/commands/src/zetachain/pools/create.ts
📚 Learning: in the zetachain toolkit bitcoin commands, the transactioninfo interface uses `amount: "0"` for call...
Learnt from: hernan-clich
PR: zeta-chain/toolkit#332
File: utils/bitcoin.command.helpers.ts:27-27
Timestamp: 2025-05-28T00:01:54.010Z
Learning: In the ZetaChain toolkit Bitcoin commands, the TransactionInfo interface uses `amount: "0"` for call-only operations that don't involve actual Bitcoin transfers, rather than making the amount field optional. This maintains interface consistency.
Applied to files:
packages/commands/src/zetachain/pools/liquidity/add.ts
🧬 Code Graph Analysis (2)
utils/uniswap.ts (2)
types/pools.types.ts (1)
Pool
(27-31)types/foreignCoins.types.ts (1)
ForeignCoin
(1-12)
packages/client/src/getPools.ts (1)
utils/uniswap.ts (1)
formatPoolsWithTokenDetails
(291-345)
🪛 ESLint
packages/commands/src/zetachain/pools/create.ts
[error] 22-22: 'SCALE' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 23-23: 'TWO_192' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 24-34: Expected a function expression.
(func-style)
[error] 36-55: Expected a function expression.
(func-style)
packages/commands/src/zetachain/pools/liquidity/add.ts
[error] 42-42: Irregular whitespace not allowed.
(no-irregular-whitespace)
⏰ 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). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
packages/client/src/getPools.ts (1)
42-47
: LGTM! Correctly handles async token detail fetching.The changes properly await the now-asynchronous
formatPoolsWithTokenDetails
function and pass the requiredprovider
parameter. This ensures complete token metadata is fetched before returning the pools data.utils/uniswap.ts (1)
291-345
: LGTM! Clean implementation of async token detail enrichment.The function correctly:
- Maintains backward compatibility by preserving the existing token mapping logic
- Adds the async capability to fetch missing token details from the blockchain
- Properly handles both ZRC20 tokens and the ZETA token
); | ||
|
||
/** | ||
* 4. Locate (or verify) the pool — order agnostic |
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.
Fix irregular whitespace.
Line 42 contains irregular whitespace that should be replaced with regular spaces.
- * 4. Locate (or verify) the pool — order agnostic
+ * 4. Locate (or verify) the pool - order agnostic
📝 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.
* 4. Locate (or verify) the pool — order agnostic | |
* 4. Locate (or verify) the pool - order agnostic |
🧰 Tools
🪛 ESLint
[error] 42-42: Irregular whitespace not allowed.
(no-irregular-whitespace)
🤖 Prompt for AI Agents
In packages/commands/src/zetachain/pools/liquidity/add.ts at line 42, there is
irregular whitespace present in the comment. Replace all irregular whitespace
characters with standard space characters to ensure consistent formatting.
Summary by CodeRabbit
New Features
Chores
Documentation