From 11b9ebe43b5a15d7524683a24d5e0ea83e691b32 Mon Sep 17 00:00:00 2001 From: "magiodev.eth" <31893902+magiodev@users.noreply.github.com> Date: Thu, 18 Jul 2024 13:19:27 +0200 Subject: [PATCH] CL Vault optimize range math operations (#657) ## 1. Overview This PR aims to streamline and optimize the math operations related to `range.rs` in the scenario of `ModifyRange` message. ## 2. Implementation Details Simplified the math operation on withdrawal of position reply in the context of re-ranging. Considering that on the reply, the balances are already incremented with the withdrawn assets from the previous step, it is unnecessary to subtract that with the withdrawn amount, only to re-add them with a `saturating_sub` in between. Deprecated `CURRENT_BALANCE` state which was unnecessary. It was used before creating a position, and we were loading it to subtract the created position amounts from it. This has been replaced with a simple balances query which, in the context of a reply, already takes into account previous operations such as creating a position that already removes funds from the contract balance. --- .../contracts/cl-vault/src/contract.rs | 31 +- .../contracts/cl-vault/src/helpers/getters.rs | 303 ++---------------- .../contracts/cl-vault/src/state.rs | 28 +- .../cl-vault/src/test_tube/helpers.rs | 2 +- .../contracts/cl-vault/src/vault/admin.rs | 4 +- .../contracts/cl-vault/src/vault/range.rs | 126 +++----- 6 files changed, 119 insertions(+), 375 deletions(-) diff --git a/smart-contracts/contracts/cl-vault/src/contract.rs b/smart-contracts/contracts/cl-vault/src/contract.rs index cc8c743e1..8eff27106 100644 --- a/smart-contracts/contracts/cl-vault/src/contract.rs +++ b/smart-contracts/contracts/cl-vault/src/contract.rs @@ -21,10 +21,13 @@ use crate::query::{ }; use crate::reply::Replies; #[allow(deprecated)] +use crate::state::CURRENT_BALANCE; +#[allow(deprecated)] use crate::state::{ MigrationStatus, VaultConfig, MIGRATION_STATUS, OLD_VAULT_CONFIG, STRATEGIST_REWARDS, VAULT_CONFIG, }; +#[allow(deprecated)] use crate::state::{Position, OLD_POSITION, POSITION}; use crate::vault::admin::execute_admin; use crate::vault::any_deposit::{execute_any_deposit, handle_any_deposit_swap_reply}; @@ -207,7 +210,7 @@ pub fn reply(deps: DepsMut, env: Env, msg: Reply) -> Result handle_collect_incentives_reply(deps, env, msg.result), Replies::CollectSpreadRewards => handle_collect_spread_rewards_reply(deps, env, msg.result), - Replies::WithdrawPosition => handle_withdraw_position_reply(deps, env, msg.result), + Replies::WithdrawPosition => handle_withdraw_position_reply(deps, env), Replies::RangeInitialCreatePosition => { handle_initial_create_position_reply(deps, env, msg.result) } @@ -228,6 +231,7 @@ pub fn reply(deps: DepsMut, env: Env, msg: Reply) -> Result Result { + #[allow(deprecated)] let old_vault_config = OLD_VAULT_CONFIG.load(deps.storage)?; let new_vault_config = VaultConfig { performance_fee: old_vault_config.performance_fee, @@ -236,6 +240,7 @@ pub fn migrate(deps: DepsMut, _env: Env, msg: MigrateMsg) -> Result Result Result Result Result { + #[allow(deprecated)] let old_vault_config = OLD_VAULT_CONFIG.load(deps.storage)?; let new_vault_config = VaultConfig { performance_fee: old_vault_config.performance_fee, @@ -316,6 +327,7 @@ mod tests { dex_router: deps.api.addr_validate(msg.dex_router.as_str())?, }; + #[allow(deprecated)] OLD_VAULT_CONFIG.remove(deps.storage); VAULT_CONFIG.save(deps.storage, &new_vault_config)?; @@ -337,6 +349,7 @@ mod tests { // Remove the state #[allow(deprecated)] STRATEGIST_REWARDS.remove(deps.storage); + #[allow(deprecated)] let old_position = OLD_POSITION.load(deps.storage)?; let new_position: Position = Position { @@ -347,7 +360,10 @@ mod tests { POSITION.save(deps.storage, &new_position)?; + #[allow(deprecated)] OLD_POSITION.remove(deps.storage); + #[allow(deprecated)] + CURRENT_BALANCE.remove(deps.storage); Ok(response) } @@ -360,9 +376,11 @@ mod tests { let new_dex_router = Addr::unchecked("dex_router"); // new field nested in existing VaultConfig state // Mock a previous state item + #[allow(deprecated)] OLD_POSITION .save(deps.as_mut().storage, &OldPosition { position_id: 1 }) .unwrap(); + #[allow(deprecated)] OLD_VAULT_CONFIG .save( deps.as_mut().storage, @@ -393,8 +411,13 @@ mod tests { assert_eq!(position.join_time, 0); assert!(position.claim_after.is_none()); + #[allow(deprecated)] let old_position = OLD_POSITION.may_load(deps.as_mut().storage).unwrap(); assert!(old_position.is_none()); + + #[allow(deprecated)] + let current_balance = CURRENT_BALANCE.may_load(deps.as_mut().storage).unwrap(); + assert!(current_balance.is_none()); } #[test] @@ -406,9 +429,11 @@ mod tests { let new_dex_router = Addr::unchecked("dex_router"); // new field nested in existing VaultConfig state // Mock a previous state item + #[allow(deprecated)] OLD_POSITION .save(deps.as_mut().storage, &OldPosition { position_id: 1 }) .unwrap(); + #[allow(deprecated)] OLD_VAULT_CONFIG .save( deps.as_mut().storage, @@ -434,6 +459,7 @@ mod tests { .unwrap(); // Assert OLD_VAULT_CONFIG have been correctly removed by unwrapping the error + #[allow(deprecated)] OLD_VAULT_CONFIG.load(deps.as_mut().storage).unwrap_err(); // Assert new VAULT_CONFIG.dex_router field have correct value @@ -465,6 +491,7 @@ mod tests { let new_dex_router = Addr::unchecked("dex_router"); // new field nested in existing VaultConfig state // Mock a previous state item + #[allow(deprecated)] OLD_VAULT_CONFIG .save( deps.as_mut().storage, @@ -476,6 +503,7 @@ mod tests { ) .unwrap(); + #[allow(deprecated)] OLD_POSITION .save(deps.as_mut().storage, &OldPosition { position_id: 1 }) .unwrap(); @@ -530,6 +558,7 @@ mod tests { STRATEGIST_REWARDS.load(deps.as_mut().storage).unwrap_err(); // Assert OLD_VAULT_CONFIG have been correctly removed by unwrapping the error + #[allow(deprecated)] OLD_VAULT_CONFIG.load(deps.as_mut().storage).unwrap_err(); // Assert new VAULT_CONFIG.dex_router field have correct value diff --git a/smart-contracts/contracts/cl-vault/src/helpers/getters.rs b/smart-contracts/contracts/cl-vault/src/helpers/getters.rs index c860cccf9..58ea3ab12 100644 --- a/smart-contracts/contracts/cl-vault/src/helpers/getters.rs +++ b/smart-contracts/contracts/cl-vault/src/helpers/getters.rs @@ -1,12 +1,12 @@ use crate::math::tick::tick_to_price; -use crate::state::RANGE_ADMIN; +use crate::state::{PoolConfig, RANGE_ADMIN}; use std::str::FromStr; use osmosis_std::shim::Timestamp as OsmoTimestamp; use osmosis_std::types::osmosis::poolmanager::v1beta1::PoolmanagerQuerier; use osmosis_std::types::osmosis::twap::v1beta1::TwapQuerier; -use crate::vault::concentrated_liquidity::{get_cl_pool_info, get_position}; +use crate::vault::concentrated_liquidity::get_position; use crate::{state::POOL_CONFIG, ContractError}; use cosmwasm_std::{ Addr, Coin, Decimal, Decimal256, Deps, DepsMut, Env, Fraction, QuerierWrapper, Storage, @@ -41,22 +41,6 @@ pub fn get_asset0_value( Ok(total) } -/// get_spot_price -/// -/// gets the spot price of the pool which this vault is managing funds in. This will always return token0 in terms of token1 (or would it be the other way around?) -pub fn get_spot_price( - storage: &dyn Storage, - querier: &QuerierWrapper, -) -> Result { - let pool_config = POOL_CONFIG.load(storage)?; - - let pm_querier = PoolmanagerQuerier::new(querier); - let spot_price = - pm_querier.spot_price(pool_config.pool_id, pool_config.token0, pool_config.token1)?; - - Ok(Decimal::from_str(&spot_price.spot_price)?) -} - pub fn get_twap_price( storage: &dyn Storage, querier: &QuerierWrapper, @@ -153,85 +137,6 @@ pub fn get_depositable_tokens( } } -// /// get_liquidity_needed_for_tokens -// /// -// /// this function calculates the liquidity needed for depositing token0 and quote token amounts respectively and returns both. -// /// depositing both tokens would result in a refund of the token with higher needed liquidity -// /// -// /// thanks: https://github.com/osmosis-labs/osmosis/blob/main/x/concentrated-liquidity/README.md#adding-liquidity -// pub fn get_liquidity_needed_for_tokens( -// delta_token0: String, -// delta_token1: String, -// lower_tick: i64, -// upper_tick: i64, -// ) -> Result<(Uint128, Uint128), ContractError> { -// // todo check that decimal casts are correct -// let delta_x = Decimal256::from_atomics(Uint128::from_str(&delta_token0)?, 18)?; -// let delta_y = Decimal256::from_atomics(Uint128::from_str(&delta_token1)?, 18)?; -// // calc liquidity needed for token - -// // save gas and read easier by calcing ahead (gas savings prob already done by compiler) -// let price_lower = tick_to_price(lower_tick)?; -// let price_upper = tick_to_price(upper_tick)?; -// let sqrt_price_lower = price_lower.sqrt(); -// let sqrt_price_upper = price_upper.sqrt(); -// let denominator = sqrt_price_upper.checked_sub(sqrt_price_lower)?; - -// // liquidity follows the formula found in the link above this function. basically this: -// // liquidity_x = (delta_x * sqrt(price_lower) * sqrt(price_upper))/(sqrt(price_upper) - sqrt(price_lower)) -// // liquidity_7 = (delta_x)/(sqrt(price_upper) - sqrt(price_lower)) -// // overflow city? -// let liquidity_x = delta_x -// .checked_mul(sqrt_price_lower)? -// .checked_mul(sqrt_price_upper)? -// .checked_div(denominator)?; - -// let liquidity_y = delta_y.checked_div(denominator)?; - -// Ok(( -// liquidity_x.atomics().try_into()?, -// liquidity_y.atomics().try_into()?, -// )) -// } - -// pub fn get_deposit_amounts_for_liquidity_needed( -// liquidity_needed_token0: Uint128, -// liquidity_needed_token1: Uint128, -// token0_amount: String, -// token1_amount: String, -// // i hate this type but it's arguably a good way to write this -// ) -> Result<((Uint128, Uint128), (Uint128, Uint128)), ContractError> { -// // calc deposit amounts for liquidity needed -// let amount_0 = Uint128::from_str(&token0_amount)?; -// let amount_1 = Uint128::from_str(&token1_amount)?; - -// // one of these will be zero -// let mut remainder_0 = Uint128::zero(); -// let mut remainder_1 = Uint128::zero(); - -// let (deposit_amount_0, deposit_amount_1) = if liquidity_needed_token0 > liquidity_needed_token1 -// { -// // scale base token amount down by L1/L0, take full amount of quote token -// let new_amount_0 = -// amount_0.multiply_ratio(liquidity_needed_token1, liquidity_needed_token0); -// remainder_0 = amount_0.checked_sub(new_amount_0).unwrap(); - -// (new_amount_0, amount_1) -// } else { -// // scale quote token amount down by L0/L1, take full amount of base token -// let new_amount_1 = -// amount_1.multiply_ratio(liquidity_needed_token0, liquidity_needed_token1); -// remainder_1 = amount_1.checked_sub(new_amount_1)?; - -// (amount_0, new_amount_1) -// }; - -// Ok(( -// (deposit_amount_0, deposit_amount_1), -// (remainder_0, remainder_1), -// )) -// } - // this math is straight from the readme pub fn get_single_sided_deposit_0_to_1_swap_amount( token0_balance: Uint128, @@ -312,182 +217,42 @@ pub fn get_unused_balances(querier: &QuerierWrapper, env: &Env) -> Result Result<(Uint256, Uint256), ContractError> { - // maxdep1 = T0 / R - let max_deposit1_from_0 = - token0.checked_multiply_ratio(ratio.denominator(), ratio.numerator())?; - // maxdep0 = T1 * R - let max_deposit0_from_1 = - token1.checked_multiply_ratio(ratio.numerator(), ratio.denominator())?; - - if max_deposit0_from_1 > token0 { - Ok((token0, max_deposit1_from_0)) - } else if max_deposit1_from_0 > token1 { - Ok((max_deposit0_from_1, token1)) - } else { - Ok((token0, token1)) - } -} - -pub fn get_liquidity_amount_for_unused_funds( - deps: DepsMut, +pub fn get_unused_pair_balances( + deps: &DepsMut, env: &Env, - additional_excluded_funds: (Uint128, Uint128), -) -> Result { - // first get the ratio of token0:token1 in the position. - let p = get_position(deps.storage, &deps.querier)?; - // if there is no position, then we can assume that there are 0 unused funds - if p.position.is_none() { - return Ok(Decimal256::zero()); - } - let position_unwrapped = p.position.ok_or(ContractError::MissingPosition {})?; + pool_config: &PoolConfig, +) -> Result<(Uint128, Uint128), ContractError> { + // Get unused balances from the contract. This is the amount of tokens that are not currently in a position. + // This amount already includes the withdrawn amounts from previous steps as in this reply those funds already compose the contract balance. + let unused_balances = get_unused_balances(&deps.querier, env)?; - // Safely unwrap asset0 and asset1, handle absence through errors - let token0_str = p.asset0.ok_or(ContractError::MissingAssetInfo { - asset: "asset0".to_string(), - })?; - let token1_str = p.asset1.ok_or(ContractError::MissingAssetInfo { - asset: "asset1".to_string(), - })?; + // Use the unused balances to get the token0 and token1 amounts that we can use to create a new position + let amount0 = unused_balances.find_coin(pool_config.token0.clone()).amount; + let amount1 = unused_balances.find_coin(pool_config.token1.clone()).amount; - let token0: Coin = token0_str - .try_into() - .map_err(|_| ContractError::ConversionError { - asset: "asset0".to_string(), - msg: "Failed to convert asset0 to Coin".to_string(), - })?; - let token1: Coin = token1_str - .try_into() - .map_err(|_| ContractError::ConversionError { - asset: "asset1".to_string(), - msg: "Failed to convert asset1 to Coin".to_string(), - })?; - - // if any of the values are 0, we fill 1 - let ratio = if token0.amount.is_zero() { - Decimal256::from_ratio(1_u128, token1.amount) - } else if token1.amount.is_zero() { - Decimal256::from_ratio(token0.amount, 1_u128) - } else { - Decimal256::from_ratio(token0.amount, token1.amount) - }; - let pool_config = POOL_CONFIG.load(deps.storage)?; - let pool_details = get_cl_pool_info(&deps.querier, pool_config.pool_id)?; - - // then figure out based on current unused balance, what the max initial deposit could be - // (with the ratio, what is the max tokens we can deposit) - let tokens = get_unused_balances(&deps.querier, env)?; - - // Notice: checked_sub has been replaced with saturating_sub due to overflowing response from dex - let unused_t0: Uint256 = tokens - .find_coin(token0.denom) - .amount - .saturating_sub(additional_excluded_funds.0) - .into(); - let unused_t1: Uint256 = tokens - .find_coin(token1.denom) - .amount - .saturating_sub(additional_excluded_funds.1) - .into(); - - let max_initial_deposit = get_max_utilization_for_ratio(unused_t0, unused_t1, ratio)?; - - // then figure out how much liquidity this would give us. - // Formula: current_position_liquidity * token0_initial_deposit_amount / token0_in_current_position - // EDGE CASE: what if it's a one-sided position with only token1? - // SOLUTION: take whichever token is greater than the other to plug into the formula 1 line above - let position_liquidity = Decimal256::from_str(&position_unwrapped.liquidity)?; - let max_initial_deposit_liquidity = if token0.amount > token1.amount { - position_liquidity - .checked_mul(Decimal256::new(max_initial_deposit.0))? - .checked_div(Decimal256::new(token0.amount.into()))? - } else { - position_liquidity - .checked_mul(Decimal256::new(max_initial_deposit.1))? - .checked_div(Decimal256::new(token1.amount.into()))? - }; - - // subtract out the max deposit from both tokens, which will leave us with only one token, lets call this leftover_balance0 or 1 - let leftover_balance0 = unused_t0.checked_sub(max_initial_deposit.0)?; - let leftover_balance1 = unused_t1.checked_sub(max_initial_deposit.1)?; - - // call get_single_sided_deposit_0_to_1_swap_amount or get_single_sided_deposit_1_to_0_swap_amount to see how much we would swap to enter with the rest of our funds - let post_swap_liquidity = if leftover_balance0 > leftover_balance1 { - let swap_amount = if pool_details.current_tick > position_unwrapped.upper_tick { - leftover_balance0.try_into()? - } else { - get_single_sided_deposit_0_to_1_swap_amount( - leftover_balance0.try_into()?, - position_unwrapped.lower_tick, - pool_details.current_tick, - position_unwrapped.upper_tick, - )? - }; - // let swap_amount = get_single_sided_deposit_0_to_1_swap_amount( - // leftover_balance0.try_into()?, - // position_unwrapped.lower_tick, - // pool_details.current_tick, - // position_unwrapped.upper_tick, - // )?; - - // subtract the resulting swap_amount from leftover_balance0 or 1, we can then use the same formula as above to get the correct liquidity amount. - // we are also mindful of the same edge case - let leftover_balance0 = leftover_balance0.checked_sub(swap_amount.into())?; - - if leftover_balance0.is_zero() { - // in this case we need to get the expected token1 from doing a full swap, meaning we need to multiply by the spot price - let token1_from_swap_amount = Decimal256::new(swap_amount.into()) - .checked_mul(tick_to_price(pool_details.current_tick)?)?; - position_liquidity - .checked_mul(token1_from_swap_amount)? - .checked_div(Decimal256::new(token1.amount.into()))? - } else { - position_liquidity - .checked_mul(Decimal256::new(leftover_balance0))? - .checked_div(Decimal256::new(token0.amount.into()))? - } - } else { - let swap_amount = if pool_details.current_tick < position_unwrapped.lower_tick { - leftover_balance1.try_into()? - } else { - get_single_sided_deposit_1_to_0_swap_amount( - leftover_balance1.try_into()?, - position_unwrapped.lower_tick, - pool_details.current_tick, - position_unwrapped.upper_tick, - )? - }; - // let swap_amount = get_single_sided_deposit_1_to_0_swap_amount( - // leftover_balance1.try_into()?, - // position_unwrapped.lower_tick, - // pool_details.current_tick, - // position_unwrapped.upper_tick, - // )?; - - // subtract the resulting swap_amount from leftover_balance0 or 1, we can then use the same formula as above to get the correct liquidity amount. - // we are also mindful of the same edge case - let leftover_balance1 = leftover_balance1.checked_sub(swap_amount.into())?; - - if leftover_balance1.is_zero() { - // in this case we need to get the expected token0 from doing a full swap, meaning we need to multiply by the spot price - let token0_from_swap_amount = Decimal256::new(swap_amount.into()) - .checked_div(tick_to_price(pool_details.current_tick)?)?; - position_liquidity - .checked_mul(token0_from_swap_amount)? - .checked_div(Decimal256::new(token0.amount.into()))? - } else { - position_liquidity - .checked_mul(Decimal256::new(leftover_balance1))? - .checked_div(Decimal256::new(token1.amount.into()))? - } - }; + Ok((amount0, amount1)) +} + +pub fn get_tokens_provided( + amount0: Uint128, + amount1: Uint128, + pool_config: &PoolConfig, +) -> Result, ContractError> { + let mut tokens_provided = vec![]; + if !amount0.is_zero() { + tokens_provided.push(Coin { + denom: pool_config.token0.clone(), + amount: amount0, + }) + } + if !amount1.is_zero() { + tokens_provided.push(Coin { + denom: pool_config.token1.clone(), + amount: amount1, + }) + } - // add together the liquidity from the initial deposit and the swap deposit and return that - Ok(max_initial_deposit_liquidity.checked_add(post_swap_liquidity)?) + Ok(tokens_provided) } #[cfg(test)] diff --git a/smart-contracts/contracts/cl-vault/src/state.rs b/smart-contracts/contracts/cl-vault/src/state.rs index b5e247f98..4631302e7 100644 --- a/smart-contracts/contracts/cl-vault/src/state.rs +++ b/smart-contracts/contracts/cl-vault/src/state.rs @@ -7,6 +7,23 @@ use cosmwasm_std::{Addr, Decimal, Decimal256, Uint128}; use cw_storage_plus::{Deque, Item, Map}; use osmosis_std::types::osmosis::poolmanager::v1beta1::SwapAmountInRoute; +#[deprecated] +pub const OLD_VAULT_CONFIG: Item = Item::new("vault_config"); + +#[deprecated] +pub const OLD_POSITION: Item = Item::new("position"); + +/// REWARDS: Current rewards are the rewards being gathered, these can be both spread rewards as well as incentives +#[deprecated] +pub const STRATEGIST_REWARDS: Item = Item::new("strategist_rewards"); + +/// Shared collection+distribution states +#[deprecated] +pub const USER_REWARDS: Map = Map::new("user_rewards"); + +#[deprecated] +pub const CURRENT_BALANCE: Item<(Uint128, Uint128)> = Item::new("current_balance"); // CURRENT_BALANCE is intended as CURRENT_SWAP_BALANCE + /// metadata useful for display purposes #[cw_serde] pub struct Metadata { @@ -45,7 +62,6 @@ pub struct OldVaultConfig { pub swap_max_slippage: Decimal, } -pub const OLD_VAULT_CONFIG: Item = Item::new("vault_config"); pub const VAULT_CONFIG: Item = Item::new("vault_config_v2"); pub const VAULT_DENOM: Item = Item::new("vault_denom"); @@ -86,7 +102,6 @@ pub struct Position { pub claim_after: Option, // this should be off chain computed and set in order to avoid forfeiting incentives } -pub const OLD_POSITION: Item = Item::new("position"); pub const POSITION: Item = Item::new("position_v2"); pub const SHARES: Map = Map::new("shares"); @@ -118,16 +133,7 @@ pub enum RewardsStatus { Distributing, } -/// REWARDS: Current rewards are the rewards being gathered, these can be both spread rewards as well as incentives -#[deprecated] -pub const STRATEGIST_REWARDS: Item = Item::new("strategist_rewards"); - -/// Shared collection+distribution states -#[deprecated] -pub const USER_REWARDS: Map = Map::new("user_rewards"); - /// Swap helper states -pub const CURRENT_BALANCE: Item<(Uint128, Uint128)> = Item::new("current_balance"); // CURRENT_BALANCE is intended as CURRENT_SWAP_BALANCE pub const CURRENT_SWAP: Item<(SwapDirection, Uint128)> = Item::new("current_swap"); pub const CURRENT_SWAP_ANY_DEPOSIT: Item<(SwapDirection, Uint128, Addr, (Uint128, Uint128))> = Item::new("current_swap_any_deposit"); diff --git a/smart-contracts/contracts/cl-vault/src/test_tube/helpers.rs b/smart-contracts/contracts/cl-vault/src/test_tube/helpers.rs index 09cb09b04..06892f4d5 100644 --- a/smart-contracts/contracts/cl-vault/src/test_tube/helpers.rs +++ b/smart-contracts/contracts/cl-vault/src/test_tube/helpers.rs @@ -87,7 +87,7 @@ pub fn calculate_deposit_ratio( }; // TODO: Compute this based on tokens_provided size - let ratio_approx = "0.00005".to_string(); + let ratio_approx: String = "0.00005".to_string(); (ratio, ratio_approx) } diff --git a/smart-contracts/contracts/cl-vault/src/vault/admin.rs b/smart-contracts/contracts/cl-vault/src/vault/admin.rs index 2e3833ef8..61e239095 100644 --- a/smart-contracts/contracts/cl-vault/src/vault/admin.rs +++ b/smart-contracts/contracts/cl-vault/src/vault/admin.rs @@ -88,7 +88,6 @@ pub fn execute_update_dex_router( nonpayable(&info).map_err(|_| ContractError::NonPayable {})?; assert_admin(deps.as_ref(), &info.sender)?; - let previous_router = RANGE_ADMIN.load(deps.storage)?; match address.clone() { Some(address) => { let new_router = deps.api.addr_validate(&address)?; @@ -102,8 +101,7 @@ pub fn execute_update_dex_router( Ok(Response::new() .add_attribute("method", "execute") .add_attribute("action", "update_dex_router") - .add_attribute("previous_router", previous_router) - .add_attribute("new_router", address.unwrap_or("none".to_owned()))) + .add_attribute("new_router", address.unwrap_or_default().to_string())) } /// Updates the configuration of the contract. diff --git a/smart-contracts/contracts/cl-vault/src/vault/range.rs b/smart-contracts/contracts/cl-vault/src/vault/range.rs index 22cc23c7f..2e30c120b 100644 --- a/smart-contracts/contracts/cl-vault/src/vault/range.rs +++ b/smart-contracts/contracts/cl-vault/src/vault/range.rs @@ -4,7 +4,8 @@ use crate::{ generic::extract_attribute_value_by_ty_and_key, getters::{ get_single_sided_deposit_0_to_1_swap_amount, - get_single_sided_deposit_1_to_0_swap_amount, get_twap_price, get_unused_balances, + get_single_sided_deposit_1_to_0_swap_amount, get_tokens_provided, get_twap_price, + get_unused_pair_balances, }, msgs::swap_msg, }, @@ -12,8 +13,8 @@ use crate::{ msg::{ExecuteMsg, MergePositionMsg}, reply::Replies, state::{ - ModifyRangeState, Position, SwapDepositMergeState, CURRENT_BALANCE, CURRENT_SWAP, - MODIFY_RANGE_STATE, POOL_CONFIG, POSITION, SWAP_DEPOSIT_MERGE_STATE, + ModifyRangeState, Position, SwapDepositMergeState, CURRENT_SWAP, MODIFY_RANGE_STATE, + POOL_CONFIG, POSITION, SWAP_DEPOSIT_MERGE_STATE, }, vault::{ concentrated_liquidity::{create_position, get_position}, @@ -27,9 +28,7 @@ use cosmwasm_std::{ SubMsg, SubMsgResult, Uint128, }; use osmosis_std::types::osmosis::{ - concentratedliquidity::v1beta1::{ - MsgCreatePositionResponse, MsgWithdrawPosition, MsgWithdrawPositionResponse, - }, + concentratedliquidity::v1beta1::{MsgCreatePositionResponse, MsgWithdrawPosition}, gamm::v1beta1::MsgSwapExactAmountInResponse, poolmanager::v1beta1::SwapAmountInRoute, }; @@ -147,50 +146,15 @@ pub fn execute_update_range_ticks( } // do create new position -pub fn handle_withdraw_position_reply( - deps: DepsMut, - env: Env, - data: SubMsgResult, -) -> Result { - let msg: MsgWithdrawPositionResponse = data.try_into()?; - +pub fn handle_withdraw_position_reply(deps: DepsMut, env: Env) -> Result { let modify_range_state = MODIFY_RANGE_STATE.load(deps.storage)?.unwrap(); let pool_config = POOL_CONFIG.load(deps.storage)?; - - let mut amount0: Uint128 = msg.amount0.parse()?; - let mut amount1: Uint128 = msg.amount1.parse()?; - - let unused_balances = get_unused_balances(&deps.querier, &env)?; - let unused_balance0 = unused_balances - .find_coin(pool_config.token0.clone()) - .amount - .saturating_sub(amount0); - let unused_balance1 = unused_balances - .find_coin(pool_config.token1.clone()) - .amount - .saturating_sub(amount1); - - amount0 = amount0.checked_add(unused_balance0)?; - amount1 = amount1.checked_add(unused_balance1)?; - - CURRENT_BALANCE.save(deps.storage, &(amount0, amount1))?; - - let mut tokens_provided = vec![]; - if !amount0.is_zero() { - tokens_provided.push(Coin { - denom: pool_config.token0.clone(), - amount: amount0, - }) - } - if !amount1.is_zero() { - tokens_provided.push(Coin { - denom: pool_config.token1.clone(), - amount: amount1, - }) - } - let pool_details = get_cl_pool_info(&deps.querier, pool_config.pool_id)?; + let unused_pair_balances = get_unused_pair_balances(&deps, &env, &pool_config)?; + let tokens_provided = + get_tokens_provided(unused_pair_balances.0, unused_pair_balances.1, &pool_config)?; + // if only one token is being deposited, and we are moving into a position where any amount of the other token is needed, // creating the position here will fail because liquidityNeeded is calculated as 0 on chain level // we can fix this by going straight into a swap-deposit-merge before creating any positions @@ -200,21 +164,24 @@ pub fn handle_withdraw_position_reply( // if (lower < current < upper) && amount0 == 0 || amount1 == 0 // also onesided but wrong token // bad complexity demon, grug no like - if (amount0.is_zero() && pool_details.current_tick < modify_range_state.upper_tick) - || (amount1.is_zero() && pool_details.current_tick > modify_range_state.lower_tick) + if (unused_pair_balances.0.is_zero() + && pool_details.current_tick < modify_range_state.upper_tick) + || (unused_pair_balances.1.is_zero() + && pool_details.current_tick > modify_range_state.lower_tick) { do_swap_deposit_merge( deps, env, modify_range_state.lower_tick, modify_range_state.upper_tick, - (amount0, amount1), + (unused_pair_balances.0, unused_pair_balances.1), None, // we just withdrew our only position modify_range_state.ratio_of_swappable_funds_to_use, modify_range_state.twap_window_seconds, ) } else { - // we can naively re-deposit up to however much keeps the proportion of tokens the same. Then swap & re-deposit the proper ratio with the remaining tokens + // we can naively re-deposit up to however much keeps the proportion of tokens the same. + // Then swap & re-deposit the proper ratio with the remaining tokens let create_position_msg = create_position( deps, &env, @@ -232,10 +199,16 @@ pub fn handle_withdraw_position_reply( )) .add_attribute("method", "reply") .add_attribute("action", "handle_withdraw_position") - .add_attribute("lower_tick", format!("{:?}", modify_range_state.lower_tick)) - .add_attribute("upper_tick", format!("{:?}", modify_range_state.upper_tick)) - .add_attribute("token0", format!("{}{}", amount0, pool_config.token0)) - .add_attribute("token1", format!("{}{}", amount1, pool_config.token1))) + .add_attribute("lower_tick", modify_range_state.lower_tick.to_string()) + .add_attribute("upper_tick", modify_range_state.upper_tick.to_string()) + .add_attribute( + "token0", + format!("{}{}", unused_pair_balances.0, pool_config.token0), + ) + .add_attribute( + "token1", + format!("{}{}", unused_pair_balances.1, pool_config.token1), + )) } } @@ -247,31 +220,21 @@ pub fn handle_initial_create_position_reply( ) -> Result { let create_position_message: MsgCreatePositionResponse = data.try_into()?; let modify_range_state = MODIFY_RANGE_STATE.load(deps.storage)?.unwrap(); + let pool_config = POOL_CONFIG.load(deps.storage)?; // target range for our imminent swap // taking from response message is important because they may differ from the ones in our request let target_lower_tick = create_position_message.lower_tick; let target_upper_tick = create_position_message.upper_tick; - // get refunded amounts - // TODO added saturating sub as work around for https://github.com/osmosis-labs/osmosis/issues/6843 - // should be a checked sub eventually - let current_balance = CURRENT_BALANCE.load(deps.storage)?; - let refunded_amounts = ( - current_balance - .0 - .saturating_sub(Uint128::from_str(&create_position_message.amount0)?), - current_balance - .1 - .saturating_sub(Uint128::from_str(&create_position_message.amount1)?), - ); + let unused_pair_balances = get_unused_pair_balances(&deps, &env, &pool_config)?; do_swap_deposit_merge( deps, env, target_lower_tick, target_upper_tick, - refunded_amounts, + (unused_pair_balances.0, unused_pair_balances.1), Some(create_position_message.position_id), modify_range_state.ratio_of_swappable_funds_to_use, modify_range_state.twap_window_seconds, @@ -287,7 +250,7 @@ pub fn do_swap_deposit_merge( env: Env, target_lower_tick: i64, target_upper_tick: i64, - refunded_amounts: (Uint128, Uint128), + tokens_provided: (Uint128, Uint128), position_id: Option, ratio_of_swappable_funds_to_use: Decimal, twap_window_seconds: u64, @@ -297,11 +260,11 @@ pub fn do_swap_deposit_merge( } let (balance0, balance1) = ( - refunded_amounts.0.checked_multiply_ratio( + tokens_provided.0.checked_multiply_ratio( ratio_of_swappable_funds_to_use.numerator(), ratio_of_swappable_funds_to_use.denominator(), )?, - refunded_amounts.1.checked_multiply_ratio( + tokens_provided.1.checked_multiply_ratio( ratio_of_swappable_funds_to_use.numerator(), ratio_of_swappable_funds_to_use.denominator(), )?, @@ -352,10 +315,7 @@ pub fn do_swap_deposit_merge( calculated.token_in_amount, calculated.token_in_denom ), ), - attr( - "token_out_min", - format!("{}", calculated.token_out_min_amount), - ), + attr("token_out_min", calculated.token_out_min_amount.to_string()), ])) } else { // If no swap message, add a new position attribute @@ -671,9 +631,8 @@ mod tests { use cosmwasm_std::{ coin, testing::{mock_dependencies, mock_env, mock_info, MOCK_CONTRACT_ADDR}, - Addr, Decimal, SubMsgResponse, SubMsgResult, + Addr, Decimal, }; - use osmosis_std::types::osmosis::concentratedliquidity::v1beta1::MsgWithdrawPositionResponse; use crate::{ helpers::getters::get_range_admin, @@ -824,20 +783,7 @@ mod tests { ) .unwrap(); - // now test two-sided withdraw - let data = SubMsgResult::Ok(SubMsgResponse { - events: vec![], - data: Some( - MsgWithdrawPositionResponse { - amount0: "10000".to_string(), - amount1: "10000".to_string(), - } - .try_into() - .unwrap(), - ), - }); - - let res = super::handle_withdraw_position_reply(deps.as_mut(), env, data).unwrap(); + let res = super::handle_withdraw_position_reply(deps.as_mut(), env).unwrap(); // verify that we did create_position first assert_eq!(res.messages.len(), 1);