Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the concentrated liquidity vault from a single-pool architecture to support multiple managed pools. The vault can now handle liquidity across multiple Ekubo pools simultaneously, with proportional deposit/withdrawal logic across all pools.
Key changes:
- Introduces
ManagedPool,SqrtValues, andInitValuesstructs to manage multiple pools - Converts storage from single pool variables to vectors of pools with associated data
- Updates all core functions (deposit, withdraw, rebalance, etc.) to operate across multiple pools
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/strategies/cl_vault/interface.cairo | Adds new data structures for multi-pool support and updates function signatures to accept pool parameters |
| src/strategies/cl_vault/cl_vault.cairo | Implements multi-pool logic with loops over managed pools, proportional deposit/withdrawal calculations, and per-pool fee handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ); | ||
| return MyPosition { | ||
| liquidity: userPosition.liquidity, amount0: amt0.into(), amount1: amt1.into() | ||
| liquidity: userPosition.liquidity, amount0: total_amt0.into(), amount1: total_amt0.into() |
There was a problem hiding this comment.
The amount1 field is incorrectly set to total_amt0 instead of total_amt1. This will return the wrong amount for token1.
| liquidity: userPosition.liquidity, amount0: total_amt0.into(), amount1: total_amt0.into() | |
| liquidity: userPosition.liquidity, amount0: total_amt0.into(), amount1: total_amt1.into() |
| while i != self.managed_pools.len() { | ||
| let pool = self.managed_pools[i].read(); | ||
| self.handle_fees(pool); | ||
| let pool_liq = self._convert_to_assets(shares, pool); | ||
|
|
||
| let old_liq = self.get_position(pool).liquidity; | ||
|
|
||
| let (amt0, amt1) = self._withdraw_position(pool_liq, pool); | ||
| total_amt0 += amt0.into(); | ||
| total_amt1 += amt1.into(); | ||
|
|
||
| let current_liq = self.get_position(pool).liquidity; | ||
|
|
||
| if current_liq == 0 { | ||
| self.managed_pools[i].write( | ||
| ManagedPool { | ||
| pool_key: pool.pool_key, | ||
| bounds: pool.bounds, | ||
| nft_id: 0 | ||
| } | ||
| ); | ||
| } | ||
|
|
||
| // withdraw | ||
| let (amt0, amt1) = self._withdraw_position(userPosition.liquidity.try_into().unwrap()); | ||
| assert( | ||
| (old_liq - current_liq).into() == userPosition.liquidity, | ||
| 'invalid liquidity removed' | ||
| ); | ||
| } |
There was a problem hiding this comment.
The assertion on line 284 compares per-pool liquidity change against total userPosition.liquidity, which will fail for multi-pool scenarios. The assertion should compare (old_liq - current_liq).into() == pool_liq or accumulate liquidity changes across pools before asserting. Also, the loop is missing an increment statement i += 1; at the end.
| let deposit_amt0 = (amount0 * (*range_amount0).into()) / total_amount0.into(); | ||
| let deposit_amt1 = (amount1 * (*range_amount1).into()) / total_amount1.into(); |
There was a problem hiding this comment.
Division by zero is possible if total_amount0 or total_amount1 is zero (e.g., when all pools are empty or unbalanced). Add checks to handle the case when totals are zero before performing division.
| let deposit_amt0 = (amount0 * (*range_amount0).into()) / total_amount0.into(); | |
| let deposit_amt1 = (amount1 * (*range_amount1).into()) / total_amount1.into(); | |
| let deposit_amt0 = if total_amount0 != 0 { | |
| (amount0 * (*range_amount0).into()) / total_amount0.into() | |
| } else { | |
| 0 | |
| }; | |
| let deposit_amt1 = if total_amount1 != 0 { | |
| (amount1 * (*range_amount1).into()) / total_amount1.into() | |
| } else { | |
| 0 | |
| }; |
| let user_new_liq = (*range_liq * deposit_amt0.try_into().unwrap()) / *range_amount0; | ||
| let mut range_shares = 0; |
There was a problem hiding this comment.
Division by zero is possible if *range_amount0 is zero. Add a check to handle empty pool ranges before performing this calculation.
| let user_new_liq = (*range_liq * deposit_amt0.try_into().unwrap()) / *range_amount0; | |
| let mut range_shares = 0; | |
| let user_new_liq = if *range_amount0 != 0 { |
| // implement arrakis ratio for shares | ||
| let init_values = self.init_values.read(); | ||
| let shares_from_token0 = if init_values.init0 != 0 { | ||
| deposit_amt0 * 1000000000000000000_u256 / init_values.init0 } |
There was a problem hiding this comment.
The magic number 1000000000000000000_u256 (1e18) should be defined as a named constant to improve code readability and maintainability.
| pub bounds: Bounds, | ||
| pub nft_id: u64 | ||
| } | ||
| // todo : add sqrt values if possible |
There was a problem hiding this comment.
[nitpick] Inconsistent spacing around colon. Should be // todo: add sqrt values if possible or // TODO: add sqrt values if possible for consistency with typical comment formatting.
| // todo : add sqrt values if possible | |
| // TODO: add sqrt values if possible |
| } | ||
|
|
||
| shares += range_shares; | ||
| total_assets += self._convert_to_assets(range_shares, pool); |
There was a problem hiding this comment.
instead of total_assets, call as total_liquidity. Also, no point in adding liquidity. there are not comparable. So, dont do this. just return shares from this function.
#[derive(Drop, Copy, Serde, starknet::Store, starknet::Event)]
pub struct RangeInstruction {
pub liquidity_mint: u128,
pub liquidity_burn: u128,
pub pool_key: PoolKey,
pub bounds: Bounds,
}
return MyPosition {
liquidity: liquidities, amount0: total_amt0.into(), amount1: total_amt0.into()
};
fn rebalance_pool(ref self: ContractState, rebalance_params: RebalanceParams) {
// Add validation loop
let mut i = 0;
while i != self.managed_pools.len() {
let stored_pool = self.managed_pools[i].read();
let input_pool_key = rebalance_params.rebal.at(i).pool_key; // Add pool_key to RangeInstruction
assert(stored_pool.pool_key.token0 == input_pool_key.token0, 'pool_key mismatch token0');
assert(stored_pool.pool_key.token1 == input_pool_key.token1, 'pool_key mismatch token1');
assert(stored_pool.pool_key.fee == input_pool_key.fee, 'pool_key mismatch fee');
assert(stored_pool.pool_key.tick_spacing == input_pool_key.tick_spacing, 'pool_key mismatch tick_spacing');
assert(stored_pool.pool_key.extension == input_pool_key.extension, 'pool_key mismatch extension');
i += 1;
}
// ... rest of rebalance logic
}
|
…rm reward_shares from cl_vault
2 participants
No description provided.